From 8bcd9c2594ba51ada3af20e0049299f5504759b8 Mon Sep 17 00:00:00 2001 From: Karl Persson Date: Tue, 13 Aug 2024 10:18:28 +0200 Subject: [PATCH] Identity: Remove typed id (#91801) * Refactor identity struct to store type in separate field * Update ResolveIdentity to take string representation of typedID * Add IsIdentityType to requester interface * Use IsIdentityType from interface * Remove usage of TypedID * Remote typedID struct * fix GetInternalID --- pkg/api/common_test.go | 4 +- pkg/api/dashboard.go | 4 +- pkg/api/folder.go | 4 +- pkg/api/index.go | 2 +- pkg/api/login_test.go | 4 +- pkg/api/org.go | 5 +- pkg/api/org_test.go | 5 +- pkg/api/user.go | 19 ++-- pkg/api/user_token.go | 9 +- pkg/apimachinery/identity/requester.go | 51 ++++++----- pkg/apimachinery/identity/static.go | 9 +- pkg/apimachinery/identity/typed_id.go | 87 ++----------------- pkg/middleware/auth_test.go | 14 +-- pkg/middleware/quota_test.go | 4 +- .../apis/dashboard/legacy/sql_dashboards.go | 13 +-- pkg/registry/apis/dashboard/sub_dto.go | 2 +- pkg/services/accesscontrol/accesscontrol.go | 14 +-- pkg/services/accesscontrol/acimpl/service.go | 8 +- pkg/services/accesscontrol/api/api.go | 13 +-- .../accesscontrol/authorize_in_org_test.go | 5 +- .../accesscontrol/database/database.go | 2 +- pkg/services/anonymous/anonimpl/client.go | 8 +- .../anonymous/anonimpl/client_test.go | 20 +++-- pkg/services/auth/idimpl/service.go | 4 +- pkg/services/auth/idimpl/service_test.go | 15 ++-- pkg/services/authn/authn.go | 8 +- pkg/services/authn/authnimpl/service.go | 49 +++++++---- pkg/services/authn/authnimpl/service_test.go | 54 ++++++------ .../authn/authnimpl/sync/oauth_token_sync.go | 8 +- .../authnimpl/sync/oauth_token_sync_test.go | 11 +-- pkg/services/authn/authnimpl/sync/org_sync.go | 16 ++-- .../authn/authnimpl/sync/org_sync_test.go | 23 ++--- .../authn/authnimpl/sync/rbac_sync.go | 4 +- .../authn/authnimpl/sync/rbac_sync_test.go | 33 ++++--- .../authn/authnimpl/sync/user_sync.go | 29 +++---- .../authn/authnimpl/sync/user_sync_test.go | 44 ++++++---- pkg/services/authn/authntest/fake.go | 2 +- pkg/services/authn/authntest/mock.go | 8 +- pkg/services/authn/clients/api_key.go | 21 +++-- pkg/services/authn/clients/api_key_test.go | 60 ++++++++----- pkg/services/authn/clients/basic_test.go | 6 +- pkg/services/authn/clients/ext_jwt.go | 27 +++--- pkg/services/authn/clients/ext_jwt_test.go | 26 +++--- pkg/services/authn/clients/grafana.go | 5 +- pkg/services/authn/clients/grafana_test.go | 5 +- pkg/services/authn/clients/oauth_test.go | 2 +- pkg/services/authn/clients/password_test.go | 10 +-- pkg/services/authn/clients/proxy.go | 13 +-- pkg/services/authn/clients/proxy_test.go | 8 +- pkg/services/authn/clients/render.go | 8 +- pkg/services/authn/clients/render_test.go | 8 +- pkg/services/authn/clients/session.go | 5 +- pkg/services/authn/clients/session_test.go | 15 ++-- pkg/services/authn/identity.go | 50 ++++++----- pkg/services/authz/server.go | 16 ++-- pkg/services/contexthandler/contexthandler.go | 11 ++- .../contexthandler/contexthandler_test.go | 9 +- pkg/services/dashboards/database/database.go | 2 +- .../dashboards/service/dashboard_service.go | 12 +-- .../dashboardsnapshots/database/database.go | 5 +- pkg/services/folder/folderimpl/sqlstore.go | 4 +- pkg/services/live/live.go | 3 +- pkg/services/ngalert/api/api_ruler.go | 10 ++- pkg/services/oauthtoken/oauth_token.go | 8 +- pkg/services/oauthtoken/oauth_token_test.go | 26 +++--- .../user_header_middleware.go | 3 +- pkg/services/serviceaccounts/api/api.go | 5 +- pkg/services/team/teamapi/team.go | 12 +-- pkg/services/user/identity.go | 20 ++--- pkg/services/user/userimpl/verifier.go | 4 +- .../unified/resource/grpc/authenticator.go | 16 ++-- pkg/util/proxyutil/proxyutil.go | 2 +- 72 files changed, 530 insertions(+), 521 deletions(-) diff --git a/pkg/api/common_test.go b/pkg/api/common_test.go index bc1b44a7a87..f94fb4dd29d 100644 --- a/pkg/api/common_test.go +++ b/pkg/api/common_test.go @@ -11,12 +11,12 @@ import ( "path/filepath" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/infra/fs" "github.com/grafana/grafana/pkg/infra/tracing" @@ -190,7 +190,7 @@ func getContextHandler(t *testing.T, cfg *setting.Cfg) *contexthandler.ContextHa return contexthandler.ProvideService( cfg, tracing.InitializeTracerForTest(), - &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: identity.AnonymousTypedID, SessionToken: &usertoken.UserToken{}}}, + &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: "0", Type: claims.TypeAnonymous, SessionToken: &usertoken.UserToken{}}}, ) } diff --git a/pkg/api/dashboard.go b/pkg/api/dashboard.go index 97a8693fb22..df13025462d 100644 --- a/pkg/api/dashboard.go +++ b/pkg/api/dashboard.go @@ -44,11 +44,11 @@ func (hs *HTTPServer) isDashboardStarredByUser(c *contextmodel.ReqContext, dashI return false, nil } - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return false, nil } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return false, err } diff --git a/pkg/api/folder.go b/pkg/api/folder.go index 3b379cd5291..889168ccdd9 100644 --- a/pkg/api/folder.go +++ b/pkg/api/folder.go @@ -195,8 +195,8 @@ func (hs *HTTPServer) setDefaultFolderPermissions(ctx context.Context, orgID int var permissions []accesscontrol.SetResourcePermissionCommand - if identity.IsIdentityType(user.GetID(), claims.TypeUser) { - userID, err := identity.UserIdentifier(user.GetID()) + if user.IsIdentityType(claims.TypeUser) { + userID, err := user.GetInternalID() if err != nil { return err } diff --git a/pkg/api/index.go b/pkg/api/index.go index 8f1eebad990..69aef128295 100644 --- a/pkg/api/index.go +++ b/pkg/api/index.go @@ -171,7 +171,7 @@ func (hs *HTTPServer) setIndexViewData(c *contextmodel.ReqContext) (*dtos.IndexV func (hs *HTTPServer) buildUserAnalyticsSettings(c *contextmodel.ReqContext) dtos.AnalyticsSettings { // Anonymous users do not have an email or auth info - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return dtos.AnalyticsSettings{Identifier: "@" + hs.Cfg.AppURL} } diff --git a/pkg/api/login_test.go b/pkg/api/login_test.go index c442b35cb3e..4e4fbcad787 100644 --- a/pkg/api/login_test.go +++ b/pkg/api/login_test.go @@ -12,13 +12,13 @@ import ( "strings" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/login/social" @@ -332,7 +332,7 @@ func TestLoginPostRedirect(t *testing.T) { HooksService: &hooks.HooksService{}, License: &licensing.OSSLicensingService{}, authnService: &authntest.FakeService{ - ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:42"), SessionToken: &usertoken.UserToken{}}, + ExpectedIdentity: &authn.Identity{ID: "42", Type: claims.TypeUser, SessionToken: &usertoken.UserToken{}}, }, AuthTokenService: authtest.NewFakeUserAuthTokenService(), Features: featuremgmt.WithFeatures(), diff --git a/pkg/api/org.go b/pkg/api/org.go index 88181f7e48a..882ebfb866d 100644 --- a/pkg/api/org.go +++ b/pkg/api/org.go @@ -9,7 +9,6 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/metrics" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/org" @@ -133,11 +132,11 @@ func (hs *HTTPServer) CreateOrg(c *contextmodel.ReqContext) response.Response { return response.Error(http.StatusBadRequest, "bad request data", err) } - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return response.Error(http.StatusForbidden, "Only users can create organizations", nil) } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "Failed to parse user id", err) } diff --git a/pkg/api/org_test.go b/pkg/api/org_test.go index 246b7cd303c..77042fb896c 100644 --- a/pkg/api/org_test.go +++ b/pkg/api/org_test.go @@ -6,10 +6,10 @@ import ( "strings" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/authn" @@ -267,7 +267,8 @@ func TestAPIEndpoint_GetOrg(t *testing.T) { for _, tt := range tests { t.Run(tt.desc, func(t *testing.T) { expectedIdentity := &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, OrgID: 1, Permissions: map[int64]map[string][]string{ 0: accesscontrol.GroupScopesByActionContext(context.Background(), tt.permissions), diff --git a/pkg/api/user.go b/pkg/api/user.go index 6179f71f0b2..3f66caeef38 100644 --- a/pkg/api/user.go +++ b/pkg/api/user.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/apimachinery/identity" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -32,18 +31,18 @@ import ( // 404: notFoundError // 500: internalServerError func (hs *HTTPServer) GetSignedInUser(c *contextmodel.ReqContext) response.Response { - if !identity.IsIdentityType(c.GetID(), claims.TypeUser) { + if !c.IsIdentityType(claims.TypeUser) { return response.JSON(http.StatusOK, user.UserProfileDTO{ IsGrafanaAdmin: c.SignedInUser.GetIsGrafanaAdmin(), OrgID: c.SignedInUser.GetOrgID(), - UID: c.SignedInUser.GetID().String(), + UID: c.SignedInUser.GetID(), Name: c.SignedInUser.NameOrFallback(), Email: c.SignedInUser.GetEmail(), Login: c.SignedInUser.GetLogin(), }) } - userID, err := identity.UserIdentifier(c.GetID()) + userID, err := c.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "Failed to parse user id", err) } @@ -278,7 +277,7 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserC } func (hs *HTTPServer) StartEmailVerificaton(c *contextmodel.ReqContext) response.Response { - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return response.Error(http.StatusBadRequest, "Only users can verify their email", nil) } @@ -287,7 +286,7 @@ func (hs *HTTPServer) StartEmailVerificaton(c *contextmodel.ReqContext) response return response.Respond(http.StatusNotModified, nil) } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "Got invalid user id", err) } @@ -505,12 +504,12 @@ func (hs *HTTPServer) ChangeActiveOrgAndRedirectToHome(c *contextmodel.ReqContex return } - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { c.JsonApiErr(http.StatusForbidden, "Endpoint only available for users", nil) return } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { c.JsonApiErr(http.StatusInternalServerError, "Failed to parse user id", err) return @@ -630,11 +629,11 @@ func (hs *HTTPServer) ClearHelpFlags(c *contextmodel.ReqContext) response.Respon } func getUserID(c *contextmodel.ReqContext) (int64, *response.NormalResponse) { - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return 0, response.Error(http.StatusForbidden, "Endpoint only available for users", nil) } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return 0, response.Error(http.StatusInternalServerError, "Failed to parse user id", err) } diff --git a/pkg/api/user_token.go b/pkg/api/user_token.go index 41ba01e7a11..166412292fe 100644 --- a/pkg/api/user_token.go +++ b/pkg/api/user_token.go @@ -11,7 +11,6 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/network" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" @@ -33,11 +32,11 @@ import ( // 403: forbiddenError // 500: internalServerError func (hs *HTTPServer) GetUserAuthTokens(c *contextmodel.ReqContext) response.Response { - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return response.Error(http.StatusForbidden, "entity not allowed to get tokens", nil) } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "failed to parse user id", err) } @@ -63,11 +62,11 @@ func (hs *HTTPServer) RevokeUserAuthToken(c *contextmodel.ReqContext) response.R return response.Error(http.StatusBadRequest, "bad request data", err) } - if !identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { + if !c.SignedInUser.IsIdentityType(claims.TypeUser) { return response.Error(http.StatusForbidden, "entity not allowed to revoke tokens", nil) } - userID, err := identity.UserIdentifier(c.SignedInUser.GetID()) + userID, err := c.SignedInUser.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "failed to parse user id", err) } diff --git a/pkg/apimachinery/identity/requester.go b/pkg/apimachinery/identity/requester.go index ff1e248368d..318fc0befed 100644 --- a/pkg/apimachinery/identity/requester.go +++ b/pkg/apimachinery/identity/requester.go @@ -15,13 +15,15 @@ type Requester interface { // GetIdentityType returns the type for the requester GetIdentityType() claims.IdentityType + // IsIdentityType returns true if identity type for requester matches any expected identity type + IsIdentityType(expected ...claims.IdentityType) bool // GetRawIdentifier returns only the identifier part of the UID, excluding the type GetRawIdentifier() string - // Deprecated: use GetUID instead + // GetInternalID returns only the identifier part of the ID, excluding the type GetInternalID() (int64, error) // GetID returns namespaced internalID for the entity // Deprecated: use GetUID instead - GetID() TypedID + GetID() string // GetDisplayName returns the display name of the active entity. // The display name is the name if it is set, otherwise the login or email. GetDisplayName() string @@ -81,11 +83,32 @@ type Requester interface { // IntIdentifier converts a typeID to an int64. // Applicable for users, service accounts, api keys and renderer service. // Errors if the identifier is not initialized or if type is not recognized. -func IntIdentifier(typedID TypedID) (int64, error) { - if claims.IsIdentityType(typedID.t, claims.TypeUser, claims.TypeAPIKey, claims.TypeServiceAccount, claims.TypeRenderService) { - id, err := strconv.ParseInt(typedID.ID(), 10, 64) +func IntIdentifier(typedID string) (int64, error) { + typ, id, err := ParseTypeAndID(typedID) + if err != nil { + return 0, err + } + + return intIdentifier(typ, id, claims.TypeUser, claims.TypeAPIKey, claims.TypeServiceAccount, claims.TypeRenderService) +} + +// UserIdentifier converts a typeID to an int64. +// Errors if the identifier is not initialized or if namespace is not recognized. +// Returns 0 if the type is not user or service account +func UserIdentifier(typedID string) (int64, error) { + typ, id, err := ParseTypeAndID(typedID) + if err != nil { + return 0, err + } + + return intIdentifier(typ, id, claims.TypeUser, claims.TypeServiceAccount) +} + +func intIdentifier(typ claims.IdentityType, id string, expected ...claims.IdentityType) (int64, error) { + if claims.IsIdentityType(typ, expected...) { + id, err := strconv.ParseInt(id, 10, 64) if err != nil { - return 0, fmt.Errorf("unrecognized format for valid type %s: %w", typedID.Type(), err) + return 0, fmt.Errorf("unrecognized format for valid type %s: %w", typ, err) } if id < 1 { @@ -97,19 +120,3 @@ func IntIdentifier(typedID TypedID) (int64, error) { return 0, ErrNotIntIdentifier } - -// UserIdentifier converts a typeID to an int64. -// Errors if the identifier is not initialized or if namespace is not recognized. -// Returns 0 if the type is not user or service account -func UserIdentifier(typedID TypedID) (int64, error) { - userID, err := IntIdentifier(typedID) - if err != nil { - return 0, err - } - - if claims.IsIdentityType(typedID.t, claims.TypeUser, claims.TypeServiceAccount) { - return userID, nil - } - - return 0, ErrInvalidIDType -} diff --git a/pkg/apimachinery/identity/static.go b/pkg/apimachinery/identity/static.go index 5d4f25e9a8a..443c8fb0a61 100644 --- a/pkg/apimachinery/identity/static.go +++ b/pkg/apimachinery/identity/static.go @@ -69,6 +69,11 @@ func (u *StaticRequester) GetIdentityType() claims.IdentityType { return u.Type } +// IsIdentityType implements Requester. +func (u *StaticRequester) IsIdentityType(expected ...claims.IdentityType) bool { + return claims.IsIdentityType(u.GetIdentityType(), expected...) +} + // GetExtra implements Requester. func (u *StaticRequester) GetExtra() map[string][]string { return map[string][]string{} @@ -158,8 +163,8 @@ func (u *StaticRequester) HasUniqueId() bool { return u.UserID > 0 } -// GetID returns namespaced id for the entity -func (u *StaticRequester) GetID() TypedID { +// GetID returns typed id for the entity +func (u *StaticRequester) GetID() string { return NewTypedIDString(u.Type, fmt.Sprintf("%d", u.UserID)) } diff --git a/pkg/apimachinery/identity/typed_id.go b/pkg/apimachinery/identity/typed_id.go index a2e81179396..753975d2411 100644 --- a/pkg/apimachinery/identity/typed_id.go +++ b/pkg/apimachinery/identity/typed_id.go @@ -2,101 +2,30 @@ package identity import ( "fmt" - "strconv" "strings" "github.com/grafana/authlib/claims" ) -// IsIdentityType returns true if typedID matches any expected identity type -func IsIdentityType(typedID TypedID, expected ...claims.IdentityType) bool { - for _, e := range expected { - if typedID.Type() == e { - return true - } - } - - return false -} - -var AnonymousTypedID = NewTypedID(claims.TypeAnonymous, 0) - -func ParseTypedID(str string) (TypedID, error) { - var typeID TypedID - +func ParseTypeAndID(str string) (claims.IdentityType, string, error) { parts := strings.Split(str, ":") if len(parts) != 2 { - return typeID, ErrInvalidTypedID.Errorf("expected typed id to have 2 parts") + return "", "", ErrInvalidTypedID.Errorf("expected typed id to have 2 parts") } t, err := claims.ParseType(parts[0]) if err != nil { - return typeID, err + return "", "", err } - typeID.id = parts[1] - typeID.t = t - - return typeID, nil + return t, parts[1], nil } -// MustParseTypedID 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 MustParseTypedID(str string) TypedID { - typeID, err := ParseTypedID(str) - if err != nil { - panic(err) - } - return typeID -} - -func NewTypedID(t claims.IdentityType, id int64) TypedID { - return TypedID{ - id: strconv.FormatInt(id, 10), - t: t, - } +func NewTypedID(t claims.IdentityType, id int64) string { + return fmt.Sprintf("%s:%d", t, id) } // NewTypedIDString creates a new TypedID with a string id -func NewTypedIDString(t claims.IdentityType, id string) TypedID { - return TypedID{ - id: id, - t: t, - } -} - -// FIXME: use this instead of encoded string through the codebase -type TypedID struct { - id string - t claims.IdentityType -} - -func (ni TypedID) ID() string { - return ni.id -} - -// UserID will try to parse and int64 identifier if namespace is either user or service-account. -// For all other namespaces '0' will be returned. -func (ni TypedID) UserID() (int64, error) { - if ni.IsType(claims.TypeUser, claims.TypeServiceAccount) { - return ni.ParseInt() - } - return 0, nil -} - -// ParseInt will try to parse the id as an int64 identifier. -func (ni TypedID) ParseInt() (int64, error) { - return strconv.ParseInt(ni.id, 10, 64) -} - -func (ni TypedID) Type() claims.IdentityType { - return ni.t -} - -func (ni TypedID) IsType(expected ...claims.IdentityType) bool { - return IsIdentityType(ni, expected...) -} - -func (ni TypedID) String() string { - return fmt.Sprintf("%s:%s", ni.t, ni.id) +func NewTypedIDString(t claims.IdentityType, id string) string { + return fmt.Sprintf("%s:%s", t, id) } diff --git a/pkg/middleware/auth_test.go b/pkg/middleware/auth_test.go index ee75387d963..9af27a76c98 100644 --- a/pkg/middleware/auth_test.go +++ b/pkg/middleware/auth_test.go @@ -7,10 +7,10 @@ import ( "net/http/httptest" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log/logtest" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" @@ -63,7 +63,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedIn should return 200 for anonymous user", path: "/api/secure", authMiddleware: ReqSignedIn, - identity: &authn.Identity{ID: identity.AnonymousTypedID}, + identity: &authn.Identity{Type: claims.TypeAnonymous}, expecedReached: true, expectedCode: http.StatusOK, }, @@ -71,7 +71,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedIn should return redirect anonymous user with forceLogin query string", path: "/secure?forceLogin=true", authMiddleware: ReqSignedIn, - identity: &authn.Identity{ID: identity.AnonymousTypedID}, + identity: &authn.Identity{Type: claims.TypeAnonymous}, expecedReached: false, expectedCode: http.StatusFound, }, @@ -79,7 +79,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedIn should return redirect anonymous user when orgId in query string is different from currently used", path: "/secure?orgId=2", authMiddleware: ReqSignedIn, - identity: &authn.Identity{ID: identity.AnonymousTypedID, OrgID: 1}, + identity: &authn.Identity{Type: claims.TypeAnonymous}, expecedReached: false, expectedCode: http.StatusFound, }, @@ -87,7 +87,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedInNoAnonymous should return 401 for anonymous user", path: "/api/secure", authMiddleware: ReqSignedInNoAnonymous, - identity: &authn.Identity{ID: identity.AnonymousTypedID}, + identity: &authn.Identity{Type: claims.TypeAnonymous}, expecedReached: false, expectedCode: http.StatusUnauthorized, }, @@ -95,7 +95,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "ReqSignedInNoAnonymous should return 200 for authenticated user", path: "/api/secure", authMiddleware: ReqSignedInNoAnonymous, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, expecedReached: true, expectedCode: http.StatusOK, }, @@ -103,7 +103,7 @@ func TestAuth_Middleware(t *testing.T) { desc: "snapshot public mode disabled should return 200 for authenticated user", path: "/api/secure", authMiddleware: SnapshotPublicModeOrSignedIn(&setting.Cfg{SnapshotPublicMode: false}), - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, expecedReached: true, expectedCode: http.StatusOK, }, diff --git a/pkg/middleware/quota_test.go b/pkg/middleware/quota_test.go index 94b6ee3e0e3..8a1f15a4118 100644 --- a/pkg/middleware/quota_test.go +++ b/pkg/middleware/quota_test.go @@ -3,9 +3,9 @@ package middleware import ( "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/quota/quotatest" @@ -53,7 +53,7 @@ func TestMiddlewareQuota(t *testing.T) { t.Run("with user logged in", func(t *testing.T) { setUp := func(sc *scenarioContext) { - sc.withIdentity(&authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{UserId: 12}}) + sc.withIdentity(&authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{UserId: 12}}) } middlewareScenario(t, "global datasource quota reached", func(t *testing.T, sc *scenarioContext) { diff --git a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go index 85426711c63..8a17e906f3f 100644 --- a/pkg/registry/apis/dashboard/legacy/sql_dashboards.go +++ b/pkg/registry/apis/dashboard/legacy/sql_dashboards.go @@ -339,10 +339,10 @@ func (a *dashboardSqlAccess) scanRow(rows *sql.Rows) (*dashboardRow, error) { func getUserID(v sql.NullString, id sql.NullInt64) string { if v.Valid && v.String != "" { - return identity.NewTypedIDString(claims.TypeUser, v.String).String() + return identity.NewTypedIDString(claims.TypeUser, v.String) } if id.Valid && id.Int64 == -1 { - return identity.NewTypedIDString(claims.TypeProvisioning, "").String() + return identity.NewTypedIDString(claims.TypeProvisioning, "") } return "" } @@ -395,9 +395,12 @@ func (a *dashboardSqlAccess) SaveDashboard(ctx context.Context, orgId int64, das dash.Spec.Remove("uid") } - userID, err := user.GetID().UserID() - if err != nil { - return nil, false, err + var userID int64 + if user.IsIdentityType(claims.TypeUser) { + userID, err = user.GetInternalID() + if err != nil { + return nil, false, err + } } meta, err := utils.MetaAccessor(dash) diff --git a/pkg/registry/apis/dashboard/sub_dto.go b/pkg/registry/apis/dashboard/sub_dto.go index 7e0cd3a60a1..b71374d8178 100644 --- a/pkg/registry/apis/dashboard/sub_dto.go +++ b/pkg/registry/apis/dashboard/sub_dto.go @@ -87,7 +87,7 @@ func (r *DTOConnector) Connect(ctx context.Context, name string, opts runtime.Ob access.CanSave, _ = guardian.CanSave() access.CanAdmin, _ = guardian.CanAdmin() access.CanDelete, _ = guardian.CanDelete() - access.CanStar = user.GetID().Type() == claims.TypeUser // not anon + access.CanStar = user.IsIdentityType(claims.TypeUser) access.AnnotationsPermissions = &dashboard.AnnotationPermission{} r.getAnnotationPermissionsByScope(ctx, user, &access.AnnotationsPermissions.Dashboard, accesscontrol.ScopeAnnotationsTypeDashboard) diff --git a/pkg/services/accesscontrol/accesscontrol.go b/pkg/services/accesscontrol/accesscontrol.go index a4a48dbb71f..a3fea676c1c 100644 --- a/pkg/services/accesscontrol/accesscontrol.go +++ b/pkg/services/accesscontrol/accesscontrol.go @@ -3,6 +3,7 @@ package accesscontrol import ( "context" "fmt" + "strconv" "strings" "go.opentelemetry.io/otel" @@ -84,8 +85,8 @@ type SearchOptions struct { Action string ActionSets []string Scope string - TypedID identity.TypedID // ID of the identity (ex: user:3, service-account:4) - wildcards Wildcards // private field computed based on the Scope + TypedID string // ID of the identity (ex: user:3, service-account:4) + wildcards Wildcards // private field computed based on the Scope RolePrefixes []string } @@ -105,17 +106,16 @@ func (s *SearchOptions) Wildcards() []string { } func (s *SearchOptions) ComputeUserID() (int64, error) { - id, err := s.TypedID.ParseInt() + typ, id, err := identity.ParseTypeAndID(s.TypedID) if err != nil { return 0, err } - // Validate namespace type is user or service account - if s.TypedID.Type() != claims.TypeUser && s.TypedID.Type() != claims.TypeServiceAccount { - return 0, fmt.Errorf("invalid type: %s", s.TypedID.Type()) + if !claims.IsIdentityType(typ, claims.TypeUser, claims.TypeServiceAccount) { + return 0, fmt.Errorf("invalid type: %s", typ) } - return id, nil + return strconv.ParseInt(id, 10, 64) } type SyncUserRolesCommand struct { diff --git a/pkg/services/accesscontrol/acimpl/service.go b/pkg/services/accesscontrol/acimpl/service.go index 07e9168bf6e..875ec43e234 100644 --- a/pkg/services/accesscontrol/acimpl/service.go +++ b/pkg/services/accesscontrol/acimpl/service.go @@ -212,9 +212,9 @@ func (s *Service) getUserDirectPermissions(ctx context.Context, user identity.Re defer span.End() var userID int64 - if identity.IsIdentityType(user.GetID(), claims.TypeUser, claims.TypeServiceAccount) { + if user.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { var err error - userID, err = identity.UserIdentifier(user.GetID()) + userID, err = user.GetInternalID() if err != nil { return nil, err } @@ -493,7 +493,7 @@ func (s *Service) SearchUsersPermissions(ctx context.Context, usr identity.Reque // Limit roles to available in OSS options.RolePrefixes = OSSRolesPrefixes - if options.TypedID.Type() != "" { + if options.TypedID != "" { userID, err := options.ComputeUserID() if err != nil { s.log.Error("Failed to resolve user ID", "error", err) @@ -608,7 +608,7 @@ func (s *Service) SearchUserPermissions(ctx context.Context, orgID int64, search timer := prometheus.NewTimer(metrics.MAccessPermissionsSummary) defer timer.ObserveDuration() - if searchOptions.TypedID.Type() == "" { + if searchOptions.TypedID == "" { return nil, fmt.Errorf("expected namespaced ID to be specified") } diff --git a/pkg/services/accesscontrol/api/api.go b/pkg/services/accesscontrol/api/api.go index a56f21b4f61..88241e0d1cf 100644 --- a/pkg/services/accesscontrol/api/api.go +++ b/pkg/services/accesscontrol/api/api.go @@ -5,7 +5,6 @@ import ( "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/middleware" "github.com/grafana/grafana/pkg/middleware/requestmeta" ac "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -72,23 +71,17 @@ func (api *AccessControlAPI) searchUsersPermissions(c *contextmodel.ReqContext) ActionPrefix: c.Query("actionPrefix"), Action: c.Query("action"), Scope: c.Query("scope"), + TypedID: c.Query("namespacedId"), } - namespacedId := c.Query("namespacedId") // Validate inputs if searchOptions.ActionPrefix != "" && searchOptions.Action != "" { return response.JSON(http.StatusBadRequest, "'action' and 'actionPrefix' are mutually exclusive") } - if namespacedId == "" && searchOptions.ActionPrefix == "" && searchOptions.Action == "" { + + if searchOptions.TypedID == "" && searchOptions.ActionPrefix == "" && searchOptions.Action == "" { return response.JSON(http.StatusBadRequest, "at least one search option must be provided") } - if namespacedId != "" { - var err error - searchOptions.TypedID, err = identity.ParseTypedID(namespacedId) - if err != nil { - return response.Error(http.StatusBadGateway, "invalid namespacedId", err) - } - } // Compute metadata permissions, err := api.Service.SearchUsersPermissions(c.Req.Context(), c.SignedInUser, searchOptions) diff --git a/pkg/services/accesscontrol/authorize_in_org_test.go b/pkg/services/accesscontrol/authorize_in_org_test.go index e85ced6c2bd..5c5819b1b26 100644 --- a/pkg/services/accesscontrol/authorize_in_org_test.go +++ b/pkg/services/accesscontrol/authorize_in_org_test.go @@ -5,12 +5,12 @@ import ( "fmt" "net/http" "net/http/httptest" + "strconv" "testing" "github.com/stretchr/testify/assert" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" @@ -188,7 +188,8 @@ func TestAuthorizeInOrgMiddleware(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/api/endpoint", nil) expectedIdentity := &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, tc.ctxSignedInUser.UserID), + ID: strconv.FormatInt(tc.ctxSignedInUser.UserID, 10), + Type: claims.TypeUser, OrgID: tc.targetOrgId, Permissions: map[int64]map[string][]string{}, } diff --git a/pkg/services/accesscontrol/database/database.go b/pkg/services/accesscontrol/database/database.go index 43c3fa448ee..afa8814c109 100644 --- a/pkg/services/accesscontrol/database/database.go +++ b/pkg/services/accesscontrol/database/database.go @@ -164,7 +164,7 @@ func (s *AccessControlStore) SearchUsersPermissions(ctx context.Context, orgID i dbPerms := make([]UserRBACPermission, 0) userID := int64(-1) - if options.TypedID.Type() != "" { + if options.TypedID != "" { var err error userID, err = options.ComputeUserID() if err != nil { diff --git a/pkg/services/anonymous/anonimpl/client.go b/pkg/services/anonymous/anonimpl/client.go index 9af940ead3d..57b4c9873fb 100644 --- a/pkg/services/anonymous/anonimpl/client.go +++ b/pkg/services/anonymous/anonimpl/client.go @@ -8,7 +8,6 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/errutil" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/anonymous" "github.com/grafana/grafana/pkg/services/anonymous/anonimpl/anonstore" @@ -74,7 +73,7 @@ func (a *Anonymous) IdentityType() claims.IdentityType { return claims.TypeAnonymous } -func (a *Anonymous) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (a *Anonymous) ResolveIdentity(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*authn.Identity, error) { o, err := a.orgService.GetByName(ctx, &org.GetOrgByNameQuery{Name: a.cfg.AnonymousOrgName}) if err != nil { return nil, err @@ -85,7 +84,7 @@ func (a *Anonymous) ResolveIdentity(ctx context.Context, orgID int64, namespaceI } // Anonymous identities should always have the same namespace id. - if namespaceID != identity.AnonymousTypedID { + if !claims.IsIdentityType(typ, claims.TypeAnonymous) || id != "0" { return nil, errInvalidID } @@ -110,7 +109,8 @@ func (a *Anonymous) Priority() uint { func (a *Anonymous) newAnonymousIdentity(o *org.Org) *authn.Identity { return &authn.Identity{ - ID: identity.AnonymousTypedID, + ID: "0", + Type: claims.TypeAnonymous, OrgID: o.ID, OrgName: o.Name, OrgRoles: map[int64]org.RoleType{o.ID: org.RoleType(a.cfg.AnonymousOrgRole)}, diff --git a/pkg/services/anonymous/anonimpl/client_test.go b/pkg/services/anonymous/anonimpl/client_test.go index 7a57a51cb74..1cff62e6a25 100644 --- a/pkg/services/anonymous/anonimpl/client_test.go +++ b/pkg/services/anonymous/anonimpl/client_test.go @@ -8,7 +8,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/anonymous/anontest" "github.com/grafana/grafana/pkg/services/authn" @@ -60,7 +60,7 @@ func TestAnonymous_Authenticate(t *testing.T) { } else { require.Nil(t, err) - assert.Equal(t, identity.AnonymousTypedID, user.ID) + assert.Equal(t, "anonymous:0", user.GetID()) assert.Equal(t, tt.org.ID, user.OrgID) assert.Equal(t, tt.org.Name, user.OrgName) assert.Equal(t, tt.cfg.AnonymousOrgRole, string(user.GetOrgRole())) @@ -74,7 +74,8 @@ func TestAnonymous_ResolveIdentity(t *testing.T) { desc string cfg *setting.Cfg orgID int64 - namespaceID identity.TypedID + typ claims.IdentityType + id string org *org.Org orgErr error expectedErr error @@ -88,7 +89,8 @@ func TestAnonymous_ResolveIdentity(t *testing.T) { AnonymousOrgName: "some org", }, orgID: 1, - namespaceID: identity.AnonymousTypedID, + typ: claims.TypeAnonymous, + id: "0", expectedErr: errInvalidOrg, }, { @@ -98,7 +100,8 @@ func TestAnonymous_ResolveIdentity(t *testing.T) { AnonymousOrgName: "some org", }, orgID: 1, - namespaceID: identity.MustParseTypedID("anonymous:1"), + typ: claims.TypeAnonymous, + id: "1", expectedErr: errInvalidID, }, { @@ -107,8 +110,9 @@ func TestAnonymous_ResolveIdentity(t *testing.T) { cfg: &setting.Cfg{ AnonymousOrgName: "some org", }, - orgID: 1, - namespaceID: identity.AnonymousTypedID, + orgID: 1, + typ: claims.TypeAnonymous, + id: "0", }, } @@ -121,7 +125,7 @@ func TestAnonymous_ResolveIdentity(t *testing.T) { anonDeviceService: anontest.NewFakeService(), } - identity, err := c.ResolveIdentity(context.Background(), tt.orgID, tt.namespaceID) + identity, err := c.ResolveIdentity(context.Background(), tt.orgID, tt.typ, tt.id) if tt.expectedErr != nil { assert.ErrorIs(t, err, tt.expectedErr) assert.Nil(t, identity) diff --git a/pkg/services/auth/idimpl/service.go b/pkg/services/auth/idimpl/service.go index 1c696255b34..81481f1b77d 100644 --- a/pkg/services/auth/idimpl/service.go +++ b/pkg/services/auth/idimpl/service.go @@ -91,7 +91,7 @@ func (s *Service) SignIdentity(ctx context.Context, id identity.Requester) (stri Claims: &jwt.Claims{ Issuer: s.cfg.AppURL, Audience: getAudience(id.GetOrgID()), - Subject: id.GetID().String(), + Subject: id.GetID(), Expiry: jwt.NewNumericDate(now.Add(tokenTTL)), IssuedAt: jwt.NewNumericDate(now), }, @@ -100,7 +100,7 @@ func (s *Service) SignIdentity(ctx context.Context, id identity.Requester) (stri }, } - if identity.IsIdentityType(id.GetID(), authnlibclaims.TypeUser) { + if id.IsIdentityType(authnlibclaims.TypeUser) { claims.Rest.Email = id.GetEmail() claims.Rest.EmailVerified = id.IsEmailVerified() claims.Rest.AuthenticatedBy = id.GetAuthenticatedBy() diff --git a/pkg/services/auth/idimpl/service_test.go b/pkg/services/auth/idimpl/service_test.go index b0407743679..4ae1ea628a7 100644 --- a/pkg/services/auth/idimpl/service_test.go +++ b/pkg/services/auth/idimpl/service_test.go @@ -10,7 +10,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/idtest" @@ -71,7 +70,7 @@ func TestService_SignIdentity(t *testing.T) { featuremgmt.WithFeatures(featuremgmt.FlagIdForwarding), &authntest.FakeService{}, nil, ) - token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ID: identity.MustParseTypedID("user:1")}) + token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ID: "1", Type: claims.TypeUser}) require.NoError(t, err) require.NotEmpty(t, token) }) @@ -83,10 +82,12 @@ func TestService_SignIdentity(t *testing.T) { &authntest.FakeService{}, nil, ) token, _, err := s.SignIdentity(context.Background(), &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, AuthenticatedBy: login.AzureADAuthModule, Login: "U1", - UID: identity.NewTypedIDString(claims.TypeUser, "edpu3nnt61se8e")}) + UID: "edpu3nnt61se8e", + }) require.NoError(t, err) parsed, err := jwt.ParseSigned(token) @@ -106,10 +107,12 @@ func TestService_SignIdentity(t *testing.T) { &authntest.FakeService{}, nil, ) _, gotClaims, err := s.SignIdentity(context.Background(), &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, AuthenticatedBy: login.AzureADAuthModule, Login: "U1", - UID: identity.NewTypedIDString(claims.TypeUser, "edpu3nnt61se8e")}) + UID: "edpu3nnt61se8e", + }) require.NoError(t, err) assert.Equal(t, login.AzureADAuthModule, gotClaims.Rest.AuthenticatedBy) diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index f7f2686a134..9ec4ac6de35 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -95,8 +95,8 @@ type Service interface { Logout(ctx context.Context, user identity.Requester, sessionToken *usertoken.UserToken) (*Redirect, error) // RegisterPreLogoutHook registers a hook that is called before a logout request. RegisterPreLogoutHook(hook PreLogoutHookFn, priority uint) - // ResolveIdentity resolves an identity from org and namespace id. - ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*Identity, error) + // ResolveIdentity resolves an identity from orgID and typedID. + ResolveIdentity(ctx context.Context, orgID int64, typedID string) (*Identity, error) // RegisterClient will register a new authn.Client that can be used for authentication RegisterClient(c Client) @@ -177,11 +177,11 @@ type UsageStatClient interface { } // IdentityResolverClient is an optional interface that auth clients can implement. -// Clients that implements this interface can resolve an full identity from an orgID and namespaceID. +// Clients that implements this interface can resolve an full identity from an orgID and typedID. type IdentityResolverClient interface { Client IdentityType() claims.IdentityType - ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*Identity, error) + ResolveIdentity(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*Identity, error) } type Request struct { diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 63f587e5ca4..9d6e047f22e 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -7,12 +7,12 @@ import ( "strconv" "strings" + "github.com/grafana/authlib/claims" "github.com/prometheus/client_golang/prometheus" "go.opentelemetry.io/otel/attribute" "go.opentelemetry.io/otel/codes" "go.opentelemetry.io/otel/trace" - "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" @@ -145,9 +145,9 @@ func (s *Service) authenticate(ctx context.Context, c authn.Client, r *authn.Req } span.SetAttributes( - attribute.String("identity.ID", identity.ID.String()), - attribute.String("identity.AuthID", identity.AuthID), - attribute.String("identity.AuthenticatedBy", identity.AuthenticatedBy), + attribute.String("identity.ID", identity.GetID()), + attribute.String("identity.AuthID", identity.GetAuthID()), + attribute.String("identity.AuthenticatedBy", identity.GetAuthenticatedBy()), ) if len(identity.ClientParams.FetchPermissionsParams.ActionsLookup) > 0 { @@ -218,12 +218,12 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i } // Login is only supported for users - if !id.ID.IsType(claims.TypeUser) { + if !id.IsIdentityType(claims.TypeUser) { s.metrics.failedLogin.WithLabelValues(client).Inc() - return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", id.ID.Type()) + return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", id.GetIdentityType()) } - userID, err := id.ID.ParseInt() + userID, err := id.GetInternalID() if err != nil { return nil, err } @@ -282,11 +282,11 @@ func (s *Service) Logout(ctx context.Context, user identity.Requester, sessionTo redirect.URL = s.cfg.SignoutRedirectUrl } - if !user.GetID().IsType(claims.TypeUser) { + if !user.IsIdentityType(claims.TypeUser) { return redirect, nil } - id, err := user.GetID().ParseInt() + id, err := user.GetInternalID() if err != nil { s.log.FromContext(ctx).Debug("Invalid user id", "id", id, "err", err) return redirect, nil @@ -329,7 +329,7 @@ Default: return redirect, nil } -func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, typedID string) (*authn.Identity, error) { ctx, span := s.tracer.Start(ctx, "authn.ResolveIdentity") defer span.End() @@ -338,8 +338,12 @@ func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID // hack to not update last seen r.SetMeta(authn.MetaKeyIsLogin, "true") - identity, err := s.resolveIdenity(ctx, orgID, namespaceID) + identity, err := s.resolveIdenity(ctx, orgID, typedID) if err != nil { + if errors.Is(err, claims.ErrInvalidTypedID) { + return nil, authn.ErrUnsupportedIdentity.Errorf("invalid identity type") + } + return nil, err } @@ -377,14 +381,20 @@ func (s *Service) SyncIdentity(ctx context.Context, identity *authn.Identity) er return s.runPostAuthHooks(ctx, identity, r) } -func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (s *Service) resolveIdenity(ctx context.Context, orgID int64, typedID string) (*authn.Identity, error) { ctx, span := s.tracer.Start(ctx, "authn.resolveIdentity") defer span.End() - if namespaceID.IsType(claims.TypeUser) { + t, i, err := identity.ParseTypeAndID(typedID) + if err != nil { + return nil, err + } + + if claims.IsIdentityType(t, claims.TypeUser) { return &authn.Identity{ OrgID: orgID, - ID: namespaceID, + ID: i, + Type: claims.TypeUser, ClientParams: authn.ClientParams{ AllowGlobalOrg: true, FetchSyncedUser: true, @@ -392,9 +402,10 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID i }}, nil } - if namespaceID.IsType(claims.TypeServiceAccount) { + if claims.IsIdentityType(t, claims.TypeServiceAccount) { return &authn.Identity{ - ID: namespaceID, + ID: i, + Type: claims.TypeServiceAccount, OrgID: orgID, ClientParams: authn.ClientParams{ AllowGlobalOrg: true, @@ -403,11 +414,11 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID i }}, nil } - resolver, ok := s.idenityResolverClients[string(namespaceID.Type())] + resolver, ok := s.idenityResolverClients[string(t)] if !ok { - return nil, authn.ErrUnsupportedIdentity.Errorf("no resolver for : %s", namespaceID.Type()) + return nil, authn.ErrUnsupportedIdentity.Errorf("no resolver for : %s", t) } - return resolver.ResolveIdentity(ctx, orgID, namespaceID) + return resolver.ResolveIdentity(ctx, orgID, t, i) } func (s *Service) errorLogFunc(ctx context.Context, err error) func(msg string, ctx ...any) { diff --git a/pkg/services/authn/authnimpl/service_test.go b/pkg/services/authn/authnimpl/service_test.go index 8eb0f49846c..216de8300d8 100644 --- a/pkg/services/authn/authnimpl/service_test.go +++ b/pkg/services/authn/authnimpl/service_test.go @@ -9,13 +9,13 @@ import ( "slices" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.opentelemetry.io/otel/codes" sdktrace "go.opentelemetry.io/otel/sdk/trace" "go.opentelemetry.io/otel/sdk/trace/tracetest" - "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -45,9 +45,9 @@ func TestService_Authenticate(t *testing.T) { { desc: "should succeed with authentication for configured client", clients: []authn.Client{ - &authntest.FakeClient{ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}}, + &authntest.FakeClient{ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}}, }, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + expectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}, }, { desc: "should succeed with authentication for configured client for identity with fetch permissions params", @@ -55,7 +55,8 @@ func TestService_Authenticate(t *testing.T) { &authntest.FakeClient{ ExpectedTest: true, ExpectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, ClientParams: authn.ClientParams{ FetchPermissionsParams: authn.FetchPermissionsParams{ ActionsLookup: []string{ @@ -71,7 +72,8 @@ func TestService_Authenticate(t *testing.T) { }, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, ClientParams: authn.ClientParams{ FetchPermissionsParams: authn.FetchPermissionsParams{ ActionsLookup: []string{ @@ -93,19 +95,19 @@ func TestService_Authenticate(t *testing.T) { ExpectedName: "2", ExpectedPriority: 2, ExpectedTest: true, - ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), AuthID: "service:some-service", AuthenticatedBy: "service_auth"}, + ExpectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser, AuthID: "service:some-service", AuthenticatedBy: "service_auth"}, }, }, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), AuthID: "service:some-service", AuthenticatedBy: "service_auth"}, + expectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser, AuthID: "service:some-service", AuthenticatedBy: "service_auth"}, }, { desc: "should succeed with authentication for third client when error happened in first", clients: []authn.Client{ &authntest.FakeClient{ExpectedName: "1", ExpectedPriority: 2, ExpectedTest: false}, &authntest.FakeClient{ExpectedName: "2", ExpectedPriority: 1, ExpectedTest: true, ExpectedErr: errors.New("some error")}, - &authntest.FakeClient{ExpectedName: "3", ExpectedPriority: 3, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:3")}}, + &authntest.FakeClient{ExpectedName: "3", ExpectedPriority: 3, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: "3", Type: claims.TypeUser}}, }, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:3")}, + expectedIdentity: &authn.Identity{ID: "3", Type: claims.TypeUser}, }, { desc: "should return error when no client could authenticate the request", @@ -180,7 +182,7 @@ func TestService_Authenticate(t *testing.T) { for _, attr := range passedAuthnSpan.Attributes() { switch attr.Key { case "identity.ID": - assert.Equal(t, tt.expectedIdentity.ID.String(), attr.Value.AsString()) + assert.Equal(t, tt.expectedIdentity.GetID(), attr.Value.AsString()) case "identity.AuthID": assert.Equal(t, tt.expectedIdentity.AuthID, attr.Value.AsString()) case "identity.AuthenticatedBy": @@ -316,10 +318,12 @@ func TestService_Login(t *testing.T) { client: "fake", expectedClientOK: true, expectedClientIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, SessionToken: &auth.UserToken{UserId: 1}, }, }, @@ -332,7 +336,7 @@ func TestService_Login(t *testing.T) { desc: "should not login non user identity", client: "fake", expectedClientOK: true, - expectedClientIdentity: &authn.Identity{ID: identity.MustParseTypedID("api-key:1")}, + expectedClientIdentity: &authn.Identity{ID: "1", Type: claims.TypeAPIKey}, expectedErr: authn.ErrUnsupportedIdentity, }, } @@ -421,31 +425,31 @@ func TestService_Logout(t *testing.T) { tests := []TestCase{ { desc: "should redirect to default redirect url when identity is not a user", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeServiceAccount, 1)}, + identity: &authn.Identity{ID: "1", Type: claims.TypeServiceAccount}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, }, { desc: "should redirect to default redirect url when no external provider was used to authenticate", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1)}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, expectedTokenRevoked: true, }, { desc: "should redirect to default redirect url when client is not found", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1), AuthenticatedBy: "notfound"}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, AuthenticatedBy: "notfound"}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, expectedTokenRevoked: true, }, { desc: "should redirect to default redirect url when client do not implement logout extension", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1), AuthenticatedBy: "azuread"}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, AuthenticatedBy: "azuread"}, expectedRedirect: &authn.Redirect{URL: "http://localhost:3000/login"}, client: &authntest.FakeClient{ExpectedName: "auth.client.azuread"}, expectedTokenRevoked: true, }, { desc: "should use signout redirect url if configured", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1), AuthenticatedBy: "azuread"}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, AuthenticatedBy: "azuread"}, expectedRedirect: &authn.Redirect{URL: "some-url"}, client: &authntest.FakeClient{ExpectedName: "auth.client.azuread"}, signoutRedirectURL: "some-url", @@ -453,7 +457,7 @@ func TestService_Logout(t *testing.T) { }, { desc: "should redirect to client specific url", - identity: &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1), AuthenticatedBy: "azuread"}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, AuthenticatedBy: "azuread"}, expectedRedirect: &authn.Redirect{URL: "http://idp.com/logout"}, client: &authntest.MockClient{ NameFunc: func() string { return "auth.client.azuread" }, @@ -501,26 +505,26 @@ func TestService_Logout(t *testing.T) { 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, identity.NewTypedID("some", 1)) + _, err := svc.ResolveIdentity(context.Background(), 1, "some:1") assert.ErrorIs(t, err, authn.ErrUnsupportedIdentity) }) t.Run("should return error for for namespace that don't have a resolver", func(t *testing.T) { svc := setupTests(t) - _, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("api-key:1")) + _, err := svc.ResolveIdentity(context.Background(), 1, "api-key:1") assert.ErrorIs(t, err, authn.ErrUnsupportedIdentity) }) t.Run("should resolve for user", func(t *testing.T) { svc := setupTests(t) - identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("user:1")) + identity, err := svc.ResolveIdentity(context.Background(), 1, "user:1") assert.NoError(t, err) assert.NotNil(t, identity) }) t.Run("should resolve for service account", func(t *testing.T) { svc := setupTests(t) - identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("service-account:1")) + identity, err := svc.ResolveIdentity(context.Background(), 1, "service-account:1") assert.NoError(t, err) assert.NotNil(t, identity) }) @@ -529,13 +533,13 @@ func TestService_ResolveIdentity(t *testing.T) { svc := setupTests(t, func(svc *Service) { svc.RegisterClient(&authntest.MockClient{ IdentityTypeFunc: func() claims.IdentityType { return claims.TypeAPIKey }, - ResolveIdentityFunc: func(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { + ResolveIdentityFunc: func(_ context.Context, _ int64, _ claims.IdentityType, _ string) (*authn.Identity, error) { return &authn.Identity{}, nil }, }) }) - identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("api-key:1")) + identity, err := svc.ResolveIdentity(context.Background(), 1, "api-key:1") assert.NoError(t, err) assert.NotNil(t, identity) }) diff --git a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go index cecc3cc5218..d68c3925317 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync.go @@ -6,9 +6,9 @@ import ( "strings" "time" + "github.com/grafana/authlib/claims" "golang.org/x/sync/singleflight" - "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/login/social" @@ -42,7 +42,7 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Ident defer span.End() // only perform oauth token check if identity is a user - if !id.ID.IsType(claims.TypeUser) { + if !id.IsIdentityType(claims.TypeUser) { return nil } @@ -56,9 +56,9 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Ident return nil } - ctxLogger := s.log.FromContext(ctx).New("userID", id.ID.ID()) + ctxLogger := s.log.FromContext(ctx).New("userID", id.GetID()) - _, err, _ := s.singleflightGroup.Do(id.ID.String(), func() (interface{}, error) { + _, err, _ := s.singleflightGroup.Do(id.GetID(), func() (interface{}, error) { ctxLogger.Debug("Singleflight request for OAuth token sync") // FIXME: Consider using context.WithoutCancel instead of context.Background after Go 1.21 update diff --git a/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go b/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go index 326a8d1eea7..8c35cf652c1 100644 --- a/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "golang.org/x/sync/singleflight" @@ -42,17 +43,17 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { tests := []testCase{ { desc: "should skip sync when identity is not a user", - identity: &authn.Identity{ID: identity.MustParseTypedID("service-account:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeServiceAccount}, expectTryRefreshTokenCalled: false, }, { desc: "should skip sync when identity is a user but is not authenticated with session token", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, expectTryRefreshTokenCalled: false, }, { desc: "should invalidate access token and session token if token refresh fails", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: true, expectedTryRefreshErr: errors.New("some err"), expectTryRefreshTokenCalled: true, @@ -63,7 +64,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { }, { desc: "should refresh the token successfully", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: false, expectTryRefreshTokenCalled: true, expectInvalidateOauthTokensCalled: false, @@ -71,7 +72,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) { }, { desc: "should not invalidate the token if the token has already been refreshed by another request (singleflight)", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser, SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule}, expectHasEntryCalled: true, expectTryRefreshTokenCalled: true, expectInvalidateOauthTokensCalled: false, diff --git a/pkg/services/authn/authnimpl/sync/org_sync.go b/pkg/services/authn/authnimpl/sync/org_sync.go index 0ef1d5150d2..87d79215ce5 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync.go +++ b/pkg/services/authn/authnimpl/sync/org_sync.go @@ -39,14 +39,14 @@ func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *a ctxLogger := s.log.FromContext(ctx).New("id", id.ID, "login", id.Login) - if !id.ID.IsType(claims.TypeUser) { - ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "type", id.ID.Type()) + if !id.IsIdentityType(claims.TypeUser) { + ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "type", id.GetIdentityType()) return nil } - userID, err := id.ID.ParseInt() + userID, err := id.GetInternalID() if err != nil { - ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "type", id.ID.Type(), "err", err) + ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "type", id.GetIdentityType(), "err", err) return nil } @@ -145,14 +145,14 @@ func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn. ctxLogger := s.log.FromContext(ctx) - if !currentIdentity.ID.IsType(claims.TypeUser) { - ctxLogger.Debug("Skipping default org sync, not a user", "type", currentIdentity.ID.Type()) + if !currentIdentity.IsIdentityType(claims.TypeUser) { + ctxLogger.Debug("Skipping default org sync, not a user", "type", currentIdentity.GetIdentityType()) return } - userID, err := currentIdentity.ID.ParseInt() + userID, err := currentIdentity.GetInternalID() if err != nil { - ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "type", currentIdentity.ID.Type(), "err", err) + ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "type", currentIdentity.GetIdentityType(), "err", err) return } diff --git a/pkg/services/authn/authnimpl/sync/org_sync_test.go b/pkg/services/authn/authnimpl/sync/org_sync_test.go index b6d14096783..b62bcbcbf00 100644 --- a/pkg/services/authn/authnimpl/sync/org_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/org_sync_test.go @@ -5,6 +5,7 @@ import ( "fmt" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -76,7 +77,8 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { args: args{ ctx: context.Background(), id: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, Login: "test", Name: "test", Email: "test", @@ -92,7 +94,8 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) { }, }, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, Login: "test", Name: "test", Email: "test", @@ -139,7 +142,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should set default org", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { userService.On("Update", mock.Anything, mock.MatchedBy(func(cmd *user.UpdateUserCommand) bool { return cmd.UserID == 1 && *cmd.OrgID == 2 @@ -149,7 +152,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip setting the default org when default org is not set", defaultOrgSetting: -1, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, }, { name: "should skip setting the default org when identity is nil", @@ -159,28 +162,28 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip setting the default org when input err is not nil", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, inputErr: fmt.Errorf("error"), }, { name: "should skip setting the default org when identity is not a user", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("service-account:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeServiceAccount}, }, { name: "should skip setting the default org when user id is not valid", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:invalid")}, + identity: &authn.Identity{ID: "invalid", Type: claims.TypeUser}, }, { name: "should skip setting the default org when user is not allowed to use the configured default org", defaultOrgSetting: 3, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, }, { name: "should skip setting the default org when validateUsingOrg returns error", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { orgService.ExpectedError = fmt.Errorf("error") }, @@ -188,7 +191,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) { { name: "should skip the hook when the user org update was unsuccessful", defaultOrgSetting: 2, - identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + identity: &authn.Identity{ID: "1", Type: claims.TypeUser}, setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) { userService.On("Update", mock.Anything, mock.Anything).Return(fmt.Errorf("error")) }, diff --git a/pkg/services/authn/authnimpl/sync/rbac_sync.go b/pkg/services/authn/authnimpl/sync/rbac_sync.go index e9ed94ff0d6..f5b11c687ca 100644 --- a/pkg/services/authn/authnimpl/sync/rbac_sync.go +++ b/pkg/services/authn/authnimpl/sync/rbac_sync.go @@ -148,12 +148,12 @@ func (s *RBACSync) SyncCloudRoles(ctx context.Context, ident *authn.Identity, r return nil } - if !ident.ID.IsType(claims.TypeUser) { + if !ident.IsIdentityType(claims.TypeUser) { s.log.FromContext(ctx).Debug("Skip syncing cloud role", "id", ident.ID) return nil } - userID, err := ident.ID.ParseInt() + userID, err := ident.GetInternalID() if err != nil { return err } diff --git a/pkg/services/authn/authnimpl/sync/rbac_sync_test.go b/pkg/services/authn/authnimpl/sync/rbac_sync_test.go index f38b83679ed..08e3434d15d 100644 --- a/pkg/services/authn/authnimpl/sync/rbac_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/rbac_sync_test.go @@ -4,10 +4,10 @@ import ( "context" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" @@ -27,14 +27,14 @@ func TestRBACSync_SyncPermission(t *testing.T) { testCases := []testCase{ { name: "enriches the identity successfully when SyncPermissions is true", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, + identity: &authn.Identity{ID: "2", Type: claims.TypeUser, OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, expectedPermissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionUsersRead}, }, }, { name: "does not load the permissions when SyncPermissions is false", - identity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, + identity: &authn.Identity{ID: "2", Type: claims.TypeUser, OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}}, expectedPermissions: []accesscontrol.Permission{ {Action: accesscontrol.ActionUsersRead}, }, @@ -68,7 +68,8 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has viewer role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, }, @@ -79,7 +80,8 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has editor role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, }, @@ -90,7 +92,8 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should call sync when authenticated with grafana com and has admin role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, }, @@ -101,7 +104,8 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should not call sync when authenticated with grafana com and has invalid role", module: login.GrafanaComAuthModule, identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleType("something else")}, }, @@ -112,7 +116,8 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) { desc: "should not call sync when not authenticated with grafana com", module: login.LDAPAuthModule, identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, }, @@ -158,7 +163,8 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) { { desc: "should map Cloud Viewer to Grafana Cloud Viewer and Support ticket reader", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, }, @@ -177,7 +183,8 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) { { desc: "should map Cloud Editor to Grafana Cloud Editor and Support ticket admin", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, }, @@ -195,7 +202,8 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) { { desc: "should map Cloud Admin to Grafana Cloud Admin and Support ticket admin", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, }, @@ -213,7 +221,8 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) { { desc: "should return an error for not supported role", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleNone}, }, diff --git a/pkg/services/authn/authnimpl/sync/user_sync.go b/pkg/services/authn/authnimpl/sync/user_sync.go index 4c00d809b38..fa682806ea3 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync.go +++ b/pkg/services/authn/authnimpl/sync/user_sync.go @@ -4,10 +4,11 @@ import ( "context" "errors" "fmt" + "strconv" "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/apimachinery/errutil" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/authn" @@ -119,11 +120,11 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, id *authn.Identity, return nil } - if !id.ID.IsType(claims.TypeUser, claims.TypeServiceAccount) { + if !id.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { return nil } - userID, err := id.ID.ParseInt() + userID, err := id.GetInternalID() if err != nil { s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err) return nil @@ -160,11 +161,11 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, id *authn.Identity, r * return nil } - if !id.ID.IsType(claims.TypeUser, claims.TypeServiceAccount) { + if !id.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { return nil } - userID, err := id.ID.ParseInt() + userID, err := id.GetInternalID() if err != nil { s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err) return nil @@ -196,11 +197,11 @@ func (s *UserSync) EnableUserHook(ctx context.Context, id *authn.Identity, _ *au return nil } - if !id.ID.IsType(claims.TypeUser) { + if !id.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { return nil } - userID, err := id.ID.ParseInt() + userID, err := id.GetInternalID() if err != nil { s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err) return nil @@ -419,8 +420,9 @@ func (s *UserSync) lookupByOneOf(ctx context.Context, params login.UserLookupPar // syncUserToIdentity syncs a user to an identity. // This is used to update the identity with the latest user information. func syncUserToIdentity(usr *user.User, id *authn.Identity) { - id.ID = identity.NewTypedID(claims.TypeUser, usr.ID) - id.UID = identity.NewTypedIDString(claims.TypeUser, usr.UID) + id.ID = strconv.FormatInt(usr.ID, 10) + id.UID = usr.UID + id.Type = claims.TypeUser id.Login = usr.Login id.Email = usr.Email id.Name = usr.Name @@ -430,14 +432,7 @@ func syncUserToIdentity(usr *user.User, id *authn.Identity) { // syncSignedInUserToIdentity syncs a user to an identity. func syncSignedInUserToIdentity(usr *user.SignedInUser, id *authn.Identity) { - var ns claims.IdentityType - if id.ID.IsType(claims.TypeServiceAccount) { - ns = claims.TypeServiceAccount - } else { - ns = claims.TypeUser - } - id.UID = identity.NewTypedIDString(ns, usr.UserUID) - + id.UID = usr.UserUID id.Name = usr.Name id.Login = usr.Login id.Email = usr.Email diff --git a/pkg/services/authn/authnimpl/sync/user_sync_test.go b/pkg/services/authn/authnimpl/sync/user_sync_test.go index 82d76e729aa..a03729420ab 100644 --- a/pkg/services/authn/authnimpl/sync/user_sync_test.go +++ b/pkg/services/authn/authnimpl/sync/user_sync_test.go @@ -4,11 +4,10 @@ import ( "context" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" @@ -165,8 +164,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), - UID: identity.MustParseTypedID("user:1"), + ID: "1", + UID: "1", + Type: claims.TypeUser, Login: "test", Name: "test", Email: "test", @@ -204,8 +204,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), - UID: identity.MustParseTypedID("user:1"), + ID: "1", + UID: "1", + Type: claims.TypeUser, Login: "test", Name: "test", Email: "test", @@ -245,8 +246,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), - UID: identity.MustParseTypedID("user:1"), + ID: "1", + UID: "1", + Type: claims.TypeUser, AuthID: "2032", AuthenticatedBy: "oauth", Login: "test", @@ -317,8 +319,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), - UID: identity.MustParseTypedID("user:2"), + ID: "2", + UID: "2", + Type: claims.TypeUser, Login: "test_create", Name: "test_create", Email: "test_create", @@ -363,8 +366,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:3"), - UID: identity.MustParseTypedID("user:3"), + ID: "3", + UID: "3", + Type: claims.TypeUser, Login: "test_mod", Name: "test_mod", Email: "test_mod", @@ -408,8 +412,9 @@ func TestUserSync_SyncUserHook(t *testing.T) { }, wantErr: false, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:3"), - UID: identity.MustParseTypedID("user:3"), + ID: "3", + UID: "3", + Type: claims.TypeUser, Name: "test", Login: "test", Email: "test_mod@test.com", @@ -459,7 +464,7 @@ func TestUserSync_FetchSyncedUserHook(t *testing.T) { { desc: "should skip hook when identity is not a user", req: &authn.Request{}, - identity: &authn.Identity{ID: identity.MustParseTypedID("api-key:1"), ClientParams: authn.ClientParams{FetchSyncedUser: true}}, + identity: &authn.Identity{ID: "1", Type: claims.TypeAPIKey, ClientParams: authn.ClientParams{FetchSyncedUser: true}}, }, } @@ -485,7 +490,8 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should skip if correct flag is not set", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: false}, }, @@ -494,7 +500,8 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should skip if identity is not a user", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeAPIKey, 1), + ID: "1", + Type: claims.TypeAPIKey, IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: true}, }, @@ -503,7 +510,8 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) { { desc: "should enabled disabled user", identity: &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, 1), + ID: "1", + Type: claims.TypeUser, IsDisabled: true, ClientParams: authn.ClientParams{EnableUser: true}, }, diff --git a/pkg/services/authn/authntest/fake.go b/pkg/services/authn/authntest/fake.go index 8589ae9a812..91fc6473032 100644 --- a/pkg/services/authn/authntest/fake.go +++ b/pkg/services/authn/authntest/fake.go @@ -78,7 +78,7 @@ func (f *FakeService) Logout(_ context.Context, _ identity.Requester, _ *usertok panic("unimplemented") } -func (f *FakeService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (f *FakeService) ResolveIdentity(ctx context.Context, orgID int64, typedID string) (*authn.Identity, error) { if f.ExpectedIdentities != nil { if f.CurrentIndex >= len(f.ExpectedIdentities) { panic("ExpectedIdentities is empty") diff --git a/pkg/services/authn/authntest/mock.go b/pkg/services/authn/authntest/mock.go index 81dad579b61..f9422291f1d 100644 --- a/pkg/services/authn/authntest/mock.go +++ b/pkg/services/authn/authntest/mock.go @@ -55,7 +55,7 @@ func (*MockService) Logout(_ context.Context, _ identity.Requester, _ *usertoken panic("unimplemented") } -func (m *MockService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (m *MockService) ResolveIdentity(ctx context.Context, orgID int64, typedID string) (*authn.Identity, error) { panic("unimplemented") } @@ -79,7 +79,7 @@ type MockClient struct { HookFunc func(ctx context.Context, identity *authn.Identity, r *authn.Request) error LogoutFunc func(ctx context.Context, user identity.Requester) (*authn.Redirect, bool) IdentityTypeFunc func() claims.IdentityType - ResolveIdentityFunc func(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) + ResolveIdentityFunc func(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*authn.Identity, error) } func (m MockClient) Name() string { @@ -136,9 +136,9 @@ func (m *MockClient) IdentityType() claims.IdentityType { } // ResolveIdentity implements authn.IdentityResolverClient. -func (m *MockClient) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { +func (m *MockClient) ResolveIdentity(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*authn.Identity, error) { if m.ResolveIdentityFunc != nil { - return m.ResolveIdentityFunc(ctx, orgID, namespaceID) + return m.ResolveIdentityFunc(ctx, orgID, typ, id) } return nil, nil } diff --git a/pkg/services/authn/clients/api_key.go b/pkg/services/authn/clients/api_key.go index fdab410c979..3750c87d71a 100644 --- a/pkg/services/authn/clients/api_key.go +++ b/pkg/services/authn/clients/api_key.go @@ -3,6 +3,7 @@ package clients import ( "context" "errors" + "strconv" "strings" "time" @@ -140,12 +141,12 @@ func (s *APIKey) IdentityType() claims.IdentityType { return claims.TypeAPIKey } -func (s *APIKey) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) { - if !namespaceID.IsType(claims.TypeAPIKey) { - return nil, identity.ErrInvalidTypedID.Errorf("got unspected namespace: %s", namespaceID.Type()) +func (s *APIKey) ResolveIdentity(ctx context.Context, orgID int64, typ claims.IdentityType, id string) (*authn.Identity, error) { + if !claims.IsIdentityType(typ, claims.TypeAPIKey) { + return nil, identity.ErrInvalidTypedID.Errorf("got unexpected type: %s", typ) } - apiKeyID, err := namespaceID.ParseInt() + apiKeyID, err := strconv.ParseInt(id, 10, 64) if err != nil { return nil, err } @@ -190,17 +191,17 @@ func (s *APIKey) Hook(ctx context.Context, identity *authn.Identity, r *authn.Re } func (s *APIKey) getAPIKeyID(ctx context.Context, id *authn.Identity, r *authn.Request) (apiKeyID int64, exists bool) { - internalId, err := id.ID.ParseInt() + internalId, err := id.GetInternalID() if err != nil { s.log.Warn("Failed to parse ID from identifier", "err", err) return -1, false } - if id.ID.IsType(claims.TypeAPIKey) { + if id.IsIdentityType(claims.TypeAPIKey) { return internalId, true } - if id.ID.IsType(claims.TypeServiceAccount) { + if id.IsIdentityType(claims.TypeServiceAccount) { // 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)) @@ -257,7 +258,8 @@ func validateApiKey(orgID int64, key *apikey.APIKey) error { func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity { return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeAPIKey, key.ID), + ID: strconv.FormatInt(key.ID, 10), + Type: claims.TypeAPIKey, OrgID: key.OrgID, OrgRoles: map[int64]org.RoleType{key.OrgID: key.Role}, ClientParams: authn.ClientParams{SyncPermissions: true}, @@ -267,7 +269,8 @@ func newAPIKeyIdentity(key *apikey.APIKey) *authn.Identity { func newServiceAccountIdentity(key *apikey.APIKey) *authn.Identity { return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeServiceAccount, *key.ServiceAccountId), + ID: strconv.FormatInt(*key.ServiceAccountId, 10), + Type: claims.TypeServiceAccount, OrgID: key.OrgID, AuthenticatedBy: login.APIKeyAuthModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, diff --git a/pkg/services/authn/clients/api_key_test.go b/pkg/services/authn/clients/api_key_test.go index 97e295f72b9..fc4fe215215 100644 --- a/pkg/services/authn/clients/api_key_test.go +++ b/pkg/services/authn/clients/api_key_test.go @@ -9,6 +9,7 @@ import ( "github.com/stretchr/testify/assert" + "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/apikeygen" "github.com/grafana/grafana/pkg/components/satokengen" @@ -48,7 +49,8 @@ func TestAPIKey_Authenticate(t *testing.T) { Role: org.RoleAdmin, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("api-key:1"), + ID: "1", + Type: claims.TypeAPIKey, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin}, ClientParams: authn.ClientParams{ @@ -71,7 +73,8 @@ func TestAPIKey_Authenticate(t *testing.T) { ServiceAccountId: intPtr(1), }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("service-account:1"), + ID: "1", + Type: claims.TypeServiceAccount, OrgID: 1, ClientParams: authn.ClientParams{ FetchSyncedUser: true, @@ -206,7 +209,8 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { ServiceAccountId: intPtr(1), }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("service-account:1"), + ID: "1", + Type: claims.TypeServiceAccount, OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -222,7 +226,8 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("api-key:2"), + ID: "2", + Type: claims.TypeAPIKey, OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -238,7 +243,8 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -254,7 +260,8 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { Key: hash, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("service-account:2"), + ID: "2", + Type: claims.TypeServiceAccount, OrgID: 1, Name: "test", AuthenticatedBy: login.APIKeyAuthModule, @@ -286,8 +293,9 @@ func TestAPIKey_GetAPIKeyIDFromIdentity(t *testing.T) { func TestAPIKey_ResolveIdentity(t *testing.T) { type testCase struct { - desc string - namespaceID identity.TypedID + desc string + typ claims.IdentityType + id string exptedApiKey *apikey.APIKey @@ -297,13 +305,15 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { tests := []testCase{ { - desc: "should return error for invalid namespace", - namespaceID: identity.MustParseTypedID("user:1"), + desc: "should return error for invalid type", + id: "1", + typ: claims.TypeUser, expectedErr: identity.ErrInvalidTypedID, }, { - desc: "should return error when api key has expired", - namespaceID: identity.MustParseTypedID("api-key:1"), + desc: "should return error when api key has expired", + id: "1", + typ: claims.TypeAPIKey, exptedApiKey: &apikey.APIKey{ ID: 1, OrgID: 1, @@ -312,8 +322,9 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedErr: errAPIKeyExpired, }, { - desc: "should return error when api key is revoked", - namespaceID: identity.MustParseTypedID("api-key:1"), + desc: "should return error when api key is revoked", + id: "1", + typ: claims.TypeAPIKey, exptedApiKey: &apikey.APIKey{ ID: 1, OrgID: 1, @@ -322,8 +333,10 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedErr: errAPIKeyRevoked, }, { - desc: "should return error when api key is connected to service account", - namespaceID: identity.MustParseTypedID("api-key:1"), + desc: "should return error when api key is connected to service account", + id: "1", + typ: claims.TypeAPIKey, + exptedApiKey: &apikey.APIKey{ ID: 1, OrgID: 1, @@ -332,8 +345,9 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedErr: identity.ErrInvalidTypedID, }, { - desc: "should return error when api key is belongs to different org", - namespaceID: identity.MustParseTypedID("api-key:1"), + desc: "should return error when api key is belongs to different org", + id: "1", + typ: claims.TypeAPIKey, exptedApiKey: &apikey.APIKey{ ID: 1, OrgID: 2, @@ -342,8 +356,9 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedErr: errAPIKeyOrgMismatch, }, { - desc: "should return valid idenitty", - namespaceID: identity.MustParseTypedID("api-key:1"), + desc: "should return valid idenitty", + id: "1", + typ: claims.TypeAPIKey, exptedApiKey: &apikey.APIKey{ ID: 1, OrgID: 1, @@ -352,7 +367,8 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { expectedIdenity: &authn.Identity{ OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleEditor}, - ID: identity.MustParseTypedID("api-key:1"), + ID: "1", + Type: claims.TypeAPIKey, AuthenticatedBy: login.APIKeyAuthModule, ClientParams: authn.ClientParams{SyncPermissions: true}, }, @@ -365,7 +381,7 @@ func TestAPIKey_ResolveIdentity(t *testing.T) { ExpectedAPIKey: tt.exptedApiKey, }) - identity, err := c.ResolveIdentity(context.Background(), 1, tt.namespaceID) + identity, err := c.ResolveIdentity(context.Background(), 1, tt.typ, tt.id) if tt.expectedErr != nil { assert.Nil(t, identity) assert.ErrorIs(t, err, tt.expectedErr) diff --git a/pkg/services/authn/clients/basic_test.go b/pkg/services/authn/clients/basic_test.go index 7521aa7e966..6b73512dfde 100644 --- a/pkg/services/authn/clients/basic_test.go +++ b/pkg/services/authn/clients/basic_test.go @@ -5,9 +5,9 @@ import ( "net/http" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" ) @@ -25,8 +25,8 @@ func TestBasic_Authenticate(t *testing.T) { { desc: "should success when password client return identity", req: &authn.Request{HTTPRequest: &http.Request{Header: map[string][]string{authorizationHeaderName: {encodeBasicAuth("user", "password")}}}}, - client: authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}}, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + client: authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}}, + expectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}, }, { desc: "should fail when basic auth header could not be decoded", diff --git a/pkg/services/authn/clients/ext_jwt.go b/pkg/services/authn/clients/ext_jwt.go index f44d1705d6b..f1fd557fc91 100644 --- a/pkg/services/authn/clients/ext_jwt.go +++ b/pkg/services/authn/clients/ext_jwt.go @@ -9,6 +9,7 @@ import ( "github.com/go-jose/go-jose/v3/jwt" authlib "github.com/grafana/authlib/authn" authlibclaims "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/apimachinery/errutil" "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" @@ -109,22 +110,22 @@ func (s *ExtendedJWT) authenticateAsUser( return nil, errExtJWTMisMatchedNamespaceClaims.Errorf("unexpected access token namespace: %s", accessTokenClaims.Rest.Namespace) } - accessID, err := identity.ParseTypedID(accessTokenClaims.Subject) + accessType, _, err := identity.ParseTypeAndID(accessTokenClaims.Subject) if err != nil { - return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", accessID.String()) + return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", accessTokenClaims.Subject) } - if !accessID.IsType(authlibclaims.TypeAccessPolicy) { - return nil, errExtJWTInvalid.Errorf("unexpected identity: %s", accessID.String()) + if !authlibclaims.IsIdentityType(accessType, authlibclaims.TypeAccessPolicy) { + return nil, errExtJWTInvalid.Errorf("unexpected identity: %s", accessTokenClaims.Subject) } - userID, err := identity.ParseTypedID(idTokenClaims.Subject) + t, id, err := identity.ParseTypeAndID(idTokenClaims.Subject) if err != nil { return nil, errExtJWTInvalid.Errorf("failed to parse id token subject: %w", err) } - if !userID.IsType(authlibclaims.TypeUser) { - return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", userID.String()) + if !authlibclaims.IsIdentityType(t, authlibclaims.TypeUser) { + return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", idTokenClaims.Subject) } // For use in service layer, allow higher privilege @@ -135,10 +136,11 @@ func (s *ExtendedJWT) authenticateAsUser( } return &authn.Identity{ - ID: userID, + ID: id, + Type: t, OrgID: s.getDefaultOrgID(), AuthenticatedBy: login.ExtendedJWTModule, - AuthID: accessID.String(), + AuthID: accessTokenClaims.Subject, AllowedKubernetesNamespace: allowedKubernetesNamespace, ClientParams: authn.ClientParams{ SyncPermissions: true, @@ -155,18 +157,19 @@ func (s *ExtendedJWT) authenticateAsService(claims *authlib.Claims[authlib.Acces return nil, errExtJWTDisallowedNamespaceClaim.Errorf("unexpected access token namespace: %s", claims.Rest.Namespace) } - id, err := identity.ParseTypedID(claims.Subject) + t, id, err := identity.ParseTypeAndID(claims.Subject) if err != nil { return nil, fmt.Errorf("failed to parse access token subject: %w", err) } - if !id.IsType(authlibclaims.TypeAccessPolicy) { - return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", id.String()) + if !authlibclaims.IsIdentityType(t, authlibclaims.TypeAccessPolicy) { + return nil, errExtJWTInvalidSubject.Errorf("unexpected identity: %s", claims.Subject) } return &authn.Identity{ ID: id, UID: id, + Type: t, OrgID: s.getDefaultOrgID(), AuthenticatedBy: login.ExtendedJWTModule, AuthID: claims.Subject, diff --git a/pkg/services/authn/clients/ext_jwt_test.go b/pkg/services/authn/clients/ext_jwt_test.go index e87feec4c61..bc249ac6e46 100644 --- a/pkg/services/authn/clients/ext_jwt_test.go +++ b/pkg/services/authn/clients/ext_jwt_test.go @@ -12,11 +12,12 @@ import ( "github.com/go-jose/go-jose/v3" "github.com/go-jose/go-jose/v3/jwt" authnlib "github.com/grafana/authlib/authn" - "github.com/grafana/grafana/pkg/apimachinery/identity" - "github.com/grafana/grafana/pkg/services/authn" - "github.com/grafana/grafana/pkg/setting" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/grafana/grafana/pkg/services/authn" + "github.com/grafana/grafana/pkg/setting" ) type ( @@ -205,8 +206,9 @@ func TestExtendedJWT_Authenticate(t *testing.T) { accessToken: &validAccessTokenClaims, orgID: 1, want: &authn.Identity{ - ID: identity.MustParseTypedID("access-policy:this-uid"), - UID: identity.MustParseTypedID("access-policy:this-uid"), + ID: "this-uid", + UID: "this-uid", + Type: claims.TypeAccessPolicy, OrgID: 1, AllowedKubernetesNamespace: "default", AuthenticatedBy: "extendedjwt", @@ -221,8 +223,9 @@ func TestExtendedJWT_Authenticate(t *testing.T) { accessToken: &validAcessTokenClaimsWildcard, orgID: 1, want: &authn.Identity{ - ID: identity.MustParseTypedID("access-policy:this-uid"), - UID: identity.MustParseTypedID("access-policy:this-uid"), + ID: "this-uid", + UID: "this-uid", + Type: claims.TypeAccessPolicy, OrgID: 1, AllowedKubernetesNamespace: "*", AuthenticatedBy: "extendedjwt", @@ -238,7 +241,8 @@ func TestExtendedJWT_Authenticate(t *testing.T) { idToken: &validIDTokenClaims, orgID: 1, want: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, OrgID: 1, AllowedKubernetesNamespace: "default", AuthenticatedBy: "extendedjwt", @@ -258,7 +262,8 @@ func TestExtendedJWT_Authenticate(t *testing.T) { idToken: &validIDTokenClaims, orgID: 1, want: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, OrgID: 1, AllowedKubernetesNamespace: "*", AuthenticatedBy: "extendedjwt", @@ -283,7 +288,8 @@ func TestExtendedJWT_Authenticate(t *testing.T) { }, }, want: &authn.Identity{ - ID: identity.MustParseTypedID("user:2"), + ID: "2", + Type: claims.TypeUser, OrgID: 1, AllowedKubernetesNamespace: "stack-1234", AuthenticatedBy: "extendedjwt", diff --git a/pkg/services/authn/clients/grafana.go b/pkg/services/authn/clients/grafana.go index 04de6c88151..f41d40ee2b0 100644 --- a/pkg/services/authn/clients/grafana.go +++ b/pkg/services/authn/clients/grafana.go @@ -5,9 +5,9 @@ import ( "crypto/subtle" "errors" "net/mail" + "strconv" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -107,7 +107,8 @@ func (c *Grafana) AuthenticatePassword(ctx context.Context, r *authn.Request, us } return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, usr.ID), + ID: strconv.FormatInt(usr.ID, 10), + Type: claims.TypeUser, OrgID: r.OrgID, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, AuthenticatedBy: login.PasswordAuthModule, diff --git a/pkg/services/authn/clients/grafana_test.go b/pkg/services/authn/clients/grafana_test.go index 4a1f5f2955c..aef2e165f9a 100644 --- a/pkg/services/authn/clients/grafana_test.go +++ b/pkg/services/authn/clients/grafana_test.go @@ -7,7 +7,7 @@ import ( "github.com/stretchr/testify/assert" - "github.com/grafana/grafana/pkg/apimachinery/identity" + "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -141,7 +141,8 @@ func TestGrafana_AuthenticatePassword(t *testing.T) { password: "password", findUser: true, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, OrgID: 1, AuthenticatedBy: login.PasswordAuthModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, diff --git a/pkg/services/authn/clients/oauth_test.go b/pkg/services/authn/clients/oauth_test.go index 9ae0de62a3a..4f5347428e1 100644 --- a/pkg/services/authn/clients/oauth_test.go +++ b/pkg/services/authn/clients/oauth_test.go @@ -486,7 +486,7 @@ func TestOAuth_Logout(t *testing.T) { } c := ProvideOAuth(authn.ClientWithPrefix("azuread"), tt.cfg, mockService, fakeSocialSvc, &setting.OSSImpl{Cfg: tt.cfg}, featuremgmt.WithFeatures()) - redirect, ok := c.Logout(context.Background(), &authn.Identity{ID: identity.NewTypedIDString(claims.TypeUser, "1")}) + redirect, ok := c.Logout(context.Background(), &authn.Identity{ID: "1", Type: claims.TypeUser}) assert.Equal(t, tt.expectedOK, ok) if tt.expectedOK { diff --git a/pkg/services/authn/clients/password_test.go b/pkg/services/authn/clients/password_test.go index 4719ee2cc9c..e8a90692229 100644 --- a/pkg/services/authn/clients/password_test.go +++ b/pkg/services/authn/clients/password_test.go @@ -4,9 +4,9 @@ import ( "context" "testing" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" "github.com/grafana/grafana/pkg/services/loginattempt/loginattempttest" @@ -30,16 +30,16 @@ func TestPassword_AuthenticatePassword(t *testing.T) { username: "test", password: "test", req: &authn.Request{}, - clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}}}, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}, + clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}}}, + expectedIdentity: &authn.Identity{ID: "1", Type: claims.TypeUser}, }, { desc: "should success when found in second client", username: "test", password: "test", req: &authn.Request{}, - clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:2")}}}, - expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:2")}, + clients: []authn.PasswordClient{authntest.FakePasswordClient{ExpectedErr: errIdentityNotFound}, authntest.FakePasswordClient{ExpectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser}}}, + expectedIdentity: &authn.Identity{ID: "2", Type: claims.TypeUser}, }, { desc: "should fail for empty password", diff --git a/pkg/services/authn/clients/proxy.go b/pkg/services/authn/clients/proxy.go index 7c838d119ef..90c44bafeee 100644 --- a/pkg/services/authn/clients/proxy.go +++ b/pkg/services/authn/clients/proxy.go @@ -13,8 +13,8 @@ import ( "time" "github.com/grafana/authlib/claims" + "github.com/grafana/grafana/pkg/apimachinery/errutil" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/remotecache" "github.com/grafana/grafana/pkg/services/authn" @@ -120,13 +120,14 @@ func (c *Proxy) retrieveIDFromCache(ctx context.Context, cacheKey string, r *aut return nil, err } - uid, err := strconv.ParseInt(string(entry), 10, 64) + _, err = strconv.ParseInt(string(entry), 10, 64) if err != nil { return nil, fmt.Errorf("failed to parse user id from cache: %w - entry: %s", err, string(entry)) } return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, uid), + ID: string(entry), + Type: claims.TypeUser, OrgID: r.OrgID, // FIXME: This does not match the actual auth module used, but should not have any impact // Maybe caching the auth module used with the user ID would be a good idea @@ -151,13 +152,13 @@ func (c *Proxy) Hook(ctx context.Context, id *authn.Identity, r *authn.Request) return nil } - if !id.ID.IsType(claims.TypeUser) { + if !id.IsIdentityType(claims.TypeUser) { return nil } - internalId, err := id.ID.ParseInt() + internalId, err := id.GetInternalID() if err != nil { - c.log.Warn("Failed to cache proxy user", "error", err, "userId", id.ID.ID(), "err", err) + c.log.Warn("Failed to cache proxy user", "error", err, "userId", id.GetID(), "err", err) return nil } diff --git a/pkg/services/authn/clients/proxy_test.go b/pkg/services/authn/clients/proxy_test.go index 7aa39efaa71..24d5004f441 100644 --- a/pkg/services/authn/clients/proxy_test.go +++ b/pkg/services/authn/clients/proxy_test.go @@ -12,7 +12,6 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn/authntest" "github.com/grafana/grafana/pkg/setting" @@ -204,8 +203,6 @@ func TestProxy_Hook(t *testing.T) { proxyFieldRole: "X-Role", } cache := &fakeCache{data: make(map[string][]byte)} - userId := int64(1) - userID := identity.NewTypedID(claims.TypeUser, userId) // withRole creates a test case for a user with a specific role. withRole := func(role string) func(t *testing.T) { @@ -214,7 +211,8 @@ func TestProxy_Hook(t *testing.T) { c, err := ProvideProxy(cfg, cache, authntest.MockProxyClient{}) require.NoError(t, err) userIdentity := &authn.Identity{ - ID: userID, + ID: "1", + Type: claims.TypeUser, ClientParams: authn.ClientParams{ CacheAuthProxyKey: cacheKey, }, @@ -230,7 +228,7 @@ func TestProxy_Hook(t *testing.T) { err = c.Hook(context.Background(), userIdentity, userReq) assert.NoError(t, err) expectedCache := map[string][]byte{ - cacheKey: []byte(fmt.Sprintf("%d", userId)), + cacheKey: []byte("1"), fmt.Sprintf("%s:%s", proxyCachePrefix, "johndoe"): []byte(fmt.Sprintf("users:johndoe-%s", role)), } assert.Equal(t, expectedCache, cache.data) diff --git a/pkg/services/authn/clients/render.go b/pkg/services/authn/clients/render.go index 6ed57114af0..e63507ff710 100644 --- a/pkg/services/authn/clients/render.go +++ b/pkg/services/authn/clients/render.go @@ -2,11 +2,11 @@ package clients import ( "context" + "strconv" "time" "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/errutil" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -44,7 +44,8 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide if renderUsr.UserID <= 0 { return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeRenderService, 0), + ID: "0", + Type: claims.TypeRenderService, OrgID: renderUsr.OrgID, OrgRoles: map[int64]org.RoleType{renderUsr.OrgID: org.RoleType(renderUsr.OrgRole)}, ClientParams: authn.ClientParams{SyncPermissions: true}, @@ -54,7 +55,8 @@ func (c *Render) Authenticate(ctx context.Context, r *authn.Request) (*authn.Ide } return &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, renderUsr.UserID), + ID: strconv.FormatInt(renderUsr.UserID, 10), + Type: claims.TypeUser, LastSeenAt: time.Now(), AuthenticatedBy: login.RenderModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, diff --git a/pkg/services/authn/clients/render_test.go b/pkg/services/authn/clients/render_test.go index 7ab176e2aeb..c30bdfe48d2 100644 --- a/pkg/services/authn/clients/render_test.go +++ b/pkg/services/authn/clients/render_test.go @@ -7,9 +7,9 @@ import ( "time" "github.com/golang/mock/gomock" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/login" "github.com/grafana/grafana/pkg/services/org" @@ -36,7 +36,8 @@ func TestRender_Authenticate(t *testing.T) { }, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("render:0"), + ID: "0", + Type: claims.TypeRenderService, OrgID: 1, OrgRoles: map[int64]org.RoleType{1: org.RoleViewer}, AuthenticatedBy: login.RenderModule, @@ -57,7 +58,8 @@ func TestRender_Authenticate(t *testing.T) { }, }, expectedIdentity: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, AuthenticatedBy: login.RenderModule, ClientParams: authn.ClientParams{FetchSyncedUser: true, SyncPermissions: true}, }, diff --git a/pkg/services/authn/clients/session.go b/pkg/services/authn/clients/session.go index 9945612c79f..43ce7dfddcf 100644 --- a/pkg/services/authn/clients/session.go +++ b/pkg/services/authn/clients/session.go @@ -4,10 +4,10 @@ import ( "context" "errors" "net/url" + "strconv" "time" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" @@ -59,7 +59,8 @@ func (s *Session) Authenticate(ctx context.Context, r *authn.Request) (*authn.Id } ident := &authn.Identity{ - ID: identity.NewTypedID(claims.TypeUser, token.UserId), + ID: strconv.FormatInt(token.UserId, 10), + Type: claims.TypeUser, SessionToken: token, ClientParams: authn.ClientParams{ FetchSyncedUser: true, diff --git a/pkg/services/authn/clients/session_test.go b/pkg/services/authn/clients/session_test.go index 28b14b5bf41..e290f55441f 100644 --- a/pkg/services/authn/clients/session_test.go +++ b/pkg/services/authn/clients/session_test.go @@ -6,10 +6,10 @@ import ( "testing" "time" + "github.com/grafana/authlib/claims" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/models/usertoken" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/auth/authtest" @@ -97,7 +97,8 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, SessionToken: validToken, ClientParams: authn.ClientParams{ SyncPermissions: true, @@ -130,7 +131,9 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, + SessionToken: validToken, ClientParams: authn.ClientParams{ SyncPermissions: true, @@ -149,7 +152,8 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, AuthID: "1", AuthenticatedBy: "oauth_azuread", SessionToken: validToken, @@ -171,7 +175,8 @@ func TestSession_Authenticate(t *testing.T) { }, args: args{r: &authn.Request{HTTPRequest: validHTTPReq}}, wantID: &authn.Identity{ - ID: identity.MustParseTypedID("user:1"), + ID: "1", + Type: claims.TypeUser, SessionToken: validToken, ClientParams: authn.ClientParams{ diff --git a/pkg/services/authn/identity.go b/pkg/services/authn/identity.go index aa16ebb636f..776283bf92d 100644 --- a/pkg/services/authn/identity.go +++ b/pkg/services/authn/identity.go @@ -2,6 +2,7 @@ package authn import ( "fmt" + "strconv" "time" "github.com/grafana/authlib/authn" @@ -21,10 +22,11 @@ var _ identity.Requester = (*Identity)(nil) type Identity struct { // ID is the unique identifier for the entity in the Grafana database. - // If the entity is not found in the DB or this entity is non-persistent, this field will be empty. - ID identity.TypedID + ID string // UID is a unique identifier stored for the entity in Grafana database. Not all entities support uid so it can be empty. - UID identity.TypedID + UID string + // Type is the IdentityType of entity. + Type claims.IdentityType // OrgID is the active organization for the entity. OrgID int64 // OrgName is the name of the active organization. @@ -90,17 +92,22 @@ func (i *Identity) GetIdentity() claims.IdentityClaims { // GetRawIdentifier implements Requester. func (i *Identity) GetRawIdentifier() string { - return i.UID.ID() + return i.UID } // GetInternalID implements Requester. func (i *Identity) GetInternalID() (int64, error) { - return i.ID.UserID() + return identity.IntIdentifier(i.GetID()) } // GetIdentityType implements Requester. func (i *Identity) GetIdentityType() claims.IdentityType { - return i.UID.Type() + return i.Type +} + +// GetIdentityType implements Requester. +func (i *Identity) IsIdentityType(expected ...claims.IdentityType) bool { + return claims.IsIdentityType(i.GetIdentityType(), expected...) } // GetExtra implements identity.Requester. @@ -125,12 +132,12 @@ func (i *Identity) GetName() string { return i.Name } -func (i *Identity) GetID() identity.TypedID { - return i.ID +func (i *Identity) GetID() string { + return identity.NewTypedIDString(i.Type, i.ID) } func (i *Identity) GetUID() string { - return i.UID.String() + return identity.NewTypedIDString(i.Type, i.UID) } func (i *Identity) GetAuthID() string { @@ -142,14 +149,14 @@ func (i *Identity) GetAuthenticatedBy() string { } func (i *Identity) GetCacheKey() string { - id := i.GetID().ID() + id := i.ID if !i.HasUniqueId() { // Hack use the org role as id for identities that do not have a unique id // e.g. anonymous and render key. id = string(i.GetOrgRole()) } - return fmt.Sprintf("%d-%s-%s", i.GetOrgID(), i.GetID().Type(), id) + return fmt.Sprintf("%d-%s-%s", i.GetOrgID(), i.Type, id) } func (i *Identity) GetDisplayName() string { @@ -242,10 +249,7 @@ func (i *Identity) HasRole(role org.RoleType) bool { } func (i *Identity) HasUniqueId() bool { - typ := i.GetID().Type() - return typ == claims.TypeUser || - typ == claims.TypeServiceAccount || - typ == claims.TypeAPIKey + return i.IsIdentityType(claims.TypeUser, claims.TypeAPIKey, claims.TypeServiceAccount) } func (i *Identity) IsAuthenticatedBy(providers ...string) bool { @@ -273,31 +277,31 @@ func (i *Identity) SignedInUser() *user.SignedInUser { AuthID: i.AuthID, AuthenticatedBy: i.AuthenticatedBy, IsGrafanaAdmin: i.GetIsGrafanaAdmin(), - IsAnonymous: i.ID.IsType(claims.TypeAnonymous), + IsAnonymous: i.IsIdentityType(claims.TypeAnonymous), IsDisabled: i.IsDisabled, HelpFlags1: i.HelpFlags1, LastSeenAt: i.LastSeenAt, Teams: i.Teams, Permissions: i.Permissions, IDToken: i.IDToken, - FallbackType: i.ID.Type(), + FallbackType: i.Type, } - if i.ID.IsType(claims.TypeAPIKey) { - id, _ := i.ID.ParseInt() + if i.IsIdentityType(claims.TypeAPIKey) { + id, _ := i.GetInternalID() u.ApiKeyID = id } else { - id, _ := i.ID.UserID() + id, _ := i.GetInternalID() u.UserID = id - u.UserUID = i.UID.ID() - u.IsServiceAccount = i.ID.IsType(claims.TypeServiceAccount) + u.UserUID = i.UID + u.IsServiceAccount = i.IsIdentityType(claims.TypeServiceAccount) } return u } func (i *Identity) ExternalUserInfo() login.ExternalUserInfo { - id, _ := i.ID.UserID() + id, _ := strconv.ParseInt(i.ID, 10, 64) return login.ExternalUserInfo{ OAuthToken: i.OAuthToken, AuthModule: i.AuthenticatedBy, diff --git a/pkg/services/authz/server.go b/pkg/services/authz/server.go index 42f0523df98..c9a4fb16c84 100644 --- a/pkg/services/authz/server.go +++ b/pkg/services/authz/server.go @@ -5,7 +5,6 @@ import ( authzv1 "github.com/grafana/authlib/authz/proto/v1" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -57,16 +56,11 @@ func (s *legacyServer) Read(ctx context.Context, req *authzv1.ReadRequest) (*aut ctxLogger := s.logger.FromContext(ctx) ctxLogger.Debug("Read", "action", action, "subject", subject, "stackID", stackID) - var err error - opts := accesscontrol.SearchOptions{Action: action} - if subject != "" { - opts.TypedID, err = identity.ParseTypedID(subject) - if err != nil { - return nil, err - } - } - - permissions, err := s.acSvc.SearchUserPermissions(ctx, stackID, opts) + permissions, err := s.acSvc.SearchUserPermissions( + ctx, + stackID, + accesscontrol.SearchOptions{Action: action, TypedID: subject}, + ) if err != nil { ctxLogger.Error("failed to search user permissions", "error", err) return nil, tracing.Errorf(span, "failed to search user permissions: %w", err) diff --git a/pkg/services/contexthandler/contexthandler.go b/pkg/services/contexthandler/contexthandler.go index d5ea06013fa..a30f560b414 100644 --- a/pkg/services/contexthandler/contexthandler.go +++ b/pkg/services/contexthandler/contexthandler.go @@ -155,22 +155,21 @@ func (h *ContextHandler) addIDHeaderEndOfRequestFunc(ident identity.Requester) w return } - id := ident.GetID() - if !identity.IsIdentityType( - id, + id, _ := ident.GetInternalID() + if !ident.IsIdentityType( claims.TypeUser, claims.TypeServiceAccount, claims.TypeAPIKey, - ) || id.ID() == "0" { + ) || id == 0 { return } - if _, ok := h.Cfg.IDResponseHeaderNamespaces[id.Type().String()]; !ok { + if _, ok := h.Cfg.IDResponseHeaderNamespaces[string(ident.GetIdentityType())]; !ok { return } headerName := fmt.Sprintf("%s-Identity-Id", h.Cfg.IDResponseHeaderPrefix) - w.Header().Add(headerName, id.String()) + w.Header().Add(headerName, ident.GetID()) } } diff --git a/pkg/services/contexthandler/contexthandler_test.go b/pkg/services/contexthandler/contexthandler_test.go index 27f3e1cd03c..1a0458a48fe 100644 --- a/pkg/services/contexthandler/contexthandler_test.go +++ b/pkg/services/contexthandler/contexthandler_test.go @@ -44,7 +44,7 @@ func TestContextHandler(t *testing.T) { }) t.Run("should set identity on successful authentication", func(t *testing.T) { - id := &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, 1), OrgID: 1} + id := &authn.Identity{ID: "1", Type: claims.TypeUser, OrgID: 1} handler := contexthandler.ProvideService( setting.NewCfg(), tracing.InitializeTracerForTest(), @@ -69,7 +69,7 @@ func TestContextHandler(t *testing.T) { }) t.Run("should not set IsSignedIn on anonymous identity", func(t *testing.T) { - identity := &authn.Identity{ID: identity.AnonymousTypedID, OrgID: 1} + identity := &authn.Identity{ID: "0", Type: claims.TypeAnonymous, OrgID: 1} handler := contexthandler.ProvideService( setting.NewCfg(), tracing.InitializeTracerForTest(), @@ -146,10 +146,13 @@ func TestContextHandler(t *testing.T) { t.Run("id response headers", func(t *testing.T) { run := func(cfg *setting.Cfg, id string) *http.Response { + typ, i, err := identity.ParseTypeAndID(id) + require.NoError(t, err) + handler := contexthandler.ProvideService( cfg, tracing.InitializeTracerForTest(), - &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID(id)}}, + &authntest.FakeService{ExpectedIdentity: &authn.Identity{ID: i, Type: typ}}, ) server := webtest.NewServer(t, routing.NewRouteRegister()) diff --git a/pkg/services/dashboards/database/database.go b/pkg/services/dashboards/database/database.go index 4b85d539a8a..8e91674cb35 100644 --- a/pkg/services/dashboards/database/database.go +++ b/pkg/services/dashboards/database/database.go @@ -879,7 +879,7 @@ func (d *dashboardStore) FindDashboards(ctx context.Context, query *dashboards.F } // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users - if query.SignedInUser == nil || query.SignedInUser.GetID().Type() != claims.TypeServiceAccount { + if query.SignedInUser == nil || !query.SignedInUser.IsIdentityType(claims.TypeServiceAccount) { filters = append(filters, searchstore.K6FolderFilter{}) } diff --git a/pkg/services/dashboards/service/dashboard_service.go b/pkg/services/dashboards/service/dashboard_service.go index d3dc27e2b28..f1e40b2f98a 100644 --- a/pkg/services/dashboards/service/dashboard_service.go +++ b/pkg/services/dashboards/service/dashboard_service.go @@ -489,11 +489,11 @@ func (dr *DashboardServiceImpl) setDefaultPermissions(ctx context.Context, dto * inFolder := dash.FolderID > 0 var permissions []accesscontrol.SetResourcePermissionCommand - if !provisioned { - userID, err := identity.IntIdentifier(dto.User.GetID()) + if !provisioned && dto.User.IsIdentityType(claims.TypeUser) { + userID, err := dto.User.GetInternalID() if err != nil { dr.log.Error("Could not make user admin", "dashboard", dash.Title, "id", dto.User.GetID(), "error", err) - } else if identity.IsIdentityType(dto.User.GetID(), claims.TypeUser) { + } else { permissions = append(permissions, accesscontrol.SetResourcePermissionCommand{ UserID: userID, Permission: dashboardaccess.PERMISSION_ADMIN.String(), }) @@ -525,11 +525,11 @@ func (dr *DashboardServiceImpl) setDefaultFolderPermissions(ctx context.Context, inFolder := f.ParentUID != "" var permissions []accesscontrol.SetResourcePermissionCommand - if !provisioned { - userID, err := identity.IntIdentifier(cmd.SignedInUser.GetID()) + if !provisioned && cmd.SignedInUser.IsIdentityType(claims.TypeUser) { + userID, err := cmd.SignedInUser.GetInternalID() if err != nil { dr.log.Error("Could not make user admin", "folder", cmd.Title, "id", cmd.SignedInUser.GetID()) - } else if identity.IsIdentityType(cmd.SignedInUser.GetID(), claims.TypeUser) { + } else { permissions = append(permissions, accesscontrol.SetResourcePermissionCommand{ UserID: userID, Permission: dashboardaccess.PERMISSION_ADMIN.String(), }) diff --git a/pkg/services/dashboardsnapshots/database/database.go b/pkg/services/dashboardsnapshots/database/database.go index bcc7c9c32e8..728883a9408 100644 --- a/pkg/services/dashboardsnapshots/database/database.go +++ b/pkg/services/dashboardsnapshots/database/database.go @@ -5,7 +5,6 @@ import ( "time" "github.com/grafana/authlib/claims" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/components/simplejson" "github.com/grafana/grafana/pkg/infra/db" "github.com/grafana/grafana/pkg/services/dashboardsnapshots" @@ -124,9 +123,9 @@ func (d *DashboardSnapshotStore) SearchDashboardSnapshots(ctx context.Context, q } var userID int64 - if identity.IsIdentityType(query.SignedInUser.GetID(), claims.TypeUser, claims.TypeServiceAccount) { + if query.SignedInUser.IsIdentityType(claims.TypeUser, claims.TypeServiceAccount) { var err error - userID, err = identity.UserIdentifier(query.SignedInUser.GetID()) + userID, err = query.SignedInUser.GetInternalID() if err != nil { return err } diff --git a/pkg/services/folder/folderimpl/sqlstore.go b/pkg/services/folder/folderimpl/sqlstore.go index d2b0c7a2bc3..c4464087840 100644 --- a/pkg/services/folder/folderimpl/sqlstore.go +++ b/pkg/services/folder/folderimpl/sqlstore.go @@ -323,7 +323,7 @@ func (ss *sqlStore) GetChildren(ctx context.Context, q folder.GetChildrenQuery) } // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users - if q.SignedInUser == nil || q.SignedInUser.GetID().Type() != claims.TypeServiceAccount { + if q.SignedInUser == nil || !q.SignedInUser.IsIdentityType(claims.TypeServiceAccount) { sql.WriteString(" AND uid != ?") args = append(args, accesscontrol.K6FolderUID) } @@ -484,7 +484,7 @@ func (ss *sqlStore) GetFolders(ctx context.Context, q getFoldersQuery) ([]*folde } // only list k6 folders when requested by a service account - prevents showing k6 folders in the UI for users - if q.SignedInUser == nil || q.SignedInUser.GetID().Type() != claims.TypeServiceAccount { + if q.SignedInUser == nil || !q.SignedInUser.IsIdentityType(claims.TypeServiceAccount) { s.WriteString(" AND f0.uid != ? AND (f0.parent_uid != ? OR f0.parent_uid IS NULL)") args = append(args, accesscontrol.K6FolderUID, accesscontrol.K6FolderUID) } diff --git a/pkg/services/live/live.go b/pkg/services/live/live.go index 18eb4296b10..5ea854ac625 100644 --- a/pkg/services/live/live.go +++ b/pkg/services/live/live.go @@ -284,9 +284,10 @@ func ProvideService(plugCtxProvider *plugincontext.Provider, cfg *setting.Cfg, r g.websocketHandler = func(ctx *contextmodel.ReqContext) { user := ctx.SignedInUser + id, _ := user.GetInternalID() // Centrifuge expects Credentials in context with a current user ID. cred := ¢rifuge.Credentials{ - UserID: user.GetID().ID(), + UserID: strconv.FormatInt(id, 10), } newCtx := centrifuge.SetCredentials(ctx.Req.Context(), cred) newCtx = livecontext.SetContextSignedUser(newCtx, user) diff --git a/pkg/services/ngalert/api/api_ruler.go b/pkg/services/ngalert/api/api_ruler.go index 27aca213e35..4cc0d4ab611 100644 --- a/pkg/services/ngalert/api/api_ruler.go +++ b/pkg/services/ngalert/api/api_ruler.go @@ -76,7 +76,8 @@ func (srv RulerSrv) RouteDeleteAlertRules(c *contextmodel.ReqContext, namespaceU return toNamespaceErrorResponse(err) } - userNamespace, id := c.SignedInUser.GetID().Type(), c.SignedInUser.GetID().ID() + id, _ := c.SignedInUser.GetInternalID() + userNamespace := c.SignedInUser.GetIdentityType() var loggerCtx = []any{ "identity", id, @@ -294,7 +295,8 @@ func (srv RulerSrv) RouteGetRulesConfig(c *contextmodel.ReqContext) response.Res for groupKey, rules := range configs { folder, ok := namespaceMap[groupKey.NamespaceUID] if !ok { - userNamespace, id := c.SignedInUser.GetID().Type(), c.SignedInUser.GetID().ID() + id, _ := c.SignedInUser.GetInternalID() + userNamespace := c.SignedInUser.GetIdentityType() srv.log.Error("Namespace not visible to the user", "user", id, "userNamespace", userNamespace, "namespace", groupKey.NamespaceUID) continue } @@ -370,7 +372,9 @@ func (srv RulerSrv) updateAlertRulesInGroup(c *contextmodel.ReqContext, groupKey var finalChanges *store.GroupDelta var dbConfig *ngmodels.AlertConfiguration err := srv.xactManager.InTransaction(c.Req.Context(), func(tranCtx context.Context) error { - userNamespace, id := c.SignedInUser.GetID().Type(), c.SignedInUser.GetID().ID() + id, _ := c.SignedInUser.GetInternalID() + userNamespace := c.SignedInUser.GetIdentityType() + logger := srv.log.New("namespace_uid", groupKey.NamespaceUID, "group", groupKey.RuleGroup, "org_id", groupKey.OrgID, "user_id", id, "userNamespace", userNamespace) groupChanges, err := store.CalculateChanges(tranCtx, srv.store, groupKey, rules) diff --git a/pkg/services/oauthtoken/oauth_token.go b/pkg/services/oauthtoken/oauth_token.go index cc736d959b4..ea1572f7ea9 100644 --- a/pkg/services/oauthtoken/oauth_token.go +++ b/pkg/services/oauthtoken/oauth_token.go @@ -95,12 +95,12 @@ func (o *Service) HasOAuthEntry(ctx context.Context, usr identity.Requester) (*l return nil, false, nil } - if !identity.IsIdentityType(usr.GetID(), claims.TypeUser) { + if !usr.IsIdentityType(claims.TypeUser) { return nil, false, nil } ctxLogger := logger.FromContext(ctx) - userID, err := identity.UserIdentifier(usr.GetID()) + userID, err := usr.GetInternalID() if err != nil { ctxLogger.Error("Failed to convert user id to int", "id", usr.GetID(), "error", err) return nil, false, err @@ -136,12 +136,12 @@ func (o *Service) TryTokenRefresh(ctx context.Context, usr identity.Requester) e return nil } - if !identity.IsIdentityType(usr.GetID(), claims.TypeUser) { + if !usr.IsIdentityType(claims.TypeUser) { ctxLogger.Warn("Can only refresh OAuth tokens for users", "id", usr.GetID()) return nil } - userID, err := identity.UserIdentifier(usr.GetID()) + userID, err := usr.GetInternalID() if err != nil { ctxLogger.Warn("Failed to convert user id to int", "id", usr.GetID(), "error", err) return nil diff --git a/pkg/services/oauthtoken/oauth_token_test.go b/pkg/services/oauthtoken/oauth_token_test.go index aee92134f95..59117a4aacc 100644 --- a/pkg/services/oauthtoken/oauth_token_test.go +++ b/pkg/services/oauthtoken/oauth_token_test.go @@ -7,6 +7,7 @@ import ( "testing" "time" + "github.com/grafana/authlib/claims" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" @@ -174,13 +175,13 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip sync when identity is not a user", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("service-account:1")} + env.identity = &authn.Identity{ID: "1", Type: claims.TypeServiceAccount} }, }, { desc: "should skip token refresh and return nil if namespace and id cannot be converted to user ID", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:invalidIdentifierFormat")} + env.identity = &authn.Identity{ID: "invalid", Type: claims.TypeUser} }, }, { @@ -203,28 +204,29 @@ func TestService_TryTokenRefresh(t *testing.T) { env.identity = &authn.Identity{ AuthenticatedBy: login.GenericOAuthModule, - ID: identity.MustParseTypedID("user:1234"), + ID: "1234", + Type: claims.TypeUser, } }, }, { desc: "should skip token refresh if the expiration check has already been cached", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.cache.Set("oauth-refresh-token-1234", true, 1*time.Minute) }, }, { desc: "should skip token refresh if there's an unexpected error while looking up the user oauth entry, additionally, no error should be returned", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedError = errors.New("some error") }, }, { desc: "should skip token refresh if the user doesn't have an oauth entry", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.SAMLAuthModule, } @@ -233,7 +235,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should do token refresh if access token or id token have not expired yet", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, } @@ -242,7 +244,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when no oauth provider was found", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -252,7 +254,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when oauth provider token handling is disabled (UseRefreshToken is false)", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -265,7 +267,7 @@ func TestService_TryTokenRefresh(t *testing.T) { { desc: "should skip token refresh when there is no refresh token", setup: func(env *environment) { - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.authInfoService.ExpectedUserAuth = &login.UserAuth{ AuthModule: login.GenericOAuthModule, OAuthIdToken: EXPIRED_JWT, @@ -285,7 +287,7 @@ func TestService_TryTokenRefresh(t *testing.T) { Expiry: time.Now().Add(-time.Hour), TokenType: "Bearer", } - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{ UseRefreshToken: true, } @@ -310,7 +312,7 @@ func TestService_TryTokenRefresh(t *testing.T) { Expiry: time.Now().Add(time.Hour), TokenType: "Bearer", } - env.identity = &authn.Identity{ID: identity.MustParseTypedID("user:1234")} + env.identity = &authn.Identity{ID: "1234", Type: claims.TypeUser} env.socialService.ExpectedAuthInfoProvider = &social.OAuthInfo{ UseRefreshToken: true, } diff --git a/pkg/services/pluginsintegration/clientmiddleware/user_header_middleware.go b/pkg/services/pluginsintegration/clientmiddleware/user_header_middleware.go index e15ae29e7db..214b1305979 100644 --- a/pkg/services/pluginsintegration/clientmiddleware/user_header_middleware.go +++ b/pkg/services/pluginsintegration/clientmiddleware/user_header_middleware.go @@ -6,7 +6,6 @@ import ( "github.com/grafana/authlib/claims" "github.com/grafana/grafana-plugin-sdk-go/backend" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/contexthandler" "github.com/grafana/grafana/pkg/util/proxyutil" @@ -36,7 +35,7 @@ func (m *UserHeaderMiddleware) applyUserHeader(ctx context.Context, h backend.Fo } h.DeleteHTTPHeader(proxyutil.UserHeaderName) - if !identity.IsIdentityType(reqCtx.SignedInUser.GetID(), claims.TypeAnonymous) { + if !reqCtx.SignedInUser.IsIdentityType(claims.TypeAnonymous) { h.SetHTTPHeader(proxyutil.UserHeaderName, reqCtx.SignedInUser.GetLogin()) } } diff --git a/pkg/services/serviceaccounts/api/api.go b/pkg/services/serviceaccounts/api/api.go index a14f05993bd..a0db940a2bc 100644 --- a/pkg/services/serviceaccounts/api/api.go +++ b/pkg/services/serviceaccounts/api/api.go @@ -8,7 +8,6 @@ import ( "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/api/response" "github.com/grafana/grafana/pkg/api/routing" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/middleware/requestmeta" "github.com/grafana/grafana/pkg/services/accesscontrol" @@ -100,8 +99,8 @@ func (api *ServiceAccountsAPI) CreateServiceAccount(c *contextmodel.ReqContext) } if api.cfg.RBAC.PermissionsOnCreation("service-account") { - if identity.IsIdentityType(c.SignedInUser.GetID(), claims.TypeUser) { - userID, err := c.SignedInUser.GetID().ParseInt() + if c.SignedInUser.IsIdentityType(claims.TypeUser) { + userID, err := c.SignedInUser.GetInternalID() if err != nil { return response.Error(http.StatusInternalServerError, "Failed to parse user id", err) } diff --git a/pkg/services/team/teamapi/team.go b/pkg/services/team/teamapi/team.go index 0fc23039fc0..71d90504c38 100644 --- a/pkg/services/team/teamapi/team.go +++ b/pkg/services/team/teamapi/team.go @@ -49,20 +49,12 @@ func (tapi *TeamAPI) createTeam(c *contextmodel.ReqContext) response.Response { // if the request is authenticated using API tokens // the SignedInUser is an empty struct therefore // an additional check whether it is an actual user is required - namespace, identifier := c.SignedInUser.GetID().Type(), c.SignedInUser.GetID().ID() - switch namespace { - case claims.TypeUser: - userID, err := strconv.ParseInt(identifier, 10, 64) - if err != nil { - c.Logger.Error("Could not add creator to team because user id is not a number", "error", err) - break - } + if c.SignedInUser.IsIdentityType(claims.TypeUser) { + userID, _ := c.SignedInUser.GetInternalID() if err := addOrUpdateTeamMember(c.Req.Context(), tapi.teamPermissionsService, userID, c.SignedInUser.GetOrgID(), t.ID, dashboardaccess.PERMISSION_ADMIN.String()); err != nil { c.Logger.Error("Could not add creator to team", "error", err) } - default: - c.Logger.Warn("Could not add creator to team because is not a real user") } return response.JSON(http.StatusOK, &util.DynMap{ diff --git a/pkg/services/user/identity.go b/pkg/services/user/identity.go index e9b0a4c2011..ece7ed08d3c 100644 --- a/pkg/services/user/identity.go +++ b/pkg/services/user/identity.go @@ -76,16 +76,9 @@ func (u *SignedInUser) GetRawIdentifier() string { return u.UserUID } -// Deprecated: use GetUID +// GetInternalID implements Requester. func (u *SignedInUser) GetInternalID() (int64, error) { - switch { - case u.ApiKeyID != 0: - return u.ApiKeyID, nil - case u.IsAnonymous: - return 0, nil - default: - } - return u.UserID, nil + return identity.IntIdentifier(u.GetID()) } // GetIdentityType implements Requester. @@ -105,6 +98,11 @@ func (u *SignedInUser) GetIdentityType() claims.IdentityType { return u.FallbackType } +// IsIdentityType implements Requester. +func (u *SignedInUser) IsIdentityType(expected ...claims.IdentityType) bool { + return claims.IsIdentityType(u.GetIdentityType(), expected...) +} + // GetName implements identity.Requester. func (u *SignedInUser) GetName() string { return u.Name @@ -183,7 +181,7 @@ func (u *SignedInUser) GetAllowedKubernetesNamespace() string { // GetCacheKey returns a unique key for the entity. // Add an extra prefix to avoid collisions with other caches func (u *SignedInUser) GetCacheKey() string { - typ, id := u.GetID().Type(), u.GetID().ID() + typ, id := u.getTypeAndID() if !u.HasUniqueId() { // Hack use the org role as id for identities that do not have a unique id // e.g. anonymous and render key. @@ -258,7 +256,7 @@ func (u *SignedInUser) GetOrgRole() identity.RoleType { } // GetID returns namespaced id for the entity -func (u *SignedInUser) GetID() identity.TypedID { +func (u *SignedInUser) GetID() string { ns, id := u.getTypeAndID() return identity.NewTypedIDString(ns, id) } diff --git a/pkg/services/user/userimpl/verifier.go b/pkg/services/user/userimpl/verifier.go index 1569ba46285..ad681e52845 100644 --- a/pkg/services/user/userimpl/verifier.go +++ b/pkg/services/user/userimpl/verifier.go @@ -5,11 +5,11 @@ import ( "errors" "fmt" "net/mail" + "strconv" "time" "github.com/grafana/authlib/claims" "github.com/grafana/grafana/pkg/apimachinery/errutil" - "github.com/grafana/grafana/pkg/apimachinery/identity" "github.com/grafana/grafana/pkg/services/auth" "github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/notifications" @@ -154,6 +154,6 @@ func (s *Verifier) Complete(ctx context.Context, cmd user.CompleteEmailVerifyCom // remove the current token, so a new one can be generated with correct values. return s.is.RemoveIDToken( ctx, - &authn.Identity{ID: identity.NewTypedID(claims.TypeUser, usr.ID), OrgID: usr.OrgID}, + &authn.Identity{ID: strconv.FormatInt(usr.ID, 10), Type: claims.TypeUser, OrgID: usr.OrgID}, ) } diff --git a/pkg/storage/unified/resource/grpc/authenticator.go b/pkg/storage/unified/resource/grpc/authenticator.go index 23322c998af..10f3c656508 100644 --- a/pkg/storage/unified/resource/grpc/authenticator.go +++ b/pkg/storage/unified/resource/grpc/authenticator.go @@ -89,21 +89,21 @@ func (f *Authenticator) decodeMetadata(ctx context.Context, meta metadata.MD) (i return user, nil } - ns, err := identity.ParseTypedID(getter(mdUserID)) + typ, id, err := identity.ParseTypeAndID(getter(mdUserID)) if err != nil { return nil, fmt.Errorf("invalid user id: %w", err) } - user.Type = ns.Type() - user.UserID, err = ns.ParseInt() + user.Type = typ + user.UserID, err = strconv.ParseInt(id, 10, 64) if err != nil { return nil, fmt.Errorf("invalid user id: %w", err) } - ns, err = identity.ParseTypedID(getter(mdUserUID)) + _, id, err = identity.ParseTypeAndID(getter(mdUserUID)) if err != nil { return nil, fmt.Errorf("invalid user id: %w", err) } - user.UserUID = ns.ID() + user.UserUID = id user.OrgName = getter(mdOrgName) user.OrgID, err = strconv.ParseInt(getter(mdOrgID), 10, 64) @@ -145,12 +145,14 @@ func wrapContext(ctx context.Context) (context.Context, error) { } func encodeIdentityInMetadata(user identity.Requester) metadata.MD { + id, _ := user.GetInternalID() + return metadata.Pairs( // This should be everything needed to recreate the user mdToken, user.GetIDToken(), // Or we can create it directly - mdUserID, user.GetID().String(), + mdUserID, user.GetID(), mdUserUID, user.GetUID(), mdOrgName, user.GetOrgName(), mdOrgID, strconv.FormatInt(user.GetOrgID(), 10), @@ -158,7 +160,7 @@ func encodeIdentityInMetadata(user identity.Requester) metadata.MD { mdLogin, user.GetLogin(), // TODO, Remove after this is deployed to unified storage - "grafana-userid", user.GetID().ID(), + "grafana-userid", strconv.FormatInt(id, 10), "grafana-useruid", user.GetRawIdentifier(), ) } diff --git a/pkg/util/proxyutil/proxyutil.go b/pkg/util/proxyutil/proxyutil.go index 4323d7c3f1b..67957abf608 100644 --- a/pkg/util/proxyutil/proxyutil.go +++ b/pkg/util/proxyutil/proxyutil.go @@ -115,7 +115,7 @@ func ApplyUserHeader(sendUserHeader bool, req *http.Request, user identity.Reque return } - if identity.IsIdentityType(user.GetID(), claims.TypeUser) { + if user.IsIdentityType(claims.TypeUser) { req.Header.Set(UserHeaderName, user.GetLogin()) } }