diff --git a/pkg/api/admin_users.go b/pkg/api/admin_users.go index bfd6e96cda0..4c76c026c6b 100644 --- a/pkg/api/admin_users.go +++ b/pkg/api/admin_users.go @@ -167,6 +167,17 @@ func (hs *HTTPServer) AdminUpdateUserPermissions(c *contextmodel.ReqContext) res return response.Error(http.StatusBadRequest, "id is invalid", err) } + 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) { + return response.Error(http.StatusForbidden, "Cannot change Grafana Admin role for externally synced user", nil) + } + } + err = hs.userService.UpdatePermissions(c.Req.Context(), userID, form.IsGrafanaAdmin) if err != nil { if errors.Is(err, user.ErrLastGrafanaAdmin) { diff --git a/pkg/api/admin_users_test.go b/pkg/api/admin_users_test.go index 27f715384db..2c2f8a49aa7 100644 --- a/pkg/api/admin_users_test.go +++ b/pkg/api/admin_users_test.go @@ -2,6 +2,7 @@ package api import ( "fmt" + "net/http" "testing" "github.com/stretchr/testify/assert" @@ -13,9 +14,12 @@ import ( "github.com/grafana/grafana/pkg/components/simplejson" "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/socialtest" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/authtest" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" + "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/user" @@ -231,6 +235,122 @@ func TestAdminAPIEndpoint(t *testing.T) { }) } +func Test_AdminUpdateUserPermissions(t *testing.T) { + testcases := []struct { + name string + authModule string + allowAssignGrafanaAdmin bool + authEnabled bool + skipOrgRoleSync bool + expectedRespCode int + }{ + { + name: "Should allow updating an externally synced OAuth user if Grafana Admin role is not synced", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: false, + skipOrgRoleSync: false, + expectedRespCode: http.StatusOK, + }, + { + name: "Should allow updating an externally synced OAuth user if OAuth provider is not enabled", + authModule: login.GenericOAuthModule, + authEnabled: false, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedRespCode: http.StatusOK, + }, + { + name: "Should allow updating an externally synced OAuth user if org roles are not being synced", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: true, + expectedRespCode: http.StatusOK, + }, + { + name: "Should not allow updating an externally synced OAuth user", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedRespCode: http.StatusForbidden, + }, + { + name: "Should allow updating an externally synced JWT user if Grafana Admin role is not synced", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: false, + skipOrgRoleSync: false, + expectedRespCode: http.StatusOK, + }, + { + name: "Should allow updating an externally synced JWT user if JWT provider is not enabled", + authModule: login.JWTModule, + authEnabled: false, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedRespCode: http.StatusOK, + }, + { + name: "Should allow updating an externally synced JWT user if org roles are not being synced", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: true, + expectedRespCode: http.StatusOK, + }, + { + name: "Should not allow updating an externally synced JWT user", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedRespCode: http.StatusForbidden, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + userAuth := &login.UserAuth{AuthModule: tc.authModule} + authInfoService := &logintest.AuthInfoServiceFake{ExpectedUserAuth: userAuth} + socialService := &socialtest.FakeSocialService{} + cfg := setting.NewCfg() + + switch tc.authModule { + case login.GenericOAuthModule: + socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled} + cfg.GenericOAuthAuthEnabled = tc.authEnabled + cfg.GenericOAuthSkipOrgRoleSync = tc.skipOrgRoleSync + case login.JWTModule: + cfg.JWTAuthEnabled = tc.authEnabled + cfg.JWTAuthSkipOrgRoleSync = tc.skipOrgRoleSync + cfg.JWTAuthAllowAssignGrafanaAdmin = tc.allowAssignGrafanaAdmin + } + + hs := &HTTPServer{ + Cfg: cfg, + authInfoService: authInfoService, + SocialService: socialService, + userService: usertest.NewUserServiceFake(), + } + + sc := setupScenarioContext(t, "/api/admin/users/1/permissions") + sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { + c.Req.Body = mockRequestBody(dtos.AdminUpdateUserPermissionsForm{IsGrafanaAdmin: true}) + c.Req.Header.Add("Content-Type", "application/json") + sc.context = c + return hs.AdminUpdateUserPermissions(c) + }) + + sc.m.Put("/api/admin/users/:id/permissions", sc.defaultHandler) + + sc.fakeReqWithParams("PUT", sc.url, map[string]string{}).exec() + + assert.Equal(t, tc.expectedRespCode, sc.resp.Code) + }) + } +} + func putAdminScenario(t *testing.T, desc string, url string, routePattern string, role org.RoleType, cmd dtos.AdminUpdateUserPermissionsForm, fn scenarioFunc, sqlStore db.DB, userSvc user.Service) { t.Run(fmt.Sprintf("%s %s", desc, url), func(t *testing.T) { diff --git a/pkg/api/user.go b/pkg/api/user.go index a6b5ae38022..7ce89333105 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -68,6 +68,11 @@ func (hs *HTTPServer) getUserUserProfile(c *contextmodel.ReqContext, userID int6 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) } userProfile.AccessControl = hs.getAccessControlMetadata(c, c.OrgID, "global.users:id:", strconv.FormatInt(userID, 10)) diff --git a/pkg/api/user_test.go b/pkg/api/user_test.go index f0a396d7111..e55105145f6 100644 --- a/pkg/api/user_test.go +++ b/pkg/api/user_test.go @@ -19,6 +19,8 @@ import ( "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/db/dbtest" "github.com/grafana/grafana/pkg/infra/usagestats" + "github.com/grafana/grafana/pkg/login/social" + "github.com/grafana/grafana/pkg/login/socialtest" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" @@ -216,6 +218,125 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) { }, mock) } +func Test_GetUserByID(t *testing.T) { + testcases := []struct { + name string + authModule string + allowAssignGrafanaAdmin bool + authEnabled bool + skipOrgRoleSync bool + expectedIsGrafanaAdminSynced bool + }{ + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if Grafana Admin role is not synced", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: false, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if OAuth provider is not enabled", + authModule: login.GenericOAuthModule, + authEnabled: false, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced OAuth user if org roles are not being synced", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: true, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = true for an externally synced OAuth user", + authModule: login.GenericOAuthModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: true, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if Grafana Admin role is not synced", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: false, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if JWT provider is not enabled", + authModule: login.JWTModule, + authEnabled: false, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = false for an externally synced JWT user if org roles are not being synced", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: true, + expectedIsGrafanaAdminSynced: false, + }, + { + name: "Should return IsGrafanaAdminExternallySynced = true for an externally synced JWT user", + authModule: login.JWTModule, + authEnabled: true, + allowAssignGrafanaAdmin: true, + skipOrgRoleSync: false, + expectedIsGrafanaAdminSynced: true, + }, + } + for _, tc := range testcases { + t.Run(tc.name, func(t *testing.T) { + userAuth := &login.UserAuth{AuthModule: tc.authModule} + authInfoService := &logintest.AuthInfoServiceFake{ExpectedUserAuth: userAuth} + socialService := &socialtest.FakeSocialService{} + userService := &usertest.FakeUserService{ExpectedUserProfileDTO: &user.UserProfileDTO{}} + cfg := setting.NewCfg() + + switch tc.authModule { + case login.GenericOAuthModule: + socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{AllowAssignGrafanaAdmin: tc.allowAssignGrafanaAdmin, Enabled: tc.authEnabled} + cfg.GenericOAuthAuthEnabled = tc.authEnabled + cfg.GenericOAuthSkipOrgRoleSync = tc.skipOrgRoleSync + case login.JWTModule: + cfg.JWTAuthEnabled = tc.authEnabled + cfg.JWTAuthSkipOrgRoleSync = tc.skipOrgRoleSync + cfg.JWTAuthAllowAssignGrafanaAdmin = tc.allowAssignGrafanaAdmin + } + + hs := &HTTPServer{ + Cfg: cfg, + authInfoService: authInfoService, + SocialService: socialService, + userService: userService, + } + + sc := setupScenarioContext(t, "/api/users/1") + sc.defaultHandler = routing.Wrap(func(c *contextmodel.ReqContext) response.Response { + sc.context = c + return hs.GetUserByID(c) + }) + + sc.m.Get("/api/users/:id", sc.defaultHandler) + sc.fakeReqWithParams("GET", sc.url, map[string]string{}).exec() + + var resp user.UserProfileDTO + require.Equal(t, http.StatusOK, sc.resp.Code) + err := json.Unmarshal(sc.resp.Body.Bytes(), &resp) + require.NoError(t, err) + + assert.Equal(t, tc.expectedIsGrafanaAdminSynced, resp.IsGrafanaAdminExternallySynced) + }) + } +} + func TestHTTPServer_UpdateUser(t *testing.T) { settings := setting.NewCfg() sqlStore := db.InitTestDB(t) diff --git a/pkg/services/login/authinfo.go b/pkg/services/login/authinfo.go index 16b25945979..5174cbde75e 100644 --- a/pkg/services/login/authinfo.go +++ b/pkg/services/login/authinfo.go @@ -114,6 +114,24 @@ func IsExternallySynced(cfg *setting.Cfg, authModule string) bool { return true } +// 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) { + return false + } + + switch authModule { + case JWTModule: + return cfg.JWTAuthAllowAssignGrafanaAdmin + case LDAPAuthModule: + return true + default: + return oAuthAndAllowAssignGrafanaAdmin + } +} + func IsProviderEnabled(cfg *setting.Cfg, authModule string) bool { switch authModule { case SAMLAuthModule: diff --git a/pkg/services/user/model.go b/pkg/services/user/model.go index 292a951b51c..c3a80ddd61d 100644 --- a/pkg/services/user/model.go +++ b/pkg/services/user/model.go @@ -142,21 +142,22 @@ type GetUserProfileQuery struct { } type UserProfileDTO struct { - ID int64 `json:"id"` - Email string `json:"email"` - Name string `json:"name"` - Login string `json:"login"` - Theme string `json:"theme"` - OrgID int64 `json:"orgId,omitempty"` - IsGrafanaAdmin bool `json:"isGrafanaAdmin"` - IsDisabled bool `json:"isDisabled"` - IsExternal bool `json:"isExternal"` - IsExternallySynced bool `json:"isExternallySynced"` - AuthLabels []string `json:"authLabels"` - UpdatedAt time.Time `json:"updatedAt"` - CreatedAt time.Time `json:"createdAt"` - AvatarURL string `json:"avatarUrl"` - AccessControl map[string]bool `json:"accessControl,omitempty"` + ID int64 `json:"id"` + Email string `json:"email"` + Name string `json:"name"` + Login string `json:"login"` + Theme string `json:"theme"` + OrgID int64 `json:"orgId,omitempty"` + IsGrafanaAdmin bool `json:"isGrafanaAdmin"` + IsDisabled bool `json:"isDisabled"` + IsExternal bool `json:"isExternal"` + IsExternallySynced bool `json:"isExternallySynced"` + IsGrafanaAdminExternallySynced bool `json:"isGrafanaAdminExternallySynced"` + AuthLabels []string `json:"authLabels"` + UpdatedAt time.Time `json:"updatedAt"` + CreatedAt time.Time `json:"createdAt"` + AvatarURL string `json:"avatarUrl"` + AccessControl map[string]bool `json:"accessControl,omitempty"` } // implement Conversion interface to define custom field mapping (xorm feature) diff --git a/public/app/features/admin/UserAdminPage.tsx b/public/app/features/admin/UserAdminPage.tsx index 2d5b013f92b..7dfa4ad8728 100644 --- a/public/app/features/admin/UserAdminPage.tsx +++ b/public/app/features/admin/UserAdminPage.tsx @@ -132,7 +132,11 @@ export class UserAdminPage extends PureComponent { canReadLDAPStatus && ( )} - + )} diff --git a/public/app/features/admin/UserPermissions.tsx b/public/app/features/admin/UserPermissions.tsx index 41f6ac321ca..2eef2449dc1 100644 --- a/public/app/features/admin/UserPermissions.tsx +++ b/public/app/features/admin/UserPermissions.tsx @@ -6,6 +6,7 @@ import { AccessControlAction } from 'app/types'; interface Props { isGrafanaAdmin: boolean; + isExternalUser?: boolean; onGrafanaAdminChange: (isGrafanaAdmin: boolean) => void; } @@ -15,7 +16,7 @@ const adminOptions = [ { label: 'No', value: false }, ]; -export function UserPermissions({ isGrafanaAdmin, onGrafanaAdminChange }: Props) { +export function UserPermissions({ isGrafanaAdmin, isExternalUser, onGrafanaAdminChange }: Props) { const [isEditing, setIsEditing] = useState(false); const [currentAdminOption, setCurrentAdminOption] = useState(isGrafanaAdmin); @@ -28,7 +29,7 @@ export function UserPermissions({ isGrafanaAdmin, onGrafanaAdminChange }: Props) const handleGrafanaAdminChange = () => onGrafanaAdminChange(currentAdminOption); - const canChangePermissions = contextSrv.hasPermission(AccessControlAction.UsersPermissionsUpdate); + const canChangePermissions = contextSrv.hasPermission(AccessControlAction.UsersPermissionsUpdate) && !isExternalUser; return ( <> diff --git a/public/app/types/user.ts b/public/app/types/user.ts index 694c897a76f..50356321c2d 100644 --- a/public/app/types/user.ts +++ b/public/app/types/user.ts @@ -48,6 +48,7 @@ export interface UserDTO extends WithAccessControlMetadata { teams?: Unit[]; orgs?: Unit[]; isExternallySynced?: boolean; + isGrafanaAdminExternallySynced?: boolean; } export interface Invitee {