Skip to content

Commit

Permalink
Merge #135634
Browse files Browse the repository at this point in the history
135634: sql: fix statement generation when ensuring roles r=souravcrl a=rafiss

Since we started skipping over non-existent roles in 35f723e, we need to make sure we only add commas at the appropriate point.

The test update demonstrates that there was a bug before this patch, as it would fail with:
```
expected:
ok defaultdb

found:
ERROR: LDAP authorization: error assigning roles to user ldap_user: EnsureUserOnlyBelongsToRoles-grant: at or near ",": syntax error (SQLSTATE 42601)
HINT: try \h GRANT
DETAIL: source SQL:
GRANT , "ldap-parent-synced" TO ldap_user
```

informs #133779
Release note: None

Co-authored-by: Rafi Shamim <[email protected]>
  • Loading branch information
craig[bot] and rafiss committed Nov 19, 2024
2 parents 9927a9a + 55a138f commit 0061bb1
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 5 deletions.
2 changes: 1 addition & 1 deletion pkg/ccl/testccl/authccl/testdata/ldap
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ CREATE ROLE "ldap-parent-synced";
----
ok

ldap_mock set_groups=(ldap_user,cn=ldap-parent-synced,cn=ldap-parent-unsynced)
ldap_mock set_groups=(ldap_user,cn=ldap-parent-unsynced,cn=ldap-parent-synced)
----

connect user=ldap_user password="ldap_pwd"
Expand Down
12 changes: 8 additions & 4 deletions pkg/sql/authorization.go
Original file line number Diff line number Diff line change
Expand Up @@ -739,11 +739,13 @@ func EnsureUserOnlyBelongsToRoles(
if len(rolesToRevoke) > 0 {
revokeStmt := strings.Builder{}
revokeStmt.WriteString("REVOKE ")
for i, role := range rolesToRevoke {
if i > 0 {
addComma := false
for _, role := range rolesToRevoke {
if addComma {
revokeStmt.WriteString(", ")
}
revokeStmt.WriteString(role.SQLIdentifier())
addComma = true
}
revokeStmt.WriteString(" FROM ")
revokeStmt.WriteString(user.SQLIdentifier())
Expand All @@ -757,12 +759,14 @@ func EnsureUserOnlyBelongsToRoles(
if len(rolesToGrant) > 0 {
grantStmt := strings.Builder{}
grantStmt.WriteString("GRANT ")
for i, role := range rolesToGrant {
addComma := false
for _, role := range rolesToGrant {
if roleExists, _ := RoleExists(ctx, txn, role); roleExists {
if i > 0 {
if addComma {
grantStmt.WriteString(", ")
}
grantStmt.WriteString(role.SQLIdentifier())
addComma = true
}
}
grantStmt.WriteString(" TO ")
Expand Down

0 comments on commit 0061bb1

Please sign in to comment.