Backport 105046 to 12.0.1 (#105337)
This commit is contained in:
@@ -156,8 +156,8 @@ func (s *UserSync) ValidateUserProvisioningHook(ctx context.Context, id *authn.I
|
||||
|
||||
// Validate the provisioned user.ExternalUID with the authinfo.ExternalUID
|
||||
if usr.IsProvisioned {
|
||||
// The user is provisioned via SAML and the identity is empty, meaning this request is not from the SAML auth flow
|
||||
if authInfo.AuthModule == login.SAMLAuthModule && authInfo.ExternalUID != "" && id.ExternalUID == "" {
|
||||
// Allow non-SAML requests for SAML-provisioned users to proceed if incoming ExternalUID is empty (e.g. session access).
|
||||
if authInfo.AuthModule == login.SAMLAuthModule && id.AuthenticatedBy != login.SAMLAuthModule && authInfo.ExternalUID != "" && id.ExternalUID == "" {
|
||||
log.Debug("Skipping ExternalUID validation for non-SAML request to SAML-provisioned user")
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -439,6 +439,94 @@ func TestUserSync_SyncUserHook(t *testing.T) {
|
||||
},
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "SyncUserHook: Provisioned user, Incoming ExternalUID is empty, DB ExternalUID non-empty - expect errEmptyExternalUID",
|
||||
fields: fields{
|
||||
userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}},
|
||||
authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}},
|
||||
quotaService: "atest.FakeQuotaService{},
|
||||
},
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
id: &authn.Identity{
|
||||
AuthID: "1",
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
ExternalUID: "",
|
||||
ClientParams: authn.ClientParams{SyncUser: true},
|
||||
},
|
||||
},
|
||||
wantErr: true, // Expecting errEmptyExternalUID
|
||||
},
|
||||
{
|
||||
name: "SyncUserHook: Provisioned user, Incoming ExternalUID is empty, DB ExternalUID also empty - expect errEmptyExternalUID",
|
||||
fields: fields{
|
||||
userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}},
|
||||
authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}}, // DB empty
|
||||
quotaService: "atest.FakeQuotaService{},
|
||||
},
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
id: &authn.Identity{
|
||||
AuthID: "1",
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
ExternalUID: "",
|
||||
ClientParams: authn.ClientParams{SyncUser: true},
|
||||
},
|
||||
},
|
||||
wantErr: true, // Expecting errEmptyExternalUID
|
||||
},
|
||||
{
|
||||
name: "SyncUserHook: Provisioned user, Incoming and DB ExternalUIDs non-empty and mismatch - expect errMismatchedExternalUID",
|
||||
fields: fields{
|
||||
userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}},
|
||||
authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}},
|
||||
quotaService: "atest.FakeQuotaService{},
|
||||
},
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
id: &authn.Identity{
|
||||
AuthID: "1",
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
ExternalUID: "incoming-uid",
|
||||
ClientParams: authn.ClientParams{SyncUser: true},
|
||||
},
|
||||
},
|
||||
wantErr: true, // Expecting errMismatchedExternalUID
|
||||
},
|
||||
{
|
||||
name: "SyncUserHook: Provisioned user, Incoming and DB ExternalUIDs non-empty and match - expect success",
|
||||
fields: fields{
|
||||
userService: &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, Login: "user1", Email: "user1@test.com", Name: "User One", IsProvisioned: true}},
|
||||
authInfoService: &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, AuthId: "1", ExternalUID: "matching-uid"}},
|
||||
quotaService: "atest.FakeQuotaService{},
|
||||
},
|
||||
args: args{
|
||||
ctx: context.Background(),
|
||||
id: &authn.Identity{
|
||||
AuthID: "1",
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
Login: "user1",
|
||||
Email: "user1@test.com",
|
||||
Name: "User One",
|
||||
ExternalUID: "matching-uid",
|
||||
ClientParams: authn.ClientParams{SyncUser: true},
|
||||
},
|
||||
},
|
||||
wantErr: false,
|
||||
wantID: &authn.Identity{
|
||||
ID: "1",
|
||||
UID: "",
|
||||
Type: claims.TypeUser,
|
||||
AuthID: "1",
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
Login: "user1",
|
||||
Email: "user1@test.com",
|
||||
Name: "User One",
|
||||
ExternalUID: "matching-uid",
|
||||
IsGrafanaAdmin: ptrBool(false),
|
||||
ClientParams: authn.ClientParams{SyncUser: true},
|
||||
},
|
||||
},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
@@ -812,12 +900,118 @@ func TestUserSync_ValidateUserProvisioningHook(t *testing.T) {
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
AuthenticatedBy: login.GenericOAuthModule,
|
||||
AuthID: "1",
|
||||
ExternalUID: "",
|
||||
},
|
||||
expectedErr: nil,
|
||||
},
|
||||
{
|
||||
desc: "ValidateProvisioning: DB ExternalUID is empty, Incoming ExternalUID is empty - expect mismatch (stricter logic)",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: ""},
|
||||
expectedErr: errUserExternalUIDMismatch,
|
||||
},
|
||||
{
|
||||
desc: "ValidateProvisioning: DB ExternalUID non-empty, Incoming ExternalUID is empty - expect mismatch",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "valid-uid"}}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: ""},
|
||||
expectedErr: errUserExternalUIDMismatch,
|
||||
},
|
||||
{
|
||||
desc: "ValidateProvisioning: DB ExternalUID is empty, Incoming ExternalUID non-empty - expect mismatch (stricter logic)",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: ""}}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: "valid-uid"},
|
||||
expectedErr: errUserExternalUIDMismatch,
|
||||
},
|
||||
{
|
||||
desc: "ValidateProvisioning: DB and Incoming ExternalUIDs non-empty and mismatch - expect mismatch",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{ExpectedUser: &user.User{ID: 1, IsProvisioned: true}}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{ExpectedUserAuth: &login.UserAuth{UserId: 1, AuthModule: login.SAMLAuthModule, ExternalUID: "db-uid"}}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{AuthenticatedBy: login.SAMLAuthModule, AuthID: "1", ExternalUID: "incoming-uid"},
|
||||
expectedErr: errUserExternalUIDMismatch,
|
||||
},
|
||||
{
|
||||
desc: "it should skip ExternalUID validation for a SAML-provisioned user accessed by a non-SAML method with an empty incoming ExternalUID",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.allowNonProvisionedUsers = false
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{
|
||||
ExpectedUser: &user.User{
|
||||
ID: 1,
|
||||
IsProvisioned: true,
|
||||
},
|
||||
}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{
|
||||
ExpectedUserAuth: &login.UserAuth{
|
||||
UserId: 1,
|
||||
AuthModule: login.SAMLAuthModule,
|
||||
AuthId: "1",
|
||||
ExternalUID: "saml-originated-uid",
|
||||
},
|
||||
}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{
|
||||
AuthenticatedBy: login.GenericOAuthModule,
|
||||
AuthID: "1",
|
||||
ExternalUID: "",
|
||||
},
|
||||
expectedErr: nil,
|
||||
},
|
||||
{
|
||||
desc: "it should fail validation when a provisioned user is accessed by SAML with an empty incoming ExternalUID",
|
||||
userSyncServiceSetup: func() *UserSync {
|
||||
userSyncService := initUserSyncService()
|
||||
userSyncService.allowNonProvisionedUsers = false
|
||||
userSyncService.isUserProvisioningEnabled = true
|
||||
userSyncService.userService = &usertest.FakeUserService{
|
||||
ExpectedUser: &user.User{
|
||||
ID: 1,
|
||||
IsProvisioned: true,
|
||||
},
|
||||
}
|
||||
userSyncService.authInfoService = &authinfotest.FakeService{
|
||||
ExpectedUserAuth: &login.UserAuth{
|
||||
UserId: 1,
|
||||
AuthModule: login.SAMLAuthModule,
|
||||
AuthId: "1",
|
||||
ExternalUID: "saml-originated-uid",
|
||||
},
|
||||
}
|
||||
return userSyncService
|
||||
},
|
||||
identity: &authn.Identity{
|
||||
AuthenticatedBy: login.SAMLAuthModule,
|
||||
AuthID: "1",
|
||||
ExternalUID: "",
|
||||
},
|
||||
expectedErr: errUserExternalUIDMismatch.Errorf("the provisioned user.ExternalUID does not match the authinfo.ExternalUID"),
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
Reference in New Issue
Block a user