Skip to content

Commit

Permalink
fix: remove email requirement for the user, and use the username if n…
Browse files Browse the repository at this point in the history
…o email provided (#900)

* fix: remove email requirement for the user, and use the username if no email provided

* fix: update translations

* fix: remove useless console.log

* test: fix user list test

* fix: disallow Plex users from changing their email
  • Loading branch information
gauthier-th authored Jul 29, 2024
1 parent 4220855 commit d5f817e
Show file tree
Hide file tree
Showing 10 changed files with 85 additions and 45 deletions.
4 changes: 2 additions & 2 deletions cypress/e2e/user/user-list.cy.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
const testUser = {
displayName: 'Test User',
username: 'Test User',
emailAddress: '[email protected]',
password: 'test1234',
};
Expand Down Expand Up @@ -32,7 +32,7 @@ describe('User List', () => {

cy.get('[data-testid=modal-title]').should('contain', 'Create Local User');

cy.get('#displayName').type(testUser.displayName);
cy.get('#username').type(testUser.username);
cy.get('#email').type(testUser.emailAddress);
cy.get('#password').type(testUser.password);

Expand Down
18 changes: 10 additions & 8 deletions server/routes/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -320,15 +320,18 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
// with admin permission
settings.main.mediaServerType = MediaServerType.JELLYFIN;
user = new User({
email: body.email,
email: body.email || account.User.Name,
jellyfinUsername: account.User.Name,
jellyfinUserId: account.User.Id,
jellyfinDeviceId: deviceId,
jellyfinAuthToken: account.AccessToken,
permissions: Permission.ADMIN,
avatar: account.User.PrimaryImageTag
? `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
: gravatarUrl(body.email ?? '', { default: 'mm', size: 200 }),
: gravatarUrl(body.email || account.User.Name, {
default: 'mm',
size: 200,
}),
userType: UserType.JELLYFIN,
});

Expand Down Expand Up @@ -371,7 +374,7 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
if (account.User.PrimaryImageTag) {
user.avatar = `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`;
} else {
user.avatar = gravatarUrl(user.email, {
user.avatar = gravatarUrl(user.email || account.User.Name, {
default: 'mm',
size: 200,
});
Expand Down Expand Up @@ -413,10 +416,6 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
}
);

if (!body.email) {
throw new Error('add_email');
}

user = new User({
email: body.email,
jellyfinUsername: account.User.Name,
Expand All @@ -426,7 +425,10 @@ authRoutes.post('/jellyfin', async (req, res, next) => {
permissions: settings.main.defaultPermissions,
avatar: account.User.PrimaryImageTag
? `${jellyfinHost}/Users/${account.User.Id}/Images/Primary/?tag=${account.User.PrimaryImageTag}&quality=90`
: gravatarUrl(body.email, { default: 'mm', size: 200 }),
: gravatarUrl(body.email || account.User.Name, {
default: 'mm',
size: 200,
}),
userType: UserType.JELLYFIN,
});
//initialize Jellyfin/Emby users with local login
Expand Down
21 changes: 17 additions & 4 deletions server/routes/user/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,19 @@ router.get('/', async (req, res, next) => {
break;
case 'displayname':
query = query.orderBy(
"(CASE WHEN (user.username IS NULL OR user.username = '') THEN (CASE WHEN (user.plexUsername IS NULL OR user.plexUsername = '') THEN user.email ELSE LOWER(user.plexUsername) END) ELSE LOWER(user.username) END)",
`CASE WHEN (user.username IS NULL OR user.username = '') THEN (
CASE WHEN (user.plexUsername IS NULL OR user.plexUsername = '') THEN (
CASE WHEN (user.jellyfinUsername IS NULL OR user.jellyfinUsername = '') THEN
user.email
ELSE
LOWER(user.jellyfinUsername)
END)
ELSE
LOWER(user.jellyfinUsername)
END)
ELSE
LOWER(user.username)
END`,
'ASC'
);
break;
Expand Down Expand Up @@ -90,12 +102,13 @@ router.post(
const settings = getSettings();

const body = req.body;
const email = body.email || body.username;
const userRepository = getRepository(User);

const existingUser = await userRepository
.createQueryBuilder('user')
.where('user.email = :email', {
email: body.email.toLowerCase(),
email: email.toLowerCase(),
})
.getOne();

Expand All @@ -108,7 +121,7 @@ router.post(
}

const passedExplicitPassword = body.password && body.password.length > 0;
const avatar = gravatarUrl(body.email, { default: 'mm', size: 200 });
const avatar = gravatarUrl(email, { default: 'mm', size: 200 });

if (
!passedExplicitPassword &&
Expand All @@ -118,9 +131,9 @@ router.post(
}

const user = new User({
email,
avatar: body.avatar ?? avatar,
username: body.username,
email: body.email,
password: body.password,
permissions: settings.main.defaultPermissions,
plexToken: '',
Expand Down
4 changes: 3 additions & 1 deletion server/routes/user/usersettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ userSettingsRoutes.post<
}

user.username = req.body.username;
user.email = req.body.email ?? user.email;
if (user.jellyfinUsername) {
user.email = req.body.email || user.jellyfinUsername || user.email;
}

// Update quota values only if the user has the correct permissions
if (
Expand Down
8 changes: 5 additions & 3 deletions src/components/Layout/UserDropdown/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,11 @@ const UserDropdown = () => {
<span className="truncate text-xl font-semibold text-gray-200">
{user?.displayName}
</span>
<span className="truncate text-sm text-gray-400">
{user?.email}
</span>
{user?.displayName?.toLowerCase() !== user?.email && (
<span className="truncate text-sm text-gray-400">
{user?.email}
</span>
)}
</div>
</div>
{user && <MiniQuotaDisplay userId={user?.id} />}
Expand Down
2 changes: 1 addition & 1 deletion src/components/UserList/JellyfinImportModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ const JellyfinImportModal: React.FC<JellyfinImportProps> = ({
}),
});
if (!res.ok) throw new Error();
const { data: createdUsers } = await res.json();
const createdUsers = await res.json();

if (!createdUsers.length) {
throw new Error('No users were imported from Jellyfin.');
Expand Down
46 changes: 28 additions & 18 deletions src/components/UserList/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -68,14 +68,15 @@ const messages = defineMessages('components.UserList', {
usercreatedfailedexisting:
'The provided email address is already in use by another user.',
usercreatedsuccess: 'User created successfully!',
displayName: 'Display Name',
username: 'Username',
email: 'Email Address',
password: 'Password',
passwordinfodescription:
'Configure an application URL and enable email notifications to allow automatic password generation.',
autogeneratepassword: 'Automatically Generate Password',
autogeneratepasswordTip: 'Email a server-generated password to the user',
validationEmail: 'You must provide a valid email address',
validationUsername: 'You must provide an username',
validationEmail: 'Email required',
sortCreated: 'Join Date',
sortDisplayName: 'Display Name',
sortRequests: 'Request Count',
Expand Down Expand Up @@ -208,9 +209,10 @@ const UserList = () => {
}

const CreateUserSchema = Yup.object().shape({
email: Yup.string()
.required(intl.formatMessage(messages.validationEmail))
.email(intl.formatMessage(messages.validationEmail)),
username: Yup.string().required(
intl.formatMessage(messages.validationUsername)
),
email: Yup.string().email(intl.formatMessage(messages.validationEmail)),
password: Yup.lazy((value) =>
!value
? Yup.string()
Expand Down Expand Up @@ -258,7 +260,7 @@ const UserList = () => {
setDeleteModal({ isOpen: false, user: deleteModal.user })
}
title={intl.formatMessage(messages.deleteuser)}
subTitle={deleteModal.user?.displayName}
subTitle={deleteModal.user?.username}
>
{intl.formatMessage(messages.deleteconfirm)}
</Modal>
Expand All @@ -276,7 +278,7 @@ const UserList = () => {
>
<Formik
initialValues={{
displayName: '',
username: '',
email: '',
password: '',
genpassword: false,
Expand All @@ -290,7 +292,7 @@ const UserList = () => {
'Content-Type': 'application/json',
},
body: JSON.stringify({
username: values.displayName,
username: values.username,
email: values.email,
password: values.genpassword ? null : values.password,
}),
Expand Down Expand Up @@ -370,23 +372,24 @@ const UserList = () => {
)}
<Form className="section">
<div className="form-row">
<label htmlFor="displayName" className="text-label">
{intl.formatMessage(messages.displayName)}
<label htmlFor="username" className="text-label">
{intl.formatMessage(messages.username)}
<span className="label-required">*</span>
</label>
<div className="form-input-area">
<div className="form-input-field">
<Field
id="displayName"
name="displayName"
type="text"
/>
<Field id="username" name="username" type="text" />
</div>
{errors.username &&
touched.username &&
typeof errors.username === 'string' && (
<div className="error">{errors.username}</div>
)}
</div>
</div>
<div className="form-row">
<label htmlFor="email" className="text-label">
{intl.formatMessage(messages.email)}
<span className="label-required">*</span>
</label>
<div className="form-input-area">
<div className="form-input-field">
Expand Down Expand Up @@ -645,9 +648,16 @@ const UserList = () => {
className="text-base font-bold leading-5 transition duration-300 hover:underline"
data-testid="user-list-username-link"
>
{user.displayName}
{user.username ||
user.jellyfinUsername ||
user.plexUsername ||
user.email}
</Link>
{user.displayName.toLowerCase() !== user.email && (
{(
user.username ||
user.jellyfinUsername ||
user.plexUsername
)?.toLowerCase() !== user.email && (
<div className="text-sm leading-5 text-gray-300">
{user.email}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,14 @@ const UserGeneralSettings = () => {
);

const UserGeneralSettingsSchema = Yup.object().shape({
email: Yup.string()
.email(intl.formatMessage(messages.validationemailformat))
.required(intl.formatMessage(messages.validationemailrequired)),
email:
user?.id === 1
? Yup.string()
.email(intl.formatMessage(messages.validationemailformat))
.required(intl.formatMessage(messages.validationemailrequired))
: Yup.string().email(
intl.formatMessage(messages.validationemailformat)
),
discordId: Yup.string()
.nullable()
.matches(/^\d{17,19}$/, intl.formatMessage(messages.validationDiscordId)),
Expand Down Expand Up @@ -134,7 +139,7 @@ const UserGeneralSettings = () => {
<Formik
initialValues={{
displayName: data?.username ?? '',
email: data?.email ?? '',
email: data?.email?.includes('@') ? data.email : '',
discordId: data?.discordId ?? '',
locale: data?.locale,
region: data?.region,
Expand All @@ -157,7 +162,8 @@ const UserGeneralSettings = () => {
},
body: JSON.stringify({
username: values.displayName,
email: values.email,
email:
values.email || user?.jellyfinUsername || user?.plexUsername,
discordId: values.discordId,
locale: values.locale,
region: values.region,
Expand Down Expand Up @@ -264,7 +270,9 @@ const UserGeneralSettings = () => {
name="displayName"
type="text"
placeholder={
user?.plexUsername ? user.plexUsername : user?.email
user?.username ||
user?.jellyfinUsername ||
user?.plexUsername
}
/>
</div>
Expand All @@ -289,6 +297,7 @@ const UserGeneralSettings = () => {
name="email"
type="text"
placeholder="[email protected]"
disabled={user?.plexUsername}
className={
user?.warnings.find((w) => w === 'userEmailRequired')
? 'border-2 border-red-400 focus:border-blue-600'
Expand Down
1 change: 1 addition & 0 deletions src/hooks/useUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ export interface User {
id: number;
warnings: string[];
plexUsername?: string;
jellyfinUsername?: string;
username?: string;
displayName: string;
email: string;
Expand Down
5 changes: 3 additions & 2 deletions src/i18n/locale/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -1111,7 +1111,6 @@
"components.UserList.creating": "Creating…",
"components.UserList.deleteconfirm": "Are you sure you want to delete this user? All of their request data will be permanently removed.",
"components.UserList.deleteuser": "Delete User",
"components.UserList.displayName": "Display Name",
"components.UserList.edituser": "Edit User Permissions",
"components.UserList.email": "Email Address",
"components.UserList.importedfromJellyfin": "<strong>{userCount}</strong> {mediaServerName} {userCount, plural, one {user} other {users}} imported successfully!",
Expand Down Expand Up @@ -1145,9 +1144,11 @@
"components.UserList.userdeleteerror": "Something went wrong while deleting the user.",
"components.UserList.userfail": "Something went wrong while saving user permissions.",
"components.UserList.userlist": "User List",
"components.UserList.username": "Username",
"components.UserList.users": "Users",
"components.UserList.userssaved": "User permissions saved successfully!",
"components.UserList.validationEmail": "You must provide a valid email address",
"components.UserList.validationEmail": "Email required",
"components.UserList.validationUsername": "You must provide an username",
"components.UserList.validationpasswordminchars": "Password is too short; should be a minimum of 8 characters",
"components.UserProfile.ProfileHeader.joindate": "Joined {joindate}",
"components.UserProfile.ProfileHeader.profile": "View Profile",
Expand Down

0 comments on commit d5f817e

Please sign in to comment.