diff --git a/pkg/api/pluginproxy/ds_proxy_test.go b/pkg/api/pluginproxy/ds_proxy_test.go index eeb533bf4a1..59776394ab4 100644 --- a/pkg/api/pluginproxy/ds_proxy_test.go +++ b/pkg/api/pluginproxy/ds_proxy_test.go @@ -30,6 +30,7 @@ import ( "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" "github.com/grafana/grafana/pkg/services/accesscontrol/actest" "github.com/grafana/grafana/pkg/services/auth/identity" + "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/datasources" datasourceservice "github.com/grafana/grafana/pkg/services/datasources/service" @@ -514,7 +515,7 @@ func TestDataSourceProxy_routeRule(t *testing.T) { &contextmodel.ReqContext{ SignedInUser: &user.SignedInUser{ Login: "test_user", - NamespacedID: "user:1", + NamespacedID: authn.MustParseNamespaceID("user:1"), }, }, &setting.Cfg{SendUserHeader: true}, diff --git a/pkg/api/pluginproxy/pluginproxy_test.go b/pkg/api/pluginproxy/pluginproxy_test.go index 8b6185510ef..556f0a7265f 100644 --- a/pkg/api/pluginproxy/pluginproxy_test.go +++ b/pkg/api/pluginproxy/pluginproxy_test.go @@ -15,6 +15,7 @@ import ( "github.com/grafana/grafana/pkg/infra/tracing" "github.com/grafana/grafana/pkg/plugins" "github.com/grafana/grafana/pkg/services/accesscontrol/acimpl" + "github.com/grafana/grafana/pkg/services/authn" contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model" "github.com/grafana/grafana/pkg/services/featuremgmt" "github.com/grafana/grafana/pkg/services/org" @@ -77,7 +78,7 @@ func TestPluginProxy(t *testing.T) { &contextmodel.ReqContext{ SignedInUser: &user.SignedInUser{ Login: "test_user", - NamespacedID: "user:1", + NamespacedID: authn.MustParseNamespaceID("user:1"), }, Context: &web.Context{ Req: httpReq, diff --git a/pkg/services/accesscontrol/acimpl/service_test.go b/pkg/services/accesscontrol/acimpl/service_test.go index c222de708ca..8ce30216268 100644 --- a/pkg/services/accesscontrol/acimpl/service_test.go +++ b/pkg/services/accesscontrol/acimpl/service_test.go @@ -756,7 +756,7 @@ func TestPermissionCacheKey(t *testing.T) { signedInUser: &user.SignedInUser{ OrgID: 1, UserID: 1, - NamespacedID: "user:1", + NamespacedID: identity.MustParseNamespaceID("user:1"), }, expected: "rbac-permissions-1-user-1", }, @@ -766,7 +766,7 @@ func TestPermissionCacheKey(t *testing.T) { OrgID: 1, ApiKeyID: 1, IsServiceAccount: false, - NamespacedID: "user:1", + NamespacedID: identity.MustParseNamespaceID("user:1"), }, expected: "rbac-permissions-1-api-key-1", }, @@ -776,7 +776,7 @@ func TestPermissionCacheKey(t *testing.T) { OrgID: 1, UserID: 1, IsServiceAccount: true, - NamespacedID: "serviceaccount:1", + NamespacedID: identity.MustParseNamespaceID("service-account:1"), }, expected: "rbac-permissions-1-service-account-1", }, @@ -786,7 +786,7 @@ func TestPermissionCacheKey(t *testing.T) { OrgID: 1, UserID: -1, IsServiceAccount: true, - NamespacedID: "serviceaccount:-1", + NamespacedID: identity.MustParseNamespaceID("service-account:-1"), }, expected: "rbac-permissions-1-service-account--1", }, @@ -795,7 +795,7 @@ func TestPermissionCacheKey(t *testing.T) { signedInUser: &user.SignedInUser{ OrgID: 1, OrgRole: org.RoleNone, - NamespacedID: "user:1", + NamespacedID: identity.MustParseNamespaceID("user:1"), }, expected: "rbac-permissions-1-user-None", }, diff --git a/pkg/services/auth/identity/requester.go b/pkg/services/auth/identity/requester.go index 87140b85d4f..3be473ec0a3 100644 --- a/pkg/services/auth/identity/requester.go +++ b/pkg/services/auth/identity/requester.go @@ -9,7 +9,7 @@ import ( type Requester interface { // GetID returns namespaced id for the entity - GetID() string + GetID() NamespaceID // GetNamespacedID returns the namespace and ID of the active entity. // The namespace is one of the constants defined in pkg/services/auth/identity. GetNamespacedID() (namespace string, identifier string) diff --git a/pkg/services/authn/authn.go b/pkg/services/authn/authn.go index 83cbc622bf9..69e7faaaa55 100644 --- a/pkg/services/authn/authn.go +++ b/pkg/services/authn/authn.go @@ -90,7 +90,7 @@ type Service interface { Logout(ctx context.Context, user identity.Requester, sessionToken *usertoken.UserToken) (*Redirect, error) // ResolveIdentity resolves an identity from org and namespace id. - ResolveIdentity(ctx context.Context, orgID int64, namespaceID string) (*Identity, error) + ResolveIdentity(ctx context.Context, orgID int64, namespaceID NamespaceID) (*Identity, error) // RegisterClient will register a new authn.Client that can be used for authentication RegisterClient(c Client) diff --git a/pkg/services/authn/authnimpl/service.go b/pkg/services/authn/authnimpl/service.go index 2b12e1021f6..425236a5ef6 100644 --- a/pkg/services/authn/authnimpl/service.go +++ b/pkg/services/authn/authnimpl/service.go @@ -308,18 +308,13 @@ Default: return redirect, nil } -func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID string) (*authn.Identity, error) { +func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*authn.Identity, error) { r := &authn.Request{} r.OrgID = orgID // hack to not update last seen r.SetMeta(authn.MetaKeyIsLogin, "true") - id, err := authn.ParseNamespaceID(namespaceID) - if err != nil { - return nil, err - } - - identity, err := s.resolveIdenity(ctx, orgID, id) + identity, err := s.resolveIdenity(ctx, orgID, namespaceID) if err != nil { return nil, err } diff --git a/pkg/services/authn/authnimpl/service_test.go b/pkg/services/authn/authnimpl/service_test.go index dd2592f89d7..9795d136343 100644 --- a/pkg/services/authn/authnimpl/service_test.go +++ b/pkg/services/authn/authnimpl/service_test.go @@ -487,26 +487,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, "some:1") - assert.ErrorIs(t, err, authn.ErrInvalidNamespaceID) + _, err := svc.ResolveIdentity(context.Background(), 1, authn.NewNamespaceIDUnchecked("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, "api-key:1") + _, err := svc.ResolveIdentity(context.Background(), 1, authn.MustParseNamespaceID("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, "user:1") + identity, err := svc.ResolveIdentity(context.Background(), 1, authn.MustParseNamespaceID("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, "service-account:1") + identity, err := svc.ResolveIdentity(context.Background(), 1, authn.MustParseNamespaceID("service-account:1")) assert.NoError(t, err) assert.NotNil(t, identity) }) @@ -521,7 +521,7 @@ func TestService_ResolveIdentity(t *testing.T) { }) }) - identity, err := svc.ResolveIdentity(context.Background(), 1, "api-key:1") + identity, err := svc.ResolveIdentity(context.Background(), 1, authn.MustParseNamespaceID("api-key:1")) assert.NoError(t, err) assert.NotNil(t, identity) }) diff --git a/pkg/services/authn/authntest/fake.go b/pkg/services/authn/authntest/fake.go index c65b2d7f7fb..f88e3384291 100644 --- a/pkg/services/authn/authntest/fake.go +++ b/pkg/services/authn/authntest/fake.go @@ -76,7 +76,7 @@ func (f *FakeService) Logout(_ context.Context, _ identity.Requester, _ *usertok panic("unimplemented") } -func (f *FakeService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID string) (*authn.Identity, error) { +func (f *FakeService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*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 f2d7d77d9b1..63dc12a8745 100644 --- a/pkg/services/authn/authntest/mock.go +++ b/pkg/services/authn/authntest/mock.go @@ -50,7 +50,7 @@ func (*MockService) Logout(_ context.Context, _ identity.Requester, _ *usertoken panic("unimplemented") } -func (m *MockService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID string) (*authn.Identity, error) { +func (m *MockService) ResolveIdentity(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*authn.Identity, error) { panic("unimplemented") } diff --git a/pkg/services/authn/identity.go b/pkg/services/authn/identity.go index 38d9a51c67b..51fc542fae9 100644 --- a/pkg/services/authn/identity.go +++ b/pkg/services/authn/identity.go @@ -73,8 +73,8 @@ type Identity struct { IDToken string } -func (i *Identity) GetID() string { - return i.ID.String() +func (i *Identity) GetID() NamespaceID { + return i.ID } func (i *Identity) GetNamespacedID() (namespace string, identifier string) { @@ -220,7 +220,7 @@ func (i *Identity) SignedInUser() *user.SignedInUser { Teams: i.Teams, Permissions: i.Permissions, IDToken: i.IDToken, - NamespacedID: i.ID.String(), + NamespacedID: i.ID, } if namespace == NamespaceAPIKey { diff --git a/pkg/services/user/identity.go b/pkg/services/user/identity.go index 3d9d2fab649..5cd3ffe2622 100644 --- a/pkg/services/user/identity.go +++ b/pkg/services/user/identity.go @@ -2,7 +2,6 @@ package user import ( "fmt" - "strings" "time" "github.com/grafana/grafana/pkg/models/roletype" @@ -40,7 +39,7 @@ type SignedInUser struct { // IDToken is a signed token representing the identity that can be forwarded to plugins and external services. // Will only be set when featuremgmt.FlagIdForwarding is enabled. IDToken string `json:"-" xorm:"-"` - NamespacedID string + NamespacedID identity.NamespaceID } func (u *SignedInUser) ShouldUpdateLastSeenAt() bool { @@ -196,7 +195,7 @@ func (u *SignedInUser) GetOrgRole() roletype.RoleType { } // GetID returns namespaced id for the entity -func (u *SignedInUser) GetID() string { +func (u *SignedInUser) GetID() identity.NamespaceID { switch { case u.ApiKeyID != 0: return namespacedID(identity.NamespaceAPIKey, u.ApiKeyID) @@ -205,7 +204,7 @@ func (u *SignedInUser) GetID() string { case u.UserID > 0: return namespacedID(identity.NamespaceUser, u.UserID) case u.IsAnonymous: - return identity.NamespaceAnonymous + ":" + return namespacedID(identity.NamespaceAnonymous, 0) case u.AuthenticatedBy == "render" && u.UserID == 0: return namespacedID(identity.NamespaceRenderService, 0) } @@ -216,13 +215,8 @@ func (u *SignedInUser) GetID() string { // GetNamespacedID returns the namespace and ID of the active entity // The namespace is one of the constants defined in pkg/services/auth/identity func (u *SignedInUser) GetNamespacedID() (string, string) { - parts := strings.Split(u.GetID(), ":") - // Safety: GetID always returns a ':' separated string - if len(parts) != 2 { - return "", "" - } - - return parts[0], parts[1] + id := u.GetID() + return id.Namespace(), id.ID() } func (u *SignedInUser) GetAuthID() string { @@ -267,6 +261,6 @@ func (u *SignedInUser) GetIDToken() string { return u.IDToken } -func namespacedID(namespace string, id int64) string { - return fmt.Sprintf("%s:%d", namespace, id) +func namespacedID(namespace string, id int64) identity.NamespaceID { + return identity.NewNamespaceIDUnchecked(namespace, id) } diff --git a/pkg/util/proxyutil/proxyutil_test.go b/pkg/util/proxyutil/proxyutil_test.go index 10eb6c536e7..d84578f638a 100644 --- a/pkg/util/proxyutil/proxyutil_test.go +++ b/pkg/util/proxyutil/proxyutil_test.go @@ -6,6 +6,7 @@ import ( "github.com/stretchr/testify/require" + "github.com/grafana/grafana/pkg/services/auth/identity" "github.com/grafana/grafana/pkg/services/user" ) @@ -174,7 +175,7 @@ func TestApplyUserHeader(t *testing.T) { require.NoError(t, err) req.Header.Set("X-Grafana-User", "admin") - ApplyUserHeader(false, req, &user.SignedInUser{Login: "admin", NamespacedID: "user:1"}) + ApplyUserHeader(false, req, &user.SignedInUser{Login: "admin", NamespacedID: identity.MustParseNamespaceID("user:1")}) require.NotContains(t, req.Header, "X-Grafana-User") }) @@ -191,7 +192,7 @@ func TestApplyUserHeader(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "/", nil) require.NoError(t, err) - ApplyUserHeader(true, req, &user.SignedInUser{IsAnonymous: true, NamespacedID: "anonymous:1"}) + ApplyUserHeader(true, req, &user.SignedInUser{IsAnonymous: true, NamespacedID: identity.MustParseNamespaceID("anonymous:0")}) require.NotContains(t, req.Header, "X-Grafana-User") }) @@ -199,7 +200,7 @@ func TestApplyUserHeader(t *testing.T) { req, err := http.NewRequest(http.MethodGet, "/", nil) require.NoError(t, err) - ApplyUserHeader(true, req, &user.SignedInUser{Login: "admin", NamespacedID: "user:1"}) + ApplyUserHeader(true, req, &user.SignedInUser{Login: "admin", NamespacedID: identity.MustParseNamespaceID("user:1")}) require.Equal(t, "admin", req.Header.Get("X-Grafana-User")) }) }