diff --git a/pkg/models/context.go b/pkg/models/context.go index 432dee40e99..eaf1bbf2871 100644 --- a/pkg/models/context.go +++ b/pkg/models/context.go @@ -1,6 +1,7 @@ package models import ( + "net/http" "strings" "github.com/grafana/grafana/pkg/infra/log" @@ -60,16 +61,21 @@ func (ctx *ReqContext) JsonApiErr(status int, message string, err error) { if err != nil { 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 { resp["error"] = err.Error() } } switch status { - case 404: + case http.StatusNotFound: resp["message"] = "Not Found" - case 500: + case http.StatusInternalServerError: resp["message"] = "Internal Server Error" } diff --git a/pkg/services/authn/authnimpl/usersync/orgsync.go b/pkg/services/authn/authnimpl/usersync/orgsync.go index e51745f29e8..2c24e0438e0 100644 --- a/pkg/services/authn/authnimpl/usersync/orgsync.go +++ b/pkg/services/authn/authnimpl/usersync/orgsync.go @@ -3,10 +3,8 @@ package usersync import ( "context" "errors" - "fmt" "sort" - "github.com/grafana/grafana/pkg/cmd/grafana-cli/logger" "github.com/grafana/grafana/pkg/infra/log" "github.com/grafana/grafana/pkg/services/accesscontrol" "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 { if !id.ClientParams.SyncUser { - s.log.Debug("Not syncing org user", "auth_module", id.AuthModule, "auth_id", id.AuthID) return nil } namespace, userID := id.NamespacedID() - if namespace != "user" && userID <= 0 { - return fmt.Errorf("invalid namespace %q for user ID %q", namespace, userID) + if namespace != "user" || userID <= 0 { + 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 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 } orgsQuery := &org.GetUserOrgListQuery{UserID: userID} result, err := s.orgService.GetUserOrgList(ctx, orgsQuery) if err != nil { - return err + s.log.Error("failed to get user's organizations", "userId", userID, "error", err) + return nil } handledOrgIds := map[int64]bool{} @@ -64,7 +63,8 @@ func (s *OrgSync) SyncOrgUser(ctx context.Context, id *authn.Identity, _ *authn. // update role cmd := &org.UpdateOrgUserCommand{OrgID: orga.OrgID, UserID: userID, Role: extRole} 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} err := s.orgService.AddOrgUser(ctx, cmd) 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} if err := s.orgService.RemoveOrgUser(ctx, cmd); err != nil { 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 } - 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 { - logger.Error("failed to delete permissions for user", "error", err, "userID", cmd.UserID, "orgID", orgId) - return err + s.log.Error("failed to delete permissions for user", "error", err, "userID", cmd.UserID, "orgID", orgId) + return nil } } diff --git a/pkg/services/authn/authnimpl/usersync/usersync.go b/pkg/services/authn/authnimpl/usersync/usersync.go index 0736cc2891a..a292f6c2f2d 100644 --- a/pkg/services/authn/authnimpl/usersync/usersync.go +++ b/pkg/services/authn/authnimpl/usersync/usersync.go @@ -16,8 +16,12 @@ import ( ) 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, - "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, @@ -49,14 +53,18 @@ func (s *UserSync) SyncUser(ctx context.Context, id *authn.Identity, _ *authn.Re // Does user exist in the database? usr, errUserInDB := s.UserInDB(ctx, &id.AuthModule, &id.AuthID, id.ClientParams.LookUpParams) 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 !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) - return login.ErrSignupNotAllowed + return errSyncUserForbidden.Errorf("%w", login.ErrSignupNotAllowed) } // 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} { limitReached, errLimit := s.quotaService.CheckQuotaReached(ctx, quota.TargetSrv(srv), nil) if errLimit != nil { - s.log.Warn("error getting user quota.", "error", errLimit) - return login.ErrGettingUserQuota + s.log.Error("error getting user quota", "error", errLimit) + return errSyncUserInternal.Errorf("%w", login.ErrGettingUserQuota) } 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 usr, errCreate = s.createUser(ctx, id) 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 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) // persist latest auth info token 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