forked from cockroachdb/cockroach
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ccl/oidcccl: support principal matching on list claims
Previously, matching on ID token claims was not possible if the claim key specified had a corresponding value that was a list, not a string. With this change, matching can now occur on claims that are list valued in order to add login capabilities to DB Console. It is important to note that this change does NOT offer the user the ability to choose between possible matches; it simply selects the first match to log the user in. This change also adds more verbose logging about ID token details. Epic: none Fixes: cockroachdb#97301, cockroachdb#97468 Release note (general change): Increasing the logging verbosity is more helpful with troubleshooting DB Console SSO issues.
- Loading branch information
1 parent
0205bfe
commit 1d20cae
Showing
4 changed files
with
173 additions
and
16 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,10 +13,12 @@ import ( | |
| "crypto/hmac" | ||
| "crypto/sha256" | ||
| "encoding/base64" | ||
| "encoding/json" | ||
| "fmt" | ||
| "net/http" | ||
| "net/http/httptest" | ||
| "net/url" | ||
| "regexp" | ||
| "strings" | ||
| "testing" | ||
|
|
||
|
|
@@ -289,6 +291,67 @@ func TestOIDCStateEncodeDecode(t *testing.T) { | |
| } | ||
| } | ||
|
|
||
| func TestOIDCClaimMatch(t *testing.T) { | ||
| ctx := context.Background() | ||
|
|
||
| for _, tc := range []struct { | ||
| testName string | ||
| claimKey string | ||
| principalRegex string | ||
| claims map[string]json.RawMessage | ||
| wantError bool | ||
| }{ | ||
| { | ||
| testName: "string valued claim", | ||
| claimKey: "email", | ||
| principalRegex: "^([^@]+)@[^@]+$", | ||
| claims: map[string]json.RawMessage{ | ||
| "email": json.RawMessage(`"[email protected]"`), | ||
| }, | ||
| }, | ||
| { | ||
| testName: "string valued claim with no match", | ||
| claimKey: "email", | ||
| principalRegex: "^([^@]+)@[^@]+$", | ||
| claims: map[string]json.RawMessage{ | ||
| "email": json.RawMessage(`"bademail"`), | ||
| }, | ||
| wantError: true, | ||
| }, | ||
| { | ||
| testName: "list valued claim", | ||
| claimKey: "groups", | ||
| principalRegex: "^([^@]+)@[^@]+$", | ||
| claims: map[string]json.RawMessage{ | ||
| "groups": json.RawMessage( | ||
| `["badgroupname", "[email protected]", "anotherbadgroupname"]`, | ||
| ), | ||
| }, | ||
| }, | ||
| { | ||
| testName: "list valued claim with no matches", | ||
| claimKey: "groups", | ||
| principalRegex: "^([^@]+)@[^@]+$", | ||
| claims: map[string]json.RawMessage{ | ||
| "groups": json.RawMessage(`["badgroupname", "anotherbadgroupname"]`), | ||
| }, | ||
| wantError: true, | ||
| }, | ||
| } { | ||
| t.Run(tc.testName, func(t *testing.T) { | ||
| sqlUsername, err := extractUsernameFromClaims( | ||
| ctx, tc.claims, tc.claimKey, regexp.MustCompile(tc.principalRegex), | ||
| ) | ||
| if !tc.wantError { | ||
| require.NoError(t, err) | ||
| require.Equal(t, "myfakeemail", sqlUsername) | ||
| } else { | ||
| require.ErrorContains(t, err, "expected one group in regexp") | ||
| } | ||
| }) | ||
| } | ||
| } | ||
|
|
||
| func Test_getRegionSpecificRedirectURL(t *testing.T) { | ||
| type args struct { | ||
| locality roachpb.Locality | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| package oidcccl | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "regexp" | ||
|
|
||
| "github.com/cockroachdb/cockroach/pkg/util/log" | ||
| "github.com/cockroachdb/errors" | ||
| ) | ||
|
|
||
| // extractUsernameFromClaims uses a regex to strip out elements of the value | ||
| // corresponding to the token claim claimKey. | ||
| func extractUsernameFromClaims( | ||
| ctx context.Context, | ||
| claims map[string]json.RawMessage, | ||
| claimKey string, | ||
| principalRE *regexp.Regexp, | ||
| ) (string, error) { | ||
| targetClaim, ok := claims[claimKey] | ||
| if !ok { | ||
| log.Errorf( | ||
| ctx, "OIDC: failed to complete authentication: invalid JSON claim key: %s", claimKey, | ||
| ) | ||
| } | ||
|
|
||
| var principal string | ||
| if err := json.Unmarshal(targetClaim, &principal); err != nil { | ||
| // Try parsing assuming the claim value is a list and not a string. | ||
| var principals []string | ||
| if err = json.Unmarshal(targetClaim, &principals); err != nil { | ||
| log.Errorf(ctx, | ||
| "OIDC: failed to complete authentication: failed to parse value for the claim %s: %v", | ||
| claimKey, err, | ||
| ) | ||
| return "", err | ||
| } | ||
| return matchOnListClaim(ctx, principals, principalRE) | ||
| } | ||
|
|
||
| match := principalRE.FindStringSubmatch(principal) | ||
| numGroups := len(match) | ||
| if numGroups != 2 { | ||
| err := errors.Newf("expected one group in regexp, got %d", numGroups) | ||
| log.Errorf(ctx, "OIDC: failed to complete authentication: %v", err) | ||
| if log.V(1) { | ||
| log.Infof(ctx, | ||
| "check OIDC cluster settings: %s, %s", | ||
| OIDCPrincipalRegexSettingName, OIDCClaimJSONKeySettingName, | ||
| ) | ||
| } | ||
| return "", err | ||
| } | ||
|
|
||
| return match[1], nil | ||
| } | ||
|
|
||
| func matchOnListClaim( | ||
| ctx context.Context, principals []string, principalRE *regexp.Regexp, | ||
| ) (string, error) { | ||
| // This is the case where the claim key specified is the "groups" claim. | ||
| // The first matching principal is selected as the SQL username. | ||
| if log.V(1) { | ||
| log.Infof(ctx, | ||
| "multiple principals in the claim found; selecting first matching principal...", | ||
| ) | ||
| } | ||
|
|
||
| var match []string | ||
| for _, principal := range principals { | ||
| match = principalRE.FindStringSubmatch(principal) | ||
| if len(match) == 2 { | ||
| return match[1], nil | ||
| } | ||
| } | ||
|
|
||
| // Error when there is not a match. | ||
| err := errors.Newf("expected one group in regexp") | ||
| log.Errorf(ctx, "OIDC: failed to complete authentication: %v", err) | ||
| if log.V(1) { | ||
| log.Infof(ctx, | ||
| "check OIDC cluster settings: %s, %s", | ||
| OIDCPrincipalRegexSettingName, OIDCClaimJSONKeySettingName, | ||
| ) | ||
| } | ||
| return "", err | ||
| } |