Skip to content

Commit 9be69f8

Browse files
craig[bot]shriramters
andcommitted
Merge #150721
150721: pgwire: decouple jwt authentication and authorization logic r=souravcrl a=shriramters Previously, the logic for JWT authorization (extracting group claims and synchronizing roles) was located within the `Authenticator` behavior. This was a necessary design when the JWT token was only exchanged from the `AuthConn` within the authenticator. This was inadequate because it bundled authentication and authorization logic, violating the separation of concerns intended by the `AuthBehaviors` framework and creating an inconsistency with other methods like `AuthLDAP`. A recent change (#149415) made the token available via closure capture to all behaviors, removing the original constraint and making this refactoring possible. To address this, this patch decouples the logic. The `Authenticator` is now solely responsible for validating the token (authentication). The authorization logic has been moved to a new `Authorizer` behavior, which aligns the implementation with the framework's design and improves code clarity and maintainability. Fixes: #150720 Release note: None Co-authored-by: Shriram Ravindranathan <[email protected]>
2 parents 5898b5d + bd5d634 commit 9be69f8

File tree

2 files changed

+32
-3
lines changed

2 files changed

+32
-3
lines changed

pkg/security/provisioning/settings.go

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,18 +24,28 @@ const (
2424

2525
baseCounterPrefix = "auth.provisioning."
2626
ldapCounterPrefix = baseCounterPrefix + "ldap."
27+
jwtCounterPrefix = baseCounterPrefix + "jwt."
2728

2829
beginLDAPProvisionCounterName = ldapCounterPrefix + "begin"
2930
provisionLDAPSuccessCounterName = ldapCounterPrefix + "success"
3031
enableLDAPProvisionCounterName = ldapCounterPrefix + "enable"
3132

33+
beginJWTProvisionCounterName = jwtCounterPrefix + "begin"
34+
provisionJWTSuccessCounterName = jwtCounterPrefix + "success"
35+
enableJWTProvisionCounterName = jwtCounterPrefix + "enable"
36+
3237
provisionedUserLoginSuccessCounterName = baseCounterPrefix + "login_success"
3338
)
3439

3540
var (
36-
BeginLDAPProvisionUseCounter = telemetry.GetCounterOnce(beginLDAPProvisionCounterName)
37-
ProvisionLDAPSuccessCounter = telemetry.GetCounterOnce(provisionLDAPSuccessCounterName)
38-
enableLDAPProvisionCounter = telemetry.GetCounterOnce(enableLDAPProvisionCounterName)
41+
BeginLDAPProvisionUseCounter = telemetry.GetCounterOnce(beginLDAPProvisionCounterName)
42+
ProvisionLDAPSuccessCounter = telemetry.GetCounterOnce(provisionLDAPSuccessCounterName)
43+
enableLDAPProvisionCounter = telemetry.GetCounterOnce(enableLDAPProvisionCounterName)
44+
45+
BeginJWTProvisionUseCounter = telemetry.GetCounterOnce(beginJWTProvisionCounterName)
46+
ProvisionJWTSuccessCounter = telemetry.GetCounterOnce(provisionJWTSuccessCounterName)
47+
enableJWTProvisionCounter = telemetry.GetCounterOnce(enableJWTProvisionCounterName)
48+
3949
ProvisionedUserLoginSuccessCounter = telemetry.GetCounterOnce(provisionedUserLoginSuccessCounterName)
4050
)
4151

@@ -99,5 +109,10 @@ func ClusterProvisioningConfig(settings *cluster.Settings) UserProvisioningConfi
99109
telemetry.Inc(enableLDAPProvisionCounter)
100110
}
101111
})
112+
jwtProvisioningEnabled.SetOnChange(&settings.SV, func(_ context.Context) {
113+
if jwtProvisioningEnabled.Get(&settings.SV) {
114+
telemetry.Inc(enableJWTProvisionCounter)
115+
}
116+
})
102117
return clusterProvisioningConfig{settings: settings}
103118
}

pkg/sql/pgwire/auth_methods.go

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,6 +860,9 @@ func authJwtToken(
860860
}
861861

862862
b.SetProvisioner(func(ctx context.Context) error {
863+
c.LogAuthInfof(ctx, "Starting JWT provisioning; attempting to verify token")
864+
telemetry.Inc(provisioning.BeginJWTProvisionUseCounter)
865+
863866
if validationErr != nil {
864867
return validationErr
865868
}
@@ -893,6 +896,8 @@ func authJwtToken(
893896
c.LogAuthFailed(ctx, eventpb.AuthFailReason_PROVISIONING_ERROR, err)
894897
return err
895898
}
899+
900+
telemetry.Inc(provisioning.ProvisionJWTSuccessCounter)
896901
return nil
897902
})
898903

@@ -905,6 +910,8 @@ func authJwtToken(
905910
return err
906911
}
907912

913+
c.LogAuthInfof(ctx, "JWT Provided; attempting to validate token for authentication")
914+
908915
// Validate the token and, if there's an error, log it with the correct reason.
909916
if detailedErrors, authError := jwtVerifier.ValidateJWTLogin(
910917
sctx, execCfg.Settings, user, []byte(token), identMap); authError != nil {
@@ -916,6 +923,12 @@ func authJwtToken(
916923
return authError
917924
}
918925

926+
return nil
927+
})
928+
929+
b.SetAuthorizer(func(ctx context.Context, systemIdentity string, clientConnection bool) error {
930+
c.LogAuthInfof(ctx, "JWT authentication succeeded; attempting authorization")
931+
919932
// Ask the CCL verifier for groups (nil slice means feature disabled).
920933
groups, err := jwtVerifier.ExtractGroups(ctx, execCfg.Settings, []byte(token))
921934
if err != nil {
@@ -959,6 +972,7 @@ func authJwtToken(
959972

960973
return nil
961974
})
975+
962976
return b, nil
963977
}
964978

0 commit comments

Comments
 (0)