Skip to content

Commit 5e36b69

Browse files
committed
oidcccl, server: add role synchronisation for DB Console OIDC logins
Previously, the OIDC callback (`/oidc/v1/callback`) authenticated Admin-UI users but did not reconcile their IdP groups with SQL roles. This was inadequate because operators who already rely on automatic GRANT/REVOKE for JWT and LDAP flows had no equivalent mechanism for OIDC log-ins, leading to inconsistent privileges and manual role management. To address this, this patch introduces group-to-role synchronisation for OIDC: * Adds three cluster settings `server.oidc_authentication.authorization.enabled`, `server.oidc_authentication.group_claim`, and `server.oidc_authentication.userinfo_group_key`, mirroring the JWT knobs. * Parses the configured claim from the verified ID-token (array or string), normalises, dedupes and sorts it. * Falls back to the user-info endpoint when the claim is absent and a group key is configured. * Converts groups to validated SQL usernames and invokes `EnsureUserOnlyBelongsToRoles`. * Threads the node’s `*sql.ExecutorConfig` into the OIDC stack. Epic: CRDB-48763 Fixes: CRDB-51161 Release note (security update): CockroachDB can now synchronise SQL role membership from the groups claim provided by an OpenID Connect (OIDC) Identity Provider when `server.oidc_authentication.authorization.enabled = true`. . At login the DB Console gathers the `groups` claim from both the verified ID token and, when available, the access token (if it is a JWT). Any groups found in either token are combined and de-duplicated. If no claim is present in either token, the server will query the provider's `/userinfo` endpoint and extract groups from there (using `server.oidc_authentication.userinfo_group_key`) as a final fallback. . The resulting list of groups is normalised to SQL role names and compared to the user’s current role memberships. Any newly required roles are GRANTed and any stale ones are REVOKEd, matching the behaviour already available for JWT and LDAP-based role synchronisation.
1 parent 8403878 commit 5e36b69

File tree

10 files changed

+1082
-47
lines changed

10 files changed

+1082
-47
lines changed

pkg/ccl/oidcccl/BUILD.bazel

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ go_library(
44
name = "oidcccl",
55
srcs = [
66
"authentication_oidc.go",
7+
"authorization_oidc.go",
78
"claim_match.go",
89
"settings.go",
910
"state.go",
@@ -12,6 +13,7 @@ go_library(
1213
visibility = ["//visibility:public"],
1314
deps = [
1415
"//pkg/ccl/jwtauthccl",
16+
"//pkg/ccl/securityccl/jwthelper",
1517
"//pkg/ccl/utilccl",
1618
"//pkg/roachpb",
1719
"//pkg/security/username",
@@ -21,6 +23,7 @@ go_library(
2123
"//pkg/server/telemetry",
2224
"//pkg/settings",
2325
"//pkg/settings/cluster",
26+
"//pkg/sql",
2427
"//pkg/sql/pgwire",
2528
"//pkg/sql/pgwire/identmap",
2629
"//pkg/ui",
@@ -31,6 +34,7 @@ go_library(
3134
"//pkg/util/uuid",
3235
"@com_github_cockroachdb_errors//:errors",
3336
"@com_github_coreos_go_oidc//:go-oidc",
37+
"@com_github_lestrrat_go_jwx_v2//jwt",
3438
"@org_golang_x_oauth2//:oauth2",
3539
],
3640
)
@@ -40,6 +44,7 @@ go_test(
4044
size = "small",
4145
srcs = [
4246
"authentication_oidc_test.go",
47+
"authorization_oidc_test.go",
4348
"main_test.go",
4449
"settings_test.go",
4550
],
@@ -64,7 +69,11 @@ go_test(
6469
"//pkg/util/leaktest",
6570
"//pkg/util/log",
6671
"//pkg/util/randutil",
72+
"@com_github_cockroachdb_errors//:errors",
6773
"@com_github_coreos_go_oidc//:go-oidc",
74+
"@com_github_lestrrat_go_jwx_v2//jwa",
75+
"@com_github_lestrrat_go_jwx_v2//jwk",
76+
"@com_github_lestrrat_go_jwx_v2//jwt",
6877
"@com_github_stretchr_testify//require",
6978
"@org_golang_x_oauth2//:oauth2",
7079
],

pkg/ccl/oidcccl/authentication_oidc.go

Lines changed: 134 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@ import (
1919
"github.com/cockroachdb/cockroach/pkg/ccl/jwtauthccl"
2020
"github.com/cockroachdb/cockroach/pkg/ccl/utilccl"
2121
"github.com/cockroachdb/cockroach/pkg/roachpb"
22-
"github.com/cockroachdb/cockroach/pkg/security/username"
22+
secuser "github.com/cockroachdb/cockroach/pkg/security/username"
2323
"github.com/cockroachdb/cockroach/pkg/server"
2424
"github.com/cockroachdb/cockroach/pkg/server/authserver"
2525
"github.com/cockroachdb/cockroach/pkg/server/serverpb"
2626
"github.com/cockroachdb/cockroach/pkg/server/telemetry"
2727
"github.com/cockroachdb/cockroach/pkg/settings/cluster"
28+
"github.com/cockroachdb/cockroach/pkg/sql"
2829
"github.com/cockroachdb/cockroach/pkg/sql/pgwire"
2930
"github.com/cockroachdb/cockroach/pkg/sql/pgwire/identmap"
3031
"github.com/cockroachdb/cockroach/pkg/ui"
@@ -34,6 +35,7 @@ import (
3435
"github.com/cockroachdb/cockroach/pkg/util/uuid"
3536
"github.com/cockroachdb/errors"
3637
oidc "github.com/coreos/go-oidc"
38+
"github.com/lestrrat-go/jwx/v2/jwt"
3739
"golang.org/x/oauth2"
3840
)
3941

@@ -131,6 +133,7 @@ type oidcAuthenticationServer struct {
131133
// to help us gracefully recover from auth provider downtime without operator intervention.
132134
enabled bool
133135
initialized bool
136+
execCfg *sql.ExecutorConfig
134137
}
135138

136139
type oidcAuthenticationConf struct {
@@ -152,6 +155,9 @@ type oidcAuthenticationConf struct {
152155
generateJWTAuthTokenSQLPort int64
153156
providerCustomCA string
154157
httpClient *httputil.Client
158+
authZEnabled bool
159+
groupClaim string
160+
userinfoGroupKey string
155161
}
156162

157163
// GetOIDCConf is used to extract certain parts of the OIDC
@@ -187,37 +193,107 @@ type oidcManager struct {
187193
oauth2Config *oauth2.Config
188194
verifier *oidc.IDTokenVerifier
189195
httpClient *httputil.Client
196+
provider *oidc.Provider
190197
}
191198

192-
func (o *oidcManager) ExchangeVerifyGetClaims(
193-
ctx context.Context, code string, idTokenKey string,
194-
) (map[string]json.RawMessage, error) {
199+
// ExchangeVerifyGetTokenInfo exchanges the auth-code, verifies the ID-token,
200+
// and extracts claims from both the ID and Access tokens if they are JWTs.
201+
// Access tokens are only processed if OIDC authorization is enabled for the
202+
// cluster.
203+
func (o *oidcManager) ExchangeVerifyGetTokenInfo(
204+
ctx context.Context, code, idTokenKey string, authZEnabled bool,
205+
) (
206+
idTokenClaims, accessTokenClaims map[string]json.RawMessage,
207+
rawIDToken, rawAccessToken string,
208+
err error,
209+
) {
195210
credentials, err := o.Exchange(ctx, code)
196211
if err != nil {
197-
log.Errorf(ctx, "OIDC: failed to exchange code for token: %v", err)
198-
return nil, err
212+
return nil, nil, "", "", errors.Wrap(err, "OIDC: failed to exchange code for token")
199213
}
200214

201-
rawIDToken, ok := credentials.Extra(idTokenKey).(string)
202-
if !ok {
203-
err := errors.New("OIDC: failed to extract ID token from the token credentials")
204-
log.Error(ctx, "OIDC: failed to extract ID token from the token credentials")
205-
return nil, err
215+
// Build the list of tokens we must handle. For the ID token we verify
216+
// signature and claims up-front; the access token is copied as-is.
217+
// Since we use the OIDC Authorization Code flow (response_type=code),
218+
// The token endpoint response must contain both id_token and access_token
219+
// (refresh_token is optional). Their presence is therefore guaranteed here.
220+
tokensToProcess := []struct {
221+
name string
222+
getVerifiedToken func() (string, error) // fetches the raw token; verifies when needed
223+
claims *map[string]json.RawMessage // destination for parsed claims
224+
rawToken *string // destination for the raw token string
225+
required bool
226+
}{
227+
{
228+
name: idTokenKey,
229+
required: true,
230+
getVerifiedToken: func() (string, error) {
231+
val, _ := credentials.Extra(idTokenKey).(string)
232+
if val == "" {
233+
log.Ops.Warning(ctx, "OIDC: required id_token missing from credentials")
234+
return "", errors.New("OIDC: required id_token missing from credentials")
235+
}
236+
// The ID token must be verified against the provider.
237+
if _, err := o.Verify(ctx, val); err != nil {
238+
return "", err
239+
}
240+
return val, nil
241+
},
242+
claims: &idTokenClaims,
243+
rawToken: &rawIDToken,
244+
},
245+
{
246+
name: "access_token",
247+
required: authZEnabled,
248+
getVerifiedToken: func() (string, error) {
249+
val := credentials.AccessToken
250+
if val == "" {
251+
log.Ops.Warning(ctx, "OIDC: required access_token missing from credentials")
252+
return "", errors.New("OIDC: required access_token missing from credentials")
253+
}
254+
return val, nil
255+
},
256+
claims: &accessTokenClaims,
257+
rawToken: &rawAccessToken,
258+
},
206259
}
207260

208-
idToken, err := o.Verify(ctx, rawIDToken)
209-
if err != nil {
210-
log.Errorf(ctx, "OIDC: unable to verify ID token: %v", err)
211-
return nil, err
212-
}
261+
for _, tokenInfo := range tokensToProcess {
262+
if !tokenInfo.required {
263+
continue
264+
}
265+
var err error
266+
*tokenInfo.rawToken, err = tokenInfo.getVerifiedToken()
267+
if err != nil {
268+
return nil, nil, "", "", errors.Wrapf(err, "OIDC: failed to verify %s", tokenInfo.name)
269+
}
270+
// Attempt to parse the token as a JWT to extract its claims.
271+
// We keep this at INFO(1) because opaque tokens are normal for many IdPs;
272+
// the message is useful for troubleshooting but not an operator-actionable error.
273+
parsedToken, err := jwt.ParseInsecure([]byte(*tokenInfo.rawToken))
274+
if err != nil {
275+
log.VInfof(ctx, 1, "OIDC: could not parse %s as JWT (this is expected for opaque tokens): %v", tokenInfo.name, err)
276+
continue // Not a JWT, so we can't get claims from it.
277+
}
213278

214-
var claims map[string]json.RawMessage
215-
if err := idToken.Claims(&claims); err != nil {
216-
log.Errorf(ctx, "OIDC: unable to deserialize token claims: %v", err)
217-
return nil, err
279+
// Convert the jwt.Token into a map to extract all claims.
280+
claimsMap, err := parsedToken.AsMap(ctx)
281+
if err != nil {
282+
return nil, nil, "", "", errors.Wrapf(err, "OIDC: failed to get claims as map from %s", tokenInfo.name)
283+
}
284+
// The claims map must be marshaled and unmarshaled to convert it to the desired type.
285+
jsonBytes, err := json.Marshal(claimsMap)
286+
if err != nil {
287+
return nil, nil, "", "", errors.Wrapf(err, "OIDC: failed to marshal claims from %s", tokenInfo.name)
288+
}
289+
var claimsData map[string]json.RawMessage
290+
if err := json.Unmarshal(jsonBytes, &claimsData); err != nil {
291+
return nil, nil, "", "", errors.Wrapf(err, "OIDC: failed to deserialize claims from %s", tokenInfo.name)
292+
}
293+
*tokenInfo.claims = claimsData
218294
}
219295

220-
return claims, nil
296+
return idTokenClaims, accessTokenClaims, rawIDToken, rawAccessToken, nil
221297
}
222298

223299
func (o *oidcManager) Verify(ctx context.Context, s string) (*oidc.IDToken, error) {
@@ -247,11 +323,17 @@ func (o oidcManager) AuthCodeURL(s string, option ...oauth2.AuthCodeOption) stri
247323
return o.oauth2Config.AuthCodeURL(s, option...)
248324
}
249325

326+
func (o *oidcManager) UserInfo(ctx context.Context, ts oauth2.TokenSource) (*oidc.UserInfo, error) {
327+
octx := oidc.ClientContext(ctx, o.httpClient.Client)
328+
return o.provider.UserInfo(octx, ts)
329+
}
330+
250331
type IOIDCManager interface {
251332
Verify(context.Context, string) (*oidc.IDToken, error)
252333
Exchange(context.Context, string, ...oauth2.AuthCodeOption) (*oauth2.Token, error)
253334
AuthCodeURL(string, ...oauth2.AuthCodeOption) string
254-
ExchangeVerifyGetClaims(context.Context, string, string) (map[string]json.RawMessage, error)
335+
ExchangeVerifyGetTokenInfo(context.Context, string, string, bool) (idTokenClaims, accessTokenClaims map[string]json.RawMessage, rawIDToken, rawAccessToken string, err error)
336+
UserInfo(context.Context, oauth2.TokenSource) (*oidc.UserInfo, error)
255337
}
256338

257339
var _ IOIDCManager = &oidcManager{}
@@ -291,6 +373,7 @@ var NewOIDCManager func(context.Context, oidcAuthenticationConf, string, []strin
291373
verifier: verifier,
292374
oauth2Config: oauth2Config,
293375
httpClient: conf.httpClient,
376+
provider: provider,
294377
}, nil
295378
}
296379

@@ -337,6 +420,9 @@ func reloadConfigLocked(
337420
httputil.WithDialerTimeout(clientTimeout),
338421
httputil.WithCustomCAPEM(OIDCProviderCustomCA.Get(&st.SV)),
339422
),
423+
authZEnabled: OIDCAuthZEnabled.Get(&st.SV),
424+
groupClaim: OIDCAuthGroupClaim.Get(&st.SV),
425+
userinfoGroupKey: OIDCAuthUserinfoGroupKey.Get(&st.SV),
340426
}
341427

342428
if !oidcAuthServer.conf.enabled && conf.enabled {
@@ -421,8 +507,11 @@ var ConfigureOIDC = func(
421507
userLoginFromSSO func(ctx context.Context, username string) (*http.Cookie, error),
422508
ambientCtx log.AmbientContext,
423509
cluster uuid.UUID,
510+
execCfg *sql.ExecutorConfig,
424511
) (authserver.OIDC, error) {
425-
oidcAuthentication := &oidcAuthenticationServer{}
512+
oidcAuthentication := &oidcAuthenticationServer{
513+
execCfg: execCfg,
514+
}
426515

427516
// Don't want to use GRPC here since these endpoints require HTTP-Redirect behaviors and the
428517
// callback endpoint will be receiving specialized parameters that grpc-gateway will only get
@@ -493,8 +582,11 @@ var ConfigureOIDC = func(
493582
return
494583
}
495584

496-
claims, err := oidcAuthentication.manager.ExchangeVerifyGetClaims(ctx, r.URL.Query().Get(codeKey), idTokenKey)
585+
idTokenClaims, _, rawIDToken, rawAccessToken, err := oidcAuthentication.manager.
586+
ExchangeVerifyGetTokenInfo(ctx, r.URL.Query().Get(codeKey), idTokenKey, oidcAuthentication.conf.authZEnabled)
587+
497588
if err != nil {
589+
log.Errorf(ctx, "OIDC: failed to get and verify token: %v", err)
498590
http.Error(w, genericCallbackHTTPError, http.StatusInternalServerError)
499591
return
500592
}
@@ -509,13 +601,20 @@ var ConfigureOIDC = func(
509601
}
510602

511603
username, err := extractUsernameFromClaims(
512-
ctx, claims, oidcAuthentication.conf.claimJSONKey, oidcAuthentication.conf.principalRegex,
604+
ctx, idTokenClaims, oidcAuthentication.conf.claimJSONKey, oidcAuthentication.conf.principalRegex,
513605
)
514606
if err != nil {
515607
http.Error(w, genericCallbackHTTPError, http.StatusInternalServerError)
516608
return
517609
}
518610

611+
// OIDC authorization
612+
if err := oidcAuthentication.authorize(ctx, rawIDToken, rawAccessToken, username); err != nil {
613+
log.Errorf(ctx, "OIDC authorization failed with error: %v", err)
614+
http.Error(w, genericCallbackHTTPError, http.StatusForbidden)
615+
return
616+
}
617+
519618
cookie, err := userLoginFromSSO(ctx, username)
520619
if err != nil {
521620
log.Errorf(ctx, "OIDC: failed to complete authentication: unable to create session for %s: %v", username, err)
@@ -705,7 +804,7 @@ var ConfigureOIDC = func(
705804
}
706805
} else {
707806
log.Infof(ctx, "OIDC: no identity map found for issuer %s; using %s without mapping", token.Issuer, tokenPrincipal)
708-
if username, err := username.MakeSQLUsernameFromUserInput(tokenPrincipal, username.PurposeValidation); err != nil {
807+
if username, err := secuser.MakeSQLUsernameFromUserInput(tokenPrincipal, secuser.PurposeValidation); err != nil {
709808
acceptedUsernames = append(acceptedUsernames, username.Normalized())
710809
}
711810
}
@@ -829,6 +928,15 @@ var ConfigureOIDC = func(
829928
OIDCAuthClientTimeout.SetOnChange(&st.SV, func(ctx context.Context) {
830929
reloadConfig(ambientCtx.AnnotateCtx(ctx), oidcAuthentication, locality, st)
831930
})
931+
OIDCAuthZEnabled.SetOnChange(&st.SV, func(ctx context.Context) {
932+
reloadConfig(ambientCtx.AnnotateCtx(ctx), oidcAuthentication, locality, st)
933+
})
934+
OIDCAuthGroupClaim.SetOnChange(&st.SV, func(ctx context.Context) {
935+
reloadConfig(ambientCtx.AnnotateCtx(ctx), oidcAuthentication, locality, st)
936+
})
937+
OIDCAuthUserinfoGroupKey.SetOnChange(&st.SV, func(ctx context.Context) {
938+
reloadConfig(ambientCtx.AnnotateCtx(ctx), oidcAuthentication, locality, st)
939+
})
832940

833941
return oidcAuthentication, nil
834942
}

pkg/ccl/oidcccl/authentication_oidc_test.go

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,13 +87,21 @@ func (m mockOidcManager) AuthCodeURL(s string, option ...oauth2.AuthCodeOption)
8787
return m.oauth2Config.AuthCodeURL(s, option...)
8888
}
8989

90-
func (m mockOidcManager) ExchangeVerifyGetClaims(
91-
ctx context.Context, s string, s2 string,
92-
) (map[string]json.RawMessage, error) {
93-
x := map[string]json.RawMessage{}
94-
// The email is surrounded by double quotes because it's a serialized JSON string.
95-
x["email"] = json.RawMessage([]byte(fmt.Sprintf(`"%s"`, m.claimEmail)))
96-
return x, nil
90+
func (m mockOidcManager) ExchangeVerifyGetTokenInfo(
91+
ctx context.Context, code, idTokenKey string, _ bool,
92+
) (map[string]json.RawMessage, map[string]json.RawMessage, string, string, error) {
93+
claims := map[string]json.RawMessage{
94+
"email": json.RawMessage(`"[email protected]"`),
95+
}
96+
// Return nil for access token claims, and the raw token strings.
97+
return claims, nil, "dummy-id-token", "dummy-access-token", nil
98+
}
99+
100+
func (m mockOidcManager) UserInfo(
101+
ctx context.Context, ts oauth2.TokenSource,
102+
) (*oidc.UserInfo, error) {
103+
// This mock is not used in this test file, so it can return nil.
104+
return nil, nil
97105
}
98106

99107
var _ IOIDCManager = &mockOidcManager{}

0 commit comments

Comments
 (0)