mirror of https://github.com/grafana/grafana.git
				
				
				
			Auth: Add disable of team sync for JWT Authentication (#62191)
* fix: disable team sync for JWT Authentication * add: comment to test * change test to conform to new expected behavior * fix: spelling * formatting
This commit is contained in:
		
							parent
							
								
									df2db3bb23
								
							
						
					
					
						commit
						5531e22f46
					
				|  | @ -37,6 +37,7 @@ type ExternalUserInfo struct { | ||||||
| 	OrgRoles       map[int64]org.RoleType | 	OrgRoles       map[int64]org.RoleType | ||||||
| 	IsGrafanaAdmin *bool // This is a pointer to know if we should sync this or not (nil = ignore sync)
 | 	IsGrafanaAdmin *bool // This is a pointer to know if we should sync this or not (nil = ignore sync)
 | ||||||
| 	IsDisabled     bool | 	IsDisabled     bool | ||||||
|  | 	SkipTeamSync   bool | ||||||
| } | } | ||||||
| 
 | 
 | ||||||
| func (e *ExternalUserInfo) String() string { | func (e *ExternalUserInfo) String() string { | ||||||
|  |  | ||||||
|  | @ -70,7 +70,8 @@ func (s *JWT) Authenticate(ctx context.Context, r *authn.Request) (*authn.Identi | ||||||
| 		OrgRoles:   map[int64]org.RoleType{}, | 		OrgRoles:   map[int64]org.RoleType{}, | ||||||
| 		ClientParams: authn.ClientParams{ | 		ClientParams: authn.ClientParams{ | ||||||
| 			SyncUser: true, | 			SyncUser: true, | ||||||
| 			SyncTeamMembers:     true, | 			// We do not allow team member sync from JWT Authentication
 | ||||||
|  | 			SyncTeamMembers:     false, | ||||||
| 			AllowSignUp:         s.cfg.JWTAuthAutoSignUp, | 			AllowSignUp:         s.cfg.JWTAuthAutoSignUp, | ||||||
| 			EnableDisabledUsers: false, | 			EnableDisabledUsers: false, | ||||||
| 		}} | 		}} | ||||||
|  |  | ||||||
|  | @ -49,9 +49,9 @@ func TestAuthenticateJWT(t *testing.T) { | ||||||
| 		IsDisabled:     false, | 		IsDisabled:     false, | ||||||
| 		HelpFlags1:     0, | 		HelpFlags1:     0, | ||||||
| 		ClientParams: authn.ClientParams{ | 		ClientParams: authn.ClientParams{ | ||||||
|  | 			SyncTeamMembers: false, | ||||||
| 			SyncUser:        true, | 			SyncUser:        true, | ||||||
| 			AllowSignUp:     true, | 			AllowSignUp:     true, | ||||||
| 			SyncTeamMembers: true, |  | ||||||
| 			LookUpParams: models.UserLookupParams{ | 			LookUpParams: models.UserLookupParams{ | ||||||
| 				UserID: nil, | 				UserID: nil, | ||||||
| 				Email:  stringPtr("eai.doe@cor.po"), | 				Email:  stringPtr("eai.doe@cor.po"), | ||||||
|  |  | ||||||
|  | @ -61,10 +61,13 @@ func (h *ContextHandler) initContextWithJWT(ctx *contextmodel.ReqContext, orgId | ||||||
| 		ctx.JsonApiErr(http.StatusUnauthorized, InvalidJWT, err) | 		ctx.JsonApiErr(http.StatusUnauthorized, InvalidJWT, err) | ||||||
| 		return true | 		return true | ||||||
| 	} | 	} | ||||||
|  | 
 | ||||||
| 	extUser := &models.ExternalUserInfo{ | 	extUser := &models.ExternalUserInfo{ | ||||||
| 		AuthModule: "jwt", | 		AuthModule: "jwt", | ||||||
| 		AuthId:     sub, | 		AuthId:     sub, | ||||||
| 		OrgRoles:   map[int64]org.RoleType{}, | 		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 != "" { | 	if key := h.Cfg.JWTAuthUsernameClaim; key != "" { | ||||||
|  |  | ||||||
|  | @ -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 { | 		if errTeamSync := ls.TeamSync(cmd.Result, extUser); errTeamSync != nil { | ||||||
| 			return errTeamSync | 			return errTeamSync | ||||||
| 		} | 		} | ||||||
|  |  | ||||||
|  | @ -71,7 +71,7 @@ func Test_teamSync(t *testing.T) { | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	email := "test_user@example.org" | 	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}} | 		UserLookupParams: models.UserLookupParams{Email: &email}} | ||||||
| 	expectedUser := &user.User{ | 	expectedUser := &user.User{ | ||||||
| 		ID:    1, | 		ID:    1, | ||||||
|  | @ -85,7 +85,7 @@ func Test_teamSync(t *testing.T) { | ||||||
| 	var actualExternalUser *models.ExternalUserInfo | 	var actualExternalUser *models.ExternalUserInfo | ||||||
| 
 | 
 | ||||||
| 	t.Run("login.TeamSync should not be called when nil", func(t *testing.T) { | 	t.Run("login.TeamSync should not be called when nil", func(t *testing.T) { | ||||||
| 		err := login.UpsertUser(context.Background(), upserCmd) | 		err := login.UpsertUser(context.Background(), upsertCmd) | ||||||
| 		require.Nil(t, err) | 		require.Nil(t, err) | ||||||
| 		assert.Nil(t, actualUser) | 		assert.Nil(t, actualUser) | ||||||
| 		assert.Nil(t, actualExternalUser) | 		assert.Nil(t, actualExternalUser) | ||||||
|  | @ -97,10 +97,33 @@ func Test_teamSync(t *testing.T) { | ||||||
| 				return nil | 				return nil | ||||||
| 			} | 			} | ||||||
| 			login.TeamSync = teamSyncFunc | 			login.TeamSync = teamSyncFunc | ||||||
| 			err := login.UpsertUser(context.Background(), upserCmd) | 			err := login.UpsertUser(context.Background(), upsertCmd) | ||||||
| 			require.Nil(t, err) | 			require.Nil(t, err) | ||||||
| 			assert.Equal(t, actualUser, expectedUser) | 			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) { | 		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") | 				return errors.New("teamsync test error") | ||||||
| 			} | 			} | ||||||
| 			login.TeamSync = teamSyncFunc | 			login.TeamSync = teamSyncFunc | ||||||
| 			err := login.UpsertUser(context.Background(), upserCmd) | 			err := login.UpsertUser(context.Background(), upsertCmd) | ||||||
| 			require.Error(t, err) | 			require.Error(t, err) | ||||||
| 		}) | 		}) | ||||||
| 	}) | 	}) | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue