diff --git a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md index cff8387c515..2598387920d 100644 --- a/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md +++ b/docs/sources/setup-grafana/configure-grafana/feature-toggles/index.md @@ -89,6 +89,7 @@ Alpha features might be changed or removed without prior notice. | `lokiQuerySplitting` | Split large interval queries into subqueries with smaller time intervals | | `lokiQuerySplittingConfig` | Give users the option to configure split durations for Loki queries | | `individualCookiePreferences` | Support overriding cookie preferences per user | +| `onlyExternalOrgRoleSync` | Prohibits a user from changing organization roles synced with external auth providers | | `drawerDataSourcePicker` | Changes the user experience for data source selection to a drawer. | | `traceqlSearch` | Enables the 'TraceQL Search' tab for the Tempo datasource which provides a UI to generate TraceQL queries | | `prometheusMetricEncyclopedia` | Replaces the Prometheus query builder metric select option with a paginated and filterable component | diff --git a/packages/grafana-data/src/types/featureToggles.gen.ts b/packages/grafana-data/src/types/featureToggles.gen.ts index 5032c1963b7..481f57b3e40 100644 --- a/packages/grafana-data/src/types/featureToggles.gen.ts +++ b/packages/grafana-data/src/types/featureToggles.gen.ts @@ -77,6 +77,7 @@ export interface FeatureToggles { lokiQuerySplitting?: boolean; lokiQuerySplittingConfig?: boolean; individualCookiePreferences?: boolean; + onlyExternalOrgRoleSync?: boolean; drawerDataSourcePicker?: boolean; traceqlSearch?: boolean; prometheusMetricEncyclopedia?: boolean; diff --git a/pkg/api/frontendsettings.go b/pkg/api/frontendsettings.go index ee87d065349..fbd626e9c80 100644 --- a/pkg/api/frontendsettings.go +++ b/pkg/api/frontendsettings.go @@ -100,7 +100,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro AppSubUrl: hs.Cfg.AppSubURL, AllowOrgCreate: (setting.AllowUserOrgCreate && c.IsSignedIn) || c.IsGrafanaAdmin, AuthProxyEnabled: hs.Cfg.AuthProxyEnabled, - LdapEnabled: hs.Cfg.LDAPEnabled, + LdapEnabled: hs.Cfg.LDAPAuthEnabled, JwtHeaderName: hs.Cfg.JWTAuthHeaderName, JwtUrlLogin: hs.Cfg.JWTAuthURLLogin, AlertingErrorOrTimeout: setting.AlertingErrorOrTimeout, @@ -148,7 +148,7 @@ func (hs *HTTPServer) getFrontendSettings(c *contextmodel.ReqContext) (*dtos.Fro GrafanaComSkipOrgRoleSync: hs.Cfg.GrafanaComSkipOrgRoleSync, GenericOAuthSkipOrgRoleSync: hs.Cfg.GenericOAuthSkipOrgRoleSync, AzureADSkipOrgRoleSync: hs.Cfg.AzureADSkipOrgRoleSync, - GithubSkipOrgRoleSync: hs.Cfg.GithubSkipOrgRoleSync, + GithubSkipOrgRoleSync: hs.Cfg.GitHubSkipOrgRoleSync, GitLabSkipOrgRoleSync: hs.Cfg.GitLabSkipOrgRoleSync, OktaSkipOrgRoleSync: hs.Cfg.OktaSkipOrgRoleSync, DisableSyncLock: hs.Cfg.DisableSyncLock, diff --git a/pkg/api/org_users.go b/pkg/api/org_users.go index c44561f6357..d1214817e3c 100644 --- a/pkg/api/org_users.go +++ b/pkg/api/org_users.go @@ -11,6 +11,7 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/services/accesscontrol" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/user" @@ -296,6 +297,7 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or userIDs[fmt.Sprint(user.UserID)] = true authLabelsUserIDs = append(authLabelsUserIDs, user.UserID) + filteredUsers = append(filteredUsers, user) } @@ -313,6 +315,7 @@ func (hs *HTTPServer) searchOrgUsersHelper(c *contextmodel.ReqContext, query *or filteredUsers[i].AccessControl = accessControlMetadata[fmt.Sprint(filteredUsers[i].UserID)] if module, ok := modules[filteredUsers[i].UserID]; ok { filteredUsers[i].AuthLabels = []string{login.GetAuthProviderLabel(module)} + filteredUsers[i].IsExternallySynced = login.IsExternallySynced(hs.Cfg, module) } } @@ -386,6 +389,22 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up if !c.OrgRole.Includes(cmd.Role) && !c.IsGrafanaAdmin { return response.Error(http.StatusForbidden, "Cannot assign a role higher than user's role", nil) } + if hs.Features.IsEnabled(featuremgmt.FlagOnlyExternalOrgRoleSync) { + // we do not allow to change role for external synced users + qAuth := login.GetAuthInfoQuery{UserId: cmd.UserID} + err := hs.authInfoService.GetAuthInfo(c.Req.Context(), &qAuth) + if err != nil { + if errors.Is(err, user.ErrUserNotFound) { + hs.log.Debug("Failed to get user auth info for basic auth user", cmd.UserID, nil) + } else { + hs.log.Error("Failed to get user auth info for external sync check", cmd.UserID, err) + return response.Error(http.StatusInternalServerError, "Failed to get user auth info", nil) + } + } + if qAuth.Result != nil && qAuth.Result.AuthModule != "" && login.IsExternallySynced(hs.Cfg, qAuth.Result.AuthModule) { + return response.Err(org.ErrCannotChangeRoleForExternallySyncedUser.Errorf("Cannot change role for externally synced user")) + } + } if err := hs.orgService.UpdateOrgUser(c.Req.Context(), &cmd); err != nil { if errors.Is(err, org.ErrLastOrgAdmin) { return response.Error(http.StatusBadRequest, "Cannot change role so that there is no organization admin left", nil) diff --git a/pkg/api/org_users_test.go b/pkg/api/org_users_test.go index af8901699a2..ce5c3b0d6be 100644 --- a/pkg/api/org_users_test.go +++ b/pkg/api/org_users_test.go @@ -16,10 +16,12 @@ 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/models/roletype" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/dashboards" "github.com/grafana/grafana/pkg/services/featuremgmt" + "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/login/logintest" "github.com/grafana/grafana/pkg/services/org" "github.com/grafana/grafana/pkg/services/org/orgimpl" @@ -201,6 +203,99 @@ func TestOrgUsersAPIEndpoint_userLoggedIn(t *testing.T) { }) } +func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) { + type testCase struct { + desc string + SkipOrgRoleSync bool + AuthEnabled bool + AuthModule string + expectedCode int + } + permissions := []accesscontrol.Permission{ + {Action: accesscontrol.ActionOrgUsersRead, Scope: "users:*"}, + {Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}, + {Action: accesscontrol.ActionOrgUsersAdd, Scope: "users:*"}, + {Action: accesscontrol.ActionOrgUsersRemove, Scope: "users:*"}, + } + tests := []testCase{ + { + desc: "should be able to change basicRole when skip_org_role_sync true", + SkipOrgRoleSync: true, + AuthEnabled: true, + AuthModule: login.LDAPAuthModule, + expectedCode: http.StatusOK, + }, + { + desc: "should not be able to change basicRole when skip_org_role_sync false", + SkipOrgRoleSync: false, + AuthEnabled: true, + AuthModule: login.LDAPAuthModule, + 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 be able to change basicRole with a basic Auth", + SkipOrgRoleSync: false, + AuthEnabled: false, + AuthModule: "", + expectedCode: http.StatusOK, + }, + { + desc: "should be able to change basicRole with a basic Auth", + SkipOrgRoleSync: true, + AuthEnabled: true, + AuthModule: "", + expectedCode: http.StatusOK, + }, + } + + userWithPermissions := userWithPermissions(1, permissions) + userRequesting := &user.User{ID: 2, OrgID: 1} + reqBody := `{"userId": "1", "role": "Admin", "orgId": "1"}` + for _, tt := range tests { + t.Run(tt.desc, func(t *testing.T) { + server := SetupAPITestServer(t, func(hs *HTTPServer) { + hs.Cfg = setting.NewCfg() + hs.Cfg.LDAPAuthEnabled = tt.AuthEnabled + 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 == "" { + // authmodule empty means basic auth + } else { + t.Errorf("invalid auth module for test: %s", tt.AuthModule) + } + + hs.authInfoService = &logintest.AuthInfoServiceFake{ + ExpectedUserAuth: &login.UserAuth{AuthModule: tt.AuthModule}, + } + hs.Features = featuremgmt.WithFeatures(featuremgmt.FlagOnlyExternalOrgRoleSync, true) + hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions} + hs.orgService = &orgtest.FakeOrgService{} + hs.accesscontrolService = &actest.FakeService{ + ExpectedPermissions: permissions, + } + }) + req := server.NewRequest(http.MethodPatch, fmt.Sprintf("/api/orgs/%d/users/%d", userRequesting.OrgID, userRequesting.ID), strings.NewReader(reqBody)) + req.Header.Set("Content-Type", "application/json") + userWithPermissions.OrgRole = roletype.RoleAdmin + res, err := server.Send(webtest.RequestWithSignedInUser(req, userWithPermissions)) + require.NoError(t, err) + assert.Equal(t, tt.expectedCode, res.StatusCode) + require.NoError(t, res.Body.Close()) + }) + } +} + func TestOrgUsersAPIEndpoint_LegacyAccessControl(t *testing.T) { type testCase struct { desc string @@ -630,7 +725,11 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) { server := SetupAPITestServer(t, func(hs *HTTPServer) { hs.Cfg = setting.NewCfg() hs.orgService = &orgtest.FakeOrgService{} - hs.authInfoService = &logintest.AuthInfoServiceFake{} + hs.authInfoService = &logintest.AuthInfoServiceFake{ + ExpectedUserAuth: &login.UserAuth{ + AuthModule: "", + }, + } hs.userService = &usertest.FakeUserService{ ExpectedUser: &user.User{}, ExpectedSignedInUser: userWithPermissions(1, tt.permissions), @@ -708,7 +807,11 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) { hs.Cfg = setting.NewCfg() hs.Cfg.RBACEnabled = tt.enableAccessControl hs.orgService = &orgtest.FakeOrgService{} - hs.authInfoService = &logintest.AuthInfoServiceFake{} + hs.authInfoService = &logintest.AuthInfoServiceFake{ + ExpectedUserAuth: &login.UserAuth{ + AuthModule: "", + }, + } hs.accesscontrolService = &actest.FakeService{} hs.userService = &usertest.FakeUserService{ ExpectedUser: &user.User{}, diff --git a/pkg/api/user.go b/pkg/api/user.go index 80057c67431..4b4fe913476 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -67,7 +67,7 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6 authLabel := login.GetAuthProviderLabel(getAuthQuery.Result.AuthModule) userProfile.AuthLabels = append(userProfile.AuthLabels, authLabel) userProfile.IsExternal = true - userProfile.IsExternallySynced = login.IsExternallySynced(hs.Cfg, authLabel) + userProfile.IsExternallySynced = login.IsExternallySynced(hs.Cfg, getAuthQuery.Result.AuthModule) } userProfile.AccessControl = hs.getAccessControlMetadata(c, c.OrgID, "global.users:id:", strconv.FormatInt(userID, 10)) diff --git a/pkg/infra/usagestats/service/usage_stats_test.go b/pkg/infra/usagestats/service/usage_stats_test.go index 1c5778031cd..0ec0264e76d 100644 --- a/pkg/infra/usagestats/service/usage_stats_test.go +++ b/pkg/infra/usagestats/service/usage_stats_test.go @@ -79,7 +79,7 @@ func TestMetrics(t *testing.T) { BuildVersion: "5.0.0", AnonymousEnabled: true, BasicAuthEnabled: true, - LDAPEnabled: true, + LDAPAuthEnabled: true, AuthProxyEnabled: true, Packaging: "deb", ReportingDistributor: "hosted-grafana", diff --git a/pkg/infra/usagestats/statscollector/service_test.go b/pkg/infra/usagestats/statscollector/service_test.go index ca34d3c5496..f9118c8d748 100644 --- a/pkg/infra/usagestats/statscollector/service_test.go +++ b/pkg/infra/usagestats/statscollector/service_test.go @@ -148,7 +148,7 @@ func TestCollectingUsageStats(t *testing.T) { BuildVersion: "5.0.0", AnonymousEnabled: true, BasicAuthEnabled: true, - LDAPEnabled: true, + LDAPAuthEnabled: true, AuthProxyEnabled: true, Packaging: "deb", ReportingDistributor: "hosted-grafana", @@ -213,7 +213,7 @@ func TestElasticStats(t *testing.T) { BuildVersion: "5.0.0", AnonymousEnabled: true, BasicAuthEnabled: true, - LDAPEnabled: true, + LDAPAuthEnabled: true, AuthProxyEnabled: true, Packaging: "deb", ReportingDistributor: "hosted-grafana", diff --git a/pkg/login/auth_test.go b/pkg/login/auth_test.go index a7a064a1d48..33557e03abd 100644 --- a/pkg/login/auth_test.go +++ b/pkg/login/auth_test.go @@ -122,7 +122,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and invalid ldap credentials", func(sc *authScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc) @@ -140,7 +140,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and valid ldap credentials", func(sc *authScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, nil, sc) @@ -158,7 +158,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When a non-existing grafana user authenticate and ldap returns unexpected error", func(sc *authScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true customErr := errors.New("custom") mockLoginUsingGrafanaDB(user.ErrUserNotFound, sc) mockLoginUsingLDAP(true, customErr, sc) @@ -177,7 +177,7 @@ func TestAuthenticateUser(t *testing.T) { authScenario(t, "When grafana user authenticate with invalid credentials and invalid ldap credentials", func(sc *authScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true mockLoginUsingGrafanaDB(ErrInvalidCredentials, sc) mockLoginUsingLDAP(true, ldap.ErrInvalidCredentials, sc) diff --git a/pkg/login/ldap_login.go b/pkg/login/ldap_login.go index 746ae2f7e80..43850946411 100644 --- a/pkg/login/ldap_login.go +++ b/pkg/login/ldap_login.go @@ -25,7 +25,7 @@ var ldapLogger = log.New("login.ldap") // populated with the logged in user if successful. var loginUsingLDAP = func(ctx context.Context, query *login.LoginUserQuery, loginService login.Service, cfg *setting.Cfg) (bool, error) { - if !cfg.LDAPEnabled { + if !cfg.LDAPAuthEnabled { return false, nil } diff --git a/pkg/login/ldap_login_test.go b/pkg/login/ldap_login_test.go index 42d0baf7ee3..9b6509f7796 100644 --- a/pkg/login/ldap_login_test.go +++ b/pkg/login/ldap_login_test.go @@ -20,7 +20,7 @@ var errTest = errors.New("test error") func TestLoginUsingLDAP(t *testing.T) { LDAPLoginScenario(t, "When LDAP enabled and no server configured", func(sc *LDAPLoginScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true sc.withLoginResult(false) getLDAPConfig = func(*setting.Cfg) (*ldap.Config, error) { @@ -41,7 +41,7 @@ func TestLoginUsingLDAP(t *testing.T) { LDAPLoginScenario(t, "When LDAP disabled", func(sc *LDAPLoginScenarioContext) { cfg := setting.NewCfg() - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false sc.withLoginResult(false) loginService := &logintest.LoginServiceFake{} diff --git a/pkg/login/social/social.go b/pkg/login/social/social.go index 3729e471cac..4921badabdc 100644 --- a/pkg/login/social/social.go +++ b/pkg/login/social/social.go @@ -156,7 +156,7 @@ func ProvideService(cfg *setting.Cfg, apiUrl: info.ApiUrl, teamIds: sec.Key("team_ids").Ints(","), allowedOrganizations: util.SplitString(sec.Key("allowed_organizations").String()), - skipOrgRoleSync: cfg.GithubSkipOrgRoleSync, + skipOrgRoleSync: cfg.GitHubSkipOrgRoleSync, } } diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index fcbe9edc190..1db8054301e 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -600,7 +600,7 @@ func TestMiddlewareContext(t *testing.T) { configure := func(cfg *setting.Cfg) { cfg.AuthProxyEnabled = true cfg.AuthProxyAutoSignUp = true - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true cfg.AuthProxyHeaderName = "X-WEBAUTH-USER" cfg.AuthProxyHeaderProperty = "username" cfg.AuthProxyHeaders = map[string]string{"Groups": "X-WEBAUTH-GROUPS", "Role": "X-WEBAUTH-ROLE"} @@ -645,7 +645,7 @@ func TestMiddlewareContext(t *testing.T) { assert.Nil(t, sc.context) }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false cfg.AuthProxyAutoSignUp = false }) @@ -666,7 +666,7 @@ func TestMiddlewareContext(t *testing.T) { require.Contains(t, list.Items, "X-WEBAUTH-ROLE") }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false cfg.AuthProxyAutoSignUp = true }) @@ -689,7 +689,7 @@ func TestMiddlewareContext(t *testing.T) { assert.Equal(t, orgRole, string(sc.context.OrgRole)) }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false cfg.AuthProxyAutoSignUp = true }) @@ -715,7 +715,7 @@ func TestMiddlewareContext(t *testing.T) { assert.Equal(t, "", string(sc.context.OrgRole)) }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false cfg.AuthProxyAutoSignUp = true }) @@ -733,7 +733,7 @@ func TestMiddlewareContext(t *testing.T) { assert.Equal(t, targetOrgID, sc.context.OrgID) }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false cfg.AuthProxyAutoSignUp = true }) @@ -807,7 +807,7 @@ func TestMiddlewareContext(t *testing.T) { assert.Equal(t, orgID, sc.context.OrgID) }, func(cfg *setting.Cfg) { configure(cfg) - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false }) middlewareScenario(t, "Should allow the request from whitelist IP", func(t *testing.T, sc *scenarioContext) { @@ -825,7 +825,7 @@ func TestMiddlewareContext(t *testing.T) { }, func(cfg *setting.Cfg) { configure(cfg) cfg.AuthProxyWhitelist = "192.168.1.0/24, 2001::0/120" - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false }) middlewareScenario(t, "Should not allow the request from whitelisted IP", func(t *testing.T, sc *scenarioContext) { @@ -841,7 +841,7 @@ func TestMiddlewareContext(t *testing.T) { }, func(cfg *setting.Cfg) { configure(cfg) cfg.AuthProxyWhitelist = "8.8.8.8" - cfg.LDAPEnabled = false + cfg.LDAPAuthEnabled = false }) middlewareScenario(t, "Should return 407 status code if LDAP says no", func(t *testing.T, sc *scenarioContext) { diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index dce5e78251e..f90cfdce514 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -90,7 +90,7 @@ func ProvideService( var proxyClients []authn.ProxyClient var passwordClients []authn.PasswordClient - if s.cfg.LDAPEnabled { + if s.cfg.LDAPAuthEnabled { ldap := clients.ProvideLDAP(cfg, ldapService) proxyClients = append(proxyClients, ldap) passwordClients = append(passwordClients, ldap) diff --git a/pkg/services/authn/authnimpl/usage_stats.go b/pkg/services/authn/authnimpl/usage_stats.go index aa086adefe2..086e7e3b50b 100644 --- a/pkg/services/authn/authnimpl/usage_stats.go +++ b/pkg/services/authn/authnimpl/usage_stats.go @@ -12,7 +12,7 @@ func (s *Service) getUsageStats(ctx context.Context) (map[string]interface{}, er // Add stats about auth configuration authTypes := map[string]bool{} authTypes["basic_auth"] = s.cfg.BasicAuthEnabled - authTypes["ldap"] = s.cfg.LDAPEnabled + authTypes["ldap"] = s.cfg.LDAPAuthEnabled authTypes["auth_proxy"] = s.cfg.AuthProxyEnabled authTypes["anonymous"] = s.cfg.AnonymousEnabled authTypes["jwt"] = s.cfg.JWTAuthEnabled diff --git a/pkg/services/authn/authnimpl/usage_stats_test.go b/pkg/services/authn/authnimpl/usage_stats_test.go index 4e6b2644032..81d61426202 100644 --- a/pkg/services/authn/authnimpl/usage_stats_test.go +++ b/pkg/services/authn/authnimpl/usage_stats_test.go @@ -22,7 +22,7 @@ func TestService_getUsageStats(t *testing.T) { svc.cfg.BasicAuthEnabled = true svc.cfg.AuthProxyEnabled = true svc.cfg.JWTAuthEnabled = true - svc.cfg.LDAPEnabled = true + svc.cfg.LDAPAuthEnabled = true svc.cfg.EditorsCanAdmin = true svc.cfg.ViewersCanEdit = true diff --git a/pkg/services/contexthandler/authproxy/authproxy.go b/pkg/services/contexthandler/authproxy/authproxy.go index 5f42274e6c7..c33a27326c7 100644 --- a/pkg/services/contexthandler/authproxy/authproxy.go +++ b/pkg/services/contexthandler/authproxy/authproxy.go @@ -164,7 +164,7 @@ func (auth *AuthProxy) Login(reqCtx *contextmodel.ReqContext, ignoreCache bool) } } - if auth.cfg.LDAPEnabled { + if auth.cfg.LDAPAuthEnabled { id, err := auth.LoginViaLDAP(reqCtx) if err != nil { if errors.Is(err, ldap.ErrInvalidCredentials) { diff --git a/pkg/services/contexthandler/authproxy/authproxy_test.go b/pkg/services/contexthandler/authproxy/authproxy_test.go index 57f3f55b170..29d8616b91f 100644 --- a/pkg/services/contexthandler/authproxy/authproxy_test.go +++ b/pkg/services/contexthandler/authproxy/authproxy_test.go @@ -107,7 +107,7 @@ func TestMiddlewareContext_ldap(t *testing.T) { cache := remotecache.NewFakeStore(t) auth, reqCtx := prepareMiddleware(t, cache, nil) - auth.cfg.LDAPEnabled = true + auth.cfg.LDAPAuthEnabled = true ldapFake := &service.LDAPFakeService{ ExpectedUser: &login.ExternalUserInfo{UserId: id}, } @@ -126,7 +126,7 @@ func TestMiddlewareContext_ldap(t *testing.T) { cache := remotecache.NewFakeStore(t) auth, reqCtx := prepareMiddleware(t, cache, nil) - auth.cfg.LDAPEnabled = true + auth.cfg.LDAPAuthEnabled = true ldapFake := &service.LDAPFakeService{ ExpectedUser: nil, ExpectedError: service.ErrUnableToCreateLDAPClient, diff --git a/pkg/services/featuremgmt/registry.go b/pkg/services/featuremgmt/registry.go index 8d0b2815b98..d7516b81d31 100644 --- a/pkg/services/featuremgmt/registry.go +++ b/pkg/services/featuremgmt/registry.go @@ -393,6 +393,12 @@ var ( State: FeatureStateAlpha, Owner: grafanaBackendPlatformSquad, }, + { + Name: "onlyExternalOrgRoleSync", + Description: "Prohibits a user from changing organization roles synced with external auth providers", + State: FeatureStateAlpha, + Owner: grafanaAuthnzSquad, + }, { Name: "drawerDataSourcePicker", Description: "Changes the user experience for data source selection to a drawer.", diff --git a/pkg/services/featuremgmt/toggles_gen.csv b/pkg/services/featuremgmt/toggles_gen.csv index 837acc653f4..c7d6321d583 100644 --- a/pkg/services/featuremgmt/toggles_gen.csv +++ b/pkg/services/featuremgmt/toggles_gen.csv @@ -58,6 +58,7 @@ logsContextDatasourceUi,alpha,@grafana/observability-logs,false,false,false,true lokiQuerySplitting,alpha,@grafana/observability-logs,false,false,false,true lokiQuerySplittingConfig,alpha,@grafana/observability-logs,false,false,false,true individualCookiePreferences,alpha,@grafana/backend-platform,false,false,false,false +onlyExternalOrgRoleSync,alpha,@grafana/grafana-authnz-team,false,false,false,false drawerDataSourcePicker,alpha,@grafana/grafana-bi-squad,false,false,false,true traceqlSearch,alpha,@grafana/observability-traces-and-profiling,false,false,false,true prometheusMetricEncyclopedia,alpha,@grafana/observability-metrics,false,false,false,true diff --git a/pkg/services/featuremgmt/toggles_gen.go b/pkg/services/featuremgmt/toggles_gen.go index e87054e5ead..e26943309e9 100644 --- a/pkg/services/featuremgmt/toggles_gen.go +++ b/pkg/services/featuremgmt/toggles_gen.go @@ -243,6 +243,10 @@ const ( // Support overriding cookie preferences per user FlagIndividualCookiePreferences = "individualCookiePreferences" + // FlagOnlyExternalOrgRoleSync + // Prohibits a user from changing organization roles synced with external auth providers + FlagOnlyExternalOrgRoleSync = "onlyExternalOrgRoleSync" + // FlagDrawerDataSourcePicker // Changes the user experience for data source selection to a drawer. FlagDrawerDataSourcePicker = "drawerDataSourcePicker" diff --git a/pkg/services/ldap/api/service.go b/pkg/services/ldap/api/service.go index 9506c9b3509..805fa9e0290 100644 --- a/pkg/services/ldap/api/service.go +++ b/pkg/services/ldap/api/service.go @@ -65,7 +65,7 @@ func ProvideService(cfg *setting.Cfg, router routing.RouteRegister, accessContro adminRoute.Get("/ldap/status", authorize(reqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)), routing.Wrap(s.GetLDAPStatus)) }, middleware.ReqSignedIn) - if cfg.LDAPEnabled { + if cfg.LDAPAuthEnabled { bundleRegistry.RegisterSupportItemCollector(supportbundles.Collector{ UID: "auth-ldap", DisplayName: "LDAP", @@ -94,7 +94,7 @@ func ProvideService(cfg *setting.Cfg, router routing.RouteRegister, accessContro // 403: forbiddenError // 500: internalServerError func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } @@ -120,7 +120,7 @@ func (s *Service) ReloadLDAPCfg(c *contextmodel.ReqContext) response.Response { // 403: forbiddenError // 500: internalServerError func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } @@ -167,7 +167,7 @@ func (s *Service) GetLDAPStatus(c *contextmodel.ReqContext) response.Response { // 403: forbiddenError // 500: internalServerError func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Response { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } @@ -262,7 +262,7 @@ func (s *Service) PostSyncUserWithLDAP(c *contextmodel.ReqContext) response.Resp // 403: forbiddenError // 500: internalServerError func (s *Service) GetUserFromLDAP(c *contextmodel.ReqContext) response.Response { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return response.Error(http.StatusBadRequest, "LDAP is not enabled", nil) } diff --git a/pkg/services/ldap/api/service_test.go b/pkg/services/ldap/api/service_test.go index 078fb65cee4..8d4140b88b9 100644 --- a/pkg/services/ldap/api/service_test.go +++ b/pkg/services/ldap/api/service_test.go @@ -60,7 +60,7 @@ func setupAPITest(t *testing.T, opts ...func(a *Service)) (*Service, *webtest.Se t.Helper() router := routing.NewRouteRegister() cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true a := ProvideService(cfg, router, diff --git a/pkg/services/ldap/api/support_bundle.go b/pkg/services/ldap/api/support_bundle.go index 414001ebe26..38ea6439024 100644 --- a/pkg/services/ldap/api/support_bundle.go +++ b/pkg/services/ldap/api/support_bundle.go @@ -72,7 +72,7 @@ func (s *Service) supportBundleCollector(context.Context) (*supportbundles.Suppo bWriter.WriteString("```ini\n") - bWriter.WriteString(fmt.Sprintf("enabled = %v\n", s.cfg.LDAPEnabled)) + bWriter.WriteString(fmt.Sprintf("enabled = %v\n", s.cfg.LDAPAuthEnabled)) bWriter.WriteString(fmt.Sprintf("config_file = %s\n", s.cfg.LDAPConfigFilePath)) bWriter.WriteString(fmt.Sprintf("allow_sign_up = %v\n", s.cfg.LDAPAllowSignup)) bWriter.WriteString(fmt.Sprintf("sync_cron = %s\n", s.cfg.LDAPSyncCron)) diff --git a/pkg/services/ldap/ldap_login_test.go b/pkg/services/ldap/ldap_login_test.go index 5f987353692..a3feb6d9734 100644 --- a/pkg/services/ldap/ldap_login_test.go +++ b/pkg/services/ldap/ldap_login_test.go @@ -32,7 +32,7 @@ func TestServer_Login_UserBind_Fail(t *testing.T) { } cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, Config: &ServerConfig{ @@ -106,7 +106,7 @@ func TestServer_Login_ValidCredentials(t *testing.T) { } cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -143,7 +143,7 @@ func TestServer_Login_UnauthenticatedBind(t *testing.T) { } cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -190,7 +190,7 @@ func TestServer_Login_AuthenticatedBind(t *testing.T) { } cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -233,7 +233,7 @@ func TestServer_Login_UserWildcardBind(t *testing.T) { } cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, diff --git a/pkg/services/ldap/ldap_private_test.go b/pkg/services/ldap/ldap_private_test.go index 132e1cfd0be..24e1f3efe4c 100644 --- a/pkg/services/ldap/ldap_private_test.go +++ b/pkg/services/ldap/ldap_private_test.go @@ -54,7 +54,7 @@ func TestServer_getSearchRequest(t *testing.T) { func TestSerializeUsers(t *testing.T) { t.Run("simple case", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -93,7 +93,7 @@ func TestSerializeUsers(t *testing.T) { t.Run("without lastname", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -130,7 +130,7 @@ func TestSerializeUsers(t *testing.T) { t.Run("mark user without matching group as disabled", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -164,7 +164,7 @@ func TestSerializeUsers(t *testing.T) { func TestServer_validateGrafanaUser(t *testing.T) { t.Run("no group config", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -184,7 +184,7 @@ func TestServer_validateGrafanaUser(t *testing.T) { t.Run("user in group", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -211,7 +211,7 @@ func TestServer_validateGrafanaUser(t *testing.T) { t.Run("user not in group", func(t *testing.T) { cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, diff --git a/pkg/services/ldap/ldap_test.go b/pkg/services/ldap/ldap_test.go index 3ee4d1f8626..8e37c456e2e 100644 --- a/pkg/services/ldap/ldap_test.go +++ b/pkg/services/ldap/ldap_test.go @@ -68,7 +68,7 @@ func TestServer_Users(t *testing.T) { // Set up attribute map without surname and email cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -212,7 +212,7 @@ func TestServer_Users(t *testing.T) { }) cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, @@ -289,7 +289,7 @@ func TestServer_Users(t *testing.T) { }) cfg := setting.NewCfg() - cfg.LDAPEnabled = true + cfg.LDAPAuthEnabled = true server := &Server{ cfg: cfg, diff --git a/pkg/services/ldap/service/ldap.go b/pkg/services/ldap/service/ldap.go index ce7650c786f..8378a9d5570 100644 --- a/pkg/services/ldap/service/ldap.go +++ b/pkg/services/ldap/service/ldap.go @@ -47,7 +47,7 @@ func ProvideService(cfg *setting.Cfg) *LDAPImpl { loadingMutex: &sync.Mutex{}, } - if !cfg.LDAPEnabled { + if !cfg.LDAPAuthEnabled { return s } @@ -63,7 +63,7 @@ func ProvideService(cfg *setting.Cfg) *LDAPImpl { } func (s *LDAPImpl) ReloadConfig() error { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return nil } @@ -95,7 +95,7 @@ func (s *LDAPImpl) Config() *ldap.Config { } func (s *LDAPImpl) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, error) { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return nil, ErrLDAPNotEnabled } @@ -108,7 +108,7 @@ func (s *LDAPImpl) Login(query *login.LoginUserQuery) (*login.ExternalUserInfo, } func (s *LDAPImpl) User(username string) (*login.ExternalUserInfo, error) { - if !s.cfg.LDAPEnabled { + if !s.cfg.LDAPAuthEnabled { return nil, ErrLDAPNotEnabled } diff --git a/pkg/services/ldap/settings.go b/pkg/services/ldap/settings.go index a9acde52b5f..142a17d854c 100644 --- a/pkg/services/ldap/settings.go +++ b/pkg/services/ldap/settings.go @@ -87,10 +87,10 @@ var config *Config // the config or it reads it and caches it first. func GetConfig(cfg *setting.Cfg) (*Config, error) { if cfg != nil { - if !cfg.LDAPEnabled { + if !cfg.LDAPAuthEnabled { return nil, nil } - } else if !cfg.LDAPEnabled { + } else if !cfg.LDAPAuthEnabled { return nil, nil } diff --git a/pkg/services/login/authinfo.go b/pkg/services/login/authinfo.go index e92a0fcfb6c..bca4807821d 100644 --- a/pkg/services/login/authinfo.go +++ b/pkg/services/login/authinfo.go @@ -56,7 +56,7 @@ const ( LDAPLabel = "LDAP" JWTLabel = "JWT" // OAuth provider labels - AuthProxtLabel = "Auth Proxy" + AuthProxyLabel = "Auth Proxy" AzureADLabel = "AzureAD" GoogleLabel = "Google" GenericOAuthLabel = "Generic OAuth" @@ -72,14 +72,18 @@ 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, autoProviderLabel string) bool { +func IsExternallySynced(cfg *setting.Cfg, authModule string) bool { + // provider enabled in config + if !IsProviderEnabled(cfg, authModule) { + return false + } // first check SAML, LDAP and JWT - switch autoProviderLabel { - case SAMLLabel: + switch authModule { + case SAMLAuthModule: return !cfg.SAMLSkipOrgRoleSync - case LDAPLabel: + case LDAPAuthModule: return !cfg.LDAPSkipOrgRoleSync - case JWTLabel: + case JWTModule: return !cfg.JWTAuthSkipOrgRoleSync } // then check the rest of the oauth providers @@ -88,25 +92,52 @@ func IsExternallySynced(cfg *setting.Cfg, autoProviderLabel string) bool { if cfg.OAuthSkipOrgRoleUpdateSync { return false } - switch autoProviderLabel { - case GoogleLabel: + switch authModule { + case GoogleAuthModule: return !cfg.GoogleSkipOrgRoleSync - case OktaLabel: + case OktaAuthModule: return !cfg.OktaSkipOrgRoleSync - case AzureADLabel: + case AzureADAuthModule: return !cfg.AzureADSkipOrgRoleSync - case GitLabLabel: + case GitLabAuthModule: return !cfg.GitLabSkipOrgRoleSync - case GithubLabel: - return !cfg.GithubSkipOrgRoleSync - case GrafanaComLabel: + case GithubAuthModule: + return !cfg.GitHubSkipOrgRoleSync + case GrafanaComAuthModule: return !cfg.GrafanaComSkipOrgRoleSync - case GenericOAuthLabel: + case GenericOAuthModule: return !cfg.GenericOAuthSkipOrgRoleSync } return true } +func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool { + switch authModule { + case SAMLAuthModule: + return cfg.SAMLAuthEnabled + case LDAPAuthModule: + 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 + case GenericOAuthModule: + return cfg.GenericOAuthAuthEnabled + } + return false +} + +// used for frontend to display a more user friendly label func GetAuthProviderLabel(authModule string) string { switch authModule { case GithubAuthModule: @@ -128,7 +159,7 @@ func GetAuthProviderLabel(authModule string) string { case JWTModule: return JWTLabel case AuthProxyAuthModule: - return AuthProxtLabel + return AuthProxyLabel case GenericOAuthModule: return GenericOAuthLabel default: diff --git a/pkg/services/login/authinfo_test.go b/pkg/services/login/authinfo_test.go index 45a1ff1c37e..f65e24662ba 100644 --- a/pkg/services/login/authinfo_test.go +++ b/pkg/services/login/authinfo_test.go @@ -14,182 +14,199 @@ func TestIsExternallySynced(t *testing.T) { provider string expected bool }{ + // azure { name: "AzureAD synced user should return that it is externally synced", - cfg: &setting.Cfg{AzureADSkipOrgRoleSync: false}, - provider: AzureADLabel, + 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{AzureADSkipOrgRoleSync: true}, - provider: AzureADLabel, + 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{AzureADSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: AzureADLabel, + 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{GoogleSkipOrgRoleSync: false}, - provider: GoogleLabel, + 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{GoogleSkipOrgRoleSync: true}, - provider: GoogleLabel, + 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{GoogleSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GoogleLabel, + 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{GoogleSkipOrgRoleSync: true, OAuthSkipOrgRoleUpdateSync: true}, - provider: GoogleLabel, + 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{OktaSkipOrgRoleSync: false}, - provider: OktaLabel, + 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{OktaSkipOrgRoleSync: true}, + cfg: &setting.Cfg{OktaAuthEnabled: true, OktaSkipOrgRoleSync: true}, - provider: OktaLabel, + 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{OktaSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: OktaLabel, + 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{GithubSkipOrgRoleSync: false}, - provider: GithubLabel, + cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false}, + provider: GithubAuthModule, expected: true, }, { name: "Github synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GithubSkipOrgRoleSync: true}, - provider: GithubLabel, + cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: true}, + provider: GithubAuthModule, 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{GithubSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GithubLabel, + cfg: &setting.Cfg{GitHubAuthEnabled: true, GitHubSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, + provider: GithubAuthModule, expected: false, }, // gitlab { name: "Gitlab synced user should return that it is externally synced", - cfg: &setting.Cfg{GitLabSkipOrgRoleSync: false}, - provider: GitLabLabel, + cfg: &setting.Cfg{GitLabAuthEnabled: true, GitLabSkipOrgRoleSync: false}, + provider: GitLabAuthModule, expected: true, }, { name: "Gitlab synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{GitLabSkipOrgRoleSync: true}, - provider: GitLabLabel, + 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{GitLabSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GitLabLabel, + 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{GrafanaComSkipOrgRoleSync: false}, - provider: GrafanaComLabel, + 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{GrafanaComSkipOrgRoleSync: true}, - provider: GrafanaComLabel, + 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{GrafanaComSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GrafanaComLabel, + cfg: &setting.Cfg{GrafanaComAuthEnabled: true, GrafanaComSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, + provider: GrafanaComAuthModule, expected: false, }, + // generic oauth { - name: "Generic OAuth synced user should return that it is externally synced", - cfg: &setting.Cfg{OAuthSkipOrgRoleUpdateSync: false}, - provider: GenericOAuthLabel, + 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: "Generic OAuth synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{OAuthSkipOrgRoleUpdateSync: true}, - provider: GenericOAuthLabel, + 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{GenericOAuthSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, - provider: GenericOAuthLabel, + cfg: &setting.Cfg{GenericOAuthAuthEnabled: true, GenericOAuthSkipOrgRoleSync: false, OAuthSkipOrgRoleUpdateSync: true}, + provider: GenericOAuthModule, expected: false, }, + // saml { name: "SAML synced user should return that it is externally synced", - cfg: &setting.Cfg{SAMLSkipOrgRoleSync: false}, - provider: SAMLLabel, + cfg: &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: false}, + provider: SAMLAuthModule, expected: true, }, { name: "SAML synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{SAMLSkipOrgRoleSync: true}, - provider: SAMLLabel, + cfg: &setting.Cfg{SAMLAuthEnabled: true, SAMLSkipOrgRoleSync: true}, + provider: SAMLAuthModule, expected: false, }, + // ldap { name: "LDAP synced user should return that it is externally synced", - cfg: &setting.Cfg{LDAPSkipOrgRoleSync: false}, - provider: LDAPLabel, + cfg: &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: false}, + provider: LDAPAuthModule, expected: true, }, { name: "LDAP synced user should return that it is not externally synced when org role sync is set", - cfg: &setting.Cfg{LDAPSkipOrgRoleSync: true}, - provider: LDAPLabel, + cfg: &setting.Cfg{LDAPAuthEnabled: true, LDAPSkipOrgRoleSync: true}, + provider: LDAPAuthModule, expected: false, }, + // jwt { name: "JWT synced user should return that it is externally synced", - cfg: &setting.Cfg{JWTAuthSkipOrgRoleSync: false}, - provider: JWTLabel, + cfg: &setting.Cfg{JWTAuthEnabled: true, JWTAuthSkipOrgRoleSync: false}, + provider: JWTModule, expected: true, }, { name: "JWT synced user should return that it is not externally synced when org role sync is set", + cfg: &setting.Cfg{JWTAuthEnabled: true, JWTAuthSkipOrgRoleSync: true}, + provider: JWTModule, + expected: false, + }, + // IsProvider test + { + name: "If no provider enabled should return false", cfg: &setting.Cfg{JWTAuthSkipOrgRoleSync: true}, - provider: JWTLabel, + provider: JWTModule, expected: false, }, } @@ -200,3 +217,32 @@ func TestIsExternallySynced(t *testing.T) { }) } } + +func TestIsProviderEnabled(t *testing.T) { + testcases := []struct { + name string + cfg *setting.Cfg + 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 false if not enabled", + cfg: &setting.Cfg{}, + 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)) + }) + } +} diff --git a/pkg/services/navtree/navtreeimpl/admin.go b/pkg/services/navtree/navtreeimpl/admin.go index 0fd2df0146f..35dbda6d71f 100644 --- a/pkg/services/navtree/navtreeimpl/admin.go +++ b/pkg/services/navtree/navtreeimpl/admin.go @@ -159,7 +159,7 @@ func (s *ServiceImpl) getServerAdminNode(c *contextmodel.ReqContext) *navtree.Na adminNavLinks = append(adminNavLinks, storage) } - if s.cfg.LDAPEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) { + if s.cfg.LDAPAuthEnabled && hasAccess(ac.ReqGrafanaAdmin, ac.EvalPermission(ac.ActionLDAPStatusRead)) { adminNavLinks = append(adminNavLinks, &navtree.NavLink{ Text: "LDAP", Id: "ldap", Url: s.cfg.AppSubURL + "/admin/ldap", Icon: "book", }) diff --git a/pkg/services/org/model.go b/pkg/services/org/model.go index f523172b917..ba3379ee4ad 100644 --- a/pkg/services/org/model.go +++ b/pkg/services/org/model.go @@ -12,11 +12,12 @@ import ( // Typed errors var ( - ErrOrgNameTaken = errors.New("organization name is taken") - ErrLastOrgAdmin = errors.New("cannot remove last organization admin") - ErrOrgUserNotFound = errors.New("cannot find the organization user") - ErrOrgUserAlreadyAdded = errors.New("user is already added to organization") - ErrOrgNotFound = errutil.NewBase(errutil.StatusNotFound, "org.notFound", errutil.WithPublicMessage("organization not found")) + ErrOrgNameTaken = errors.New("organization name is taken") + ErrLastOrgAdmin = errors.New("cannot remove last organization admin") + ErrOrgUserNotFound = errors.New("cannot find the organization user") + ErrOrgUserAlreadyAdded = errors.New("user is already added to organization") + ErrOrgNotFound = errutil.NewBase(errutil.StatusNotFound, "org.notFound", errutil.WithPublicMessage("organization not found")) + ErrCannotChangeRoleForExternallySyncedUser = errutil.NewBase(errutil.StatusForbidden, "org.externallySynced", errutil.WithPublicMessage("cannot change role for externally synced user")) ) type Org struct { @@ -139,20 +140,21 @@ type UpdateOrgUserCommand struct { } type OrgUserDTO struct { - OrgID int64 `json:"orgId" xorm:"org_id"` - UserID int64 `json:"userId" xorm:"user_id"` - Email string `json:"email"` - Name string `json:"name"` - AvatarURL string `json:"avatarUrl" xorm:"avatar_url"` - Login string `json:"login"` - Role string `json:"role"` - LastSeenAt time.Time `json:"lastSeenAt"` - Updated time.Time `json:"-"` - Created time.Time `json:"-"` - LastSeenAtAge string `json:"lastSeenAtAge"` - AccessControl map[string]bool `json:"accessControl,omitempty"` - IsDisabled bool `json:"isDisabled"` - AuthLabels []string `json:"authLabels" xorm:"-"` + OrgID int64 `json:"orgId" xorm:"org_id"` + UserID int64 `json:"userId" xorm:"user_id"` + Email string `json:"email"` + Name string `json:"name"` + AvatarURL string `json:"avatarUrl" xorm:"avatar_url"` + Login string `json:"login"` + Role string `json:"role"` + LastSeenAt time.Time `json:"lastSeenAt"` + Updated time.Time `json:"-"` + Created time.Time `json:"-"` + LastSeenAtAge string `json:"lastSeenAtAge"` + AccessControl map[string]bool `json:"accessControl,omitempty"` + IsDisabled bool `json:"isDisabled"` + AuthLabels []string `json:"authLabels" xorm:"-"` + IsExternallySynced bool `json:"isExternallySynced"` } type RemoveOrgUserCommand struct { diff --git a/pkg/setting/setting.go b/pkg/setting/setting.go index d2b36676d19..589ca001917 100644 --- a/pkg/setting/setting.go +++ b/pkg/setting/setting.go @@ -405,19 +405,23 @@ type Cfg struct { IntercomSecret string // AzureAD + AzureADEnabled bool AzureADSkipOrgRoleSync bool // Google + GoogleAuthEnabled bool GoogleSkipOrgRoleSync bool // Gitlab + GitLabAuthEnabled bool GitLabSkipOrgRoleSync bool // Generic OAuth + GenericOAuthAuthEnabled bool GenericOAuthSkipOrgRoleSync bool // LDAP - LDAPEnabled bool + LDAPAuthEnabled bool LDAPSkipOrgRoleSync bool LDAPConfigFilePath string LDAPAllowSignup bool @@ -453,8 +457,9 @@ type Cfg struct { // then Live uses AppURL as the only allowed origin. LiveAllowedOrigins []string - // Github OAuth - GithubSkipOrgRoleSync bool + // GitHub OAuth + GitHubAuthEnabled bool + GitHubSkipOrgRoleSync bool // Grafana.com URL, used for OAuth redirect. GrafanaComURL string @@ -462,6 +467,8 @@ type Cfg struct { // 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 @@ -486,9 +493,11 @@ type Cfg struct { SecureSocksDSProxy SecureSocksDSProxySettings // SAML Auth + SAMLAuthEnabled bool SAMLSkipOrgRoleSync bool // Okta OAuth + OktaAuthEnabled bool OktaSkipOrgRoleSync bool // Access Control @@ -1190,6 +1199,7 @@ type RemoteCacheOptions struct { func (cfg *Cfg) readSAMLConfig() { samlSec := cfg.Raw.Section("auth.saml") + cfg.SAMLAuthEnabled = samlSec.Key("enabled").MustBool(false) cfg.SAMLSkipOrgRoleSync = samlSec.Key("skip_org_role_sync").MustBool(false) } @@ -1197,7 +1207,7 @@ func (cfg *Cfg) readLDAPConfig() { ldapSec := cfg.Raw.Section("auth.ldap") cfg.LDAPConfigFilePath = ldapSec.Key("config_file").String() cfg.LDAPSyncCron = ldapSec.Key("sync_cron").String() - cfg.LDAPEnabled = ldapSec.Key("enabled").MustBool(false) + cfg.LDAPAuthEnabled = ldapSec.Key("enabled").MustBool(false) cfg.LDAPSkipOrgRoleSync = ldapSec.Key("skip_org_role_sync").MustBool(false) cfg.LDAPActiveSyncEnabled = ldapSec.Key("active_sync_enabled").MustBool(false) cfg.LDAPAllowSignup = ldapSec.Key("allow_sign_up").MustBool(true) @@ -1376,36 +1386,43 @@ func readSecuritySettings(iniFile *ini.File, cfg *Cfg) error { } func readAuthAzureADSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.azuread") + cfg.AzureADEnabled = sec.Key("enabled").MustBool(false) cfg.AzureADSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readAuthGrafanaComSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.grafana_com") + cfg.GrafanaComAuthEnabled = sec.Key("enabled").MustBool(false) cfg.GrafanaComSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readAuthGithubSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.github") - cfg.GithubSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) + cfg.GitHubAuthEnabled = sec.Key("enabled").MustBool(false) + cfg.GitHubSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readAuthGoogleSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.google") + cfg.GoogleAuthEnabled = sec.Key("enabled").MustBool(false) cfg.GoogleSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readAuthGitlabSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.gitlab") + cfg.GitLabAuthEnabled = sec.Key("enabled").MustBool(false) cfg.GitLabSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readGenericOAuthSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.generic_oauth") + cfg.GenericOAuthAuthEnabled = sec.Key("enabled").MustBool(false) cfg.GenericOAuthSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } func readAuthOktaSettings(iniFile *ini.File, cfg *Cfg) { sec := iniFile.Section("auth.okta") + cfg.OktaAuthEnabled = sec.Key("enabled").MustBool(false) cfg.OktaSkipOrgRoleSync = sec.Key("skip_org_role_sync").MustBool(false) } diff --git a/public/app/features/admin/UserListAdminPage.tsx b/public/app/features/admin/UserListAdminPage.tsx index fde3b25c827..a1c6e1f0327 100644 --- a/public/app/features/admin/UserListAdminPage.tsx +++ b/public/app/features/admin/UserListAdminPage.tsx @@ -150,7 +150,7 @@ const UserListAdminPageUnConnected = ({ - Synced from + Origin diff --git a/public/app/features/admin/UserOrgs.tsx b/public/app/features/admin/UserOrgs.tsx index 3a97685c318..01d334a0b3b 100644 --- a/public/app/features/admin/UserOrgs.tsx +++ b/public/app/features/admin/UserOrgs.tsx @@ -426,6 +426,9 @@ const getChangeOrgButtonTheme = (theme: GrafanaTheme2) => ({ margin-left: 1.8rem; margin-right: 0.6rem; `, + icon: css` + line-height: 2; + `, }); export function ChangeOrgButton({ @@ -443,6 +446,7 @@ export function ChangeOrgButton({ {lockMessage} This user's role is not editable because it is synchronized from your auth provider. Refer to @@ -459,7 +463,9 @@ export function ChangeOrgButton({ } > - +
+ +
) : ( diff --git a/public/app/features/users/UsersTable.tsx b/public/app/features/users/UsersTable.tsx index ac8f6c7c917..de46b1989b3 100644 --- a/public/app/features/users/UsersTable.tsx +++ b/public/app/features/users/UsersTable.tsx @@ -5,6 +5,7 @@ import { Button, ConfirmModal } from '@grafana/ui'; import { UserRolePicker } from 'app/core/components/RolePicker/UserRolePicker'; import { fetchRoleOptions } from 'app/core/components/RolePicker/api'; import { TagBadge } from 'app/core/components/TagFilter/TagBadge'; +import config from 'app/core/config'; import { contextSrv } from 'app/core/core'; import { AccessControlAction, OrgUser, Role } from 'app/types'; @@ -49,12 +50,17 @@ export const UsersTable = ({ users, orgId, onRoleChange, onRemoveUser }: Props) Seen Role - + Origin {users.map((user, index) => { + let basicRoleDisabled = !contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user); + if (config.featureToggles.onlyExternalOrgRoleSync) { + const isUserSynced = user?.isExternallySynced; + basicRoleDisabled = isUserSynced || basicRoleDisabled; + } return ( @@ -86,13 +92,13 @@ export const UsersTable = ({ users, orgId, onRoleChange, onRemoveUser }: Props) roleOptions={roleOptions} basicRole={user.role} onBasicRoleChange={(newRole) => onRoleChange(newRole, user)} - basicRoleDisabled={!contextSrv.hasPermissionInMetadata(AccessControlAction.OrgUsersWrite, user)} + basicRoleDisabled={basicRoleDisabled} /> ) : ( onRoleChange(newRole, user)} /> )} diff --git a/public/app/types/user.ts b/public/app/types/user.ts index 94fee64d417..694c897a76f 100644 --- a/public/app/types/user.ts +++ b/public/app/types/user.ts @@ -13,6 +13,7 @@ export interface OrgUser extends WithAccessControlMetadata { userId: number; isDisabled: boolean; authLabels?: string[]; + isExternallySynced?: boolean; } export interface User {