From 49abd03fbdd8849719da8a340aa3991e32ae737b Mon Sep 17 00:00:00 2001 From: Gauthier Date: Wed, 22 May 2024 18:20:15 +0200 Subject: [PATCH 1/3] fix(api): save user email on the first try fix #227 --- server/routes/user/usersettings.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/server/routes/user/usersettings.ts b/server/routes/user/usersettings.ts index f9ea3de61..5620f142a 100644 --- a/server/routes/user/usersettings.ts +++ b/server/routes/user/usersettings.ts @@ -98,6 +98,7 @@ userSettingsRoutes.post< } user.username = req.body.username; + user.email = req.body.email ?? user.email; // Update quota values only if the user has the correct permissions if ( @@ -127,20 +128,21 @@ userSettingsRoutes.post< user.settings.originalLanguage = req.body.originalLanguage; user.settings.watchlistSyncMovies = req.body.watchlistSyncMovies; user.settings.watchlistSyncTv = req.body.watchlistSyncTv; - user.email = req.body.email ?? user.email; } - await userRepository.save(user); + const savedUser = await userRepository.save(user); + + // TODO: check if the email of the user has been modified after save (if the email already exists in the db) return res.status(200).json({ - username: user.username, - discordId: user.settings.discordId, - locale: user.settings.locale, - region: user.settings.region, - originalLanguage: user.settings.originalLanguage, - watchlistSyncMovies: user.settings.watchlistSyncMovies, - watchlistSyncTv: user.settings.watchlistSyncTv, - email: user.email, + username: savedUser.username, + discordId: savedUser.settings?.discordId, + locale: savedUser.settings?.locale, + region: savedUser.settings?.region, + originalLanguage: savedUser.settings?.originalLanguage, + watchlistSyncMovies: savedUser.settings?.watchlistSyncMovies, + watchlistSyncTv: savedUser.settings?.watchlistSyncTv, + email: savedUser.email, }); } catch (e) { next({ status: 500, message: e.message }); From 10e7fe9c16c23f7f4974fa7803333dce4b1cd9a0 Mon Sep 17 00:00:00 2001 From: Gauthier Date: Thu, 23 May 2024 13:55:29 +0200 Subject: [PATCH 2/3] fix(api): remove todo --- server/routes/user/usersettings.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/server/routes/user/usersettings.ts b/server/routes/user/usersettings.ts index 5620f142a..53eed9ef9 100644 --- a/server/routes/user/usersettings.ts +++ b/server/routes/user/usersettings.ts @@ -132,8 +132,6 @@ userSettingsRoutes.post< const savedUser = await userRepository.save(user); - // TODO: check if the email of the user has been modified after save (if the email already exists in the db) - return res.status(200).json({ username: savedUser.username, discordId: savedUser.settings?.discordId, From d0ec319acfdb6ead482e591bf8762b2e224ea2aa Mon Sep 17 00:00:00 2001 From: Fallenbagel <98979876+Fallenbagel@users.noreply.github.com> Date: Thu, 23 May 2024 19:34:31 +0500 Subject: [PATCH 3/3] fix(logging): handle media server connection refused error/toast (#748) * fix(logging): handle media server connection refused error/toast Properly log as connection refused if the jellyfin/emby server is unreachable. Previously it used to throw a credentials error which lead to a lot of confusion * refactor(i8n): extract translation keys * refactor(auth): error message for a more consistent format * refactor(auth/errors): use custom error types and error codes instead of abusing error messages * refactor(i8n): replace connection refused translation key with invalidurl * fix(error): combine auth and api error class into a single one called network error * fix(error): use the new network error and network error codes in auth/api * refactor(error): rename NetworkError to ApiError --- server/api/jellyfin.ts | 28 +++++++- server/constants/error.ts | 5 ++ server/routes/auth.ts | 98 ++++++++++++++++---------- server/types/error.ts | 9 +++ src/components/Login/JellyfinLogin.tsx | 26 +++++-- src/i18n/locale/en.json | 12 ++-- 6 files changed, 127 insertions(+), 51 deletions(-) create mode 100644 server/constants/error.ts create mode 100644 server/types/error.ts diff --git a/server/api/jellyfin.ts b/server/api/jellyfin.ts index cd7fb1cfc..3f7130f79 100644 --- a/server/api/jellyfin.ts +++ b/server/api/jellyfin.ts @@ -1,6 +1,8 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ +import { ApiErrorCode } from '@server/constants/error'; import availabilitySync from '@server/lib/availabilitySync'; import logger from '@server/logger'; +import { ApiError } from '@server/types/error'; import type { AxiosInstance } from 'axios'; import axios from 'axios'; @@ -129,9 +131,33 @@ class JellyfinAPI { Pw: Password, } ); + return account.data; } catch (e) { - throw new Error('Unauthorized'); + const status = e.response?.status; + + const networkErrorCodes = new Set([ + 'ECONNREFUSED', + 'EHOSTUNREACH', + 'ENOTFOUND', + 'ETIMEDOUT', + 'ECONNRESET', + 'EADDRINUSE', + 'ENETDOWN', + 'ENETUNREACH', + 'EPIPE', + 'ECONNABORTED', + 'EPROTO', + 'EHOSTDOWN', + 'EAI_AGAIN', + 'ERR_INVALID_URL', + ]); + + if (networkErrorCodes.has(e.code) || status === 404) { + throw new ApiError(status, ApiErrorCode.InvalidUrl); + } + + throw new ApiError(status, ApiErrorCode.InvalidCredentials); } } diff --git a/server/constants/error.ts b/server/constants/error.ts new file mode 100644 index 000000000..87e37e4c2 --- /dev/null +++ b/server/constants/error.ts @@ -0,0 +1,5 @@ +export enum ApiErrorCode { + InvalidUrl = 'INVALID_URL', + InvalidCredentials = 'INVALID_CREDENTIALS', + NotAdmin = 'NOT_ADMIN', +} diff --git a/server/routes/auth.ts b/server/routes/auth.ts index c20b4a5f6..9ef3ef7e4 100644 --- a/server/routes/auth.ts +++ b/server/routes/auth.ts @@ -1,5 +1,6 @@ import JellyfinAPI from '@server/api/jellyfin'; import PlexTvAPI from '@server/api/plextv'; +import { ApiErrorCode } from '@server/constants/error'; import { MediaServerType } from '@server/constants/server'; import { UserType } from '@server/constants/user'; import { getRepository } from '@server/datasource'; @@ -9,6 +10,7 @@ import { Permission } from '@server/lib/permissions'; import { getSettings } from '@server/lib/settings'; import logger from '@server/logger'; import { isAuthenticated } from '@server/middleware/auth'; +import { ApiError } from '@server/types/error'; import * as EmailValidator from 'email-validator'; import { Router } from 'express'; import gravatarUrl from 'gravatar-url'; @@ -278,7 +280,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => { if (!user && !(await userRepository.count())) { // Check if user is admin on jellyfin if (account.User.Policy.IsAdministrator === false) { - throw new Error('not_admin'); + throw new ApiError(403, ApiErrorCode.NotAdmin); } logger.info( @@ -412,43 +414,63 @@ authRoutes.post('/jellyfin', async (req, res, next) => { return res.status(200).json(user?.filter() ?? {}); } catch (e) { - if (e.message === 'Unauthorized') { - logger.warn( - 'Failed login attempt from user with incorrect Jellyfin credentials', - { - label: 'Auth', - account: { - ip: req.ip, - email: body.username, - password: '__REDACTED__', - }, - } - ); - return next({ - status: 401, - message: 'Unauthorized', - }); - } else if (e.message === 'not_admin') { - return next({ - status: 403, - message: 'CREDENTIAL_ERROR_NOT_ADMIN', - }); - } else if (e.message === 'add_email') { - return next({ - status: 406, - message: 'CREDENTIAL_ERROR_ADD_EMAIL', - }); - } else if (e.message === 'select_server_type') { - return next({ - status: 406, - message: 'CREDENTIAL_ERROR_NO_SERVER_TYPE', - }); - } else { - logger.error(e.message, { label: 'Auth' }); - return next({ - status: 500, - message: 'Something went wrong.', - }); + switch (e.errorCode) { + case ApiErrorCode.InvalidUrl: + logger.error( + `The provided ${ + process.env.JELLYFIN_TYPE == 'emby' ? 'Emby' : 'Jellyfin' + } is invalid or the server is not reachable.`, + { + label: 'Auth', + error: e.errorCode, + status: e.statusCode, + hostname: body.hostname, + } + ); + return next({ + status: e.statusCode, + message: e.errorCode, + }); + + case ApiErrorCode.InvalidCredentials: + logger.warn( + 'Failed login attempt from user with incorrect Jellyfin credentials', + { + label: 'Auth', + account: { + ip: req.ip, + email: body.username, + password: '__REDACTED__', + }, + } + ); + return next({ + status: e.statusCode, + message: e.errorCode, + }); + + case ApiErrorCode.NotAdmin: + logger.warn( + 'Failed login attempt from user without admin permissions', + { + label: 'Auth', + account: { + ip: req.ip, + email: body.username, + }, + } + ); + return next({ + status: e.statusCode, + message: e.errorCode, + }); + + default: + logger.error(e.message, { label: 'Auth' }); + return next({ + status: 500, + message: 'Something went wrong.', + }); } } }); diff --git a/server/types/error.ts b/server/types/error.ts new file mode 100644 index 000000000..4985b5c55 --- /dev/null +++ b/server/types/error.ts @@ -0,0 +1,9 @@ +import type { ApiErrorCode } from '@server/constants/error'; + +export class ApiError extends Error { + constructor(public statusCode: number, public errorCode: ApiErrorCode) { + super(); + + this.name = 'apiError'; + } +} diff --git a/src/components/Login/JellyfinLogin.tsx b/src/components/Login/JellyfinLogin.tsx index e5c01d6ef..7403392e9 100644 --- a/src/components/Login/JellyfinLogin.tsx +++ b/src/components/Login/JellyfinLogin.tsx @@ -2,6 +2,7 @@ import Button from '@app/components/Common/Button'; import Tooltip from '@app/components/Common/Tooltip'; import useSettings from '@app/hooks/useSettings'; import { InformationCircleIcon } from '@heroicons/react/24/solid'; +import { ApiErrorCode } from '@server/constants/error'; import axios from 'axios'; import { Field, Form, Formik } from 'formik'; import getConfig from 'next/config'; @@ -26,6 +27,7 @@ const messages = defineMessages({ loginerror: 'Something went wrong while trying to sign in.', adminerror: 'You must use an admin account to sign in.', credentialerror: 'The username or password is incorrect.', + invalidurlerror: 'Unable to connect to {mediaServerName} server.', signingin: 'Signing in…', signin: 'Sign In', initialsigningin: 'Connecting…', @@ -91,14 +93,24 @@ const JellyfinLogin: React.FC = ({ email: values.email, }); } catch (e) { + let errorMessage = null; + switch (e.response?.data?.message) { + case ApiErrorCode.InvalidUrl: + errorMessage = messages.invalidurlerror; + break; + case ApiErrorCode.InvalidCredentials: + errorMessage = messages.credentialerror; + break; + case ApiErrorCode.NotAdmin: + errorMessage = messages.adminerror; + break; + default: + errorMessage = messages.loginerror; + break; + } + toasts.addToast( - intl.formatMessage( - e.message == 'Request failed with status code 401' - ? messages.credentialerror - : e.message == 'Request failed with status code 403' - ? messages.adminerror - : messages.loginerror - ), + intl.formatMessage(errorMessage, mediaServerFormatValues), { autoDismiss: true, appearance: 'error', diff --git a/src/i18n/locale/en.json b/src/i18n/locale/en.json index 73e6170fb..367fabc37 100644 --- a/src/i18n/locale/en.json +++ b/src/i18n/locale/en.json @@ -219,8 +219,9 @@ "components.Layout.VersionStatus.outofdate": "Out of Date", "components.Layout.VersionStatus.streamdevelop": "Jellyseerr Develop", "components.Layout.VersionStatus.streamstable": "Jellyseerr Stable", - "components.Login.credentialerror": "The username or password is incorrect.", "components.Login.adminerror": "You must use an admin account to sign in.", + "components.Login.invalidurlerror": "Unable to connect to {mediaServerName} server.", + "components.Login.credentialerror": "The username or password is incorrect.", "components.Login.description": "Since this is your first time logging into {applicationName}, you are required to add a valid email address.", "components.Login.email": "Email Address", "components.Login.emailtooltip": "Address does not need to be associated with your {mediaServerName} instance.", @@ -752,8 +753,8 @@ "components.Settings.SettingsAbout.overseerrinformation": "About Jellyseerr", "components.Settings.SettingsAbout.preferredmethod": "Preferred", "components.Settings.SettingsAbout.runningDevelop": "You are running the develop branch of Jellyseerr, which is only recommended for those contributing to development or assisting with bleeding-edge testing.", - "components.Settings.SettingsAbout.supportoverseerr": "Support Overseerr", "components.Settings.SettingsAbout.supportjellyseerr": "Support Jellyseerr", + "components.Settings.SettingsAbout.supportoverseerr": "Support Overseerr", "components.Settings.SettingsAbout.timezone": "Time Zone", "components.Settings.SettingsAbout.totalmedia": "Total Media", "components.Settings.SettingsAbout.totalrequests": "Total Requests", @@ -938,17 +939,18 @@ "components.Settings.hostname": "Hostname or IP Address", "components.Settings.internalUrl": "Internal URL", "components.Settings.is4k": "4K", + "components.Settings.jellyfinForgotPasswordUrl": "Forgot Password URL", "components.Settings.jellyfinSettings": "{mediaServerName} Settings", "components.Settings.jellyfinSettingsDescription": "Optionally configure the internal and external endpoints for your {mediaServerName} server. In most cases, the external URL is different to the internal URL. A custom password reset URL can also be set for {mediaServerName} login, in case you would like to redirect to a different password reset page.", "components.Settings.jellyfinSettingsFailure": "Something went wrong while saving {mediaServerName} settings.", "components.Settings.jellyfinSettingsSuccess": "{mediaServerName} settings saved successfully!", + "components.Settings.jellyfinSyncFailedAutomaticGroupedFolders": "Custom authentication with Automatic Library Grouping not supported", + "components.Settings.jellyfinSyncFailedGenericError": "Something went wrong while syncing libraries", + "components.Settings.jellyfinSyncFailedNoLibrariesFound": "No libraries were found", "components.Settings.jellyfinlibraries": "{mediaServerName} Libraries", "components.Settings.jellyfinlibrariesDescription": "The libraries {mediaServerName} scans for titles. Click the button below if no libraries are listed.", "components.Settings.jellyfinsettings": "{mediaServerName} Settings", "components.Settings.jellyfinsettingsDescription": "Configure the settings for your {mediaServerName} server. {mediaServerName} scans your {mediaServerName} libraries to see what content is available.", - "components.Settings.jellyfinSyncFailedNoLibrariesFound": "No libraries were found", - "components.Settings.jellyfinSyncFailedAutomaticGroupedFolders": "Custom authentication with Automatic Library Grouping not supported", - "components.Settings.jellyfinSyncFailedGenericError": "Something went wrong while syncing libraries", "components.Settings.librariesRemaining": "Libraries Remaining: {count}", "components.Settings.manualscan": "Manual Library Scan", "components.Settings.manualscanDescription": "Normally, this will only be run once every 24 hours. Jellyseerr will check your Plex server's recently added more aggressively. If this is your first time configuring Plex, a one-time full manual library scan is recommended!",