Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jul 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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, {
Fallenbagel marked this conversation as resolved.
Show resolved Hide resolved
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, {
Fallenbagel marked this conversation as resolved.
Show resolved Hide resolved
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, {
Fallenbagel marked this conversation as resolved.
Show resolved Hide resolved
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 });
Fallenbagel marked this conversation as resolved.
Show resolved Hide resolved

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 @@ -363,23 +365,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 @@ -638,9 +641,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
Loading