diff --git a/packages/grafana-data/src/types/config.ts b/packages/grafana-data/src/types/config.ts index e1c8c8eaa40..4c9d74d049f 100644 --- a/packages/grafana-data/src/types/config.ts +++ b/packages/grafana-data/src/types/config.ts @@ -235,16 +235,27 @@ export interface SqlConnectionLimits { } export interface AuthSettings { + AuthProxyEnableLoginToken?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 OAuthSkipOrgRoleUpdateSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 SAMLSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 LDAPSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 JWTAuthSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 GrafanaComSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 GithubSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 GitLabSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 OktaSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 AzureADSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 GoogleSkipOrgRoleSync?: boolean; + // @deprecated -- this is no longer used and will be removed in Grafana 11 GenericOAuthSkipOrgRoleSync?: boolean; - AuthProxyEnableLoginToken?: boolean; } diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index 65b231323bc..416fd568fac 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -163,11 +163,8 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *contextmodel.ReqContext) res getAuthQuery := login.GetAuthInfoQuery{UserId: userID} if authInfo, err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &getAuthQuery); err == nil && authInfo != nil { - oAuthAndAllowAssignGrafanaAdmin := false - if oauthInfo := hs.SocialService.GetOAuthInfoProvider(strings.TrimPrefix(authInfo.AuthModule, "oauth_")); oauthInfo != nil { - oAuthAndAllowAssignGrafanaAdmin = oauthInfo.AllowAssignGrafanaAdmin - } - if login.IsGrafanaAdminExternallySynced(hs.Cfg, authInfo.AuthModule, oAuthAndAllowAssignGrafanaAdmin) { + oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule) + if login.IsGrafanaAdminExternallySynced(hs.Cfg, oauthInfo, authInfo.AuthModule) { return response.Error(http.StatusForbidden, "Cannot change Grafana Admin role for externally synced user", nil) } } diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index 3ec408ad1de..c449854c322 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -318,9 +318,7 @@ func Test_AdminUpdateUserPermissions(t *testing.T) { switch tc.authModule { case login.GenericOAuthModule: - socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled} - cfg.GenericOAuthAuthEnabled = tc.authEnabled - cfg.GenericOAuthSkipOrgRoleSync = tc.skipOrgRoleSync + socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled, SkipOrgRoleSync: tc.skipOrgRoleSync} case login.JWTModule: cfg.JWTAuthEnabled = tc.authEnabled cfg.JWTAuthSkipOrgRoleSync = tc.skipOrgRoleSync diff --git a/pkg/api/dtos/frontend_settings.go b/pkg/api/dtos/frontend_settings.go index 84dd91c622b..28fb545d26d 100644 --- a/pkg/api/dtos/frontend_settings.go +++ b/pkg/api/dtos/frontend_settings.go @@ -6,18 +6,29 @@ import ( ) type FrontendSettingsAuthDTO struct { - OAuthSkipOrgRoleUpdateSync bool `json:"OAuthSkipOrgRoleUpdateSync"` - SAMLSkipOrgRoleSync bool `json:"SAMLSkipOrgRoleSync"` - LDAPSkipOrgRoleSync bool `json:"LDAPSkipOrgRoleSync"` - GoogleSkipOrgRoleSync bool `json:"GoogleSkipOrgRoleSync"` + AuthProxyEnableLoginToken bool `json:"AuthProxyEnableLoginToken"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + OAuthSkipOrgRoleUpdateSync bool `json:"OAuthSkipOrgRoleUpdateSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + SAMLSkipOrgRoleSync bool `json:"SAMLSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + LDAPSkipOrgRoleSync bool `json:"LDAPSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + GoogleSkipOrgRoleSync bool `json:"GoogleSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 GenericOAuthSkipOrgRoleSync bool `json:"GenericOAuthSkipOrgRoleSync"` - JWTAuthSkipOrgRoleSync bool `json:"JWTAuthSkipOrgRoleSync"` - GrafanaComSkipOrgRoleSync bool `json:"GrafanaComSkipOrgRoleSync"` - AzureADSkipOrgRoleSync bool `json:"AzureADSkipOrgRoleSync"` - GithubSkipOrgRoleSync bool `json:"GithubSkipOrgRoleSync"` - GitLabSkipOrgRoleSync bool `json:"GitLabSkipOrgRoleSync"` - OktaSkipOrgRoleSync bool `json:"OktaSkipOrgRoleSync"` - AuthProxyEnableLoginToken bool `json:"AuthProxyEnableLoginToken"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + JWTAuthSkipOrgRoleSync bool `json:"JWTAuthSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + GrafanaComSkipOrgRoleSync bool `json:"GrafanaComSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + AzureADSkipOrgRoleSync bool `json:"AzureADSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + GithubSkipOrgRoleSync bool `json:"GithubSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + GitLabSkipOrgRoleSync bool `json:"GitLabSkipOrgRoleSync"` + // Deprecated: this is no longer used and will be removed in Grafana 11 + OktaSkipOrgRoleSync bool `json:"OktaSkipOrgRoleSync"` } type FrontendSettingsBuildInfoDTO struct { diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index 1092efe028b..e8146b80abe 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -8,6 +8,7 @@ import ( "strings" "github.com/grafana/grafana/pkg/api/dtos" + "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -159,21 +160,6 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro PublicDashboardAccessToken: c.PublicDashboardAccessToken, SharedWithMeFolderUID: folder.SharedWithMeFolderUID, - Auth: dtos.FrontendSettingsAuthDTO{ - OAuthSkipOrgRoleUpdateSync: hs.Cfg.OAuthSkipOrgRoleUpdateSync, - SAMLSkipOrgRoleSync: hs.Cfg.SAMLSkipOrgRoleSync, - LDAPSkipOrgRoleSync: hs.Cfg.LDAPSkipOrgRoleSync, - GoogleSkipOrgRoleSync: hs.Cfg.GoogleSkipOrgRoleSync, - JWTAuthSkipOrgRoleSync: hs.Cfg.JWTAuthSkipOrgRoleSync, - GrafanaComSkipOrgRoleSync: hs.Cfg.GrafanaComSkipOrgRoleSync, - GenericOAuthSkipOrgRoleSync: hs.Cfg.GenericOAuthSkipOrgRoleSync, - AzureADSkipOrgRoleSync: hs.Cfg.AzureADSkipOrgRoleSync, - GithubSkipOrgRoleSync: hs.Cfg.GitHubSkipOrgRoleSync, - GitLabSkipOrgRoleSync: hs.Cfg.GitLabSkipOrgRoleSync, - OktaSkipOrgRoleSync: hs.Cfg.OktaSkipOrgRoleSync, - AuthProxyEnableLoginToken: hs.Cfg.AuthProxyEnableLoginToken, - }, - BuildInfo: dtos.FrontendSettingsBuildInfoDTO{ HideVersion: hideVersion, Version: version, @@ -261,6 +247,30 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro frontendSettings.AlertingEnabled = *setting.AlertingEnabled } + // It returns false if the provider is not enabled or the skip org role sync is false. + parseSkipOrgRoleSyncEnabled := func(info *social.OAuthInfo) bool { + if info == nil { + return false + } + return info.SkipOrgRoleSync + } + + oauthProviders := hs.SocialService.GetOAuthInfoProviders() + frontendSettings.Auth = dtos.FrontendSettingsAuthDTO{ + AuthProxyEnableLoginToken: hs.Cfg.AuthProxyEnableLoginToken, + OAuthSkipOrgRoleUpdateSync: hs.Cfg.OAuthSkipOrgRoleUpdateSync, + SAMLSkipOrgRoleSync: hs.Cfg.SAMLSkipOrgRoleSync, + LDAPSkipOrgRoleSync: hs.Cfg.LDAPSkipOrgRoleSync, + JWTAuthSkipOrgRoleSync: hs.Cfg.JWTAuthSkipOrgRoleSync, + GoogleSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.GoogleProviderName]), + GrafanaComSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.GrafanaComProviderName]), + GenericOAuthSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.GenericOAuthProviderName]), + AzureADSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.AzureADProviderName]), + GithubSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.GitHubProviderName]), + GitLabSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.GitlabProviderName]), + OktaSkipOrgRoleSync: parseSkipOrgRoleSyncEnabled(oauthProviders[social.OktaProviderName]), + } + if hs.pluginsCDNService != nil && hs.pluginsCDNService.IsEnabled() { cdnBaseURL, err := hs.pluginsCDNService.BaseURL() if err != nil { diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index 91d70049fe1..7a8c993bce6 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -334,8 +334,9 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or for i := range filteredUsers { filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserID)] if module, ok := modules[filteredUsers[i].UserID]; ok { + oauthInfo := hs.SocialService.GetOAuthInfoProvider(module) filteredUsers[i].AuthLabels = []string{login.GetAuthProviderLabel(module)} - filteredUsers[i].IsExternallySynced = login.IsExternallySynced(hs.Cfg, module) + filteredUsers[i].IsExternallySynced = login.IsExternallySynced(hs.Cfg, module, oauthInfo) } } @@ -421,8 +422,11 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up return response.Error(http.StatusInternalServerError, "Failed to get user auth info", nil) } } - if authInfo != nil && authInfo.AuthModule != "" && login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) { - return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user")) + if authInfo != nil && authInfo.AuthModule != "" { + oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule) + if login.IsExternallySynced(hs.Cfg, authInfo.AuthModule, oauthInfo) { + return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user")) + } } if err := hs.orgService.UpdateOrgUser(c.Req.Context(), &cmd); err != nil { diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index fcc7223a8a3..d4f9951bcf6 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -15,6 +15,8 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db/dbtest" + "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/login/social/socialtest" "github.com/grafana/grafana/pkg/models/roletype" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" @@ -223,14 +225,7 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { expectedCode: http.StatusForbidden, }, { - desc: "should not be able to change basicRole with a different provider", - SkipOrgRoleSync: false, - AuthEnabled: true, - AuthModule: login.GenericOAuthModule, - expectedCode: http.StatusForbidden, - }, - { - desc: "should not be able to change basicRole for a user synced through GCom", + desc: "should not be able to change basicRole for a user synced through an OAuth provider", SkipOrgRoleSync: false, AuthEnabled: true, AuthModule: login.GrafanaComAuthModule, @@ -263,23 +258,17 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { if tt.AuthModule == login.LDAPAuthModule { hs.Cfg.LDAPAuthEnabled = tt.AuthEnabled hs.Cfg.LDAPSkipOrgRoleSync = tt.SkipOrgRoleSync - } else if tt.AuthModule == login.GenericOAuthModule { - hs.Cfg.GenericOAuthAuthEnabled = tt.AuthEnabled - hs.Cfg.GenericOAuthSkipOrgRoleSync = tt.SkipOrgRoleSync - } else if tt.AuthModule == login.GrafanaComAuthModule { - hs.Cfg.GrafanaNetAuthEnabled = tt.AuthEnabled - hs.Cfg.GrafanaComSkipOrgRoleSync = tt.SkipOrgRoleSync - } else if tt.AuthModule == "" { - // authmodule empty means basic auth - } else { - t.Errorf("invalid auth module for test: %s", tt.AuthModule) } + // AuthModule empty means basic auth hs.authInfoService = &authinfotest.FakeService{ ExpectedUserAuth: &login.UserAuth{AuthModule: tt.AuthModule}, } hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions} hs.orgService = &orgtest.FakeOrgService{} + hs.SocialService = &socialtest.FakeSocialService{ + ExpectedAuthInfoProvider: &social.OAuthInfo{Enabled: tt.AuthEnabled, SkipOrgRoleSync: tt.SkipOrgRoleSync}, + } hs.accesscontrolService = &actest.FakeService{ ExpectedPermissions: permissions, } diff --git a/pkg/api/user.go b/pkg/api/user.go index a034299060f..ba65e374a08 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -72,12 +72,10 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6 authLabel := login.GetAuthProviderLabel(authInfo.AuthModule) userProfile.AuthLabels = append(userProfile.AuthLabels, authLabel) userProfile.IsExternal = true - userProfile.IsExternallySynced = login.IsExternallySynced(hs.Cfg, authInfo.AuthModule) - oAuthAndAllowAssignGrafanaAdmin := false - if oauthInfo := hs.SocialService.GetOAuthInfoProvider(strings.TrimPrefix(authInfo.AuthModule, "oauth_")); oauthInfo != nil { - oAuthAndAllowAssignGrafanaAdmin = oauthInfo.AllowAssignGrafanaAdmin - } - userProfile.IsGrafanaAdminExternallySynced = login.IsGrafanaAdminExternallySynced(hs.Cfg, authInfo.AuthModule, oAuthAndAllowAssignGrafanaAdmin) + + oauthInfo := hs.SocialService.GetOAuthInfoProvider(authInfo.AuthModule) + userProfile.IsExternallySynced = login.IsExternallySynced(hs.Cfg, authInfo.AuthModule, oauthInfo) + userProfile.IsGrafanaAdminExternallySynced = login.IsGrafanaAdminExternallySynced(hs.Cfg, oauthInfo, authInfo.AuthModule) } userProfile.AccessControl = hs.getAccessControlMetadata(c, c.SignedInUser.GetOrgID(), "global.users:id:", strconv.FormatInt(userID, 10)) diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index 08bf2833348..16c9fcfaf51 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -298,9 +298,7 @@ func Test_GetUserByID(t *testing.T) { switch tc.authModule { case login.GenericOAuthModule: - socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled} - cfg.GenericOAuthAuthEnabled = tc.authEnabled - cfg.GenericOAuthSkipOrgRoleSync = tc.skipOrgRoleSync + socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled, SkipOrgRoleSync: tc.skipOrgRoleSync} case login.JWTModule: cfg.JWTAuthEnabled = tc.authEnabled cfg.JWTAuthSkipOrgRoleSync = tc.skipOrgRoleSync diff --git a/pkg/login/social/connectors/azuread_oauth.go b/pkg/login/social/connectors/azuread_oauth.go index d2a5f9812dd..30791bc74c2 100644 --- a/pkg/login/social/connectors/azuread_oauth.go +++ b/pkg/login/social/connectors/azuread_oauth.go @@ -40,7 +40,6 @@ type SocialAzureAD struct { cache remotecache.CacheStorage allowedOrganizations []string forceUseGraphAPI bool - skipOrgRoleSync bool } type azureClaims struct { @@ -76,13 +75,10 @@ type keySetJWKS struct { func NewAzureADProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features *featuremgmt.FeatureManager, cache remotecache.CacheStorage) *SocialAzureAD { config := createOAuthConfig(info, cfg, social.AzureADProviderName) provider := &SocialAzureAD{ - SocialBase: newSocialBase(social.AzureADProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + SocialBase: newSocialBase(social.AzureADProviderName, config, info, cfg.AutoAssignOrgRole, *features), cache: cache, allowedOrganizations: util.SplitString(info.Extra[allowedOrganizationsKey]), forceUseGraphAPI: MustBool(info.Extra[forceUseGraphAPIKey], false), - skipOrgRoleSync: cfg.AzureADSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync } if info.UseRefreshToken && features.IsEnabledGlobally(featuremgmt.FlagAccessTokenExpirationCheck) { @@ -120,7 +116,7 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token // setting the role, grafanaAdmin to empty to reflect that we are not syncronizing with the external provider var role roletype.RoleType var grafanaAdmin bool - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, err = s.extractRoleAndAdmin(claims) if err != nil { return nil, err @@ -151,7 +147,7 @@ func (s *SocialAzureAD) UserInfo(ctx context.Context, client *http.Client, token isGrafanaAdmin = &grafanaAdmin } - if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + if s.allowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } diff --git a/pkg/login/social/connectors/azuread_oauth_test.go b/pkg/login/social/connectors/azuread_oauth_test.go index 543c993c6f5..f974d132a4b 100644 --- a/pkg/login/social/connectors/azuread_oauth_test.go +++ b/pkg/login/social/connectors/azuread_oauth_test.go @@ -793,13 +793,11 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { Name: "azuread", ClientId: "client-id-example", AllowAssignGrafanaAdmin: true, - // TODO: use this setting when SkipOrgRoleSync has moved to OAuthInfo - //SkipOrgRoleSync: false, + SkipOrgRoleSync: false, }, cfg: &setting.Cfg{ AutoAssignOrgRole: "", OAuthSkipOrgRoleUpdateSync: false, - AzureADSkipOrgRoleSync: false, }, }, claims: &azureClaims{ @@ -826,13 +824,11 @@ func TestSocialAzureAD_SkipOrgRole(t *testing.T) { Name: "azuread", ClientId: "client-id-example", AllowAssignGrafanaAdmin: true, - // TODO: use this setting when SkipOrgRoleSync has moved to OAuthInfo - // SkipOrgRoleSync: false, + SkipOrgRoleSync: false, }, cfg: &setting.Cfg{ AutoAssignOrgRole: "", OAuthSkipOrgRoleUpdateSync: false, - AzureADSkipOrgRoleSync: false, }, }, claims: &azureClaims{ diff --git a/pkg/login/social/connectors/generic_oauth.go b/pkg/login/social/connectors/generic_oauth.go index dfbf80fc4f3..800ffabe1a6 100644 --- a/pkg/login/social/connectors/generic_oauth.go +++ b/pkg/login/social/connectors/generic_oauth.go @@ -45,13 +45,12 @@ type SocialGenericOAuth struct { teamIdsAttributePath string teamIds []string allowedGroups []string - skipOrgRoleSync bool } func NewGenericOAuthProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features *featuremgmt.FeatureManager) *SocialGenericOAuth { config := createOAuthConfig(info, cfg, social.GenericOAuthProviderName) provider := &SocialGenericOAuth{ - SocialBase: newSocialBase(social.GenericOAuthProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + SocialBase: newSocialBase(social.GenericOAuthProviderName, config, info, cfg.AutoAssignOrgRole, *features), apiUrl: info.ApiUrl, teamsUrl: info.TeamsUrl, emailAttributeName: info.EmailAttributeName, @@ -64,9 +63,6 @@ func NewGenericOAuthProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettin teamIds: util.SplitString(info.Extra[teamIdsKey]), allowedOrganizations: util.SplitString(info.Extra[allowedOrganizationsKey]), allowedGroups: info.AllowedGroups, - skipOrgRoleSync: cfg.GenericOAuthSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync } if features.IsEnabledGlobally(featuremgmt.FlagSsoSettingsApi) { @@ -196,7 +192,7 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, } } - if userInfo.Role == "" && !s.skipOrgRoleSync { + if userInfo.Role == "" && !s.info.SkipOrgRoleSync { role, grafanaAdmin, err := s.extractRoleAndAdminOptional(data.rawJSON, []string{}) if err != nil { s.log.Warn("Failed to extract role", "err", err) @@ -219,14 +215,14 @@ func (s *SocialGenericOAuth) UserInfo(ctx context.Context, client *http.Client, } } - if userInfo.Role == "" && !s.skipOrgRoleSync { + if userInfo.Role == "" && !s.info.SkipOrgRoleSync { if s.roleAttributeStrict { return nil, errRoleAttributeStrictViolation.Errorf("idP did not return a role attribute") } userInfo.Role = s.defaultRole() } - if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + if s.allowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } diff --git a/pkg/login/social/connectors/generic_oauth_test.go b/pkg/login/social/connectors/generic_oauth_test.go index e90c60e4355..e0d3df51aad 100644 --- a/pkg/login/social/connectors/generic_oauth_test.go +++ b/pkg/login/social/connectors/generic_oauth_test.go @@ -455,7 +455,7 @@ func TestUserInfoSearchesForEmailAndRole(t *testing.T) { for _, test := range tests { provider.roleAttributePath = test.RoleAttributePath provider.allowAssignGrafanaAdmin = test.AllowAssignGrafanaAdmin - provider.skipOrgRoleSync = test.SkipOrgRoleSync + provider.info.SkipOrgRoleSync = test.SkipOrgRoleSync t.Run(test.Name, func(t *testing.T) { body, err := json.Marshal(test.ResponseBody) diff --git a/pkg/login/social/connectors/github_oauth.go b/pkg/login/social/connectors/github_oauth.go index b1210a989e7..bd8296d1c35 100644 --- a/pkg/login/social/connectors/github_oauth.go +++ b/pkg/login/social/connectors/github_oauth.go @@ -32,7 +32,6 @@ type SocialGithub struct { allowedOrganizations []string apiUrl string teamIds []int - skipOrgRoleSync bool } type GithubTeam struct { @@ -61,13 +60,10 @@ func NewGitHubProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings sso config := createOAuthConfig(info, cfg, social.GitHubProviderName) provider := &SocialGithub{ - SocialBase: newSocialBase(social.GitHubProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + SocialBase: newSocialBase(social.GitHubProviderName, config, info, cfg.AutoAssignOrgRole, *features), apiUrl: info.ApiUrl, teamIds: teamIds, allowedOrganizations: util.SplitString(info.Extra[allowedOrganizationsKey]), - skipOrgRoleSync: cfg.GitHubSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync } if len(teamIdsSplitted) != len(teamIds) { @@ -263,7 +259,7 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token var role roletype.RoleType var isGrafanaAdmin *bool = nil - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err = s.extractRoleAndAdmin(response.Body, teams) if err != nil { @@ -276,7 +272,7 @@ func (s *SocialGithub) UserInfo(ctx context.Context, client *http.Client, token } // we skip allowing assignment of GrafanaAdmin if skipOrgRoleSync is present - if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + if s.allowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } diff --git a/pkg/login/social/connectors/github_oauth_test.go b/pkg/login/social/connectors/github_oauth_test.go index ccfff095e56..336fee64569 100644 --- a/pkg/login/social/connectors/github_oauth_test.go +++ b/pkg/login/social/connectors/github_oauth_test.go @@ -245,13 +245,13 @@ func TestSocialGitHub_UserInfo(t *testing.T) { &social.OAuthInfo{ ApiUrl: server.URL + "/user", RoleAttributePath: tt.roleAttributePath, + SkipOrgRoleSync: tt.settingSkipOrgRoleSync, Extra: map[string]string{ "allowed_organizations": "", "team_ids": "", }, }, &setting.Cfg{ - AutoAssignOrgRole: tt.autoAssignOrgRole, - GitHubSkipOrgRoleSync: tt.settingSkipOrgRoleSync, + AutoAssignOrgRole: tt.autoAssignOrgRole, }, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) diff --git a/pkg/login/social/connectors/gitlab_oauth.go b/pkg/login/social/connectors/gitlab_oauth.go index 9dcbe4830fd..6d09e8c7b57 100644 --- a/pkg/login/social/connectors/gitlab_oauth.go +++ b/pkg/login/social/connectors/gitlab_oauth.go @@ -29,8 +29,7 @@ var _ ssosettings.Reloadable = (*SocialGitlab)(nil) type SocialGitlab struct { *SocialBase - apiUrl string - skipOrgRoleSync bool + apiUrl string } type apiData struct { @@ -57,11 +56,8 @@ type userData struct { func NewGitLabProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features *featuremgmt.FeatureManager) *SocialGitlab { config := createOAuthConfig(info, cfg, social.GitlabProviderName) provider := &SocialGitlab{ - SocialBase: newSocialBase(social.GitlabProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), - apiUrl: info.ApiUrl, - skipOrgRoleSync: cfg.GitLabSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync + SocialBase: newSocialBase(social.GitlabProviderName, config, info, cfg.AutoAssignOrgRole, *features), + apiUrl: info.ApiUrl, } if features.IsEnabledGlobally(featuremgmt.FlagSsoSettingsApi) { @@ -187,7 +183,7 @@ func (s *SocialGitlab) UserInfo(ctx context.Context, client *http.Client, token return nil, errMissingGroupMembership } - if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + if s.allowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } @@ -226,7 +222,7 @@ func (s *SocialGitlab) extractFromAPI(ctx context.Context, client *http.Client, Groups: s.getGroups(ctx, client), } - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err := s.extractRoleAndAdmin(response.Body, idData.Groups) if err != nil { @@ -283,7 +279,7 @@ func (s *SocialGitlab) extractFromToken(ctx context.Context, client *http.Client data.Groups = userInfo.Groups } - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, errRole := s.extractRoleAndAdmin(rawJSON, data.Groups) if errRole != nil { return nil, errRole diff --git a/pkg/login/social/connectors/gitlab_oauth_test.go b/pkg/login/social/connectors/gitlab_oauth_test.go index b37040320f7..14271f72cea 100644 --- a/pkg/login/social/connectors/gitlab_oauth_test.go +++ b/pkg/login/social/connectors/gitlab_oauth_test.go @@ -165,7 +165,7 @@ func TestSocialGitlab_UserInfo(t *testing.T) { provider.allowAssignGrafanaAdmin = test.Cfg.AllowAssignGrafanaAdmin provider.autoAssignOrgRole = string(test.Cfg.AutoAssignOrgRole) provider.roleAttributeStrict = test.Cfg.RoleAttributeStrict - provider.skipOrgRoleSync = test.Cfg.SkipOrgRoleSync + provider.info.SkipOrgRoleSync = test.Cfg.SkipOrgRoleSync t.Run(test.Name, func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { @@ -353,15 +353,13 @@ func TestSocialGitlab_extractFromToken(t *testing.T) { AllowSignup: false, RoleAttributePath: "", RoleAttributeStrict: false, - // TODO: use this setting when SkipOrgRoleSync has moved to OAuthInfo - //SkipOrgRoleSync: false, - AuthUrl: tc.config.Endpoint.AuthURL, - TokenUrl: tc.config.Endpoint.TokenURL, + SkipOrgRoleSync: false, + AuthUrl: tc.config.Endpoint.AuthURL, + TokenUrl: tc.config.Endpoint.TokenURL, }, &setting.Cfg{ AutoAssignOrgRole: "", OAuthSkipOrgRoleUpdateSync: false, - GitLabSkipOrgRoleSync: false, }, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) diff --git a/pkg/login/social/connectors/google_oauth.go b/pkg/login/social/connectors/google_oauth.go index 69d14f27be6..a44eadc1073 100644 --- a/pkg/login/social/connectors/google_oauth.go +++ b/pkg/login/social/connectors/google_oauth.go @@ -28,9 +28,8 @@ var _ ssosettings.Reloadable = (*SocialGoogle)(nil) type SocialGoogle struct { *SocialBase - hostedDomain string - apiUrl string - skipOrgRoleSync bool + hostedDomain string + apiUrl string } type googleUserData struct { @@ -44,12 +43,9 @@ type googleUserData struct { func NewGoogleProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features *featuremgmt.FeatureManager) *SocialGoogle { config := createOAuthConfig(info, cfg, social.GoogleProviderName) provider := &SocialGoogle{ - SocialBase: newSocialBase(social.GoogleProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), - hostedDomain: info.HostedDomain, - apiUrl: info.ApiUrl, - skipOrgRoleSync: cfg.GoogleSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync + SocialBase: newSocialBase(social.GoogleProviderName, config, info, cfg.AutoAssignOrgRole, *features), + hostedDomain: info.HostedDomain, + apiUrl: info.ApiUrl, } if strings.HasPrefix(info.ApiUrl, legacyAPIURL) { @@ -112,7 +108,7 @@ func (s *SocialGoogle) UserInfo(ctx context.Context, client *http.Client, token Groups: groups, } - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role, grafanaAdmin, errRole := s.extractRoleAndAdmin(data.rawJSON, groups) if errRole != nil { return nil, errRole diff --git a/pkg/login/social/connectors/google_oauth_test.go b/pkg/login/social/connectors/google_oauth_test.go index cde6ecc8666..dffd3d74bc4 100644 --- a/pkg/login/social/connectors/google_oauth_test.go +++ b/pkg/login/social/connectors/google_oauth_test.go @@ -193,10 +193,10 @@ func TestSocialGoogle_retrieveGroups(t *testing.T) { RoleAttributePath: "", RoleAttributeStrict: false, AllowAssignGrafanaAdmin: false, + SkipOrgRoleSync: false, }, &setting.Cfg{ - AutoAssignOrgRole: "", - GoogleSkipOrgRoleSync: false, + AutoAssignOrgRole: "", }, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) @@ -647,12 +647,9 @@ func TestSocialGoogle_UserInfo(t *testing.T) { RoleAttributePath: tt.fields.roleAttributePath, RoleAttributeStrict: tt.fields.roleAttributeStrict, AllowAssignGrafanaAdmin: tt.fields.allowAssignGrafanaAdmin, - // TODO: use this setting when SkipOrgRoleSync has moved to OAuthInfo - // SkipOrgRoleSync: tt.fields.skipOrgRoleSync, - }, - &setting.Cfg{ - GoogleSkipOrgRoleSync: tt.fields.skipOrgRoleSync, + SkipOrgRoleSync: tt.fields.skipOrgRoleSync, }, + &setting.Cfg{}, &ssosettingstests.MockService{}, featuremgmt.WithFeatures()) diff --git a/pkg/login/social/connectors/grafana_com_oauth.go b/pkg/login/social/connectors/grafana_com_oauth.go index 3d6071e78d1..d3bda3607ef 100644 --- a/pkg/login/social/connectors/grafana_com_oauth.go +++ b/pkg/login/social/connectors/grafana_com_oauth.go @@ -27,7 +27,6 @@ type SocialGrafanaCom struct { *SocialBase url string allowedOrganizations []string - skipOrgRoleSync bool } type OrgRecord struct { @@ -42,12 +41,9 @@ func NewGrafanaComProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings config := createOAuthConfig(info, cfg, social.GrafanaComProviderName) provider := &SocialGrafanaCom{ - SocialBase: newSocialBase(social.GrafanaComProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + SocialBase: newSocialBase(social.GrafanaComProviderName, config, info, cfg.AutoAssignOrgRole, *features), url: cfg.GrafanaComURL, allowedOrganizations: util.SplitString(info.Extra[allowedOrganizationsKey]), - skipOrgRoleSync: cfg.GrafanaComSkipOrgRoleSync, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync } if features.IsEnabledGlobally(featuremgmt.FlagSsoSettingsApi) { @@ -109,7 +105,7 @@ func (s *SocialGrafanaCom) UserInfo(ctx context.Context, client *http.Client, _ // on login we do not want to display the role from the external provider var role roletype.RoleType - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { role = org.RoleType(data.Role) } userInfo := &social.BasicUserInfo{ diff --git a/pkg/login/social/connectors/grafana_com_oauth_test.go b/pkg/login/social/connectors/grafana_com_oauth_test.go index 35629547f00..63c0c668b89 100644 --- a/pkg/login/social/connectors/grafana_com_oauth_test.go +++ b/pkg/login/social/connectors/grafana_com_oauth_test.go @@ -68,7 +68,7 @@ func TestSocialGrafanaCom_UserInfo(t *testing.T) { } for _, test := range tests { - provider.skipOrgRoleSync = test.Cfg.skipOrgRoleSync + provider.info.SkipOrgRoleSync = test.Cfg.skipOrgRoleSync t.Run(test.Name, func(t *testing.T) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { diff --git a/pkg/login/social/connectors/okta_oauth.go b/pkg/login/social/connectors/okta_oauth.go index 1ba9b1f892e..5cc5dfa9e10 100644 --- a/pkg/login/social/connectors/okta_oauth.go +++ b/pkg/login/social/connectors/okta_oauth.go @@ -23,9 +23,8 @@ var _ ssosettings.Reloadable = (*SocialOkta)(nil) type SocialOkta struct { *SocialBase - apiUrl string - allowedGroups []string - skipOrgRoleSync bool + apiUrl string + allowedGroups []string } type OktaUserInfoJson struct { @@ -50,12 +49,9 @@ type OktaClaims struct { func NewOktaProvider(info *social.OAuthInfo, cfg *setting.Cfg, ssoSettings ssosettings.Service, features *featuremgmt.FeatureManager) *SocialOkta { config := createOAuthConfig(info, cfg, social.OktaProviderName) provider := &SocialOkta{ - SocialBase: newSocialBase(social.OktaProviderName, config, info, cfg.AutoAssignOrgRole, cfg.OAuthSkipOrgRoleUpdateSync, *features), + SocialBase: newSocialBase(social.OktaProviderName, config, info, cfg.AutoAssignOrgRole, *features), apiUrl: info.ApiUrl, allowedGroups: info.AllowedGroups, - // FIXME: Move skipOrgRoleSync to OAuthInfo - // skipOrgRoleSync: info.SkipOrgRoleSync - skipOrgRoleSync: cfg.OktaSkipOrgRoleSync, } if info.UseRefreshToken && features.IsEnabledGlobally(featuremgmt.FlagAccessTokenExpirationCheck) { @@ -119,7 +115,7 @@ func (s *SocialOkta) UserInfo(ctx context.Context, client *http.Client, token *o var role roletype.RoleType var isGrafanaAdmin *bool - if !s.skipOrgRoleSync { + if !s.info.SkipOrgRoleSync { var grafanaAdmin bool role, grafanaAdmin, err = s.extractRoleAndAdmin(data.rawJSON, groups) if err != nil { @@ -130,7 +126,7 @@ func (s *SocialOkta) UserInfo(ctx context.Context, client *http.Client, token *o isGrafanaAdmin = &grafanaAdmin } } - if s.allowAssignGrafanaAdmin && s.skipOrgRoleSync { + if s.allowAssignGrafanaAdmin && s.info.SkipOrgRoleSync { s.log.Debug("AllowAssignGrafanaAdmin and skipOrgRoleSync are both set, Grafana Admin role will not be synced, consider setting one or the other") } diff --git a/pkg/login/social/connectors/okta_oauth_test.go b/pkg/login/social/connectors/okta_oauth_test.go index db3ebd199fe..27019a03280 100644 --- a/pkg/login/social/connectors/okta_oauth_test.go +++ b/pkg/login/social/connectors/okta_oauth_test.go @@ -104,11 +104,9 @@ func TestSocialOkta_UserInfo(t *testing.T) { ApiUrl: server.URL + "/user", RoleAttributePath: tt.RoleAttributePath, AllowAssignGrafanaAdmin: tt.allowAssignGrafanaAdmin, - // TODO: use this setting when SkipOrgRoleSync has moved to OAuthInfo - // SkipOrgRoleSync: tt.settingSkipOrgRoleSync, + SkipOrgRoleSync: tt.settingSkipOrgRoleSync, }, &setting.Cfg{ - OktaSkipOrgRoleSync: tt.settingSkipOrgRoleSync, AutoAssignOrgRole: tt.autoAssignOrgRole, OAuthSkipOrgRoleUpdateSync: false, }, diff --git a/pkg/login/social/connectors/social_base.go b/pkg/login/social/connectors/social_base.go index 0ace2fb9baf..e5d67d929dc 100644 --- a/pkg/login/social/connectors/social_base.go +++ b/pkg/login/social/connectors/social_base.go @@ -32,7 +32,6 @@ type SocialBase struct { roleAttributePath string roleAttributeStrict bool autoAssignOrgRole string - skipOrgRoleSync bool features featuremgmt.FeatureManager useRefreshToken bool } @@ -41,7 +40,6 @@ func newSocialBase(name string, config *oauth2.Config, info *social.OAuthInfo, autoAssignOrgRole string, - skipOrgRoleSync bool, features featuremgmt.FeatureManager, ) *SocialBase { logger := log.New("oauth." + name) @@ -57,7 +55,6 @@ func newSocialBase(name string, roleAttributePath: info.RoleAttributePath, roleAttributeStrict: info.RoleAttributeStrict, autoAssignOrgRole: autoAssignOrgRole, - skipOrgRoleSync: skipOrgRoleSync, features: features, useRefreshToken: info.UseRefreshToken, } @@ -76,7 +73,7 @@ func (s *SocialBase) SupportBundleContent(bf *bytes.Buffer) error { bf.WriteString(fmt.Sprintf("auto_assign_org_role = %v\n", s.autoAssignOrgRole)) bf.WriteString(fmt.Sprintf("role_attribute_path = %v\n", s.roleAttributePath)) bf.WriteString(fmt.Sprintf("role_attribute_strict = %v\n", s.roleAttributeStrict)) - bf.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", s.skipOrgRoleSync)) + bf.WriteString(fmt.Sprintf("skip_org_role_sync = %v\n", s.info.SkipOrgRoleSync)) bf.WriteString(fmt.Sprintf("client_id = %v\n", s.Config.ClientID)) bf.WriteString(fmt.Sprintf("client_secret = %v ; issue if empty\n", strings.Repeat("*", len(s.Config.ClientSecret)))) bf.WriteString(fmt.Sprintf("auth_url = %v\n", s.Config.Endpoint.AuthURL)) diff --git a/pkg/login/social/socialimpl/service.go b/pkg/login/social/socialimpl/service.go index 312efe16e5b..5cf45747749 100644 --- a/pkg/login/social/socialimpl/service.go +++ b/pkg/login/social/socialimpl/service.go @@ -71,12 +71,18 @@ func ProvideService(cfg *setting.Cfg, sec := cfg.Raw.Section("auth." + name) settingsKVs := convertIniSectionToMap(sec) + info, err := connectors.CreateOAuthInfoFromKeyValues(settingsKVs) if err != nil { ss.log.Error("Failed to create OAuthInfo for provider", "error", err, "provider", name) continue } + // Workaround for moving the SkipOrgRoleSync setting to the OAuthInfo struct + withOverrides := cfg.SectionWithEnvOverrides("auth." + name) + info.Enabled = withOverrides.Key("enabled").MustBool(false) + info.SkipOrgRoleSync = withOverrides.Key("skip_org_role_sync").MustBool(false) + if !info.Enabled { continue } @@ -175,7 +181,9 @@ func (ss *SocialService) GetConnector(name string) (social.SocialConnector, erro } func (ss *SocialService) GetOAuthInfoProvider(name string) *social.OAuthInfo { - connector, ok := ss.socialMap[name] + // The socialMap keys don't have "oauth_" prefix, but everywhere else in the system does + provider := strings.TrimPrefix(name, "oauth_") + connector, ok := ss.socialMap[provider] if !ok { return nil } diff --git a/pkg/login/social/socialimpl/service_test.go b/pkg/login/social/socialimpl/service_test.go index 2fd4ad57ad6..fe3d885fedd 100644 --- a/pkg/login/social/socialimpl/service_test.go +++ b/pkg/login/social/socialimpl/service_test.go @@ -3,13 +3,98 @@ package socialimpl import ( "testing" + "github.com/stretchr/testify/require" "gopkg.in/ini.v1" + "github.com/grafana/grafana/pkg/api/routing" + "github.com/grafana/grafana/pkg/infra/db" + "github.com/grafana/grafana/pkg/infra/remotecache" + "github.com/grafana/grafana/pkg/infra/usagestats" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/login/social/connectors" - "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/featuremgmt" + secretsfake "github.com/grafana/grafana/pkg/services/secrets/fakes" + "github.com/grafana/grafana/pkg/services/ssosettings/ssosettingsimpl" + "github.com/grafana/grafana/pkg/services/supportbundles/supportbundlestest" + "github.com/grafana/grafana/pkg/setting" ) +func TestSocialService_ProvideService(t *testing.T) { + type testEnv struct { + features *featuremgmt.FeatureManager + } + testCases := []struct { + name string + setup func(t *testing.T, env *testEnv) + expectedSocialMapLength int + expectedGenericOAuthSkipOrgRoleSync bool + }{ + { + name: "should load only enabled social connectors when ssoSettingsApi is disabled", + setup: nil, + expectedSocialMapLength: 1, + expectedGenericOAuthSkipOrgRoleSync: false, + }, + { + name: "should load all social connectors when ssoSettingsApi is enabled", + setup: func(t *testing.T, env *testEnv) { + env.features = featuremgmt.WithFeatures(featuremgmt.FlagSsoSettingsApi) + }, + expectedSocialMapLength: 7, + expectedGenericOAuthSkipOrgRoleSync: false, + }, + { + name: "should load Enabled and SkipOrgRoleSync parameters from environment variables when ssoSettingsApi is disabled", + setup: func(t *testing.T, env *testEnv) { + t.Setenv("GF_AUTH_GENERIC_OAUTH_ENABLED", "true") + t.Setenv("GF_AUTH_GENERIC_OAUTH_SKIP_ORG_ROLE_SYNC", "true") + }, + expectedSocialMapLength: 2, + expectedGenericOAuthSkipOrgRoleSync: true, + }, + } + iniContent := ` + [auth.azuread] + enabled = true + skip_org_role_sync = true + + [auth.generic_oauth] + enabled = false + skip_org_role_sync = false + ` + iniFile, err := ini.Load([]byte(iniContent)) + require.NoError(t, err) + + cfg := setting.NewCfg() + cfg.Raw = iniFile + + secrets := secretsfake.NewMockService(t) + accessControl := acimpl.ProvideAccessControl(cfg) + sqlStore := db.InitTestDB(t) + + ssoSettingsSvc := ssosettingsimpl.ProvideService(cfg, sqlStore, accessControl, routing.NewRouteRegister(), featuremgmt.WithFeatures(), secrets) + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + env := &testEnv{ + features: featuremgmt.WithFeatures(), + } + if tc.setup != nil { + tc.setup(t, env) + } + + socialService := ProvideService(cfg, env.features, &usagestats.UsageStatsMock{}, supportbundlestest.NewFakeBundleService(), remotecache.NewFakeStore(t), ssoSettingsSvc) + require.Equal(t, tc.expectedSocialMapLength, len(socialService.socialMap)) + + genericOAuthInfo := socialService.GetOAuthInfoProvider("generic_oauth") + if genericOAuthInfo != nil { + require.Equal(t, tc.expectedGenericOAuthSkipOrgRoleSync, genericOAuthInfo.SkipOrgRoleSync) + } + }) + } +} + func TestMapping_IniSectionOAuthInfo(t *testing.T) { iniContent := ` [test] diff --git a/pkg/services/login/authinfo.go b/pkg/services/login/authinfo.go index fa6257633a1..f70dae8c869 100644 --- a/pkg/services/login/authinfo.go +++ b/pkg/services/login/authinfo.go @@ -3,6 +3,7 @@ package login import ( "context" + "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/setting" ) @@ -63,9 +64,9 @@ const ( // https://github.com/grafana/grafana/blob/4181acec72f76df7ad02badce13769bae4a1f840/pkg/services/login/authinfoservice/database/database.go#L61 // this means that if the user has multiple auth providers and one of them is set to sync org roles // then IsExternallySynced will be true for this one provider and false for the others -func IsExternallySynced(cfg *setting.Cfg, authModule string) bool { +func IsExternallySynced(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool { // provider enabled in config - if !IsProviderEnabled(cfg, authModule) { + if !IsProviderEnabled(cfg, authModule, oauthInfo) { return false } // first check SAML, LDAP and JWT @@ -84,20 +85,11 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string) bool { return false } switch authModule { - case GoogleAuthModule: - return !cfg.GoogleSkipOrgRoleSync - case OktaAuthModule: - return !cfg.OktaSkipOrgRoleSync - case AzureADAuthModule: - return !cfg.AzureADSkipOrgRoleSync - case GitLabAuthModule: - return !cfg.GitLabSkipOrgRoleSync - case GithubAuthModule: - return !cfg.GitHubSkipOrgRoleSync - case GrafanaComAuthModule: - return !cfg.GrafanaComSkipOrgRoleSync - case GenericOAuthModule: - return !cfg.GenericOAuthSkipOrgRoleSync + case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule: + if oauthInfo == nil { + return false + } + return !oauthInfo.SkipOrgRoleSync } return true } @@ -105,8 +97,8 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string) bool { // IsGrafanaAdminExternallySynced returns true if Grafana server admin role is being managed by an external auth provider, and false otherwise. // Grafana admin role sync is available for JWT, OAuth providers and LDAP. // For JWT and OAuth providers there is an additional config option `allow_assign_grafana_admin` that has to be enabled for Grafana Admin role to be synced. -func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, authModule string, oAuthAndAllowAssignGrafanaAdmin bool) bool { - if !IsExternallySynced(cfg, authModule) { +func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, oauthInfo *social.OAuthInfo, authModule string) bool { + if !IsExternallySynced(cfg, authModule, oauthInfo) { return false } @@ -118,11 +110,11 @@ func IsGrafanaAdminExternallySynced(cfg *setting.Cfg, authModule string, oAuthAn case LDAPAuthModule: return true default: - return oAuthAndAllowAssignGrafanaAdmin + return oauthInfo != nil && oauthInfo.AllowAssignGrafanaAdmin } } -func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool { +func IsProviderEnabled(cfg *setting.Cfg, authModule string, oauthInfo *social.OAuthInfo) bool { switch authModule { case SAMLAuthModule: return cfg.SAMLAuthEnabled @@ -130,20 +122,11 @@ func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool { return cfg.LDAPAuthEnabled case JWTModule: return cfg.JWTAuthEnabled - case GoogleAuthModule: - return cfg.GoogleAuthEnabled - case OktaAuthModule: - return cfg.OktaAuthEnabled - case AzureADAuthModule: - return cfg.AzureADEnabled - case GitLabAuthModule: - return cfg.GitLabAuthEnabled - case GithubAuthModule: - return cfg.GitHubAuthEnabled - case GrafanaComAuthModule: - return cfg.GrafanaComAuthEnabled || cfg.GrafanaNetAuthEnabled - case GenericOAuthModule: - return cfg.GenericOAuthAuthEnabled + case GoogleAuthModule, OktaAuthModule, AzureADAuthModule, GitLabAuthModule, GithubAuthModule, GrafanaComAuthModule, GenericOAuthModule: + if oauthInfo == nil { + return false + } + return oauthInfo.Enabled } return false } diff --git a/pkg/services/login/authinfo_test.go b/pkg/services/login/authinfo_test.go index f65e24662ba..bd84e471e83 100644 --- a/pkg/services/login/authinfo_test.go +++ b/pkg/services/login/authinfo_test.go @@ -3,165 +3,55 @@ package login import ( "testing" + "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/setting" "github.com/stretchr/testify/assert" ) func TestIsExternallySynced(t *testing.T) { testcases := []struct { - name string - cfg *setting.Cfg - provider string - expected bool + name string + cfg *setting.Cfg + oauthInfo *social.OAuthInfo + provider string + expected bool }{ - // azure + // Same for all of the OAuth providers { - name: "AzureAD synced user should return that it is externally synced", - cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: false}, - provider: AzureADAuthModule, - expected: true, - }, - { - name: "AzureAD synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: true}, - provider: AzureADAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "azuread external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{AzureADEnabled: true, AzureADSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: AzureADAuthModule, - expected: false, - }, - // google - { - name: "Google synced user should return that it is externally synced", - cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: false}, - provider: GoogleAuthModule, - expected: true, - }, - { - name: "Google synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: true}, - provider: GoogleAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "google external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GoogleAuthModule, - expected: false, - }, - { - name: "external user should return that it is not externally synced when oauth org role sync is set and google skip org role sync set", - cfg: &setting.Cfg{GoogleAuthEnabled: true, GoogleSkipOrgRoleSync: true, OAuthSkipOrgRoleUpdateSync: true}, - provider: GoogleAuthModule, - expected: false, - }, - // okta - { - name: "Okta synced user should return that it is externally synced", - cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: false}, - provider: OktaAuthModule, - expected: true, - }, - { - name: "Okta synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: true}, - - provider: OktaAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "okta external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: OktaAuthModule, - expected: false, - }, - // github - { - name: "Github synced user should return that it is externally synced", - cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false}, - provider: GithubAuthModule, - expected: true, + name: "AzureAD external user should return that it is externally synced", + cfg: &setting.Cfg{}, + oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false}, + provider: AzureADAuthModule, + expected: true, }, { - name: "Github synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: true}, - provider: GithubAuthModule, - expected: false, + name: "AzureAD external user should return that it is not externally synced when org role sync is set", + cfg: &setting.Cfg{}, + oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: true}, + provider: AzureADAuthModule, + expected: false, }, // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers { - name: "github external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GithubAuthModule, - expected: false, + name: "AzureAD external user should return that it is not externally synced when oauth org role sync is set", + cfg: &setting.Cfg{OAuthSkipOrgRoleUpdateSync: true}, + oauthInfo: &social.OAuthInfo{Enabled: true, SkipOrgRoleSync: false}, + provider: AzureADAuthModule, + expected: false, }, - // gitlab { - name: "Gitlab synced user should return that it is externally synced", - cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: false}, - provider: GitLabAuthModule, - expected: true, + name: "AzureAD external user should return that it is not externally synced when the provider is not enabled", + cfg: &setting.Cfg{}, + oauthInfo: &social.OAuthInfo{Enabled: false, SkipOrgRoleSync: false}, + provider: AzureADAuthModule, + expected: false, }, { - name: "Gitlab synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: true}, - provider: GitLabAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "gitlab external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GitLabAuthModule, - expected: false, - }, - // grafana.com - { - name: "Grafana.com synced user should return that it is externally synced", - cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: false}, - provider: GrafanaComAuthModule, - expected: true, - }, - { - name: "Grafana.com synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: true}, - provider: GrafanaComAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "grafanacom external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GrafanaComAuthModule, - expected: false, - }, - // generic oauth - { - name: "OAuth synced user should return that it is externally synced", - cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, OAuthSkipOrgRoleUpdateSync: false}, - // this could be any of the external oauth providers - provider: GenericOAuthModule, - expected: true, - }, - { - name: "OAuth synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, OAuthSkipOrgRoleUpdateSync: true}, - // this could be any of the external oauth providers - provider: GenericOAuthModule, - expected: false, - }, - // FIXME: remove this test as soon as we remove the deprecated setting for skipping org role sync for all external oauth providers - { - name: "generic oauth external user should return that it is not externally synced when oauth org role sync is set", - cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, GenericOAuthSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GenericOAuthModule, - expected: false, + name: "AzureAD synced user should return that it is not externally synced when the provider is not enabled and nil", + cfg: &setting.Cfg{}, + oauthInfo: nil, + provider: AzureADAuthModule, + expected: false, }, // saml { @@ -213,36 +103,36 @@ func TestIsExternallySynced(t *testing.T) { for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider)) + assert.Equal(t, tc.expected, IsExternallySynced(tc.cfg, tc.provider, tc.oauthInfo)) }) } } func TestIsProviderEnabled(t *testing.T) { testcases := []struct { - name string - cfg *setting.Cfg - provider string - expected bool + name string + oauthInfo *social.OAuthInfo + provider string + expected bool }{ // github { - name: "Github should return true if enabled", - cfg: &setting.Cfg{GitHubAuthEnabled: true}, - provider: GithubAuthModule, - expected: true, + name: "Github should return true if enabled", + oauthInfo: &social.OAuthInfo{Enabled: true}, + provider: GithubAuthModule, + expected: true, }, { - name: "Github should return false if not enabled", - cfg: &setting.Cfg{}, - provider: GithubAuthModule, - expected: false, + name: "Github should return false if not enabled", + oauthInfo: &social.OAuthInfo{Enabled: false}, + provider: GithubAuthModule, + expected: false, }, } for _, tc := range testcases { t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.expected, IsProviderEnabled(tc.cfg, tc.provider)) + assert.Equal(t, tc.expected, IsProviderEnabled(setting.NewCfg(), tc.provider, tc.oauthInfo)) }) } } diff --git a/pkg/services/ssosettings/strategies/oauth_strategy_test.go b/pkg/services/ssosettings/strategies/oauth_strategy_test.go index b7193ed242f..414f7deb9a4 100644 --- a/pkg/services/ssosettings/strategies/oauth_strategy_test.go +++ b/pkg/services/ssosettings/strategies/oauth_strategy_test.go @@ -4,11 +4,11 @@ import ( "context" "testing" + "github.com/stretchr/testify/require" "gopkg.in/ini.v1" "github.com/grafana/grafana/pkg/login/social" "github.com/grafana/grafana/pkg/setting" - "github.com/stretchr/testify/require" ) var ( diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index e142dc77709..126d9c0788a 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -443,22 +443,6 @@ type Cfg struct { RudderstackIntegrationsURL string IntercomSecret string - // AzureAD - AzureADEnabled bool - AzureADSkipOrgRoleSync bool - - // Google - GoogleAuthEnabled bool - GoogleSkipOrgRoleSync bool - - // Gitlab - GitLabAuthEnabled bool - GitLabSkipOrgRoleSync bool - - // Generic OAuth - GenericOAuthAuthEnabled bool - GenericOAuthSkipOrgRoleSync bool - // LDAP LDAPAuthEnabled bool LDAPSkipOrgRoleSync bool @@ -497,25 +481,12 @@ type Cfg struct { // then Live uses AppURL as the only allowed origin. LiveAllowedOrigins []string - // GitHub OAuth - GitHubAuthEnabled bool - GitHubSkipOrgRoleSync bool - // Grafana.com URL, used for OAuth redirect. GrafanaComURL string // Grafana.com API URL. Can be set separately to GrafanaComURL // in case API is not publicly accessible. // Defaults to GrafanaComURL setting + "/api" if unset. GrafanaComAPIURL string - // Grafana.com Auth enabled - GrafanaComAuthEnabled bool - // GrafanaComSkipOrgRoleSync can be set for - // letting users set org roles from within Grafana and - // skip the org roles coming from GrafanaCom - GrafanaComSkipOrgRoleSync bool - - // Grafana.com Auth enabled through [auth.grafananet] config section - GrafanaNetAuthEnabled bool // Geomap base layer config GeomapDefaultBaseLayerConfig map[string]any @@ -538,10 +509,6 @@ type Cfg struct { SAMLSkipOrgRoleSync bool SAMLRoleValuesGrafanaAdmin string - // Okta OAuth - OktaAuthEnabled bool - OktaSkipOrgRoleSync bool - // OAuth2 Server OAuth2ServerEnabled bool @@ -1514,52 +1481,6 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error { return nil } -func readAuthAzureADSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.azuread") - cfg.AzureADEnabled = sec.Key("enabled").MustBool(false) - cfg.AzureADSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} - -func readAuthGrafanaComSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.grafana_com") - cfg.GrafanaComAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.GrafanaComSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} - -func readAuthGrafanaNetSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.grafananet") - cfg.GrafanaNetAuthEnabled = sec.Key("enabled").MustBool(false) -} - -func readAuthGithubSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.github") - cfg.GitHubAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.GitHubSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} - -func readAuthGoogleSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.google") - cfg.GoogleAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.GoogleSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(true) -} - -func readAuthGitlabSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.gitlab") - cfg.GitLabAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.GitLabSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} - -func readGenericOAuthSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.generic_oauth") - cfg.GenericOAuthAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.GenericOAuthSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} - -func readAuthOktaSettings(cfg *Cfg) { - sec := cfg.SectionWithEnvOverrides("auth.okta") - cfg.OktaAuthEnabled = sec.Key("enabled").MustBool(false) - cfg.OktaSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) -} func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { auth := iniFile.Section("auth") @@ -1629,27 +1550,6 @@ func readAuthSettings(iniFile *ini.File, cfg *Cfg) (err error) { cfg.IDResponseHeaderNamespaces[namespace] = struct{}{} } - readAuthAzureADSettings(cfg) - - // Google Auth - readAuthGoogleSettings(cfg) - - // GitLab Auth - readAuthGitlabSettings(cfg) - - // Generic OAuth - readGenericOAuthSettings(cfg) - - // Okta Auth - readAuthOktaSettings(cfg) - - // GrafanaCom - readAuthGrafanaComSettings(cfg) - readAuthGrafanaNetSettings(cfg) - - // Github - readAuthGithubSettings(cfg) - // anonymous access anonSection := iniFile.Section("auth.anonymous") cfg.AnonymousEnabled = anonSection.Key("enabled").MustBool(false)