mirror of
https://github.com/grafana/grafana.git
synced 2025-02-25 18:55:37 -06:00
Auth: Lock down Grafana admin role updates if the role is externally synced (#72677)
* lock down server admin role updates on the frontend if the user is externally synced * add tests * lock Grafana Server admin role updates from the backend * rename variables * check that the user has auth info * add LDAP to providers for which Grafana Server admin role can be synced * linting
This commit is contained in:
Ieva
GitHub
parent
d28bb03ebc
commit
d3b481dac8
@ -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) {
|
||||
|
||||
@ -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) {
|
||||
|
||||
@ -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))
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -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:
|
||||
|
||||
@ -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)
|
||||
|
||||
@ -132,7 +132,11 @@ export class UserAdminPage extends PureComponent<Props> {
|
||||
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}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
|
||||
|
||||
@ -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 (
|
||||
<>
|
||||
|
||||
@ -48,6 +48,7 @@ export interface UserDTO extends WithAccessControlMetadata {
|
||||
teams?: Unit[];
|
||||
orgs?: Unit[];
|
||||
isExternallySynced?: boolean;
|
||||
isGrafanaAdminExternallySynced?: boolean;
|
||||
}
|
||||
|
||||
export interface Invitee {
|
||||
|
||||
Loading…
Reference in New Issue
Block a user