-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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: #97301, #97468 Release note (enterprise change): The cluster setting `server.oidc_authentication.claim_json_key` for DB Console SSO now accepts list-valued token claims. Release note (general change): Increasing the logging verbosity is more helpful with troubleshooting DB Console SSO issues.
- Loading branch information
1 parent
8cac476
commit 496d069
Showing
4 changed files
with
183 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,97 @@ | ||
| // Copyright 2023 The Cockroach Authors. | ||
| // | ||
| // Licensed as a CockroachDB Enterprise file under the Cockroach Community | ||
| // License (the "License"); you may not use this file except in compliance with | ||
| // the License. You may obtain a copy of the License at | ||
| // | ||
| // https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt | ||
|
|
||
| package oidcccl | ||
|
|
||
| import ( | ||
| "context" | ||
| "encoding/json" | ||
| "regexp" | ||
| "strings" | ||
|
|
||
| "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) { | ||
| var ( | ||
| principal string | ||
| principals []string | ||
| ) | ||
|
|
||
| claimKeys := make([]string, len(claims)) | ||
| i := 0 | ||
| for k := range claims { | ||
| claimKeys[i] = k | ||
| i++ | ||
| } | ||
|
|
||
| targetClaim, ok := claims[claimKey] | ||
| if !ok { | ||
| log.Errorf( | ||
| ctx, "OIDC: failed to complete authentication: invalid JSON claim key: %s", claimKey, | ||
| ) | ||
| log.Infof(ctx, "token payload includes the following claims: %s", strings.Join(claimKeys, ", ")) | ||
| } | ||
|
|
||
| if err := json.Unmarshal(targetClaim, &principal); err != nil { | ||
| // Try parsing assuming the claim value is a list and not a string. | ||
| if log.V(1) { | ||
| log.Infof(ctx, | ||
| "failed parsing claim as string; attempting to parse as a list", | ||
| ) | ||
| } | ||
| 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 | ||
| } | ||
| if log.V(1) { | ||
| log.Infof(ctx, | ||
| "multiple principals in the claim found; selecting first matching principal", | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| if len(principals) == 0 { | ||
| principals = []string{principal} | ||
| } | ||
|
|
||
| var match []string | ||
| for _, principal := range principals { | ||
| match = principalRE.FindStringSubmatch(principal) | ||
| if len(match) == 2 { | ||
| log.Infof(ctx, | ||
| "extracted SQL username %s from the target claim %s", match[1], claimKey, | ||
| ) | ||
| 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, | ||
| "token payload includes the following claims: %s\n"+ | ||
| "check OIDC cluster settings: %s, %s", | ||
| strings.Join(claimKeys, ", "), | ||
| OIDCClaimJSONKeySettingName, OIDCPrincipalRegexSettingName, | ||
| ) | ||
| } | ||
| return "", err | ||
| } |