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

Identity: Remove typed id (#91801)

Browse Source 
* 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
This commit is contained in:
Karl Persson
2024-08-13 10:18:28 +02:00
committed by GitHub
parent 0258842f87
commit 8bcd9c2594
72 changed files with 530 additions and 521 deletions
Download Patch File Download Diff File Expand all files Collapse all files
pkg/services/authn/authnimpl/service.go
+30 -19
View File
@@ -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) {
pkg/services/authn/authnimpl/service_test.go
+29 -25
View File
@@ -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)
})
pkg/services/authn/authnimpl/sync/oauth_token_sync.go
+4 -4
View File
@@ -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
pkg/services/authn/authnimpl/sync/oauth_token_sync_test.go
+6 -5
View File
@@ -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,
pkg/services/authn/authnimpl/sync/org_sync.go
+8 -8
View File
@@ -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
}
pkg/services/authn/authnimpl/sync/org_sync_test.go
+13 -10
View File
@@ -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"))
},
pkg/services/authn/authnimpl/sync/rbac_sync.go
+2 -2
View File
@@ -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
}
pkg/services/authn/authnimpl/sync/rbac_sync_test.go
+21 -12
View File
@@ -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},
},
pkg/services/authn/authnimpl/sync/user_sync.go
+12 -17
View File
@@ -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
pkg/services/authn/authnimpl/sync/user_sync_test.go
+26 -18
View File
@@ -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},
},