[v10.1.x] Auth: Lock down Grafana admin role updates if the role is externally synced

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) {

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) {

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))

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)

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:

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)

public/app/features/admin/UserAdminPage.tsx

@@ -130,7 +130,11 @@ export class UserAdminPage extends PureComponent<Props> {
               {isLDAPUser && isUserSynced && featureEnabled('ldapsync') && ldapSyncInfo && canReadLDAPStatus && (
                 <UserLdapSyncInfo ldapSyncInfo={ldapSyncInfo} user={user} onUserSync={this.onUserSync} />
               )}
-              <UserPermissions isGrafanaAdmin={user.isGrafanaAdmin} onGrafanaAdminChange={this.onGrafanaAdminChange} />
+              <UserPermissions
+                isGrafanaAdmin={user.isGrafanaAdmin}
+                isExternalUser={user?.isGrafanaAdminExternallySynced}
+                onGrafanaAdminChange={this.onGrafanaAdminChange}
+              />
             </>
           )}