Chore: Cleanup namespace and ID resolution (#79360)

* Chore: Cleanup namespace ID resolution

* Check for negative userID when relevant

* Reuse existing function for parsing ID as int

* Fix imports
This commit is contained in:
Vardan Torosyan 2023-12-21 20:42:05 +01:00 committed by GitHub
parent d160638c67
commit 63cd5a5625
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
8 changed files with 65 additions and 46 deletions

View File

@ -102,7 +102,7 @@ func (s *Service) hook(ctx context.Context, identity *authn.Identity, _ *authn.R
// FIXME(kalleep): we should probably lazy load this
token, err := s.SignIdentity(ctx, identity)
if err != nil {
namespace, id := identity.NamespacedID()
namespace, id := identity.GetNamespacedID()
s.logger.Error("Failed to sign id token", "err", err, "namespace", namespace, "id", id)
// for now don't return error so we don't break authentication from this hook
return nil

View File

@ -263,7 +263,7 @@ func (s *Service) RegisterPostAuthHook(hook authn.PostAuthHookFn, priority uint)
s.postAuthHooks.insert(hook, priority)
}
func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (identity *authn.Identity, err error) {
func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (id *authn.Identity, err error) {
ctx, span := s.tracer.Start(ctx, "authn.Login", trace.WithAttributes(
attribute.String(attributeKeyClient, client),
))
@ -273,7 +273,7 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i
defer func() {
for _, hook := range s.postLoginHooks.items {
hook.v(ctx, identity, r, err)
hook.v(ctx, id, r, err)
}
}()
@ -284,36 +284,40 @@ func (s *Service) Login(ctx context.Context, client string, r *authn.Request) (i
}
r.SetMeta(authn.MetaKeyIsLogin, "true")
identity, err = s.authenticate(ctx, c, r)
id, err = s.authenticate(ctx, c, r)
if err != nil {
s.metrics.failedLogin.WithLabelValues(client).Inc()
return nil, err
}
namespace, id := identity.NamespacedID()
namespace, namespaceID := id.GetNamespacedID()
// Login is only supported for users
if namespace != authn.NamespaceUser || id <= 0 {
if namespace != authn.NamespaceUser {
s.metrics.failedLogin.WithLabelValues(client).Inc()
return nil, authn.ErrUnsupportedIdentity.Errorf("expected identity of type user but got: %s", namespace)
}
intId, err := identity.IntIdentifier(namespace, namespaceID)
if err != nil {
return nil, err
}
addr := web.RemoteAddr(r.HTTPRequest)
ip, err := network.GetIPFromAddress(addr)
if err != nil {
s.log.FromContext(ctx).Debug("Failed to parse ip from address", "client", c.Name(), "id", identity.ID, "addr", addr, "error", err)
s.log.FromContext(ctx).Debug("Failed to parse ip from address", "client", c.Name(), "id", id.ID, "addr", addr, "error", err)
}
sessionToken, err := s.sessionService.CreateToken(ctx, &user.User{ID: id}, ip, r.HTTPRequest.UserAgent())
sessionToken, err := s.sessionService.CreateToken(ctx, &user.User{ID: intId}, ip, r.HTTPRequest.UserAgent())
if err != nil {
s.metrics.failedLogin.WithLabelValues(client).Inc()
s.log.FromContext(ctx).Error("Failed to create session", "client", client, "id", identity.ID, "err", err)
s.log.FromContext(ctx).Error("Failed to create session", "client", client, "id", id.ID, "err", err)
return nil, err
}
s.metrics.successfulLogin.WithLabelValues(client).Inc()
identity.SessionToken = sessionToken
return identity, nil
id.SessionToken = sessionToken
return id, nil
}
func (s *Service) RegisterPostLoginHook(hook authn.PostLoginHookFn, priority uint) {

View File

@ -40,7 +40,7 @@ type OAuthTokenSync struct {
}
func (s *OAuthTokenSync) SyncOauthTokenHook(ctx context.Context, identity *authn.Identity, _ *authn.Request) error {
namespace, _ := identity.NamespacedID()
namespace, _ := identity.GetNamespacedID()
// only perform oauth token check if identity is a user
if namespace != authn.NamespaceUser {
return nil

View File

@ -7,6 +7,7 @@ import (
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/org"
"github.com/grafana/grafana/pkg/services/user"
@ -31,12 +32,18 @@ func (s *OrgSync) SyncOrgRolesHook(ctx context.Context, id *authn.Identity, _ *a
ctxLogger := s.log.FromContext(ctx)
namespace, userID := id.NamespacedID()
if namespace != authn.NamespaceUser || userID <= 0 {
namespace, identifier := id.GetNamespacedID()
if namespace != authn.NamespaceUser {
ctxLogger.Warn("Failed to sync org role, invalid namespace for identity", "id", id.ID, "namespace", namespace)
return nil
}
userID, err := identity.IntIdentifier(namespace, identifier)
if err != nil {
ctxLogger.Warn("Failed to sync org role, invalid ID for identity", "id", id.ID, "namespace", namespace, "err", err)
return nil
}
ctxLogger.Debug("Syncing organization roles", "id", id.ID, "extOrgRoles", id.OrgRoles)
// don't sync org roles if none is specified
if len(id.OrgRoles) == 0 {

View File

@ -6,6 +6,7 @@ import (
"fmt"
"github.com/grafana/grafana/pkg/infra/log"
authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
@ -109,13 +110,19 @@ func (s *UserSync) FetchSyncedUserHook(ctx context.Context, identity *authn.Iden
if !identity.ClientParams.FetchSyncedUser {
return nil
}
namespace, id := identity.NamespacedID()
namespace, id := identity.GetNamespacedID()
if namespace != authn.NamespaceUser {
return nil
}
userID, err := authidentity.IntIdentifier(namespace, id)
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
return nil
}
usr, err := s.userService.GetSignedInUserWithCacheCtx(ctx, &user.GetSignedInUserQuery{
UserID: id,
UserID: userID,
OrgID: r.OrgID,
})
if err != nil {
@ -135,15 +142,15 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit
return nil
}
namespace, id := identity.NamespacedID()
// do not sync invalid users
if id <= 0 {
return nil // skip sync
namespace, id := identity.GetNamespacedID()
if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount {
return nil
}
if namespace != authn.NamespaceUser && namespace != authn.NamespaceServiceAccount {
return nil // skip sync
userID, err := authidentity.IntIdentifier(namespace, id)
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
return nil
}
go func(userID int64) {
@ -158,7 +165,7 @@ func (s *UserSync) SyncLastSeenHook(ctx context.Context, identity *authn.Identit
!errors.Is(err, user.ErrLastSeenUpToDate) {
s.log.Error("Failed to update last_seen_at", "err", err, "userId", userID)
}
}(id)
}(userID)
return nil
}
@ -168,12 +175,18 @@ func (s *UserSync) EnableUserHook(ctx context.Context, identity *authn.Identity,
return nil
}
namespace, id := identity.NamespacedID()
namespace, id := identity.GetNamespacedID()
if namespace != authn.NamespaceUser {
return nil
}
return s.userService.Disable(ctx, &user.DisableUserCommand{UserID: id, IsDisabled: false})
userID, err := authidentity.IntIdentifier(namespace, id)
if err != nil {
s.log.FromContext(ctx).Warn("got invalid identity ID", "id", id, "err", err)
return nil
}
return s.userService.Disable(ctx, &user.DisableUserCommand{UserID: userID, IsDisabled: false})
}
func (s *UserSync) upsertAuthConnection(ctx context.Context, userID int64, identity *authn.Identity, createConnection bool) error {

View File

@ -10,6 +10,7 @@ import (
"github.com/grafana/grafana/pkg/components/satokengen"
"github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/apikey"
authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/org"
@ -175,8 +176,13 @@ func (s *APIKey) Hook(ctx context.Context, identity *authn.Identity, r *authn.Re
}
func (s *APIKey) getAPIKeyID(ctx context.Context, identity *authn.Identity, r *authn.Request) (apiKeyID int64, exists bool) {
namespace, id := identity.NamespacedID()
namespace, identifier := identity.GetNamespacedID()
id, err := authidentity.IntIdentifier(namespace, identifier)
if err != nil {
s.log.Warn("Failed to parse ID from identifier", "err", err)
return -1, false
}
if namespace == authn.NamespaceAPIKey {
return id, true
}

View File

@ -12,6 +12,7 @@ import (
"time"
"github.com/grafana/grafana/pkg/infra/log"
authidentity "github.com/grafana/grafana/pkg/services/auth/identity"
"github.com/grafana/grafana/pkg/services/authn"
"github.com/grafana/grafana/pkg/services/login"
"github.com/grafana/grafana/pkg/services/user"
@ -135,11 +136,16 @@ func (c *Proxy) Hook(ctx context.Context, identity *authn.Identity, r *authn.Req
return nil
}
namespace, id := identity.NamespacedID()
namespace, identifier := identity.GetNamespacedID()
if namespace != authn.NamespaceUser {
return nil
}
id, err := authidentity.IntIdentifier(namespace, identifier)
if err != nil {
c.log.Warn("Failed to cache proxy user", "error", err, "userId", identifier, "err", err)
return nil
}
c.log.FromContext(ctx).Debug("Cache proxy user", "userId", id)
bytes := []byte(strconv.FormatInt(id, 10))
if err := c.cache.Set(ctx, identity.ClientParams.CacheAuthProxyKey, bytes, time.Duration(c.cfg.AuthProxySyncTTL)*time.Minute); err != nil {

View File

@ -185,23 +185,6 @@ func (i *Identity) IsNil() bool {
return i == nil
}
// NamespacedID returns the namespace, e.g. "user" and the id for that namespace
// FIXME(kalleep): Replace with GetNamespacedID
func (i *Identity) NamespacedID() (string, int64) {
split := strings.Split(i.ID, ":")
if len(split) != 2 {
return "", -1
}
id, err := strconv.ParseInt(split[1], 10, 64)
if err != nil {
// FIXME (kalleep): Improve error handling
return "", -1
}
return split[0], id
}
// SignedInUser returns a SignedInUser from the identity.
func (i *Identity) SignedInUser() *user.SignedInUser {
namespace, id := i.GetNamespacedID()