Authn: move namespace id type (#86853)

* Use RoleType from org package

* Move to identity package and re-export from authn

* Replace usage of top level functions for identity

Co-authored-by: Misi <mgyongyosi@users.noreply.github.com>

pkg/services/auth/identity/error.go

@@ -0,0 +1,13 @@
+package identity
+
+import (
+	"errors"
+
+	"github.com/grafana/grafana/pkg/util/errutil"
+)
+
+var (
+	ErrInvalidNamespaceID       = errutil.BadRequest("auth.identity.invalid-namespace-id")
+	ErrNotIntIdentifier         = errors.New("identifier is not an int64")
+	ErrIdentifierNotInitialized = errors.New("identifier is not initialized")
+)

pkg/services/auth/identity/namespace.go

@@ -0,0 +1,113 @@
+package identity
+
+import (
+	"fmt"
+	"strconv"
+	"strings"
+)
+
+const (
+	NamespaceUser           = "user"
+	NamespaceAPIKey         = "api-key"
+	NamespaceServiceAccount = "service-account"
+	NamespaceAnonymous      = "anonymous"
+	NamespaceRenderService  = "render"
+	NamespaceAccessPolicy   = "access-policy"
+)
+
+var AnonymousNamespaceID = MustNewNamespaceID(NamespaceAnonymous, 0)
+
+var namespaceLookup = map[string]struct{}{
+	NamespaceUser:           {},
+	NamespaceAPIKey:         {},
+	NamespaceServiceAccount: {},
+	NamespaceAnonymous:      {},
+	NamespaceRenderService:  {},
+	NamespaceAccessPolicy:   {},
+}
+
+func ParseNamespaceID(str string) (NamespaceID, error) {
+	var namespaceID NamespaceID
+
+	parts := strings.Split(str, ":")
+	if len(parts) != 2 {
+		return namespaceID, ErrInvalidNamespaceID.Errorf("expected namespace id to have 2 parts")
+	}
+
+	namespace, id := parts[0], parts[1]
+
+	if _, ok := namespaceLookup[namespace]; !ok {
+		return namespaceID, ErrInvalidNamespaceID.Errorf("got invalid namespace %s", namespace)
+	}
+
+	namespaceID.id = id
+	namespaceID.namespace = namespace
+
+	return namespaceID, nil
+}
+
+// MustParseNamespaceID parses namespace id, it will panic if it fails to do so.
+// Suitable to use in tests or when we can guarantee that we pass a correct format.
+func MustParseNamespaceID(str string) NamespaceID {
+	namespaceID, err := ParseNamespaceID(str)
+	if err != nil {
+		panic(err)
+	}
+	return namespaceID
+}
+
+// NewNamespaceID creates a new NamespaceID, will fail for invalid namespace.
+func NewNamespaceID(namespace string, id int64) (NamespaceID, error) {
+	var namespaceID NamespaceID
+	if _, ok := namespaceLookup[namespace]; !ok {
+		return namespaceID, ErrInvalidNamespaceID.Errorf("got invalid namespace %s", namespace)
+	}
+	namespaceID.id = strconv.FormatInt(id, 10)
+	namespaceID.namespace = namespace
+	return namespaceID, nil
+}
+
+// MustNewNamespaceID creates a new NamespaceID, will panic for invalid namespace.
+// Suitable to use in tests or when we can guarantee that we pass a correct format.
+func MustNewNamespaceID(namespace string, id int64) NamespaceID {
+	namespaceID, err := NewNamespaceID(namespace, id)
+	if err != nil {
+		panic(err)
+	}
+	return namespaceID
+}
+
+// NewNamespaceIDUnchecked creates a new NamespaceID without checking if namespace is valid.
+// It us up to the caller to ensure that namespace is valid.
+func NewNamespaceIDUnchecked(namespace string, id int64) NamespaceID {
+	return NamespaceID{
+		id:        strconv.FormatInt(id, 10),
+		namespace: namespace,
+	}
+}
+
+// FIXME: use this instead of encoded string through the codebase
+type NamespaceID struct {
+	id        string
+	namespace string
+}
+
+func (ni NamespaceID) ID() string {
+	return ni.id
+}
+
+func (ni NamespaceID) ParseInt() (int64, error) {
+	return strconv.ParseInt(ni.id, 10, 64)
+}
+
+func (ni NamespaceID) Namespace() string {
+	return ni.namespace
+}
+
+func (ni NamespaceID) IsNamespace(expected ...string) bool {
+	return IsNamespace(ni.namespace, expected...)
+}
+
+func (ni NamespaceID) String() string {
+	return fmt.Sprintf("%s:%s", ni.namespace, ni.id)
+}

pkg/services/auth/identity/requester.go

@@ -1,25 +1,12 @@
 package identity
 
 import (
-	"errors"
 	"fmt"
 	"strconv"
 
 	"github.com/grafana/grafana/pkg/models/roletype"
 )
 
-const (
-	NamespaceUser           = "user"
-	NamespaceAPIKey         = "api-key"
-	NamespaceServiceAccount = "service-account"
-	NamespaceAnonymous      = "anonymous"
-	NamespaceRenderService  = "render"
-	NamespaceAccessPolicy   = "access-policy"
-)
-
-var ErrNotIntIdentifier = errors.New("identifier is not an int64")
-var ErrIdentifierNotInitialized = errors.New("identifier is not initialized")
-
 type Requester interface {
 	// GetID returns namespaced id for the entity
 	GetID() string

pkg/services/authn/authn.go

@@ -153,7 +153,7 @@ type RedirectClient interface {
 // that should happen during logout and supports client specific redirect URL.
 type LogoutClient interface {
 	Client
-	Logout(ctx context.Context, user identity.Requester) (*Redirect, bool)
+	Logout(ctx context.Context, user Requester) (*Redirect, bool)
 }
 
 type PasswordClient interface {

pkg/services/authn/authnimpl/service.go

@@ -260,7 +260,7 @@ func (s *Service) RedirectURL(ctx context.Context, client string, r *authn.Reque
 	return redirectClient.RedirectURL(ctx, r)
 }
 
-func (s *Service) Logout(ctx context.Context, user identity.Requester, sessionToken *auth.UserToken) (*authn.Redirect, error) {
+func (s *Service) Logout(ctx context.Context, user authn.Requester, sessionToken *auth.UserToken) (*authn.Redirect, error) {
 	ctx, span := s.tracer.Start(ctx, "authn.Logout")
 	defer span.End()
 

pkg/services/authn/authnimpl/service_test.go

@@ -488,7 +488,7 @@ func TestService_ResolveIdentity(t *testing.T) {
 	t.Run("should return error for for unknown namespace", func(t *testing.T) {
 		svc := setupTests(t)
 		_, err := svc.ResolveIdentity(context.Background(), 1, "some:1")
-		assert.ErrorIs(t, err, authn.ErrInvalidNamepsaceID)
+		assert.ErrorIs(t, err, authn.ErrInvalidNamespaceID)
 	})
 
 	t.Run("should return error for for namespace that don't have a resolver", func(t *testing.T) {

pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go

@@ -14,7 +14,6 @@ import (
 	"github.com/grafana/grafana/pkg/login/social/socialtest"
 	"github.com/grafana/grafana/pkg/services/auth"
 	"github.com/grafana/grafana/pkg/services/auth/authtest"
-	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/oauthtoken/oauthtokentest"
@@ -92,7 +91,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
 			)
 
 			service := &oauthtokentest.MockOauthTokenService{
-				HasOAuthEntryFunc: func(ctx context.Context, usr identity.Requester) (*login.UserAuth, bool, error) {
+				HasOAuthEntryFunc: func(ctx context.Context, usr authn.Requester) (*login.UserAuth, bool, error) {
 					hasEntryCalled = true
 					return tt.expectedHasEntryToken, tt.expectedHasEntryToken != nil, nil
 				},
@@ -100,7 +99,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
 					invalidateTokensCalled = true
 					return nil
 				},
-				TryTokenRefreshFunc: func(ctx context.Context, usr identity.Requester) error {
+				TryTokenRefreshFunc: func(ctx context.Context, usr authn.Requester) error {
 					tryRefreshCalled = true
 					return tt.expectedTryRefreshErr
 				},

pkg/services/authn/authnimpl/sync/rbac_sync_test.go

@@ -7,7 +7,6 @@ import (
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/accesscontrol"
 	acmock "github.com/grafana/grafana/pkg/services/accesscontrol/mock"
-	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -143,7 +142,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
 
 func setupTestEnv() *RBACSync {
 	acMock := &acmock.Mock{
-		GetUserPermissionsFunc: func(ctx context.Context, siu identity.Requester, o accesscontrol.Options) ([]accesscontrol.Permission, error) {
+		GetUserPermissionsFunc: func(ctx context.Context, siu authn.Requester, o accesscontrol.Options) ([]accesscontrol.Permission, error) {
 			return []accesscontrol.Permission{
 				{Action: accesscontrol.ActionUsersRead},
 			}, nil

pkg/services/authn/authnimpl/sync/user_sync.go

@@ -4,10 +4,8 @@ import (
 	"context"
 	"errors"
 	"fmt"
-	"strconv"
 
 	"github.com/grafana/grafana/pkg/infra/log"
-	authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -112,14 +110,13 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Iden
 		return nil
 	}
 
-	namespace, id := identity.GetNamespacedID()
+	if !identity.ID.IsNamespace(authn.NamespaceUser, authn.NamespaceServiceAccount) {
-	if !authidentity.IsNamespace(namespace, authn.NamespaceUser, authn.NamespaceServiceAccount) {
 		return nil
 	}
 
-	userID, err := strconv.ParseInt(id, 10, 64)
+	userID, err := identity.ID.ParseInt()
 	if err != nil {
-		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
+		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
 		return nil
 	}
 
@@ -151,14 +148,13 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit
 		return nil
 	}
 
-	namespace, id := identity.GetNamespacedID()
+	if !identity.ID.IsNamespace(authn.NamespaceUser, authn.NamespaceServiceAccount) {
-	if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount {
 		return nil
 	}
 
-	userID, err := authidentity.IntIdentifier(namespace, id)
+	userID, err := identity.ID.ParseInt()
 	if err != nil {
-		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
+		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
 		return nil
 	}
 
@@ -184,14 +180,13 @@ func (s *UserSync) EnableUserHook(ctx context.Context, identity *authn.Identity,
 		return nil
 	}
 
-	namespace, id := identity.GetNamespacedID()
+	if !identity.ID.IsNamespace(authn.NamespaceUser) {
-	if namespace != authn.NamespaceUser {
 		return nil
 	}
 
-	userID, err := authidentity.IntIdentifier(namespace, id)
+	userID, err := identity.ID.ParseInt()
 	if err != nil {
-		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
+		s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
 		return nil
 	}
 

pkg/services/authn/clients/api_key.go

@@ -10,7 +10,6 @@ import (
 	"github.com/grafana/grafana/pkg/components/satokengen"
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/services/apikey"
-	authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/services/org"
@@ -141,7 +140,7 @@ func (s *APIKey) Namespace() string {
 
 func (s *APIKey) ResolveIdentity(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*authn.Identity, error) {
 	if !namespaceID.IsNamespace(authn.NamespaceAPIKey) {
-		return nil, authn.ErrInvalidNamepsaceID.Errorf("got unspected namespace: %s", namespaceID.Namespace())
+		return nil, authn.ErrInvalidNamespaceID.Errorf("got unspected namespace: %s", namespaceID.Namespace())
 	}
 
 	apiKeyID, err := namespaceID.ParseInt()
@@ -161,7 +160,7 @@ func (s *APIKey) ResolveIdentity(ctx context.Context, orgID int64, namespaceID a
 	}
 
 	if key.ServiceAccountId != nil && *key.ServiceAccountId >= 1 {
-		return nil, authn.ErrInvalidNamepsaceID.Errorf("api key belongs to service account")
+		return nil, authn.ErrInvalidNamespaceID.Errorf("api key belongs to service account")
 	}
 
 	return newAPIKeyIdentity(key), nil
@@ -189,18 +188,17 @@ func (s *APIKey) Hook(ctx context.Context, identity *authn.Identity, r *authn.Re
 }
 
 func (s *APIKey) getAPIKeyID(ctx context.Context, identity *authn.Identity, r *authn.Request) (apiKeyID int64, exists bool) {
-	namespace, identifier := identity.GetNamespacedID()
+	id, err := identity.ID.ParseInt()
-
-	id, err := authidentity.IntIdentifier(namespace, identifier)
 	if err != nil {
 		s.log.Warn("Failed to parse ID from identifier", "err", err)
 		return -1, false
 	}
-	if namespace == authn.NamespaceAPIKey {
+
+	if identity.ID.IsNamespace(authn.NamespaceAPIKey) {
 		return id, true
 	}
 
-	if namespace == authn.NamespaceServiceAccount {
+	if identity.ID.IsNamespace(authn.NamespaceServiceAccount) {
 		// When the identity is service account, the ID in from the namespace is the service account ID.
 		// We need to fetch the API key in this scenario, as we could use it to uniquely identify a service account token.
 		apiKey, err := s.getAPIKey(ctx, getTokenFromRequest(r))
@@ -211,6 +209,7 @@ func (s *APIKey) getAPIKeyID(ctx context.Context, identity *authn.Identity, r *a
 
 		return apiKey.ID, true
 	}
+
 	return -1, false
 }
 

pkg/services/authn/clients/api_key_test.go

@@ -298,7 +298,7 @@ func TestAPIKey_ResolveIdentity(t *testing.T) {
 		{
 			desc:        "should return error for invalid namespace",
 			namespaceID: authn.MustParseNamespaceID("user:1"),
-			expectedErr: authn.ErrInvalidNamepsaceID,
+			expectedErr: authn.ErrInvalidNamespaceID,
 		},
 		{
 			desc:        "should return error when api key has expired",
@@ -328,7 +328,7 @@ func TestAPIKey_ResolveIdentity(t *testing.T) {
 				OrgID:            1,
 				ServiceAccountId: intPtr(1),
 			},
-			expectedErr: authn.ErrInvalidNamepsaceID,
+			expectedErr: authn.ErrInvalidNamespaceID,
 		},
 		{
 			desc:        "should return error when api key is belongs to different org",

pkg/services/authn/clients/oauth.go

@@ -250,7 +250,7 @@ func (c *OAuth) RedirectURL(ctx context.Context, r *authn.Request) (*authn.Redir
 	}, nil
 }
 
-func (c *OAuth) Logout(ctx context.Context, user identity.Requester) (*authn.Redirect, bool) {
+func (c *OAuth) Logout(ctx context.Context, user authn.Requester) (*authn.Redirect, bool) {
 	token := c.oauthService.GetCurrentOAuthToken(ctx, user)
 
 	namespace, id := user.GetNamespacedID()

pkg/services/authn/clients/proxy.go

@@ -14,7 +14,6 @@ import (
 
 	"github.com/grafana/grafana/pkg/infra/log"
 	"github.com/grafana/grafana/pkg/infra/remotecache"
-	authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/authn"
 	"github.com/grafana/grafana/pkg/services/login"
 	"github.com/grafana/grafana/pkg/setting"
@@ -150,14 +149,13 @@ func (c *Proxy) Hook(ctx context.Context, identity *authn.Identity, r *authn.Req
 		return nil
 	}
 
-	namespace, identifier := identity.GetNamespacedID()
+	if !identity.ID.IsNamespace(authn.NamespaceUser) {
-	if namespace != authn.NamespaceUser {
 		return nil
 	}
 
-	id, err := authidentity.IntIdentifier(namespace, identifier)
+	id, err := identity.ID.ParseInt()
 	if err != nil {
-		c.log.Warn("Failed to cache proxy user", "error", err, "userId", identifier, "err", err)
+		c.log.Warn("Failed to cache proxy user", "error", err, "userId", identity.ID.ID(), "err", err)
 		return nil
 	}
 

pkg/services/authn/error.go

@@ -8,5 +8,4 @@ var (
 	ErrClientNotConfigured = errutil.BadRequest("auth.client.notConfigured")
 	ErrUnsupportedIdentity = errutil.NotImplemented("auth.identity.unsupported")
 	ErrExpiredAccessToken  = errutil.Unauthorized("oauth.expired-token", errutil.WithPublicMessage("OAuth access token expired"))
-	ErrInvalidNamepsaceID  = errutil.BadRequest("auth.identity.invalid-namespace-id")
 )

pkg/services/authn/identity.go

@@ -7,7 +7,6 @@ import (
 
 	"golang.org/x/oauth2"
 
-	"github.com/grafana/grafana/pkg/models/roletype"
 	"github.com/grafana/grafana/pkg/models/usertoken"
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 	"github.com/grafana/grafana/pkg/services/login"
@@ -17,7 +16,9 @@ import (
 
 const GlobalOrgID = int64(0)
 
-var _ identity.Requester = (*Identity)(nil)
+type Requester = identity.Requester
+
+var _ Requester = (*Identity)(nil)
 
 type Identity struct {
 	// ID is the unique identifier for the entity in the Grafana database.
@@ -131,13 +132,13 @@ func (i *Identity) GetOrgName() string {
 	return i.OrgName
 }
 
-func (i *Identity) GetOrgRole() roletype.RoleType {
+func (i *Identity) GetOrgRole() org.RoleType {
 	if i.OrgRoles == nil {
-		return roletype.RoleNone
+		return org.RoleNone
 	}
 
 	if i.OrgRoles[i.GetOrgID()] == "" {
-		return roletype.RoleNone
+		return org.RoleNone
 	}
 
 	return i.OrgRoles[i.GetOrgID()]
@@ -172,7 +173,7 @@ func (i *Identity) GetTeams() []int64 {
 	return i.Teams
 }
 
-func (i *Identity) HasRole(role roletype.RoleType) bool {
+func (i *Identity) HasRole(role org.RoleType) bool {
 	if i.GetIsGrafanaAdmin() {
 		return true
 	}

pkg/services/authn/namespace.go

@@ -1,10 +1,6 @@
 package authn
 
 import (
-	"fmt"
-	"strconv"
-	"strings"
-
 	"github.com/grafana/grafana/pkg/services/auth/identity"
 )
 
@@ -19,97 +15,13 @@ const (
 
 var AnonymousNamespaceID = MustNewNamespaceID(NamespaceAnonymous, 0)
 
-var namespaceLookup = map[string]struct{}{
+type NamespaceID = identity.NamespaceID
-	NamespaceUser:           {},
-	NamespaceAPIKey:         {},
-	NamespaceServiceAccount: {},
-	NamespaceAnonymous:      {},
-	NamespaceRenderService:  {},
-	NamespaceAccessPolicy:   {},
-}
 
-func ParseNamespaceID(str string) (NamespaceID, error) {
+var (
-	var namespaceID NamespaceID
+	ParseNamespaceID        = identity.ParseNamespaceID
-
+	MustParseNamespaceID    = identity.MustParseNamespaceID
-	parts := strings.Split(str, ":")
+	NewNamespaceID          = identity.NewNamespaceID
-	if len(parts) != 2 {
+	MustNewNamespaceID      = identity.MustNewNamespaceID
-		return namespaceID, ErrInvalidNamepsaceID.Errorf("expected namespace id to have 2 parts")
+	NewNamespaceIDUnchecked = identity.NewNamespaceIDUnchecked
-	}
+	ErrInvalidNamespaceID   = identity.ErrInvalidNamespaceID
-
+)
-	namespace, id := parts[0], parts[1]
-
-	if _, ok := namespaceLookup[namespace]; !ok {
-		return namespaceID, ErrInvalidNamepsaceID.Errorf("got invalid namespace %s", namespace)
-	}
-
-	namespaceID.id = id
-	namespaceID.namespace = namespace
-
-	return namespaceID, nil
-}
-
-// MustParseNamespaceID parses namespace id, it will panic it failes to do so.
-// Sutable to use in tests or when we can garantuee that we pass a correct format.
-func MustParseNamespaceID(str string) NamespaceID {
-	namespaceID, err := ParseNamespaceID(str)
-	if err != nil {
-		panic(err)
-	}
-	return namespaceID
-}
-
-// NewNamespaceID creates a new NamespaceID, will fail for invalid namespace.
-func NewNamespaceID(namespace string, id int64) (NamespaceID, error) {
-	var namespaceID NamespaceID
-	if _, ok := namespaceLookup[namespace]; !ok {
-		return namespaceID, ErrInvalidNamepsaceID.Errorf("got invalid namespace %s", namespace)
-	}
-	namespaceID.id = strconv.FormatInt(id, 10)
-	namespaceID.namespace = namespace
-	return namespaceID, nil
-}
-
-// MustNewNamespaceID creates a new NamespaceID, will panic for invalid namespace.
-// Sutable to use in tests or when we can garantuee that we pass a correct format.
-func MustNewNamespaceID(namespace string, id int64) NamespaceID {
-	namespaceID, err := NewNamespaceID(namespace, id)
-	if err != nil {
-		panic(err)
-	}
-	return namespaceID
-}
-
-// NewNamespaceIDUnchecked creates a new NamespaceID without checking if namespace is valid.
-// It us up to the caller to ensure that namespace is valid.
-func NewNamespaceIDUnchecked(namespace string, id int64) NamespaceID {
-	return NamespaceID{
-		id:        strconv.FormatInt(id, 10),
-		namespace: namespace,
-	}
-}
-
-// FIXME: use this instead of encoded string through the codebase
-type NamespaceID struct {
-	id        string
-	namespace string
-}
-
-func (ni NamespaceID) ID() string {
-	return ni.id
-}
-
-func (ni NamespaceID) ParseInt() (int64, error) {
-	return strconv.ParseInt(ni.id, 10, 64)
-}
-
-func (ni NamespaceID) Namespace() string {
-	return ni.namespace
-}
-
-func (ni NamespaceID) IsNamespace(expected ...string) bool {
-	return identity.IsNamespace(ni.namespace, expected...)
-}
-
-func (ni NamespaceID) String() string {
-	return fmt.Sprintf("%s:%s", ni.namespace, ni.id)
-}