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

merge
2023-08-02 08:54:24 +01:00
parent ae6e5d105b
commit 6b1b34b5f3
9 changed files with 300 additions and 18 deletions
@@ -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) {