diff --git a/pkg/models/user_auth.go b/pkg/models/user_auth.go index bebdfdccf61..7b2697778dc 100644 --- a/pkg/models/user_auth.go +++ b/pkg/models/user_auth.go @@ -37,6 +37,7 @@ type ExternalUserInfo struct { OrgRoles map[int64]org.RoleType IsGrafanaAdmin *bool // This is a pointer to know if we should sync this or not (nil = ignore sync) IsDisabled bool + SkipTeamSync bool } func (e *ExternalUserInfo) String() string { diff --git a/pkg/services/authn/clients/jwt.go b/pkg/services/authn/clients/jwt.go index 79259943af1..8ad47e3105e 100644 --- a/pkg/services/authn/clients/jwt.go +++ b/pkg/services/authn/clients/jwt.go @@ -69,8 +69,9 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi AuthID: sub, OrgRoles: map[int64]org.RoleType{}, ClientParams: authn.ClientParams{ - SyncUser: true, - SyncTeamMembers: true, + SyncUser: true, + // We do not allow team member sync from JWT Authentication + SyncTeamMembers: false, AllowSignUp: s.cfg.JWTAuthAutoSignUp, EnableDisabledUsers: false, }} diff --git a/pkg/services/authn/clients/jwt_test.go b/pkg/services/authn/clients/jwt_test.go index 80df9289092..d95876fd4f6 100644 --- a/pkg/services/authn/clients/jwt_test.go +++ b/pkg/services/authn/clients/jwt_test.go @@ -49,9 +49,9 @@ func TestAuthenticateJWT(t *testing.T) { IsDisabled: false, HelpFlags1: 0, ClientParams: authn.ClientParams{ + SyncTeamMembers: false, SyncUser: true, AllowSignUp: true, - SyncTeamMembers: true, LookUpParams: models.UserLookupParams{ UserID: nil, Email: stringPtr("eai.doe@cor.po"), diff --git a/pkg/services/contexthandler/auth_jwt.go b/pkg/services/contexthandler/auth_jwt.go index 68558418a4d..24f7a2c0a22 100644 --- a/pkg/services/contexthandler/auth_jwt.go +++ b/pkg/services/contexthandler/auth_jwt.go @@ -61,10 +61,13 @@ func (h *ContextHandler) initContextWithJWT(ctx *contextmodel.ReqContext, orgId ctx.JsonApiErr(http.StatusUnauthorized, InvalidJWT, err) return true } + extUser := &models.ExternalUserInfo{ AuthModule: "jwt", AuthId: sub, OrgRoles: map[int64]org.RoleType{}, + // we do not want to sync team memberships from JWT authentication see - https://github.com/grafana/grafana/issues/62175 + SkipTeamSync: true, } if key := h.Cfg.JWTAuthUsernameClaim; key != "" { diff --git a/pkg/services/login/loginservice/loginservice.go b/pkg/services/login/loginservice/loginservice.go index 37f38af1373..89cc5351dae 100644 --- a/pkg/services/login/loginservice/loginservice.go +++ b/pkg/services/login/loginservice/loginservice.go @@ -154,7 +154,8 @@ func (ls *Implementation) UpsertUser(ctx context.Context, cmd *models.UpsertUser } } - if ls.TeamSync != nil { + // There are external providers where we want to completely skip team synchronization see - https://github.com/grafana/grafana/issues/62175 + if ls.TeamSync != nil && !extUser.SkipTeamSync { if errTeamSync := ls.TeamSync(cmd.Result, extUser); errTeamSync != nil { return errTeamSync } diff --git a/pkg/services/login/loginservice/loginservice_test.go b/pkg/services/login/loginservice/loginservice_test.go index 0798d744045..c7d5a4762bc 100644 --- a/pkg/services/login/loginservice/loginservice_test.go +++ b/pkg/services/login/loginservice/loginservice_test.go @@ -71,7 +71,7 @@ func Test_teamSync(t *testing.T) { } email := "test_user@example.org" - upserCmd := &models.UpsertUserCommand{ExternalUser: &models.ExternalUserInfo{Email: email}, + upsertCmd := &models.UpsertUserCommand{ExternalUser: &models.ExternalUserInfo{Email: email}, UserLookupParams: models.UserLookupParams{Email: &email}} expectedUser := &user.User{ ID: 1, @@ -84,8 +84,8 @@ func Test_teamSync(t *testing.T) { var actualUser *user.User var actualExternalUser *models.ExternalUserInfo - t.Run("login.TeamSync should not be called when nil", func(t *testing.T) { - err := login.UpsertUser(context.Background(), upserCmd) + t.Run("login.TeamSync should not be called when nil", func(t *testing.T) { + err := login.UpsertUser(context.Background(), upsertCmd) require.Nil(t, err) assert.Nil(t, actualUser) assert.Nil(t, actualExternalUser) @@ -97,10 +97,33 @@ func Test_teamSync(t *testing.T) { return nil } login.TeamSync = teamSyncFunc - err := login.UpsertUser(context.Background(), upserCmd) + err := login.UpsertUser(context.Background(), upsertCmd) require.Nil(t, err) assert.Equal(t, actualUser, expectedUser) - assert.Equal(t, actualExternalUser, upserCmd.ExternalUser) + assert.Equal(t, actualExternalUser, upsertCmd.ExternalUser) + }) + + t.Run("login.TeamSync should not be called when not nil and skipTeamSync is set for externalUserInfo", func(t *testing.T) { + var actualUser *user.User + var actualExternalUser *models.ExternalUserInfo + upsertCmdSkipTeamSync := &models.UpsertUserCommand{ + ExternalUser: &models.ExternalUserInfo{ + Email: email, + // sending in ExternalUserInfo with SkipTeamSync yields no team sync + SkipTeamSync: true, + }, + UserLookupParams: models.UserLookupParams{Email: &email}, + } + teamSyncFunc := func(user *user.User, externalUser *models.ExternalUserInfo) error { + actualUser = user + actualExternalUser = externalUser + return nil + } + login.TeamSync = teamSyncFunc + err := login.UpsertUser(context.Background(), upsertCmdSkipTeamSync) + require.Nil(t, err) + assert.Nil(t, actualUser) + assert.Nil(t, actualExternalUser) }) t.Run("login.TeamSync should propagate its errors to the caller", func(t *testing.T) { @@ -108,7 +131,7 @@ func Test_teamSync(t *testing.T) { return errors.New("teamsync test error") } login.TeamSync = teamSyncFunc - err := login.UpsertUser(context.Background(), upserCmd) + err := login.UpsertUser(context.Background(), upsertCmd) require.Error(t, err) }) })