AuthN: Clean errors in user/org sync (#61560)

* clean errors in user/org sync

* lower logging level for non 5xx errors
This commit is contained in:
Jo 2023-01-16 16:37:04 +00:00 committed by GitHub
parent 82d8b2036f
commit 6fec8fda39
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 55 additions and 27 deletions

View File

@ -1,6 +1,7 @@
package models package models
import ( import (
"net/http"
"strings" "strings"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
@ -60,16 +61,21 @@ func (ctx *ReqContext) JsonApiErr(status int, message string, err error) {
if err != nil { if err != nil {
resp["traceID"] = traceID resp["traceID"] = traceID
ctx.Logger.Error(message, "error", err, "traceID", traceID) if status == http.StatusInternalServerError {
ctx.Logger.Error(message, "error", err, "traceID", traceID)
} else {
ctx.Logger.Warn(message, "error", err, "traceID", traceID)
}
if setting.Env != setting.Prod { if setting.Env != setting.Prod {
resp["error"] = err.Error() resp["error"] = err.Error()
} }
} }
switch status { switch status {
case 404: case http.StatusNotFound:
resp["message"] = "Not Found" resp["message"] = "Not Found"
case 500: case http.StatusInternalServerError:
resp["message"] = "Internal Server Error" resp["message"] = "Internal Server Error"
} }

View File

@ -3,10 +3,8 @@ package usersync
import ( import (
"context" "context"
"errors" "errors"
"fmt"
"sort" "sort"
"github.com/grafana/grafana/pkg/cmd/grafana-cli/logger"
"github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/infra/log"
"github.com/grafana/grafana/pkg/services/accesscontrol" "github.com/grafana/grafana/pkg/services/accesscontrol"
"github.com/grafana/grafana/pkg/services/authn" "github.com/grafana/grafana/pkg/services/authn"
@ -28,26 +26,27 @@ type OrgSync struct {
func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error { func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.Request) error {
if !id.ClientParams.SyncUser { if !id.ClientParams.SyncUser {
s.log.Debug("Not syncing org user", "auth_module", id.AuthModule, "auth_id", id.AuthID)
return nil return nil
} }
namespace, userID := id.NamespacedID() namespace, userID := id.NamespacedID()
if namespace != "user" && userID <= 0 { if namespace != "user" || userID <= 0 {
return fmt.Errorf("invalid namespace %q for user ID %q", namespace, userID) s.log.Warn("invalid namespace %q for user ID %q", namespace, userID)
return nil
} }
s.log.Debug("Syncing organization roles", "id", userID, "extOrgRoles", id.OrgRoles) s.log.Debug("syncing organization roles", "id", userID, "extOrgRoles", id.OrgRoles)
// don't sync org roles if none is specified // don't sync org roles if none is specified
if len(id.OrgRoles) == 0 { if len(id.OrgRoles) == 0 {
s.log.Debug("Not syncing organization roles since external user doesn't have any") s.log.Debug("not syncing organization roles since external user doesn't have any")
return nil return nil
} }
orgsQuery := &org.GetUserOrgListQuery{UserID: userID} orgsQuery := &org.GetUserOrgListQuery{UserID: userID}
result, err := s.orgService.GetUserOrgList(ctx, orgsQuery) result, err := s.orgService.GetUserOrgList(ctx, orgsQuery)
if err != nil { if err != nil {
return err s.log.Error("failed to get user's organizations", "userId", userID, "error", err)
return nil
} }
handledOrgIds := map[int64]bool{} handledOrgIds := map[int64]bool{}
@ -64,7 +63,8 @@ func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.
// update role // update role
cmd := &org.UpdateOrgUserCommand{OrgID: orga.OrgID, UserID: userID, Role: extRole} cmd := &org.UpdateOrgUserCommand{OrgID: orga.OrgID, UserID: userID, Role: extRole}
if err := s.orgService.UpdateOrgUser(ctx, cmd); err != nil { if err := s.orgService.UpdateOrgUser(ctx, cmd); err != nil {
return err s.log.Error("failed to update active org user", "userId", userID, "error", err)
return nil
} }
} }
} }
@ -81,7 +81,8 @@ func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.
cmd := &org.AddOrgUserCommand{UserID: userID, Role: orgRole, OrgID: orgId} cmd := &org.AddOrgUserCommand{UserID: userID, Role: orgRole, OrgID: orgId}
err := s.orgService.AddOrgUser(ctx, cmd) err := s.orgService.AddOrgUser(ctx, cmd)
if err != nil && !errors.Is(err, org.ErrOrgNotFound) { if err != nil && !errors.Is(err, org.ErrOrgNotFound) {
return err s.log.Error("failed to update active org user", "userId", userID, "error", err)
return nil
} }
} }
@ -92,16 +93,17 @@ func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn.
cmd := &org.RemoveOrgUserCommand{OrgID: orgId, UserID: userID} cmd := &org.RemoveOrgUserCommand{OrgID: orgId, UserID: userID}
if err := s.orgService.RemoveOrgUser(ctx, cmd); err != nil { if err := s.orgService.RemoveOrgUser(ctx, cmd); err != nil {
if errors.Is(err, org.ErrLastOrgAdmin) { if errors.Is(err, org.ErrLastOrgAdmin) {
logger.Error(err.Error(), "userId", cmd.UserID, "orgId", cmd.OrgID) s.log.Error(err.Error(), "userId", cmd.UserID, "orgId", cmd.OrgID)
continue continue
} }
return err s.log.Error("failed to delete user org membership", "userId", userID, "error", err)
return nil
} }
if err := s.accessControl.DeleteUserPermissions(ctx, orgId, cmd.UserID); err != nil { if err := s.accessControl.DeleteUserPermissions(ctx, orgId, cmd.UserID); err != nil {
logger.Error("failed to delete permissions for user", "error", err, "userID", cmd.UserID, "orgID", orgId) s.log.Error("failed to delete permissions for user", "error", err, "userID", cmd.UserID, "orgID", orgId)
return err return nil
} }
} }

View File

@ -16,8 +16,12 @@ import (
) )
var ( var (
errSyncUserForbidden = errutil.NewBase(errutil.StatusForbidden,
"user.sync.forbidden", errutil.WithPublicMessage("User sync forbidden"))
errSyncUserInternal = errutil.NewBase(errutil.StatusInternal,
"user.sync.forbidden", errutil.WithPublicMessage("User sync failed"))
errUserProtection = errutil.NewBase(errutil.StatusForbidden, errUserProtection = errutil.NewBase(errutil.StatusForbidden,
"user.sync.protected role", errutil.WithPublicMessage("Unable to sync due to protected role")) "user.sync.protectedrole", errutil.WithPublicMessage("Unable to sync due to protected role"))
) )
func ProvideUserSync(userService user.Service, func ProvideUserSync(userService user.Service,
@ -49,14 +53,18 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re
// Does user exist in the database? // Does user exist in the database?
usr, errUserInDB := s.UserInDB(ctx, &id.AuthModule, &id.AuthID, id.ClientParams.LookUpParams) usr, errUserInDB := s.UserInDB(ctx, &id.AuthModule, &id.AuthID, id.ClientParams.LookUpParams)
if errUserInDB != nil && !errors.Is(errUserInDB, user.ErrUserNotFound) { if errUserInDB != nil && !errors.Is(errUserInDB, user.ErrUserNotFound) {
return errUserInDB s.log.Error("error retrieving user", "error", errUserInDB,
"auth_module", id.AuthModule, "auth_id", id.AuthID,
"lookup_params", id.ClientParams.LookUpParams,
)
return errSyncUserInternal.Errorf("unable to retrieve user")
} }
if errors.Is(errUserInDB, user.ErrUserNotFound) { if errors.Is(errUserInDB, user.ErrUserNotFound) {
if !id.ClientParams.AllowSignUp { if !id.ClientParams.AllowSignUp {
s.log.Warn("Not allowing login, user not found in internal user database and allow signup = false", s.log.Warn("not allowing login, user not found in internal user database and allow signup = false",
"auth_module", id.AuthModule) "auth_module", id.AuthModule)
return login.ErrSignupNotAllowed return errSyncUserForbidden.Errorf("%w", login.ErrSignupNotAllowed)
} }
// quota check (FIXME: (jguer) this should be done in the user service) // quota check (FIXME: (jguer) this should be done in the user service)
@ -65,11 +73,11 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re
for _, srv := range []string{user.QuotaTargetSrv, org.QuotaTargetSrv} { for _, srv := range []string{user.QuotaTargetSrv, org.QuotaTargetSrv} {
limitReached, errLimit := s.quotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil) limitReached, errLimit := s.quotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil)
if errLimit != nil { if errLimit != nil {
s.log.Warn("error getting user quota.", "error", errLimit) s.log.Error("error getting user quota", "error", errLimit)
return login.ErrGettingUserQuota return errSyncUserInternal.Errorf("%w", login.ErrGettingUserQuota)
} }
if limitReached { if limitReached {
return login.ErrUsersQuotaReached return errSyncUserForbidden.Errorf("%w", login.ErrUsersQuotaReached)
} }
} }
@ -77,7 +85,11 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re
var errCreate error var errCreate error
usr, errCreate = s.createUser(ctx, id) usr, errCreate = s.createUser(ctx, id)
if errCreate != nil { if errCreate != nil {
return errCreate s.log.Error("error creating user", "error", errCreate,
"auth_module", id.AuthModule, "auth_id", id.AuthID,
"id_login", id.Login, "id_email", id.Email,
)
return errSyncUserInternal.Errorf("unable to create user")
} }
} }
@ -87,14 +99,22 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re
// update user // update user
if errUpdate := s.updateUserAttributes(ctx, usr, id); errUpdate != nil { if errUpdate := s.updateUserAttributes(ctx, usr, id); errUpdate != nil {
return errUpdate s.log.Error("error creating user", "error", errUpdate,
"auth_module", id.AuthModule, "auth_id", id.AuthID,
"login", usr.Login, "email", usr.Email,
"id_login", id.Login, "id_email", id.Email,
)
return errSyncUserInternal.Errorf("unable to update user")
} }
syncUserToIdentity(usr, id) syncUserToIdentity(usr, id)
// persist latest auth info token // persist latest auth info token
if errAuthInfo := s.updateAuthInfo(ctx, id); errAuthInfo != nil { if errAuthInfo := s.updateAuthInfo(ctx, id); errAuthInfo != nil {
return errAuthInfo s.log.Error("error creating user", "error", errAuthInfo,
"auth_module", id.AuthModule, "auth_id", id.AuthID,
)
return errSyncUserInternal.Errorf("unable to update auth info")
} }
return nil return nil