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

Auth: Add OrgRole to ID token (#100383)

Browse Source 
* Changes for Users and ServiceAccounts

* Align tests
This commit is contained in:
Misi
2025-02-12 05:51:29 -08:00
committed by GitHub
parent a5c8b5ed83
commit ee0a1391df
19 changed files with 202 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/api/http_server.go
+3 -1
View File
@@ -207,6 +207,7 @@ type HTTPServer struct {
tempUserService tempUser.Service
loginAttemptService loginAttempt.Service
orgService org.Service
idService auth.IDService
orgDeletionService org.DeletionService
TeamService team.Service
accesscontrolService accesscontrol.Service
@@ -272,7 +273,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
annotationRepo annotations.Repository, tagService tag.Service, searchv2HTTPService searchV2.SearchHTTPService, oauthTokenService oauthtoken.OAuthTokenService,
statsService stats.Service, authnService authn.Service, pluginsCDNService *pluginscdn.Service, promGatherer prometheus.Gatherer,
starApi *starApi.API, promRegister prometheus.Registerer, clientConfigProvider grafanaapiserver.DirectRestConfigProvider, anonService anonymous.Service,
userVerifier user.Verifier, pluginPreinstall plugininstaller.Preinstall,
userVerifier user.Verifier, pluginPreinstall plugininstaller.Preinstall, idService auth.IDService,
) (*HTTPServer, error) {
web.Env = cfg.Env
m := web.New()
@@ -360,6 +361,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
tempUserService: tempUserService,
loginAttemptService: loginAttemptService,
orgService: orgService,
idService: idService,
orgDeletionService: orgDeletionService,
TeamService: teamService,
navTreeService: navTreeService,
pkg/api/org_users.go
+6
View File
@@ -7,9 +7,11 @@ import (
"net/http"
"strconv"
claims "github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/api/dtos"
"github.com/grafana/grafana/pkg/api/response"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/authn"
contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
@@ -432,6 +434,10 @@ func (hs *HTTPServer) updateOrgUserHelper(c *contextmodel.ReqContext, cmd org.Up
}
}
if err := hs.idService.RemoveIDToken(c.Req.Context(), &authn.Identity{ID: strconv.FormatInt(cmd.UserID, 10), Type: claims.TypeUser, OrgID: cmd.OrgID}); err != nil {
return response.Error(http.StatusInternalServerError, "Failed to invalidate the ID token cache", err)
}
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
+50 -24
View File
@@ -9,9 +9,12 @@ import (
"strings"
"testing"
"github.com/grafana/authlib/types"
"github.com/grafana/grafana/pkg/services/auth/idtest"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/authn/authntest"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/mock"
"github.com/stretchr/testify/require"
"github.com/grafana/grafana/pkg/api/dtos"
@@ -202,11 +205,12 @@ 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
desc string
SkipOrgRoleSync bool
AuthEnabled bool
AuthModule string
shouldInvalidateIDToken bool
expectedCode int
}
permissions := []accesscontrol.Permission{
{Action: accesscontrol.ActionOrgUsersRead, Scope: "users:*"},
@@ -216,11 +220,12 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
}
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 be able to change basicRole when skip_org_role_sync true",
SkipOrgRoleSync: true,
AuthEnabled: true,
AuthModule: login.LDAPAuthModule,
shouldInvalidateIDToken: true,
expectedCode: http.StatusOK,
},
{
desc: "should not be able to change basicRole when skip_org_role_sync false",
@@ -237,18 +242,20 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
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: false,
AuthEnabled: false,
AuthModule: "",
shouldInvalidateIDToken: true,
expectedCode: http.StatusOK,
},
{
desc: "should be able to change basicRole with a basic Auth",
SkipOrgRoleSync: true,
AuthEnabled: true,
AuthModule: "",
expectedCode: http.StatusOK,
desc: "should be able to change basicRole with a basic Auth",
SkipOrgRoleSync: true,
AuthEnabled: true,
AuthModule: "",
shouldInvalidateIDToken: true,
expectedCode: http.StatusOK,
},
}
@@ -279,6 +286,11 @@ func TestOrgUsersAPIEndpoint_updateOrgRole(t *testing.T) {
}
hs.userService = &usertest.FakeUserService{ExpectedSignedInUser: userWithPermissions}
hs.orgService = &orgtest.FakeOrgService{}
idService := &idtest.MockService{}
if tt.shouldInvalidateIDToken {
idService.On("RemoveIDToken", mock.Anything, mock.Anything).Return(nil)
}
hs.idService = idService
hs.SocialService = &socialtest.FakeSocialService{
ExpectedAuthInfoProvider: &social.OAuthInfo{Enabled: tt.AuthEnabled, SkipOrgRoleSync: tt.SkipOrgRoleSync},
}
@@ -615,6 +627,7 @@ func TestOrgUsersAPIEndpointWithSetPerms_AccessControl(t *testing.T) {
ExpectedUser: &user.User{},
ExpectedSignedInUser: userWithPermissions(1, tt.permissions),
}
hs.idService = &idtest.FakeService{}
hs.accesscontrolService = &actest.FakeService{}
})
@@ -637,16 +650,24 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
name string
role org.RoleType
permissions []accesscontrol.Permission
setup func(*testing.T, *idtest.MockService)
input string
expectedCode int
}
tests := []testCase{
{
name: "user with permissions can update org role",
permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}},
role: org.RoleAdmin,
input: `{"role": "Viewer"}`,
name: "user with permissions can update org role",
permissions: []accesscontrol.Permission{{Action: accesscontrol.ActionOrgUsersWrite, Scope: "users:*"}},
role: org.RoleAdmin,
input: `{"role": "Viewer"}`,
setup: func(t *testing.T, idService *idtest.MockService) {
idService.On("RemoveIDToken", mock.Anything, mock.MatchedBy(func(id *authn.Identity) bool {
return id.GetIdentityType() == types.TypeUser &&
id.GetID() == "user:1" &&
id.GetOrgID() == int64(1)
})).Return(nil)
},
expectedCode: http.StatusOK,
},
{
@@ -673,6 +694,11 @@ func TestPatchOrgUsersAPIEndpoint_AccessControl(t *testing.T) {
AuthModule: "",
},
}
idService := &idtest.MockService{}
if tt.setup != nil {
tt.setup(t, idService)
}
hs.idService = idService
hs.accesscontrolService = &actest.FakeService{}
hs.userService = &usertest.FakeUserService{
ExpectedUser: &user.User{},
pkg/api/user_test.go
+2 -2
View File
@@ -463,7 +463,7 @@ func setupUpdateEmailTests(t *testing.T, cfg *setting.Cfg) (*user.User, *HTTPSer
require.NoError(t, err)
nsMock := notifications.MockNotificationService()
verifier := userimpl.ProvideVerifier(cfg, userSvc, tempUserService, nsMock, &idtest.MockService{})
verifier := userimpl.ProvideVerifier(cfg, userSvc, tempUserService, nsMock, &idtest.FakeService{})
hs := &HTTPServer{
Cfg: cfg,
@@ -688,7 +688,7 @@ func TestUser_UpdateEmail(t *testing.T) {
hs.tempUserService = tempUserSvc
hs.NotificationService = nsMock
hs.SecretsService = fakes.NewFakeSecretsService()
hs.userVerifier = userimpl.ProvideVerifier(settings, userSvc, tempUserSvc, nsMock, &idtest.MockService{})
hs.userVerifier = userimpl.ProvideVerifier(settings, userSvc, tempUserSvc, nsMock, &idtest.FakeService{})
// User is internal
hs.authInfoService = &authinfotest.FakeService{ExpectedError: user.ErrUserNotFound}
})