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

Identity: Rename "namespace" to "type" in the requester interface (#90567)

Browse Source
This commit is contained in:
Ryan McKinley
2024-07-25 12:52:14 +03:00
committed by GitHub
parent 8cdf5ee824
commit 9db3bc926e
109 changed files with 649 additions and 632 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/authnimpl/service.go
+12 -11
View File
@@ -13,6 +13,7 @@ import (
"go.opentelemetry.io/otel/trace"
"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/network"
"github.com/grafana/grafana/pkg/infra/tracing"
@@ -216,9 +217,9 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i
}
// Login is only supported for users
if !id.ID.IsNamespace(authn.NamespaceUser) {
if !id.ID.IsType(identity.TypeUser) {
s.metrics.failedLogin.WithLabelValues(client).Inc()
return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", id.ID.Namespace())
return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", id.ID.Type())
}
userID, err := id.ID.ParseInt()
@@ -271,7 +272,7 @@ func (s *Service) RegisterPreLogoutHook(hook authn.PreLogoutHookFn, priority uin
s.preLogoutHooks.insert(hook, priority)
}
func (s *Service) Logout(ctx context.Context, user authn.Requester, sessionToken *auth.UserToken) (*authn.Redirect, error) {
func (s *Service) Logout(ctx context.Context, user identity.Requester, sessionToken *auth.UserToken) (*authn.Redirect, error) {
ctx, span := s.tracer.Start(ctx, "authn.Logout")
defer span.End()
@@ -280,7 +281,7 @@ func (s *Service) Logout(ctx context.Context, user authn.Requester, sessionToken
redirect.URL = s.cfg.SignoutRedirectUrl
}
if !user.GetID().IsNamespace(authn.NamespaceUser) {
if !user.GetID().IsType(identity.TypeUser) {
return redirect, nil
}
@@ -327,7 +328,7 @@ Default:
return redirect, nil
}
func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*authn.Identity, error) {
func (s *Service) ResolveIdentity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) {
ctx, span := s.tracer.Start(ctx, "authn.ResolveIdentity")
defer span.End()
@@ -352,7 +353,7 @@ func (s *Service) RegisterClient(c authn.Client) {
}
if rc, ok := c.(authn.IdentityResolverClient); ok {
s.idenityResolverClients[rc.Namespace()] = rc
s.idenityResolverClients[rc.IdentityType().String()] = rc
}
}
@@ -375,11 +376,11 @@ 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 authn.NamespaceID) (*authn.Identity, error) {
func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) {
ctx, span := s.tracer.Start(ctx, "authn.resolveIdentity")
defer span.End()
if namespaceID.IsNamespace(authn.NamespaceUser) {
if namespaceID.IsType(identity.TypeUser) {
return &authn.Identity{
OrgID: orgID,
ID: namespaceID,
@@ -390,7 +391,7 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID a
}}, nil
}
if namespaceID.IsNamespace(authn.NamespaceServiceAccount) {
if namespaceID.IsType(identity.TypeServiceAccount) {
return &authn.Identity{
ID: namespaceID,
OrgID: orgID,
@@ -401,9 +402,9 @@ func (s *Service) resolveIdenity(ctx context.Context, orgID int64, namespaceID a
}}, nil
}
resolver, ok := s.idenityResolverClients[namespaceID.Namespace().String()]
resolver, ok := s.idenityResolverClients[string(namespaceID.Type())]
if !ok {
return nil, authn.ErrUnsupportedIdentity.Errorf("no resolver for : %s", namespaceID.Namespace())
return nil, authn.ErrUnsupportedIdentity.Errorf("no resolver for : %s", namespaceID.Type())
}
return resolver.ResolveIdentity(ctx, orgID, namespaceID)
}
pkg/services/authn/authnimpl/service_test.go
+24 -24
View File
@@ -44,9 +44,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: authn.MustParseNamespaceID("user:1")}},
&authntest.FakeClient{ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")}},
},
expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")},
expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
},
{
desc: "should succeed with authentication for configured client for identity with fetch permissions params",
@@ -54,7 +54,7 @@ func TestService_Authenticate(t *testing.T) {
&authntest.FakeClient{
ExpectedTest: true,
ExpectedIdentity: &authn.Identity{
ID: authn.MustParseNamespaceID("user:2"),
ID: identity.MustParseTypedID("user:2"),
ClientParams: authn.ClientParams{
FetchPermissionsParams: authn.FetchPermissionsParams{
ActionsLookup: []string{
@@ -70,7 +70,7 @@ func TestService_Authenticate(t *testing.T) {
},
},
expectedIdentity: &authn.Identity{
ID: authn.MustParseNamespaceID("user:2"),
ID: identity.MustParseTypedID("user:2"),
ClientParams: authn.ClientParams{
FetchPermissionsParams: authn.FetchPermissionsParams{
ActionsLookup: []string{
@@ -92,19 +92,19 @@ func TestService_Authenticate(t *testing.T) {
ExpectedName: "2",
ExpectedPriority: 2,
ExpectedTest: true,
ExpectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2"), 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: authn.MustParseNamespaceID("user:2"), AuthID: "service:some-service", AuthenticatedBy: "service_auth"},
expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), 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: authn.MustParseNamespaceID("user:3")}},
&authntest.FakeClient{ExpectedName: "3", ExpectedPriority: 3, ExpectedTest: true, ExpectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:3")}},
},
expectedIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("user:3")},
expectedIdentity: &authn.Identity{ID: identity.MustParseTypedID("user:3")},
},
{
desc: "should return error when no client could authenticate the request",
@@ -315,10 +315,10 @@ func TestService_Login(t *testing.T) {
client: "fake",
expectedClientOK: true,
expectedClientIdentity: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
},
expectedIdentity: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
SessionToken: &auth.UserToken{UserId: 1},
},
},
@@ -331,7 +331,7 @@ func TestService_Login(t *testing.T) {
desc: "should not login non user identity",
client: "fake",
expectedClientOK: true,
expectedClientIdentity: &authn.Identity{ID: authn.MustParseNamespaceID("api-key:1")},
expectedClientIdentity: &authn.Identity{ID: identity.MustParseTypedID("api-key:1")},
expectedErr: authn.ErrUnsupportedIdentity,
},
}
@@ -420,31 +420,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: authn.NewNamespaceID(authn.NamespaceServiceAccount, 1)},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeServiceAccount, 1)},
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: authn.NewNamespaceID(authn.NamespaceUser, 1)},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeUser, 1)},
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: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "notfound"},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeUser, 1), 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: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeUser, 1), 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: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeUser, 1), AuthenticatedBy: "azuread"},
expectedRedirect: &authn.Redirect{URL: "some-url"},
client: &authntest.FakeClient{ExpectedName: "auth.client.azuread"},
signoutRedirectURL: "some-url",
@@ -452,7 +452,7 @@ func TestService_Logout(t *testing.T) {
},
{
desc: "should redirect to client specific url",
identity: &authn.Identity{ID: authn.NewNamespaceID(authn.NamespaceUser, 1), AuthenticatedBy: "azuread"},
identity: &authn.Identity{ID: identity.NewTypedID(identity.TypeUser, 1), AuthenticatedBy: "azuread"},
expectedRedirect: &authn.Redirect{URL: "http://idp.com/logout"},
client: &authntest.MockClient{
NameFunc: func() string { return "auth.client.azuread" },
@@ -500,26 +500,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, authn.NewNamespaceID("some", 1))
_, err := svc.ResolveIdentity(context.Background(), 1, identity.NewTypedID("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, authn.MustParseNamespaceID("api-key:1"))
_, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("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, authn.MustParseNamespaceID("user:1"))
identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("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, authn.MustParseNamespaceID("service-account:1"))
identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("service-account:1"))
assert.NoError(t, err)
assert.NotNil(t, identity)
})
@@ -527,14 +527,14 @@ func TestService_ResolveIdentity(t *testing.T) {
t.Run("should resolve for valid namespace if client is registered", func(t *testing.T) {
svc := setupTests(t, func(svc *Service) {
svc.RegisterClient(&authntest.MockClient{
NamespaceFunc: func() string { return authn.NamespaceAPIKey.String() },
ResolveIdentityFunc: func(ctx context.Context, orgID int64, namespaceID authn.NamespaceID) (*authn.Identity, error) {
IdentityTypeFunc: func() identity.IdentityType { return identity.TypeAPIKey },
ResolveIdentityFunc: func(ctx context.Context, orgID int64, namespaceID identity.TypedID) (*authn.Identity, error) {
return &authn.Identity{}, nil
},
})
})
identity, err := svc.ResolveIdentity(context.Background(), 1, authn.MustParseNamespaceID("api-key:1"))
identity, err := svc.ResolveIdentity(context.Background(), 1, identity.MustParseTypedID("api-key:1"))
assert.NoError(t, err)
assert.NotNil(t, identity)
})
pkg/services/authn/authnimpl/sync/oauth_token_sync.go
+14 -13
View File
@@ -8,6 +8,7 @@ import (
"golang.org/x/sync/singleflight"
"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/login/social"
@@ -36,42 +37,42 @@ type OAuthTokenSync struct {
tracer tracing.Tracer
}
func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error {
func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
ctx, span := s.tracer.Start(ctx, "oauth.sync.SyncOauthTokenHook")
defer span.End()
// only perform oauth token check if identity is a user
if !identity.ID.IsNamespace(authn.NamespaceUser) {
if !id.ID.IsType(identity.TypeUser) {
return nil
}
// Not authenticated through session tokens, so we can skip this hook.
if identity.SessionToken == nil {
if id.SessionToken == nil {
return nil
}
// Not authenticated with a oauth provider, so we can skip this hook.
if !strings.HasPrefix(identity.GetAuthenticatedBy(), "oauth") {
if !strings.HasPrefix(id.GetAuthenticatedBy(), "oauth") {
return nil
}
ctxLogger := s.log.FromContext(ctx).New("userID", identity.ID.ID())
ctxLogger := s.log.FromContext(ctx).New("userID", id.ID.ID())
_, err, _ := s.singleflightGroup.Do(identity.ID.String(), func() (interface{}, error) {
_, err, _ := s.singleflightGroup.Do(id.ID.String(), 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
updateCtx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
defer cancel()
if refreshErr := s.service.TryTokenRefresh(updateCtx, identity); refreshErr != nil {
if refreshErr := s.service.TryTokenRefresh(updateCtx, id); refreshErr != nil {
if errors.Is(refreshErr, context.Canceled) {
return nil, nil
}
token, _, err := s.service.HasOAuthEntry(ctx, identity)
token, _, err := s.service.HasOAuthEntry(ctx, id)
if err != nil {
ctxLogger.Error("Failed to get OAuth entry for verifying if token has already been refreshed", "id", identity.ID, "error", err)
ctxLogger.Error("Failed to get OAuth entry for verifying if token has already been refreshed", "id", id.ID, "error", err)
return nil, err
}
@@ -81,14 +82,14 @@ func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn
return nil, nil
}
ctxLogger.Error("Failed to refresh OAuth access token", "id", identity.ID, "error", refreshErr)
ctxLogger.Error("Failed to refresh OAuth access token", "id", id.ID, "error", refreshErr)
if err := s.service.InvalidateOAuthTokens(ctx, token); err != nil {
ctxLogger.Warn("Failed to invalidate OAuth tokens", "id", identity.ID, "error", err)
ctxLogger.Warn("Failed to invalidate OAuth tokens", "id", id.ID, "error", err)
}
if err := s.sessionService.RevokeToken(ctx, identity.SessionToken, false); err != nil {
ctxLogger.Warn("Failed to revoke session token", "id", identity.ID, "tokenId", identity.SessionToken.Id, "error", err)
if err := s.sessionService.RevokeToken(ctx, id.SessionToken, false); err != nil {
ctxLogger.Warn("Failed to revoke session token", "id", id.ID, "tokenId", id.SessionToken.Id, "error", err)
}
return nil, refreshErr
pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go
+8 -7
View File
@@ -9,6 +9,7 @@ import (
"github.com/stretchr/testify/assert"
"golang.org/x/sync/singleflight"
"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/login/social"
@@ -41,17 +42,17 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
tests := []testCase{
{
desc: "should skip sync when identity is not a user",
identity: &authn.Identity{ID: authn.MustParseNamespaceID("service-account:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("service-account:1")},
expectTryRefreshTokenCalled: false,
},
{
desc: "should skip sync when identity is a user but is not authenticated with session token",
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
expectTryRefreshTokenCalled: false,
},
{
desc: "should invalidate access token and session token if token refresh fails",
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: true,
expectedTryRefreshErr: errors.New("some err"),
expectTryRefreshTokenCalled: true,
@@ -62,7 +63,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
},
{
desc: "should refresh the token successfully",
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: false,
expectTryRefreshTokenCalled: true,
expectInvalidateOauthTokensCalled: false,
@@ -70,7 +71,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: authn.MustParseNamespaceID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1"), SessionToken: &auth.UserToken{}, AuthenticatedBy: login.AzureADAuthModule},
expectHasEntryCalled: true,
expectTryRefreshTokenCalled: true,
expectInvalidateOauthTokensCalled: false,
@@ -92,7 +93,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
)
service := &oauthtokentest.MockOauthTokenService{
HasOAuthEntryFunc: func(ctx context.Context, usr authn.Requester) (*login.UserAuth, bool, error) {
HasOAuthEntryFunc: func(ctx context.Context, usr identity.Requester) (*login.UserAuth, bool, error) {
hasEntryCalled = true
return tt.expectedHasEntryToken, tt.expectedHasEntryToken != nil, nil
},
@@ -100,7 +101,7 @@ func TestOAuthTokenSync_SyncOAuthTokenHook(t *testing.T) {
invalidateTokensCalled = true
return nil
},
TryTokenRefreshFunc: func(ctx context.Context, usr authn.Requester) error {
TryTokenRefreshFunc: func(ctx context.Context, usr identity.Requester) error {
tryRefreshCalled = true
return tt.expectedTryRefreshErr
},
pkg/services/authn/authnimpl/sync/org_sync.go
+7 -6
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"sort"
"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"
@@ -38,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.IsNamespace(authn.NamespaceUser) {
ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "namespace", id.ID.Namespace())
if !id.ID.IsType(identity.TypeUser) {
ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "type", id.ID.Type())
return nil
}
userID, err := id.ID.ParseInt()
if err != nil {
ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "namespace", id.ID.Namespace(), "err", err)
ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "type", id.ID.Type(), "err", err)
return nil
}
@@ -144,14 +145,14 @@ func (s *OrgSync) SetDefaultOrgHook(ctx context.Context, currentIdentity *authn.
ctxLogger := s.log.FromContext(ctx)
if !currentIdentity.ID.IsNamespace(authn.NamespaceUser) {
ctxLogger.Debug("Skipping default org sync, not a user", "namespace", currentIdentity.ID.Namespace())
if !currentIdentity.ID.IsType(identity.TypeUser) {
ctxLogger.Debug("Skipping default org sync, not a user", "type", currentIdentity.ID.Type())
return
}
userID, err := currentIdentity.ID.ParseInt()
if err != nil {
ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "namespace", currentIdentity.ID.Namespace(), "err", err)
ctxLogger.Debug("Skipping default org sync, invalid ID for identity", "id", currentIdentity.ID, "type", currentIdentity.ID.Type(), "err", err)
return
}
pkg/services/authn/authnimpl/sync/org_sync_test.go
+10 -10
View File
@@ -76,7 +76,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) {
args: args{
ctx: context.Background(),
id: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
Login: "test",
Name: "test",
Email: "test",
@@ -92,7 +92,7 @@ func TestOrgSync_SyncOrgRolesHook(t *testing.T) {
},
},
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
Login: "test",
Name: "test",
Email: "test",
@@ -139,7 +139,7 @@ func TestOrgSync_SetDefaultOrgHook(t *testing.T) {
{
name: "should set default org",
defaultOrgSetting: 2,
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
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 +149,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: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
},
{
name: "should skip setting the default org when identity is nil",
@@ -159,28 +159,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: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
inputErr: fmt.Errorf("error"),
},
{
name: "should skip setting the default org when identity is not a user",
defaultOrgSetting: 2,
identity: &authn.Identity{ID: authn.MustParseNamespaceID("service-account:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("service-account:1")},
},
{
name: "should skip setting the default org when user id is not valid",
defaultOrgSetting: 2,
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:invalid")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:invalid")},
},
{
name: "should skip setting the default org when user is not allowed to use the configured default org",
defaultOrgSetting: 3,
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
},
{
name: "should skip setting the default org when validateUsingOrg returns error",
defaultOrgSetting: 2,
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) {
orgService.ExpectedError = fmt.Errorf("error")
},
@@ -188,7 +188,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: authn.MustParseNamespaceID("user:1")},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:1")},
setupMock: func(userService *usertest.MockService, orgService *orgtest.FakeOrgService) {
userService.On("Update", mock.Anything, mock.Anything).Return(fmt.Errorf("error"))
},
pkg/services/authn/authnimpl/sync/rbac_sync.go
+2 -1
View File
@@ -5,6 +5,7 @@ import (
"errors"
"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/accesscontrol"
@@ -146,7 +147,7 @@ func (s *RBACSync) SyncCloudRoles(ctx context.Context, ident *authn.Identity, r
return nil
}
if !ident.ID.IsNamespace(authn.NamespaceUser) {
if !ident.ID.IsType(identity.TypeUser) {
s.log.FromContext(ctx).Debug("Skip syncing cloud role", "id", ident.ID)
return nil
}
pkg/services/authn/authnimpl/sync/rbac_sync_test.go
+16 -14
View File
@@ -4,6 +4,10 @@ import (
"context"
"testing"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"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"
@@ -11,8 +15,6 @@ import (
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestRBACSync_SyncPermission(t *testing.T) {
@@ -24,14 +26,14 @@ func TestRBACSync_SyncPermission(t *testing.T) {
testCases := []testCase{
{
name: "enriches the identity successfully when SyncPermissions is true",
identity: &authn.Identity{ID: authn.MustParseNamespaceID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), 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: authn.MustParseNamespaceID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
identity: &authn.Identity{ID: identity.MustParseTypedID("user:2"), OrgID: 1, ClientParams: authn.ClientParams{SyncPermissions: true}},
expectedPermissions: []accesscontrol.Permission{
{Action: accesscontrol.ActionUsersRead},
},
@@ -65,7 +67,7 @@ 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: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleViewer},
},
@@ -76,7 +78,7 @@ 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: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleEditor},
},
@@ -87,7 +89,7 @@ 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: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
},
@@ -98,7 +100,7 @@ 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: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleType("something else")},
},
@@ -109,7 +111,7 @@ func TestRBACSync_SyncCloudRoles(t *testing.T) {
desc: "should not call sync when not authenticated with grafana com",
module: login.LDAPAuthModule,
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
},
@@ -155,7 +157,7 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) {
{
desc: "should map Cloud Viewer to Grafana Cloud Viewer and Support ticket reader",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleViewer},
},
@@ -174,7 +176,7 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) {
{
desc: "should map Cloud Editor to Grafana Cloud Editor and Support ticket admin",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleEditor},
},
@@ -193,7 +195,7 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) {
{
desc: "should map Cloud Admin to Grafana Cloud Admin and Support ticket admin",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleAdmin},
},
@@ -212,7 +214,7 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) {
{
desc: "should return an error for not supported role",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
OrgID: 1,
OrgRoles: map[int64]org.RoleType{1: org.RoleNone},
},
@@ -234,7 +236,7 @@ func TestRBACSync_cloudRolesToAddAndRemove(t *testing.T) {
func setupTestEnv() *RBACSync {
acMock := &acmock.Mock{
GetUserPermissionsFunc: func(ctx context.Context, siu authn.Requester, o accesscontrol.Options) ([]accesscontrol.Permission, error) {
GetUserPermissionsFunc: func(ctx context.Context, siu identity.Requester, o accesscontrol.Options) ([]accesscontrol.Permission, error) {
return []accesscontrol.Permission{
{Action: accesscontrol.ActionUsersRead},
}, nil
pkg/services/authn/authnimpl/sync/user_sync.go
+37 -36
View File
@@ -6,6 +6,7 @@ import (
"fmt"
"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"
@@ -109,21 +110,21 @@ func (s *UserSync) SyncUserHook(ctx context.Context, id *authn.Identity, _ *auth
return nil
}
func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error {
func (s *UserSync) FetchSyncedUserHook(ctx context.Context, id *authn.Identity, r *authn.Request) error {
ctx, span := s.tracer.Start(ctx, "user.sync.FetchSyncedUserHook")
defer span.End()
if !identity.ClientParams.FetchSyncedUser {
if !id.ClientParams.FetchSyncedUser {
return nil
}
if !identity.ID.IsNamespace(authn.NamespaceUser, authn.NamespaceServiceAccount) {
if !id.ID.IsType(identity.TypeUser, identity.TypeServiceAccount) {
return nil
}
userID, err := identity.ID.ParseInt()
userID, err := id.ID.ParseInt()
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err)
return nil
}
@@ -138,18 +139,18 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Iden
return errFetchingSignedInUser.Errorf("failed to resolve user: %w", err)
}
if identity.ClientParams.AllowGlobalOrg && identity.OrgID == authn.GlobalOrgID {
if id.ClientParams.AllowGlobalOrg && id.OrgID == authn.GlobalOrgID {
usr.Teams = nil
usr.OrgName = ""
usr.OrgRole = org.RoleNone
usr.OrgID = authn.GlobalOrgID
}
syncSignedInUserToIdentity(usr, identity)
syncSignedInUserToIdentity(usr, id)
return nil
}
func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identity, r *authn.Request) error {
func (s *UserSync) SyncLastSeenHook(ctx context.Context, id *authn.Identity, r *authn.Request) error {
ctx, span := s.tracer.Start(ctx, "user.sync.SyncLastSeenHook")
defer span.End()
@@ -158,13 +159,13 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit
return nil
}
if !identity.ID.IsNamespace(authn.NamespaceUser, authn.NamespaceServiceAccount) {
if !id.ID.IsType(identity.TypeUser, identity.TypeServiceAccount) {
return nil
}
userID, err := identity.ID.ParseInt()
userID, err := id.ID.ParseInt()
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err)
return nil
}
@@ -186,21 +187,21 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit
return nil
}
func (s *UserSync) EnableUserHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error {
func (s *UserSync) EnableUserHook(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
ctx, span := s.tracer.Start(ctx, "user.sync.EnableUserHook")
defer span.End()
if !identity.ClientParams.EnableUser {
if !id.ClientParams.EnableUser {
return nil
}
if !identity.ID.IsNamespace(authn.NamespaceUser) {
if !id.ID.IsType(identity.TypeUser) {
return nil
}
userID, err := identity.ID.ParseInt()
userID, err := id.ID.ParseInt()
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", identity.ID, "err", err)
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id.ID, "err", err)
return nil
}
@@ -417,8 +418,8 @@ 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 = authn.NewNamespaceID(authn.NamespaceUser, usr.ID)
id.UID = authn.NewNamespaceIDString(authn.NamespaceUser, usr.UID)
id.ID = identity.NewTypedID(identity.TypeUser, usr.ID)
id.UID = identity.NewTypedIDString(identity.TypeUser, usr.UID)
id.Login = usr.Login
id.Email = usr.Email
id.Name = usr.Name
@@ -427,25 +428,25 @@ func syncUserToIdentity(usr *user.User, id *authn.Identity) {
}
// syncSignedInUserToIdentity syncs a user to an identity.
func syncSignedInUserToIdentity(usr *user.SignedInUser, identity *authn.Identity) {
var ns authn.Namespace
if identity.ID.IsNamespace(authn.NamespaceServiceAccount) {
ns = authn.NamespaceServiceAccount
func syncSignedInUserToIdentity(usr *user.SignedInUser, id *authn.Identity) {
var ns identity.IdentityType
if id.ID.IsType(identity.TypeServiceAccount) {
ns = identity.TypeServiceAccount
} else {
ns = authn.NamespaceUser
ns = identity.TypeUser
}
identity.UID = authn.NewNamespaceIDString(ns, usr.UserUID)
id.UID = identity.NewTypedIDString(ns, usr.UserUID)
identity.Name = usr.Name
identity.Login = usr.Login
identity.Email = usr.Email
identity.OrgID = usr.OrgID
identity.OrgName = usr.OrgName
identity.OrgRoles = map[int64]org.RoleType{identity.OrgID: usr.OrgRole}
identity.HelpFlags1 = usr.HelpFlags1
identity.Teams = usr.Teams
identity.LastSeenAt = usr.LastSeenAt
identity.IsDisabled = usr.IsDisabled
identity.IsGrafanaAdmin = &usr.IsGrafanaAdmin
identity.EmailVerified = usr.EmailVerified
id.Name = usr.Name
id.Login = usr.Login
id.Email = usr.Email
id.OrgID = usr.OrgID
id.OrgName = usr.OrgName
id.OrgRoles = map[int64]org.RoleType{id.OrgID: usr.OrgRole}
id.HelpFlags1 = usr.HelpFlags1
id.Teams = usr.Teams
id.LastSeenAt = usr.LastSeenAt
id.IsDisabled = usr.IsDisabled
id.IsGrafanaAdmin = &usr.IsGrafanaAdmin
id.EmailVerified = usr.EmailVerified
}
pkg/services/authn/authnimpl/sync/user_sync_test.go
+17 -16
View File
@@ -7,6 +7,7 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"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"
@@ -163,8 +164,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
UID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
UID: identity.MustParseTypedID("user:1"),
Login: "test",
Name: "test",
Email: "test",
@@ -202,8 +203,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
UID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
UID: identity.MustParseTypedID("user:1"),
Login: "test",
Name: "test",
Email: "test",
@@ -243,8 +244,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:1"),
UID: authn.MustParseNamespaceID("user:1"),
ID: identity.MustParseTypedID("user:1"),
UID: identity.MustParseTypedID("user:1"),
AuthID: "2032",
AuthenticatedBy: "oauth",
Login: "test",
@@ -315,8 +316,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:2"),
UID: authn.MustParseNamespaceID("user:2"),
ID: identity.MustParseTypedID("user:2"),
UID: identity.MustParseTypedID("user:2"),
Login: "test_create",
Name: "test_create",
Email: "test_create",
@@ -361,8 +362,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:3"),
UID: authn.MustParseNamespaceID("user:3"),
ID: identity.MustParseTypedID("user:3"),
UID: identity.MustParseTypedID("user:3"),
Login: "test_mod",
Name: "test_mod",
Email: "test_mod",
@@ -406,8 +407,8 @@ func TestUserSync_SyncUserHook(t *testing.T) {
},
wantErr: false,
wantID: &authn.Identity{
ID: authn.MustParseNamespaceID("user:3"),
UID: authn.MustParseNamespaceID("user:3"),
ID: identity.MustParseTypedID("user:3"),
UID: identity.MustParseTypedID("user:3"),
Name: "test",
Login: "test",
Email: "test_mod@test.com",
@@ -457,7 +458,7 @@ func TestUserSync_FetchSyncedUserHook(t *testing.T) {
{
desc: "should skip hook when identity is not a user",
req: &authn.Request{},
identity: &authn.Identity{ID: authn.MustParseNamespaceID("api-key:1"), ClientParams: authn.ClientParams{FetchSyncedUser: true}},
identity: &authn.Identity{ID: identity.MustParseTypedID("api-key:1"), ClientParams: authn.ClientParams{FetchSyncedUser: true}},
},
}
@@ -483,7 +484,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should skip if correct flag is not set",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: false},
},
@@ -492,7 +493,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should skip if identity is not a user",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceAPIKey, 1),
ID: identity.NewTypedID(identity.TypeAPIKey, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: true},
},
@@ -501,7 +502,7 @@ func TestUserSync_EnableDisabledUserHook(t *testing.T) {
{
desc: "should enabled disabled user",
identity: &authn.Identity{
ID: authn.NewNamespaceID(authn.NamespaceUser, 1),
ID: identity.NewTypedID(identity.TypeUser, 1),
IsDisabled: true,
ClientParams: authn.ClientParams{EnableUser: true},
},