SCIM: show error if SCIM-provisioned user attempts login with non-SAML auth module (#115271)

2025-12-12 13:51:37 -06:00
parent 4817ecf6a3
commit 5156177079
2 changed files with 116 additions and 0 deletions
@@ -77,6 +77,10 @@ var (
 		"user.sync.user-externalUID-mismatch",
 		errutil.WithPublicMessage("User externalUID mismatch"),
 	)
+	errSCIMAuthModuleMismatch = errutil.Unauthorized(
+		"user.sync.scim-auth-module-mismatch",
+		errutil.WithPublicMessage("User was provisioned via SCIM and must login via SAML"),
+	)
 )

 var (
@@ -308,6 +312,21 @@ func (s *UserSync) SyncUserHook(ctx context.Context, id *authn.Identity, _ *auth
 		// just try to fetch the user one more to make the other request work.
 		if errors.Is(err, user.ErrUserAlreadyExists) {
 			usr, _, err = s.getUser(ctx, id)
+
+			// Check if this is a SCIM-provisioned user trying to login via an auth module that is not SAML or GCOM
+			if err == nil && usr != nil && usr.IsProvisioned && id.AuthenticatedBy != login.GrafanaComAuthModule {
+				_, authErr := s.authInfoService.GetAuthInfo(ctx, &login.GetAuthInfoQuery{
+					UserId:     usr.ID,
+					AuthModule: id.AuthenticatedBy,
+				})
+				if errors.Is(authErr, user.ErrUserNotFound) {
+					s.log.FromContext(ctx).Error("SCIM-provisioned user attempted login via non-SAML auth module",
+						"user_id", usr.ID,
+						"attempted_module", id.AuthenticatedBy,
+					)
+					return errSCIMAuthModuleMismatch.Errorf("user was provisioned via SCIM but attempted login via %s", id.AuthenticatedBy)
+				}
+			}
 		}

 		if err != nil {
@@ -1926,3 +1926,100 @@ func TestUserSync_SCIMLoginUsageStatSet(t *testing.T) {
 	finalCount := finalStats["stats.features.scim.has_successful_login.count"].(int)
 	require.Equal(t, int(1), finalCount)
 }
+
+func TestUserSync_SyncUserHook_SCIMAuthModuleMismatch(t *testing.T) {
+	userSrv := usertest.NewMockService(t)
+	authInfoSrv := authinfotest.NewMockAuthInfoService(t)
+
+	userSrv.On("GetByEmail", mock.Anything, mock.Anything).Return(nil, user.ErrUserNotFound).Once()
+
+	userSrv.On("Create", mock.Anything, mock.Anything).Return(nil, user.ErrUserAlreadyExists).Once()
+
+	userSrv.On("GetByEmail", mock.Anything, mock.Anything).Return(&user.User{
+		ID:            1,
+		Email:         "test@test.com",
+		IsProvisioned: true,
+	}, nil).Once()
+
+	authInfoSrv.On("GetAuthInfo", mock.Anything, mock.MatchedBy(func(q *login.GetAuthInfoQuery) bool {
+		return q.AuthModule == "oauth_azuread"
+	})).Return(nil, user.ErrUserNotFound).Once()
+
+	s := ProvideUserSync(
+		userSrv,
+		authinfoimpl.ProvideOSSUserProtectionService(),
+		authInfoSrv,
+		&quotatest.FakeQuotaService{},
+		tracing.NewNoopTracerService(),
+		featuremgmt.WithFeatures(),
+		setting.NewCfg(),
+		nil,
+	)
+
+	email := "test@test.com"
+
+	err := s.SyncUserHook(context.Background(), &authn.Identity{
+		AuthenticatedBy: "oauth_azuread",
+		ClientParams: authn.ClientParams{
+			SyncUser:    true,
+			AllowSignUp: true,
+			LookUpParams: login.UserLookupParams{
+				Email: &email,
+			},
+		},
+	}, nil)
+
+	require.Error(t, err)
+	assert.ErrorIs(t, err, errSCIMAuthModuleMismatch)
+	assert.Contains(t, err.Error(), "SCIM")
+	assert.Contains(t, err.Error(), "oauth_azuread")
+}
+
+func TestUserSync_SyncUserHook_SCIMUserAllowsGCOMLogin(t *testing.T) {
+	userSrv := usertest.NewMockService(t)
+	authInfoSrv := authinfotest.NewMockAuthInfoService(t)
+
+	authInfoSrv.On("GetAuthInfo", mock.Anything, mock.MatchedBy(func(q *login.GetAuthInfoQuery) bool {
+		return q.AuthModule == login.GrafanaComAuthModule && q.AuthId == "gcom-user-123"
+	})).Return(nil, user.ErrUserNotFound).Once()
+
+	userSrv.On("GetByEmail", mock.Anything, mock.Anything).Return(nil, user.ErrUserNotFound).Once()
+	userSrv.On("Create", mock.Anything, mock.Anything).Return(nil, user.ErrUserAlreadyExists).Once()
+
+	authInfoSrv.On("GetAuthInfo", mock.Anything, mock.MatchedBy(func(q *login.GetAuthInfoQuery) bool {
+		return q.AuthModule == login.GrafanaComAuthModule && q.AuthId == "gcom-user-123"
+	})).Return(nil, user.ErrUserNotFound).Once()
+
+	userSrv.On("GetByEmail", mock.Anything, mock.Anything).Return(&user.User{
+		ID:            1,
+		Email:         "test@test.com",
+		IsProvisioned: true,
+	}, nil).Once()
+
+	s := ProvideUserSync(
+		userSrv,
+		authinfoimpl.ProvideOSSUserProtectionService(),
+		authInfoSrv,
+		&quotatest.FakeQuotaService{},
+		tracing.NewNoopTracerService(),
+		featuremgmt.WithFeatures(),
+		setting.NewCfg(),
+		nil,
+	)
+
+	email := "test@test.com"
+
+	err := s.SyncUserHook(context.Background(), &authn.Identity{
+		AuthenticatedBy: login.GrafanaComAuthModule,
+		AuthID:          "gcom-user-123",
+		ClientParams: authn.ClientParams{
+			SyncUser:    true,
+			AllowSignUp: true,
+			LookUpParams: login.UserLookupParams{
+				Email: &email,
+			},
+		},
+	}, nil)
+
+	require.NoError(t, err)
+}