From aa55cf03d475324a5f18fe922165e7ce8b45d875 Mon Sep 17 00:00:00 2001 From: Anass Bouassaba Date: Mon, 11 Nov 2024 21:45:59 +0100 Subject: [PATCH] fix(misc): consistent error reporting for IdP and API (#373) --- api/errorpkg/error_creators.go | 87 +++++++------- api/service/file_service.go | 2 +- api/service/group_service.go | 2 +- api/service/organization_service.go | 2 +- idp/src/account/service.ts | 11 +- idp/src/app.ts | 9 +- idp/src/infra/base64.ts | 10 +- idp/src/infra/{error.ts => error/core.ts} | 51 ++------ idp/src/infra/error/creators.ts | 138 ++++++++++++++++++++++ idp/src/infra/error/index.ts | 3 + idp/src/infra/error/validation.ts | 22 ++++ idp/src/token/service.ts | 58 ++++----- idp/src/user/model.ts | 2 +- idp/src/user/repo.ts | 95 +++------------ idp/src/user/router.ts | 30 ++--- idp/src/user/service.ts | 70 ++++++----- 16 files changed, 334 insertions(+), 258 deletions(-) rename idp/src/infra/{error.ts => error/core.ts} (66%) create mode 100644 idp/src/infra/error/creators.ts create mode 100644 idp/src/infra/error/index.ts create mode 100644 idp/src/infra/error/validation.ts diff --git a/api/errorpkg/error_creators.go b/api/errorpkg/error_creators.go index 304e6c00e..4289d46cb 100644 --- a/api/errorpkg/error_creators.go +++ b/api/errorpkg/error_creators.go @@ -85,7 +85,7 @@ func NewSnapshotNotFoundError(err error) *ErrorResponse { "snapshot_not_found", http.StatusNotFound, "Snapshot not found.", - "The file has no snapshots.", + "Snapshot not found.", err, ) } @@ -95,7 +95,7 @@ func NewS3ObjectNotFoundError(err error) *ErrorResponse { "s3_object_not_found", http.StatusNotFound, "S3 object not found.", - "The snapshot does not contain the S3 object requested.", + "S3 object not found.", err, ) } @@ -205,30 +205,30 @@ func NewOrganizationPermissionError(userID string, org model.Organization, permi "missing_organization_permission", http.StatusForbidden, fmt.Sprintf( - "User '%s' is missing the permission '%s' for organization '%s' (%s).", - userID, permission, org.GetName(), org.GetID(), + "User '%s' is missing permission '%s' for organization '%s'.", + userID, permission, org.GetID(), ), fmt.Sprintf("Sorry, you don't have enough permissions for organization '%s'.", org.GetName()), nil, ) } -func NewCannotRemoveLastRemainingOwnerOfOrganizationError(org model.Organization) *ErrorResponse { +func NewCannotRemoveSoleOwnerOfOrganizationError(org model.Organization) *ErrorResponse { return NewErrorResponse( - "cannot_remove_last_owner_of_organization", + "cannot_remove_sole_owner_of_organization", http.StatusBadRequest, - fmt.Sprintf("Cannot remove the last remaining owner of organization '%s'.", org.GetID()), - fmt.Sprintf("Cannot remove the last remaining owner of organization '%s'.", org.GetName()), + fmt.Sprintf("Cannot remove sole owner of organization '%s'.", org.GetID()), + fmt.Sprintf("Cannot remove sole owner of organization '%s'.", org.GetName()), nil, ) } -func NewCannotRemoveLastRemainingOwnerOfGroupError(group model.Group) *ErrorResponse { +func NewCannotRemoveSoleOwnerOfGroupError(group model.Group) *ErrorResponse { return NewErrorResponse( - "cannot_remove_last_owner_of_group", + "cannot_remove_sole_owner_of_group", http.StatusBadRequest, - fmt.Sprintf("Cannot remove the last remaining owner of group '%s'.", group.GetID()), - fmt.Sprintf("Cannot remove the last remaining owner of group '%s'.", group.GetName()), + fmt.Sprintf("Cannot remove sole owner of group '%s'.", group.GetID()), + fmt.Sprintf("Cannot remove sole owner of group '%s'.", group.GetName()), nil, ) } @@ -238,10 +238,10 @@ func NewGroupPermissionError(userID string, org model.Organization, permission s "missing_group_permission", http.StatusForbidden, fmt.Sprintf( - "User '%s' is missing the permission '%s' for the group '%s' (%s).", - userID, permission, org.GetName(), org.GetID(), + "User '%s' is missing permission '%s' for group '%s'.", + userID, permission, org.GetID(), ), - fmt.Sprintf("Sorry, you don't have enough permissions for the group '%s'.", org.GetName()), + fmt.Sprintf("Sorry, you don't have enough permissions for group '%s'.", org.GetName()), nil, ) } @@ -251,10 +251,10 @@ func NewWorkspacePermissionError(userID string, workspace model.Workspace, permi "missing_workspace_permission", http.StatusForbidden, fmt.Sprintf( - "User '%s' is missing the permission '%s' for the workspace '%s' (%s).", - userID, permission, workspace.GetName(), workspace.GetID(), + "User '%s' is missing permission '%s' for workspace '%s'.", + userID, permission, workspace.GetID(), ), - fmt.Sprintf("Sorry, you don't have enough permissions for the workspace '%s'.", workspace.GetName()), + fmt.Sprintf("Sorry, you don't have enough permissions for workspace '%s'.", workspace.GetName()), nil, ) } @@ -264,10 +264,10 @@ func NewFilePermissionError(userID string, file model.File, permission string) * "missing_file_permission", http.StatusForbidden, fmt.Sprintf( - "User '%s' is missing the permission '%s' for the file '%s' (%s).", - userID, permission, file.GetName(), file.GetID(), + "User '%s' is missing permission '%s' for file '%s'.", + userID, permission, file.GetID(), ), - fmt.Sprintf("Sorry, you don't have enough permissions for the item '%s'.", file.GetName()), + fmt.Sprintf("Sorry, you don't have enough permissions for item '%s'.", file.GetName()), nil, ) } @@ -307,7 +307,7 @@ func NewStorageLimitExceededError() *ErrorResponse { "storage_limit_exceeded", http.StatusForbidden, "Storage limit exceeded.", - "The storage limit of your workspace has been reached, please increase it and try again.", + "Storage limit of your workspace has been reached, please increase it and try again.", nil, ) } @@ -317,7 +317,8 @@ func NewInsufficientStorageCapacityError() *ErrorResponse { "insufficient_storage_capacity", http.StatusForbidden, "Insufficient storage capacity.", - "The requested storage capacity is insufficient.", + "Insufficient storage capacity.", + nil, ) } @@ -330,7 +331,7 @@ func NewRequestBodyValidationError(err error) *ErrorResponse { return NewErrorResponse( "request_validation_error", http.StatusBadRequest, - fmt.Sprintf("Failed validation for the following fields: %s.", strings.Join(fields, ",")), + fmt.Sprintf("Failed validation for fields: %s.", strings.Join(fields, ",")), MsgInvalidRequest, err, ) @@ -340,7 +341,7 @@ func NewFileAlreadyChildOfDestinationError(source model.File, target model.File) return NewErrorResponse( "file_already_child_of_destination", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) is already a child of '%s' (%s).", source.GetName(), source.GetID(), target.GetName(), target.GetID()), + fmt.Sprintf("File '%s' is already a child of '%s'.", source.GetID(), target.GetID()), fmt.Sprintf("Item '%s' is already within '%s'.", source.GetName(), target.GetName()), nil, ) @@ -350,7 +351,7 @@ func NewFileCannotBeMovedIntoItselfError(source model.File) *ErrorResponse { return NewErrorResponse( "file_cannot_be_moved_into_itself", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) cannot be moved into itself.", source.GetName(), source.GetID()), + fmt.Sprintf("File '%s' cannot be moved into itself.", source.GetID()), fmt.Sprintf("Item '%s' cannot be moved into itself.", source.GetName()), nil, ) @@ -360,7 +361,7 @@ func NewFileIsNotAFolderError(file model.File) *ErrorResponse { return NewErrorResponse( "file_is_not_a_folder", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) is not a folder.", file.GetName(), file.GetID()), + fmt.Sprintf("File '%s' is not a folder.", file.GetID()), fmt.Sprintf("Item '%s' is not a folder.", file.GetName()), nil, ) @@ -370,7 +371,7 @@ func NewFileIsNotAFileError(file model.File) *ErrorResponse { return NewErrorResponse( "file_is_not_a_file", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) is not a file.", file.GetName(), file.GetID()), + fmt.Sprintf("File '%s' is not a file.", file.GetID()), fmt.Sprintf("Item '%s' is not a file.", file.GetName()), nil, ) @@ -380,7 +381,7 @@ func NewTargetIsGrandChildOfSourceError(file model.File) *ErrorResponse { return NewErrorResponse( "target_is_grant_child_of_source", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) cannot be moved in another file within its own tree.", file.GetName(), file.GetID()), + fmt.Sprintf("File '%s' cannot be moved in another file within its own tree.", file.GetID()), fmt.Sprintf("Item '%s' cannot be moved in another item within its own tree.", file.GetName()), nil, ) @@ -390,8 +391,8 @@ func NewCannotDeleteWorkspaceRootError(file model.File, workspace model.Workspac return NewErrorResponse( "cannot_delete_workspace_root", http.StatusForbidden, - fmt.Sprintf("Cannot delete the root file (%s) of the workspace '%s' (%s).", file.GetID(), workspace.GetName(), workspace.GetID()), - fmt.Sprintf("Cannot delete the root item of the workspace '%s'.", workspace.GetName()), + fmt.Sprintf("Cannot delete root file '%s' of workspace '%s'.", file.GetID(), workspace.GetID()), + fmt.Sprintf("Cannot delete root item of workspace '%s'.", workspace.GetName()), nil, ) } @@ -400,17 +401,17 @@ func NewFileCannotBeCopiedIntoOwnSubtreeError(file model.File) *ErrorResponse { return NewErrorResponse( "file_cannot_be_coped_into_own_subtree", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) cannot be copied in another file within its own subtree.", file.GetName(), file.GetID()), + fmt.Sprintf("File '%s' cannot be copied in another file within its own subtree.", file.GetID()), fmt.Sprintf("Item '%s' cannot be copied in another item within its own subtree.", file.GetName()), nil, ) } -func NewFileCannotBeCopiedIntoIselfError(file model.File) *ErrorResponse { +func NewFileCannotBeCopiedIntoItselfError(file model.File) *ErrorResponse { return NewErrorResponse( "file_cannot_be_copied_into_itself", http.StatusForbidden, - fmt.Sprintf("File '%s' (%s) cannot be copied into itself.", file.GetName(), file.GetID()), + fmt.Sprintf("File '%s' cannot be copied into itself.", file.GetID()), fmt.Sprintf("Item '%s' cannot be copied into itself.", file.GetName()), nil, ) @@ -430,7 +431,7 @@ func NewCannotAcceptNonPendingInvitationError(invitation model.Invitation) *Erro return NewErrorResponse( "cannot_accept_non_pending_invitation", http.StatusForbidden, - fmt.Sprintf("Cannot accept an invitation which is not pending, the status of the invitation (%s) is (%s).", invitation.GetID(), invitation.GetStatus()), + fmt.Sprintf("Cannot accept an invitation which is not pending, status of invitation '%s' is '%s'.", invitation.GetID(), invitation.GetStatus()), "Cannot accept an invitation which is not pending.", nil, ) @@ -440,7 +441,7 @@ func NewCannotDeclineNonPendingInvitationError(invitation model.Invitation) *Err return NewErrorResponse( "cannot_decline_non_pending_invitation", http.StatusForbidden, - fmt.Sprintf("Cannot decline an invitation which is not pending, the status of the invitation (%s) is (%s).", invitation.GetID(), invitation.GetStatus()), + fmt.Sprintf("Cannot decline an invitation which is not pending, status of invitation '%s' is '%s'.", invitation.GetID(), invitation.GetStatus()), "Cannot decline an invitation which is not pending.", nil, ) @@ -450,7 +451,7 @@ func NewCannotResendNonPendingInvitationError(invitation model.Invitation) *Erro return NewErrorResponse( "cannot_resend_non_pending_invitation", http.StatusForbidden, - fmt.Sprintf("Cannot resend an invitation which is not pending, the status of the invitation (%s) is (%s).", invitation.GetID(), invitation.GetStatus()), + fmt.Sprintf("Cannot resend an invitation which is not pending, status of invitation '%s' is '%s'.", invitation.GetID(), invitation.GetStatus()), "Cannot resend an invitation which is not pending.", nil, ) @@ -460,7 +461,7 @@ func NewUserNotAllowedToAcceptInvitationError(user model.User, invitation model. return NewErrorResponse( "user_not_allowed_to_accept_invitation", http.StatusForbidden, - fmt.Sprintf("User '%s' (%s) is not allowed to accept the invitation (%s).", user.GetUsername(), user.GetID(), invitation.GetID()), + fmt.Sprintf("User '%s' is not allowed to accept invitation '%s'.", user.GetID(), invitation.GetID()), "Not allowed to accept this invitation.", nil, ) @@ -470,7 +471,7 @@ func NewUserNotAllowedToDeclineInvitationError(user model.User, invitation model return NewErrorResponse( "user_not_allowed_to_decline_invitation", http.StatusForbidden, - fmt.Sprintf("User '%s' (%s) is not allowed to decline the invitation (%s).", user.GetUsername(), user.GetID(), invitation.GetID()), + fmt.Sprintf("User '%s' is not allowed to decline invitation '%s'.", user.GetID(), invitation.GetID()), "Not allowed to decline this invitation.", nil, ) @@ -480,7 +481,7 @@ func NewUserNotAllowedToDeleteInvitationError(user model.User, invitation model. return NewErrorResponse( "user_not_allowed_to_delete_invitation", http.StatusForbidden, - fmt.Sprintf("User '%s' (%s) not allowed to delete the invitation (%s).", user.GetUsername(), user.GetID(), invitation.GetID()), + fmt.Sprintf("User '%s' is not allowed to delete invitation '%s'.", user.GetID(), invitation.GetID()), "Not allowed to delete this invitation.", nil, ) @@ -490,8 +491,8 @@ func NewUserAlreadyMemberOfOrganizationError(user model.User, org model.Organiza return NewErrorResponse( "user_already_member_of_organization", http.StatusForbidden, - fmt.Sprintf("User '%s' (%s) is already a member of the organization '%s' (%s).", user.GetUsername(), user.GetID(), org.GetName(), org.GetID()), - fmt.Sprintf("You are already a member of the organization '%s'.", org.GetName()), + fmt.Sprintf("User '%s' is already a member of organization '%s'.", user.GetID(), org.GetID()), + fmt.Sprintf("You are already a member of organization '%s'.", org.GetName()), nil, ) } @@ -501,7 +502,7 @@ func NewInvalidAPIKeyError() *ErrorResponse { "invalid_api_key", http.StatusUnauthorized, "Invalid API key.", - "The API key is either missing or invalid.", + "API key is either missing or invalid.", nil, ) } diff --git a/api/service/file_service.go b/api/service/file_service.go index 78f395adb..9d82af27d 100644 --- a/api/service/file_service.go +++ b/api/service/file_service.go @@ -821,7 +821,7 @@ func (svc *FileService) CopyOne(sourceID string, targetID string, userID string) return nil, err } if source.GetID() == target.GetID() { - return nil, errorpkg.NewFileCannotBeCopiedIntoIselfError(source) + return nil, errorpkg.NewFileCannotBeCopiedIntoItselfError(source) } if target.GetType() != model.FileTypeFolder { return nil, errorpkg.NewFileIsNotAFolderError(target) diff --git a/api/service/group_service.go b/api/service/group_service.go index 24fa7eded..0ebb50918 100644 --- a/api/service/group_service.go +++ b/api/service/group_service.go @@ -335,7 +335,7 @@ func (svc *GroupService) RemoveMember(id string, memberID string, userID string) return err } if svc.groupGuard.IsAuthorized(memberID, group, model.PermissionOwner) && ownerCount == 1 { - return errorpkg.NewCannotRemoveLastRemainingOwnerOfGroupError(group) + return errorpkg.NewCannotRemoveSoleOwnerOfGroupError(group) } if err := svc.groupRepo.RevokeUserPermission(id, memberID); err != nil { diff --git a/api/service/organization_service.go b/api/service/organization_service.go index bb0f39d0d..e5b12578f 100644 --- a/api/service/organization_service.go +++ b/api/service/organization_service.go @@ -269,7 +269,7 @@ func (svc *OrganizationService) RemoveMember(id string, memberID string, userID return err } if svc.orgGuard.IsAuthorized(memberID, org, model.PermissionOwner) && ownerCount == 1 { - return errorpkg.NewCannotRemoveLastRemainingOwnerOfOrganizationError(org) + return errorpkg.NewCannotRemoveSoleOwnerOfOrganizationError(org) } /* Revoke permissions from all groups belonging to this organization. */ diff --git a/idp/src/account/service.ts b/idp/src/account/service.ts index cb159d1ad..b70f5c436 100644 --- a/idp/src/account/service.ts +++ b/idp/src/account/service.ts @@ -9,7 +9,10 @@ // licenses/AGPL.txt. import { getConfig } from '@/config/config' import { newDateTime } from '@/infra/date-time' -import { ErrorCode, newError } from '@/infra/error' +import { + newInternalServerError, + newUsernameUnavailableError, +} from '@/infra/error' import { newHashId, newHyphenlessUuid } from '@/infra/id' import { sendTemplateMail } from '@/infra/mail' import { hashPassword } from '@/infra/password' @@ -52,7 +55,7 @@ export async function createUser( ): Promise { const id = newHashId() if (!(await userRepo.isUsernameAvailable(options.email))) { - throw newError({ code: ErrorCode.UsernameUnavailable }) + throw newUsernameUnavailableError() } if ((await getUserCount()) === 0) { options.isAdmin = true @@ -89,7 +92,7 @@ export async function createUser( } catch (error) { await userRepo.delete(id) await search.index(USER_SEARCH_INDEX).deleteDocuments([id]) - throw newError({ code: ErrorCode.InternalServerError, error }) + throw newInternalServerError(error) } } @@ -143,7 +146,7 @@ export async function sendResetPasswordEmail( } catch (error) { const { id } = await userRepo.findByEmail(options.email) await userRepo.update({ id, resetPasswordToken: null }) - throw newError({ code: ErrorCode.InternalServerError, error }) + throw newInternalServerError(error) } } diff --git a/idp/src/app.ts b/idp/src/app.ts index 13e191a02..1fb2db3fa 100644 --- a/idp/src/app.ts +++ b/idp/src/app.ts @@ -16,7 +16,12 @@ import { ExtractJwt, Strategy as JwtStrategy } from 'passport-jwt' import accountRouter from '@/account/router' import { getConfig } from '@/config/config' import healthRouter from '@/health/router' -import { ErrorCode, errorHandler, newError, newResponse } from '@/infra/error' +import { + ErrorCode, + errorHandler, + newError, + newResponse, +} from '@/infra/error/core' import tokenRouter from '@/token/router' import userRepo from '@/user/repo' import userRouter from '@/user/router' @@ -41,7 +46,7 @@ passport.use( }, async (payload, done) => { try { - const user = await userRepo.findByID(payload.sub) + const user = await userRepo.findById(payload.sub) return done(null, user) } catch { return done( diff --git a/idp/src/infra/base64.ts b/idp/src/infra/base64.ts index 5d442fb6a..3535abe76 100644 --- a/idp/src/infra/base64.ts +++ b/idp/src/infra/base64.ts @@ -1,4 +1,4 @@ -export function base64ToBuffer(value: string): Buffer { +export function base64ToBuffer(value: string): Buffer | null { let withoutPrefix: string if (value.includes(',')) { withoutPrefix = value.split(',')[1] @@ -7,12 +7,12 @@ export function base64ToBuffer(value: string): Buffer { } try { return Buffer.from(withoutPrefix, 'base64') - } catch (err) { - throw new Error(err as string) + } catch { + return null } } -export function base64ToMIME(value: string): string { +export function base64ToMIME(value: string): string | null { if (!value.startsWith('data:image/')) { return '' } @@ -24,7 +24,7 @@ export function base64ToMIME(value: string): string { return value.substring(colonIndex + 1, semicolonIndex) } -export function base64ToExtension(value: string): string { +export function base64ToExtension(value: string): string | null { const mime = base64ToMIME(value) switch (mime) { case 'image/jpeg': diff --git a/idp/src/infra/error.ts b/idp/src/infra/error/core.ts similarity index 66% rename from idp/src/infra/error.ts rename to idp/src/infra/error/core.ts index 42640d387..b2645de85 100644 --- a/idp/src/infra/error.ts +++ b/idp/src/infra/error/core.ts @@ -21,12 +21,12 @@ export enum ErrorCode { EmailNotConfirmed = 'email_not_confirmed', RefreshTokenExpired = 'refresh_token_expired', InvalidRequest = 'invalid_request', - UnsupportedGrantType = 'unsupported_grant_type', + InvalidGrantType = 'invalid_grant_type', PasswordValidationFailed = 'password_validation_failed', UserSuspended = 'user_suspended', - MissingPermission = 'missing_permission', + UserIsNotAdmin = 'user_is_not_admin', UserNotFound = 'user_not_found', - OrphanError = 'orphan_error', + CannotSuspendLastAdmin = 'cannot_suspend_last_admin', SearchError = 'search_error', } @@ -42,28 +42,15 @@ const statuses: { [key: string]: number } = { [ErrorCode.EmailNotConfirmed]: 401, [ErrorCode.RefreshTokenExpired]: 401, [ErrorCode.InvalidRequest]: 400, - [ErrorCode.UnsupportedGrantType]: 400, + [ErrorCode.InvalidGrantType]: 400, [ErrorCode.PasswordValidationFailed]: 400, [ErrorCode.UserSuspended]: 403, - [ErrorCode.MissingPermission]: 403, + [ErrorCode.UserIsNotAdmin]: 403, [ErrorCode.UserNotFound]: 404, - [ErrorCode.OrphanError]: 400, + [ErrorCode.CannotSuspendLastAdmin]: 400, [ErrorCode.SearchError]: 500, } -const userMessages: { [key: string]: string } = { - [ErrorCode.UsernameUnavailable]: 'Email belongs to an existing user.', - [ErrorCode.EmailNotConfirmed]: 'Email not confirmed.', - [ErrorCode.InvalidPassword]: 'Invalid password.', - [ErrorCode.InvalidUsernameOrPassword]: 'Invalid username or password.', - [ErrorCode.InvalidCredentials]: 'Invalid credentials.', - [ErrorCode.UserSuspended]: 'User suspended.', - [ErrorCode.MissingPermission]: 'You are not an console', - [ErrorCode.OrphanError]: 'You cannot suspend last console', - [ErrorCode.SearchError]: - 'Search engine encountered error or is not available', -} - export type ErrorData = { code: string status: number @@ -91,10 +78,7 @@ export type ErrorOptions = { } export function newError(options: ErrorOptions): ErrorData { - const userMessage = - options.userMessage || - userMessages[options.code] || - 'Oops! something went wrong.' + const userMessage = options.userMessage || 'Oops! something went wrong.' return { code: options.code, status: statuses[options.code], @@ -137,24 +121,3 @@ export function errorHandler( next(error) return } - -/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ -export function parseValidationError(result: any): ErrorData { - let message: string - let userMessage: string - if (result.errors) { - message = result.errors - /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ - .map((e: any) => `${e.msg} for ${e.type} '${e.path}' in ${e.location}.`) - .join(' ') - userMessage = result.errors - /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ - .map((e: any) => `${e.msg} for ${e.type} '${e.path}'.`) - .join(' ') - } - return newError({ - code: ErrorCode.RequestValidationError, - message, - userMessage, - }) -} diff --git a/idp/src/infra/error/creators.ts b/idp/src/infra/error/creators.ts new file mode 100644 index 000000000..5c8cef132 --- /dev/null +++ b/idp/src/infra/error/creators.ts @@ -0,0 +1,138 @@ +import { ErrorCode, newError } from './core' + +export function newInternalServerError(error?: unknown) { + return newError({ + code: ErrorCode.InternalServerError, + error, + message: 'Internal server error.', + userMessage: 'Internal server error.', + }) +} + +export function newUserNotFoundError() { + return newError({ + code: ErrorCode.ResourceNotFound, + message: 'User not found.', + userMessage: 'User not found.', + }) +} + +export function newPictureNotFoundError() { + return newError({ + code: ErrorCode.ResourceNotFound, + message: 'Picture not found.', + userMessage: 'Picture not found.', + }) +} + +export function newInvalidJwtError() { + return newError({ + code: ErrorCode.InvalidJwt, + message: 'Invalid JWT.', + userMessage: 'Invalid JWT.', + }) +} + +export function newUsernameUnavailableError() { + return newError({ + code: ErrorCode.UsernameUnavailable, + message: `Username is not available.`, + userMessage: `Username is not available.`, + }) +} + +export function newInvalidUsernameOrPasswordError() { + return newError({ + code: ErrorCode.InvalidUsernameOrPassword, + message: 'Invalid username or password.', + userMessage: 'Invalid username or password.', + }) +} + +export function newEmailNotConfirmedError() { + return newError({ + code: ErrorCode.EmailNotConfirmed, + message: 'Email not confirmed.', + userMessage: 'Email not confirmed.', + }) +} + +export function newUserSuspendedError() { + return newError({ + code: ErrorCode.UserSuspended, + message: 'User suspended.', + userMessage: 'User suspended.', + }) +} + +export function newRefreshTokenExpiredError() { + return newError({ + code: ErrorCode.RefreshTokenExpired, + message: 'Refresh token expired.', + userMessage: 'Refresh token expired.', + }) +} + +export function newUserIsNotAdminError() { + return newError({ + code: ErrorCode.UserIsNotAdmin, + message: 'User is not admin.', + userMessage: 'User is not admin.', + }) +} + +export function newMissingQueryParamError(param: string) { + return newError({ + code: ErrorCode.InvalidRequest, + message: `Missing query param: ${param}.`, + userMessage: `Missing query param: ${param}.`, + }) +} + +export function newMissingFormParamError(param: string) { + return newError({ + code: ErrorCode.InvalidRequest, + message: `Missing form parameter: ${param}.`, + userMessage: `Missing form parameter: ${param}.`, + }) +} + +export function newInvalidGrantType(grantType: string) { + return newError({ + code: ErrorCode.InvalidGrantType, + message: `Invalid Grant type: ${grantType}.`, + userMessage: `Invalid Grant type: ${grantType}.`, + }) +} + +export function newPasswordValidationFailedError() { + return newError({ + code: ErrorCode.PasswordValidationFailed, + message: 'Password validation failed.', + userMessage: 'Password validation failed.', + }) +} + +export function newInvalidPasswordError() { + return newError({ + code: ErrorCode.InvalidPassword, + message: 'Invalid password.', + userMessage: 'Invalid password.', + }) +} + +export function newCannotSuspendSoleAdminError() { + return newError({ + code: ErrorCode.CannotSuspendLastAdmin, + message: 'Cannot suspend sole admin.', + userMessage: 'Cannot suspend sole admin.', + }) +} + +export function newCannotDemoteSoleAdminError() { + return newError({ + code: ErrorCode.CannotSuspendLastAdmin, + message: 'Cannot demote sole admin.', + userMessage: 'Cannot demote sole admin.', + }) +} diff --git a/idp/src/infra/error/index.ts b/idp/src/infra/error/index.ts new file mode 100644 index 000000000..48d4977b8 --- /dev/null +++ b/idp/src/infra/error/index.ts @@ -0,0 +1,3 @@ +export * from './core' +export * from './creators' +export * from './validation' diff --git a/idp/src/infra/error/validation.ts b/idp/src/infra/error/validation.ts new file mode 100644 index 000000000..4bd4c7c0d --- /dev/null +++ b/idp/src/infra/error/validation.ts @@ -0,0 +1,22 @@ +import { ErrorCode, ErrorData, newError } from './core' + +/* eslint-disable-next-line @typescript-eslint/no-explicit-any */ +export function parseValidationError(result: any): ErrorData { + let message: string + let userMessage: string + if (result.errors) { + message = result.errors + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + .map((e: any) => `${e.msg} for ${e.type} '${e.path}' in ${e.location}.`) + .join(' ') + userMessage = result.errors + /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ + .map((e: any) => `${e.msg} for ${e.type} '${e.path}'.`) + .join(' ') + } + return newError({ + code: ErrorCode.RequestValidationError, + message, + userMessage, + }) +} diff --git a/idp/src/token/service.ts b/idp/src/token/service.ts index 0e8bd6c9b..718fa8736 100644 --- a/idp/src/token/service.ts +++ b/idp/src/token/service.ts @@ -9,7 +9,15 @@ // licenses/AGPL.txt. import { decodeJwt, SignJWT } from 'jose' import { getConfig } from '@/config/config' -import { ErrorCode, newError } from '@/infra/error' +import { + newEmailNotConfirmedError, + newInvalidGrantType, + newInvalidUsernameOrPasswordError, + newMissingFormParamError, + newRefreshTokenExpiredError, + newUserIsNotAdminError, + newUserSuspendedError, +} from '@/infra/error' import { newHyphenlessUuid } from '@/infra/id' import { verifyPassword } from '@/infra/password' import { User } from '@/user/model' @@ -41,18 +49,18 @@ export async function exchange(options: TokenExchangeOptions): Promise { try { user = await userRepo.findByUsername(options.username.toLocaleLowerCase()) } catch { - throw newError({ code: ErrorCode.InvalidUsernameOrPassword }) + throw newInvalidUsernameOrPasswordError() } if (!user.isEmailConfirmed) { - throw newError({ code: ErrorCode.EmailNotConfirmed }) + throw newEmailNotConfirmedError() } if (!user.isActive) { - throw newError({ code: ErrorCode.UserSuspended }) + throw newUserSuspendedError() } if (verifyPassword(options.password, user.passwordHash)) { return newToken(user.id, user.isAdmin) } else { - throw newError({ code: ErrorCode.InvalidUsernameOrPassword }) + throw newInvalidUsernameOrPasswordError() } } else if (options.grant_type === 'refresh_token') { // https://datatracker.ietf.org/doc/html/rfc6749#section-6 @@ -60,58 +68,44 @@ export async function exchange(options: TokenExchangeOptions): Promise { try { user = await userRepo.findByRefreshTokenValue(options.refresh_token) } catch { - throw newError({ code: ErrorCode.InvalidUsernameOrPassword }) + throw newInvalidUsernameOrPasswordError() } if (!user.isEmailConfirmed) { - throw newError({ code: ErrorCode.EmailNotConfirmed }) + throw newEmailNotConfirmedError() } if (new Date() >= new Date(user.refreshTokenExpiry)) { - throw newError({ code: ErrorCode.RefreshTokenExpired }) + throw newRefreshTokenExpiredError() } return newToken(user.id, user.isAdmin) } } -export const checkAdmin = (jwt) => { - if (!decodeJwt(jwt).is_admin) - throw newError({ code: ErrorCode.MissingPermission }) +export function checkAdmin(jwt: string) { + if (!decodeJwt(jwt).is_admin) { + throw newUserIsNotAdminError() + } } function validateParameters(options: TokenExchangeOptions) { if (!options.grant_type) { - throw newError({ - code: ErrorCode.InvalidRequest, - message: 'Missing parameter: grant_type', - }) + throw newMissingFormParamError('grant_type') } if ( options.grant_type !== 'password' && options.grant_type !== 'refresh_token' ) { - throw newError({ - code: ErrorCode.UnsupportedGrantType, - message: `Grant type unsupported: ${options.grant_type}`, - }) + throw newInvalidGrantType(options.grant_type) } if (options.grant_type === 'password') { if (!options.username) { - throw newError({ - code: ErrorCode.InvalidRequest, - message: 'Missing parameter: username', - }) + throw newMissingFormParamError('username') } if (!options.password) { - throw newError({ - code: ErrorCode.InvalidRequest, - message: 'Missing parameter: password', - }) + throw newMissingFormParamError('password') } } if (options.grant_type === 'refresh_token' && !options.refresh_token) { - throw newError({ - code: ErrorCode.InvalidRequest, - message: 'Missing parameter: refresh_token', - }) + throw newMissingFormParamError('refresh_token') } } @@ -131,7 +125,7 @@ async function newToken(userId: string, isAdmin: boolean): Promise { token_type: 'Bearer', refresh_token: newHyphenlessUuid(), } - const user = await userRepo.findByID(userId) + const user = await userRepo.findById(userId) await userRepo.update({ id: user.id, refreshTokenValue: token.refresh_token, diff --git a/idp/src/user/model.ts b/idp/src/user/model.ts index d7479f31e..1db0494e7 100644 --- a/idp/src/user/model.ts +++ b/idp/src/user/model.ts @@ -65,7 +65,7 @@ export type UpdateOptions = { } export interface UserRepo { - findByID(id: string): Promise + findById(id: string): Promise findByUsername(username: string): Promise findByEmail(email: string): Promise findByRefreshTokenValue(refreshTokenValue: string): Promise diff --git a/idp/src/user/repo.ts b/idp/src/user/repo.ts index 32d212edc..47d250eb4 100644 --- a/idp/src/user/repo.ts +++ b/idp/src/user/repo.ts @@ -7,22 +7,18 @@ // the Business Source License, use of this software will be governed // by the GNU Affero General Public License v3.0 only, included in the file // licenses/AGPL.txt. -import { ErrorCode, newError } from '@/infra/error' +import { newInternalServerError, newUserNotFoundError } from '@/infra/error' import { client } from '@/infra/postgres' import { InsertOptions, UpdateOptions, User } from './model' class UserRepoImpl { - async findByID(id: string): Promise { + async findById(id: string): Promise { const { rowCount, rows } = await client.query( `SELECT * FROM "user" WHERE id = $1`, [id], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with id=${id} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -33,11 +29,7 @@ class UserRepoImpl { [username], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with username=${username} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -48,11 +40,7 @@ class UserRepoImpl { [email], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with email=${email} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -63,11 +51,7 @@ class UserRepoImpl { [refreshTokenValue], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with refresh_token_value=${refreshTokenValue} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -78,11 +62,7 @@ class UserRepoImpl { [resetPasswordToken], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with reset_password_token=${resetPasswordToken} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -95,11 +75,7 @@ class UserRepoImpl { [emailConfirmationToken], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with email_confirmation_token=${emailConfirmationToken} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } @@ -110,17 +86,13 @@ class UserRepoImpl { [emailUpdateToken], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `User with email_update_token=${emailUpdateToken} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } return this.mapRow(rows[0]) } async list(page: number, size: number): Promise { - const { rowCount, rows } = await client.query( + const { rows } = await client.query( `SELECT * FROM "user" ORDER BY create_time @@ -128,46 +100,25 @@ class UserRepoImpl { LIMIT $2`, [(page - 1) * size, size], ) - if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: 'There are no users', - userMessage: 'There are no users', - }) - } return this.mapList(rows) } async findMany(ids: string[]): Promise { - const { rowCount, rows } = await client.query( + const { rows } = await client.query( `SELECT * FROM "user" WHERE id = ANY ($1) ORDER BY create_time`, [ids], ) - if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: 'There are no users', - userMessage: 'There are no users', - }) - } return this.mapList(rows) } async getCount(): Promise { - const { rowCount, rows } = await client.query( + const { rowCount } = await client.query( `SELECT COUNT(id) as count FROM "user"`, ) - if (rowCount < 1) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: `Fatal database error (no users present in database)`, - userMessage: 'There are no users', - }) - } - return parseInt(rows[0].count) + return rowCount } async isUsernameAvailable(username: string): Promise { @@ -214,23 +165,15 @@ class UserRepoImpl { ], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.InternalServerError, - message: `Inserting user with id=${data.id} failed`, - userMessage: 'Failed to insert user', - }) + throw newInternalServerError() } return this.mapRow(rows[0]) } async update(data: UpdateOptions): Promise { - const entity = await this.findByID(data.id) + const entity = await this.findById(data.id) if (!entity) { - throw newError({ - code: ErrorCode.InternalServerError, - message: `User with id=${data.id} not found`, - userMessage: 'User not found', - }) + throw newUserNotFoundError() } Object.assign(entity, data) entity.updateTime = new Date().toISOString() @@ -274,11 +217,7 @@ class UserRepoImpl { ], ) if (rowCount < 1) { - throw newError({ - code: ErrorCode.InternalServerError, - message: `Inserting user with id=${data.id} failed`, - userMessage: 'Failed to insert user', - }) + throw newInternalServerError() } return this.mapRow(rows[0]) } diff --git a/idp/src/user/router.ts b/idp/src/user/router.ts index 8d5e2d89c..43a9811d5 100644 --- a/idp/src/user/router.ts +++ b/idp/src/user/router.ts @@ -15,7 +15,12 @@ import multer from 'multer' import os from 'os' import passport from 'passport' import { getConfig } from '@/config/config' -import { ErrorCode, newError, parseValidationError } from '@/infra/error' +import { + newInvalidJwtError, + newMissingQueryParamError, + newPictureNotFoundError, + parseValidationError, +} from '@/infra/error' import { PassportRequest } from '@/infra/passport-request' import { checkAdmin } from '@/token/service' import { @@ -55,21 +60,14 @@ router.get( '/me/picture:extension', async (req: PassportRequest, res: Response) => { if (!req.query.access_token) { - throw newError({ - code: ErrorCode.InvalidRequest, - message: "Query param 'access_token' is required", - }) + throw newMissingQueryParamError('access_token') } - const userID = await getUserIDFromAccessToken( + const userId = await getUserIdFromAccessToken( req.query.access_token as string, ) - const { buffer, extension, mime } = await getUserPicture(userID) + const { buffer, extension, mime } = await getUserPicture(userId) if (extension !== req.params.extension) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: 'Picture not found', - userMessage: 'Picture not found', - }) + throw newPictureNotFoundError() } res.setHeader( 'Content-Disposition', @@ -247,7 +245,7 @@ router.get( }, ) -async function getUserIDFromAccessToken(accessToken: string): Promise { +async function getUserIdFromAccessToken(accessToken: string): Promise { try { const { payload } = await jwtVerify( accessToken, @@ -255,11 +253,7 @@ async function getUserIDFromAccessToken(accessToken: string): Promise { ) return payload.sub } catch { - throw newError({ - code: ErrorCode.InvalidJwt, - message: 'Invalid JWT', - userMessage: 'Invalid JWT', - }) + throw newInvalidJwtError() } } diff --git a/idp/src/user/service.ts b/idp/src/user/service.ts index 99ac02859..2a0d969d8 100644 --- a/idp/src/user/service.ts +++ b/idp/src/user/service.ts @@ -10,7 +10,17 @@ import fs from 'fs/promises' import { getConfig } from '@/config/config' import { base64ToBuffer, base64ToExtension, base64ToMIME } from '@/infra/base64' -import { ErrorCode, newError } from '@/infra/error' +import { + newCannotDemoteSoleAdminError, + newCannotSuspendSoleAdminError, + newInternalServerError, + newInvalidPasswordError, + newPasswordValidationFailedError, + newUsernameUnavailableError, + newUserNotFoundError, + newPictureNotFoundError, +} from '@/infra/error' +import { ErrorCode, newError } from '@/infra/error/core' import { newHyphenlessUuid } from '@/infra/id' import { sendTemplateMail } from '@/infra/mail' import { hashPassword, verifyPassword } from '@/infra/password' @@ -101,27 +111,31 @@ export type UserListOptions = { } export async function getUser(id: string): Promise { - return mapEntity(await userRepo.findByID(id)) + return mapEntity(await userRepo.findById(id)) } export async function getUserByAdmin(id: string): Promise { - return adminMapEntity(await userRepo.findByID(id)) + return adminMapEntity(await userRepo.findById(id)) } export async function getUserPicture(id: string): Promise { - const user = await userRepo.findByID(id) + const user = await userRepo.findById(id) if (!user.picture) { - throw newError({ - code: ErrorCode.ResourceNotFound, - message: 'Picture not found', - userMessage: 'Picture not found', - }) + throw newPictureNotFoundError() + } + const buffer = base64ToBuffer(user.picture) + if (!buffer) { + throw newPictureNotFoundError() } - return { - buffer: base64ToBuffer(user.picture), - extension: base64ToExtension(user.picture), - mime: base64ToMIME(user.picture), + const extension = base64ToExtension(user.picture) + if (!extension) { + throw newPictureNotFoundError() } + const mime = base64ToMIME(user.picture) + if (!mime) { + throw newPictureNotFoundError() + } + return { buffer, extension, mime } } export async function list({ @@ -173,7 +187,7 @@ export async function updateFullName( id: string, options: UserUpdateFullNameOptions, ): Promise { - let user = await userRepo.findByID(id) + let user = await userRepo.findById(id) user = await userRepo.update({ id: user.id, fullName: options.fullName }) await search.index(USER_SEARCH_INDEX).updateDocuments([ { @@ -194,7 +208,7 @@ export async function updateEmailRequest( id: string, options: UserUpdateEmailRequestOptions, ): Promise { - let user = await userRepo.findByID(id) + let user = await userRepo.findById(id) if (options.email === user.email) { user = await userRepo.update({ id: user.id, @@ -211,7 +225,7 @@ export async function updateEmailRequest( // Ignored } if (usernameUnavailable) { - throw newError({ code: ErrorCode.UsernameUnavailable }) + throw newUsernameUnavailableError() } user = await userRepo.update({ id: user.id, @@ -231,7 +245,7 @@ export async function updateEmailRequest( emailUpdateToken: null, emailUpdateValue: null, }) - throw newError({ code: ErrorCode.InternalServerError, error }) + throw newInternalServerError(error) } } } @@ -266,7 +280,7 @@ export async function updatePassword( id: string, options: UserUpdatePasswordOptions, ): Promise { - let user = await userRepo.findByID(id) + let user = await userRepo.findById(id) if (verifyPassword(options.currentPassword, user.passwordHash)) { user = await userRepo.update({ id: user.id, @@ -274,7 +288,7 @@ export async function updatePassword( }) return mapEntity(user) } else { - throw newError({ code: ErrorCode.PasswordValidationFailed }) + throw newPasswordValidationFailedError() } } @@ -284,7 +298,7 @@ export async function updatePicture( contentType: string, ): Promise { const picture = await fs.readFile(path, { encoding: 'base64' }) - const { id: userId } = await userRepo.findByID(id) + const { id: userId } = await userRepo.findById(id) const user = await userRepo.update({ id: userId, picture: `data:${contentType};base64,${picture}`, @@ -305,7 +319,7 @@ export async function updatePicture( } export async function deletePicture(id: string): Promise { - let user = await userRepo.findByID(id) + let user = await userRepo.findById(id) user = await userRepo.update({ id: user.id, picture: null }) await search.index(USER_SEARCH_INDEX).updateDocuments([ { @@ -323,23 +337,23 @@ export async function deletePicture(id: string): Promise { } export async function deleteUser(id: string, options: UserDeleteOptions) { - const user = await userRepo.findByID(id) + const user = await userRepo.findById(id) if (verifyPassword(options.password, user.passwordHash)) { await userRepo.delete(user.id) await search.index(USER_SEARCH_INDEX).deleteDocuments([user.id]) } else { - throw newError({ code: ErrorCode.InvalidPassword }) + throw newInvalidPasswordError() } } export async function suspendUser(id: string, options: UserSuspendOptions) { - const user = await userRepo.findByID(id) + const user = await userRepo.findById(id) if ( user.isAdmin && !(await userRepo.enoughActiveAdmins()) && options.suspend ) { - throw newError({ code: ErrorCode.OrphanError }) + throw newCannotSuspendSoleAdminError() } if (user) { await userRepo.suspend(user.id, options.suspend) @@ -356,18 +370,18 @@ export async function suspendUser(id: string, options: UserSuspendOptions) { }, ]) } else { - throw newError({ code: ErrorCode.UserNotFound }) + throw newUserNotFoundError() } } export async function makeAdminUser(id: string, options: UserMakeAdminOptions) { - const user = await userRepo.findByID(id) + const user = await userRepo.findById(id) if ( user.isAdmin && !(await userRepo.enoughActiveAdmins()) && options.makeAdmin ) { - throw newError({ code: ErrorCode.OrphanError }) + throw newCannotDemoteSoleAdminError() } if (user) { await userRepo.makeAdmin(user.id, options.makeAdmin)