Skip to content

Commit

Permalink
AuthZ: Extend /api/search to work with self-contained permissions (#7…
Browse files Browse the repository at this point in the history 
…0749)

* Search sql filter draft, unfinished

* Search works for empty roles

* Add current AuthModule to SignedInUser

* clean up, changes to the search

* Use constant prefixes

* Change AuthModule to AuthenticatedBy

* Add tests for using the permissions from the SignedInUser

* Refactor and simplify code

* Fix sql generation for pg and mysql

* Fixes, clean up

* Add test for empty permission list

* Fix

* Fix any vs all in case of edit permission

* Update pkg/services/authn/authn.go

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* Update pkg/services/sqlstore/permissions/dashboard_test.go

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>

* Fixes, changes based on the review

---------

Co-authored-by: Gabriel MABILLE <gamab@users.noreply.github.com>
  • Loading branch information
@mgyongyosi @gamab
mgyongyosi and gamab committed Jul 12, 2023
1 parent e56b2ca commit 5efc338
Show file tree
Hide file tree
Showing 23 changed files with 651 additions and 236 deletions.
72 changes: 37 additions & 35 deletions pkg/services/authn/authn.go
View file
Expand Up @@ -200,9 +200,9 @@ type Identity struct {
Email string
// IsGrafanaAdmin is true if the entity is a Grafana admin.
IsGrafanaAdmin *bool
// AuthModule is the name of the external system. For example, "auth_ldap" or "auth_saml".
// Empty if the identity is provided by Grafana.
AuthModule string
// AuthenticatedBy is the name of the authentication client that was used to authenticate the current Identity.
// For example, "password", "apikey", "auth_ldap" or "auth_azuread".
AuthenticatedBy string
// AuthId is the unique identifier for the entity in the external system.
// Empty if the identity is provided by Grafana.
AuthID string
Expand Down Expand Up @@ -262,21 +262,22 @@ func (i *Identity) SignedInUser() *user.SignedInUser {
}

u := &user.SignedInUser{
UserID: 0,
OrgID: i.OrgID,
OrgName: i.OrgName,
OrgRole: i.Role(),
Login: i.Login,
Name: i.Name,
Email: i.Email,
OrgCount: i.OrgCount,
IsGrafanaAdmin: isGrafanaAdmin,
IsAnonymous: i.IsAnonymous,
IsDisabled: i.IsDisabled,
HelpFlags1: i.HelpFlags1,
LastSeenAt: i.LastSeenAt,
Teams: i.Teams,
Permissions: i.Permissions,
UserID: 0,
OrgID: i.OrgID,
OrgName: i.OrgName,
OrgRole: i.Role(),
Login: i.Login,
Name: i.Name,
Email: i.Email,
AuthenticatedBy: i.AuthenticatedBy,
OrgCount: i.OrgCount,
IsGrafanaAdmin: isGrafanaAdmin,
IsAnonymous: i.IsAnonymous,
IsDisabled: i.IsDisabled,
HelpFlags1: i.HelpFlags1,
LastSeenAt: i.LastSeenAt,
Teams: i.Teams,
Permissions: i.Permissions,
}

namespace, id := i.NamespacedID()
Expand All @@ -294,7 +295,7 @@ func (i *Identity) ExternalUserInfo() login.ExternalUserInfo {
_, id := i.NamespacedID()
return login.ExternalUserInfo{
OAuthToken: i.OAuthToken,
AuthModule: i.AuthModule,
AuthModule: i.AuthenticatedBy,
AuthId: i.AuthID,
UserId: id,
Email: i.Email,
Expand All @@ -308,23 +309,24 @@ func (i *Identity) ExternalUserInfo() login.ExternalUserInfo {
}

// IdentityFromSignedInUser creates an identity from a SignedInUser.
func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientParams) *Identity {
func IdentityFromSignedInUser(id string, usr *user.SignedInUser, params ClientParams, authenticatedBy string) *Identity {
return &Identity{
ID: id,
OrgID: usr.OrgID,
OrgName: usr.OrgName,
OrgRoles: map[int64]org.RoleType{usr.OrgID: usr.OrgRole},
Login: usr.Login,
Name: usr.Name,
Email: usr.Email,
OrgCount: usr.OrgCount,
IsGrafanaAdmin: &usr.IsGrafanaAdmin,
IsDisabled: usr.IsDisabled,
HelpFlags1: usr.HelpFlags1,
LastSeenAt: usr.LastSeenAt,
Teams: usr.Teams,
ClientParams: params,
Permissions: usr.Permissions,
ID: id,
OrgID: usr.OrgID,
OrgName: usr.OrgName,
OrgRoles: map[int64]org.RoleType{usr.OrgID: usr.OrgRole},
Login: usr.Login,
Name: usr.Name,
Email: usr.Email,
AuthenticatedBy: authenticatedBy,
OrgCount: usr.OrgCount,
IsGrafanaAdmin: &usr.IsGrafanaAdmin,
IsDisabled: usr.IsDisabled,
HelpFlags1: usr.HelpFlags1,
LastSeenAt: usr.LastSeenAt,
Teams: usr.Teams,
ClientParams: params,
Permissions: usr.Permissions,
}
}

Expand Down
26 changes: 13 additions & 13 deletions pkg/services/authn/authnimpl/sync/user_sync.go
View file
Expand Up @@ -76,27 +76,27 @@ func (s *UserSync) SyncUserHook(ctx context.Context, id *authn.Identity, _ *auth
// Does user exist in the database?
usr, userAuth, errUserInDB := s.getUser(ctx, id)
if errUserInDB != nil && !errors.Is(errUserInDB, user.ErrUserNotFound) {
s.log.FromContext(ctx).Error("Failed to fetch user", "error", errUserInDB, "auth_module", id.AuthModule, "auth_id", id.AuthID)
s.log.FromContext(ctx).Error("Failed to fetch user", "error", errUserInDB, "auth_module", id.AuthenticatedBy, "auth_id", id.AuthID)
return errSyncUserInternal.Errorf("unable to retrieve user")
}

if errors.Is(errUserInDB, user.ErrUserNotFound) {
if !id.ClientParams.AllowSignUp {
s.log.FromContext(ctx).Warn("Failed to create user, signup is not allowed for module", "auth_module", id.AuthModule, "auth_id", id.AuthID)
s.log.FromContext(ctx).Warn("Failed to create user, signup is not allowed for module", "auth_module", id.AuthenticatedBy, "auth_id", id.AuthID)
return errUserSignupDisabled.Errorf("%w", login.ErrSignupNotAllowed)
}

// create user
var errCreate error
usr, errCreate = s.createUser(ctx, id)
if errCreate != nil {
s.log.FromContext(ctx).Error("Failed to create user", "error", errCreate, "auth_module", id.AuthModule, "auth_id", id.AuthID)
s.log.FromContext(ctx).Error("Failed to create user", "error", errCreate, "auth_module", id.AuthenticatedBy, "auth_id", id.AuthID)
return errSyncUserInternal.Errorf("unable to create user")
}
} else {
// update user
if errUpdate := s.updateUserAttributes(ctx, usr, id, userAuth); errUpdate != nil {
s.log.FromContext(ctx).Error("Failed to update user", "error", errUpdate, "auth_module", id.AuthModule, "auth_id", id.AuthID)
s.log.FromContext(ctx).Error("Failed to update user", "error", errUpdate, "auth_module", id.AuthenticatedBy, "auth_id", id.AuthID)
return errSyncUserInternal.Errorf("unable to update user")
}
}
Expand Down Expand Up @@ -174,7 +174,7 @@ func (s *UserSync) EnableDisabledUserHook(ctx context.Context, identity *authn.I
}

func (s *UserSync) upsertAuthConnection(ctx context.Context, userID int64, identity *authn.Identity, createConnection bool) error {
if identity.AuthModule == "" {
if identity.AuthenticatedBy == "" {
return nil
}

Expand All @@ -184,7 +184,7 @@ func (s *UserSync) upsertAuthConnection(ctx context.Context, userID int64, ident
if createConnection {
return s.authInfoService.SetAuthInfo(ctx, &login.SetAuthInfoCommand{
UserId: userID,
AuthModule: identity.AuthModule,
AuthModule: identity.AuthenticatedBy,
AuthId: identity.AuthID,
OAuthToken: identity.OAuthToken,
})
Expand All @@ -194,13 +194,13 @@ func (s *UserSync) upsertAuthConnection(ctx context.Context, userID int64, ident
return s.authInfoService.UpdateAuthInfo(ctx, &login.UpdateAuthInfoCommand{
UserId: userID,
AuthId: identity.AuthID,
AuthModule: identity.AuthModule,
AuthModule: identity.AuthenticatedBy,
OAuthToken: identity.OAuthToken,
})
}

func (s *UserSync) updateUserAttributes(ctx context.Context, usr *user.User, id *authn.Identity, userAuth *login.UserAuth) error {
if errProtection := s.userProtectionService.AllowUserMapping(usr, id.AuthModule); errProtection != nil {
if errProtection := s.userProtectionService.AllowUserMapping(usr, id.AuthenticatedBy); errProtection != nil {
return errUserProtection.Errorf("user mapping not allowed: %w", errProtection)
}
// sync user info
Expand Down Expand Up @@ -286,8 +286,8 @@ func (s *UserSync) createUser(ctx context.Context, id *authn.Identity) (*user.Us

func (s *UserSync) getUser(ctx context.Context, identity *authn.Identity) (*user.User, *login.UserAuth, error) {
// Check auth info fist
if identity.AuthID != "" && identity.AuthModule != "" {
query := &login.GetAuthInfoQuery{AuthId: identity.AuthID, AuthModule: identity.AuthModule}
if identity.AuthID != "" && identity.AuthenticatedBy != "" {
query := &login.GetAuthInfoQuery{AuthId: identity.AuthID, AuthModule: identity.AuthenticatedBy}
authInfo, errGetAuthInfo := s.authInfoService.GetAuthInfo(ctx, query)

if errGetAuthInfo != nil && !errors.Is(errGetAuthInfo, user.ErrUserNotFound) {
Expand All @@ -307,7 +307,7 @@ func (s *UserSync) getUser(ctx context.Context, identity *authn.Identity) (*user
// if the user connected to user auth does not exist try to clean it up
if errors.Is(errGetByID, user.ErrUserNotFound) {
if err := s.authInfoService.DeleteUserAuthInfo(ctx, authInfo.UserId); err != nil {
s.log.FromContext(ctx).Error("Failed to clean up user auth", "error", err, "auth_module", identity.AuthModule, "auth_id", identity.AuthID)
s.log.FromContext(ctx).Error("Failed to clean up user auth", "error", err, "auth_module", identity.AuthenticatedBy, "auth_id", identity.AuthID)
}
}
}
Expand All @@ -322,8 +322,8 @@ func (s *UserSync) getUser(ctx context.Context, identity *authn.Identity) (*user
var userAuth *login.UserAuth
// Special case for generic oauth: generic oauth does not store authID,
// so we need to find the user first then check for the userAuth connection by module and userID
if identity.AuthModule == login.GenericOAuthModule {
query := &login.GetAuthInfoQuery{AuthModule: identity.AuthModule, UserId: usr.ID}
if identity.AuthenticatedBy == login.GenericOAuthModule {
query := &login.GetAuthInfoQuery{AuthModule: identity.AuthenticatedBy, UserId: usr.ID}
userAuth, err = s.authInfoService.GetAuthInfo(ctx, query)
if err != nil && !errors.Is(err, user.ErrUserNotFound) {
return nil, nil, err
Expand Down
66 changes: 33 additions & 33 deletions pkg/services/authn/authnimpl/sync/user_sync_test.go
View file
Expand Up @@ -268,12 +268,12 @@ func TestUserSync_SyncUserHook(t *testing.T) {
args: args{
ctx: context.Background(),
id: &authn.Identity{
ID: "",
AuthID: "2032",
AuthModule: "oauth",
Login: "test",
Name: "test",
Email: "test",
ID: "",
AuthID: "2032",
AuthenticatedBy: "oauth",
Login: "test",
Name: "test",
Email: "test",
ClientParams: authn.ClientParams{
SyncUser: true,
LookUpParams: login.UserLookupParams{
Expand All @@ -286,13 +286,13 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: "user:1",
AuthID: "2032",
AuthModule: "oauth",
Login: "test",
Name: "test",
Email: "test",
IsGrafanaAdmin: ptrBool(false),
ID: "user:1",
AuthID: "2032",
AuthenticatedBy: "oauth",
Login: "test",
Name: "test",
Email: "test",
IsGrafanaAdmin: ptrBool(false),
ClientParams: authn.ClientParams{
SyncUser: true,
LookUpParams: login.UserLookupParams{
Expand All @@ -313,12 +313,12 @@ func TestUserSync_SyncUserHook(t *testing.T) {
args: args{
ctx: context.Background(),
id: &authn.Identity{
ID: "",
Login: "test",
Name: "test",
Email: "test",
AuthModule: "oauth",
AuthID: "2032",
ID: "",
Login: "test",
Name: "test",
Email: "test",
AuthenticatedBy: "oauth",
AuthID: "2032",
ClientParams: authn.ClientParams{
SyncUser: true,
LookUpParams: login.UserLookupParams{
Expand All @@ -341,13 +341,13 @@ func TestUserSync_SyncUserHook(t *testing.T) {
args: args{
ctx: context.Background(),
id: &authn.Identity{
ID: "",
Login: "test_create",
Name: "test_create",
IsGrafanaAdmin: ptrBool(true),
Email: "test_create",
AuthModule: "oauth",
AuthID: "2032",
ID: "",
Login: "test_create",
Name: "test_create",
IsGrafanaAdmin: ptrBool(true),
Email: "test_create",
AuthenticatedBy: "oauth",
AuthID: "2032",
ClientParams: authn.ClientParams{
SyncUser: true,
AllowSignUp: true,
Expand All @@ -362,13 +362,13 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: "user:2",
Login: "test_create",
Name: "test_create",
Email: "test_create",
AuthModule: "oauth",
AuthID: "2032",
IsGrafanaAdmin: ptrBool(true),
ID: "user:2",
Login: "test_create",
Name: "test_create",
Email: "test_create",
AuthenticatedBy: "oauth",
AuthID: "2032",
IsGrafanaAdmin: ptrBool(true),
ClientParams: authn.ClientParams{
SyncUser: true,
AllowSignUp: true,
Expand Down
12 changes: 7 additions & 5 deletions pkg/services/authn/clients/api_key.go
View file
Expand Up @@ -11,6 +11,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/util"
Expand Down Expand Up @@ -64,10 +65,11 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
// if the api key don't belong to a service account construct the identity and return it
if apiKey.ServiceAccountId == nil || *apiKey.ServiceAccountId < 1 {
return &authn.Identity{
ID: authn.NamespacedID(authn.NamespaceAPIKey, apiKey.ID),
OrgID: apiKey.OrgID,
OrgRoles: map[int64]org.RoleType{apiKey.OrgID: apiKey.Role},
ClientParams: authn.ClientParams{SyncPermissions: true},
ID: authn.NamespacedID(authn.NamespaceAPIKey, apiKey.ID),
OrgID: apiKey.OrgID,
OrgRoles: map[int64]org.RoleType{apiKey.OrgID: apiKey.Role},
ClientParams: authn.ClientParams{SyncPermissions: true},
AuthenticatedBy: login.APIKeyAuthModule,
}, nil
}

Expand All @@ -80,7 +82,7 @@ func (s *APIKey) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide
return nil, err
}

return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceServiceAccount, usr.UserID), usr, authn.ClientParams{SyncPermissions: true}), nil
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceServiceAccount, usr.UserID), usr, authn.ClientParams{SyncPermissions: true}, login.APIKeyAuthModule), nil
}

func (s *APIKey) getAPIKey(ctx context.Context, token string) (*apikey.APIKey, error) {
Expand Down
3 changes: 3 additions & 0 deletions pkg/services/authn/clients/api_key_test.go
View file
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/grafana/grafana/pkg/services/apikey"
"github.com/grafana/grafana/pkg/services/apikey/apikeytest"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
"github.com/grafana/grafana/pkg/services/user/usertest"
Expand Down Expand Up @@ -55,6 +56,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
ClientParams: authn.ClientParams{
SyncPermissions: true,
},
AuthenticatedBy: login.APIKeyAuthModule,
},
},
{
Expand Down Expand Up @@ -88,6 +90,7 @@ func TestAPIKey_Authenticate(t *testing.T) {
ClientParams: authn.ClientParams{
SyncPermissions: true,
},
AuthenticatedBy: login.APIKeyAuthModule,
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion pkg/services/authn/clients/ext_jwt.go
View file
Expand Up @@ -13,6 +13,7 @@ import (

"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/oauthserver"
"github.com/grafana/grafana/pkg/services/signingkeys"
"github.com/grafana/grafana/pkg/services/user"
Expand Down Expand Up @@ -100,7 +101,7 @@ func (s *ExtendedJWT) Authenticate(ctx context.Context, r *authn.Request) (*auth

signedInUser.Permissions[s.getDefaultOrgID()] = claims.Entitlements

return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{SyncPermissions: false}), nil
return authn.IdentityFromSignedInUser(authn.NamespacedID(authn.NamespaceUser, signedInUser.UserID), signedInUser, authn.ClientParams{SyncPermissions: false}, login.ExtendedJWTModule), nil
}

func (s *ExtendedJWT) Test(ctx context.Context, r *authn.Request) bool {
Expand Down

0 comments on commit 5efc338

Please sign in to comment.