public/grafana
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Auth: Fix orgrole picker disabled if isSynced user (#64033)

Browse Source 
* fix: disable orgrolepicker if externaluser is synced

* add disable to role picker

* just took me 2 hours to center the icon

* wip

* fix: check externallySyncedUser for API call

* remove check from store

* add: tests

* refactor authproxy and made tests run

* add: feature toggle

* set feature toggle for tests

* add: IsProviderEnabled

* refactor: featuretoggle name

* IsProviderEnabled tests

* add specific tests for isProviderEnabled

* fix: org_user tests

* add: owner to featuretoggle

* add missing authlabels

* remove fmt

* feature toggle

* change config

* add test for a different authmodule

* test refactor

* gen feature toggle again

* fix basic auth user able to change the org role

* test for basic auth role

* make err.base to error

* lowered lvl of log and input mesg
This commit is contained in:
Eric Leijonmarck
2023-03-22 17:41:59 +00:00
committed by GitHub
parent 13af5afaf3
commit 3cd952b8ba
38 changed files with 405 additions and 161 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/frontendsettings.go
+2 -2
View File
@@ -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,
pkg/api/org_users.go
+19
View File
@@ -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)
pkg/api/org_users_test.go
+105 -2
View File
@@ -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{},
pkg/api/user.go
+1 -1
View File
@@ -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))