feat(API): permissive email check in reset & verification

In order to not force users to be case sensitive when asking for
password reset or resend email verification. When there's multiple
emails where the only difference in the local is the capitalized
letters, in those cases the users has to be case sensitive.

closes #6570
This commit is contained in:
kontrollanten 2024-10-01 22:34:52 +02:00
parent 639feb2306
commit 714d9c4aa7
9 changed files with 121 additions and 19 deletions

View File

@ -161,6 +161,7 @@ export class UsersCommand extends AbstractCommand {
videoQuotaDaily?: number videoQuotaDaily?: number
role?: UserRoleType role?: UserRoleType
adminFlags?: UserAdminFlagType adminFlags?: UserAdminFlagType
email?: string
}) { }) {
const { const {
username, username,
@ -168,7 +169,8 @@ export class UsersCommand extends AbstractCommand {
password = 'password', password = 'password',
videoQuota, videoQuota,
videoQuotaDaily, videoQuotaDaily,
role = UserRole.USER role = UserRole.USER,
email = username + '@example.com'
} = options } = options
const path = '/api/v1/users' const path = '/api/v1/users'
@ -182,7 +184,7 @@ export class UsersCommand extends AbstractCommand {
password, password,
role, role,
adminFlags, adminFlags,
email: username + '@example.com', email,
videoQuota, videoQuota,
videoQuotaDaily videoQuotaDaily
}, },

View File

@ -28,11 +28,23 @@ describe('Test users API validators', function () {
await server.config.enableSignup(true) await server.config.enableSignup(true)
await server.users.generate('moderator2', UserRole.MODERATOR) await server.users.generate('moderator2', UserRole.MODERATOR)
await server.users.create({ username: 'user' })
await server.users.create({ username: 'user_similar', email: 'User@example.com' })
await server.users.generate('user2')
await server.registrations.requestRegistration({ await server.registrations.requestRegistration({
username: 'request1', username: 'request1',
registrationReason: 'tt' registrationReason: 'tt'
}) })
await server.registrations.requestRegistration({
username: 'request_1',
email: 'Request1@example.com',
registrationReason: 'tt'
})
await server.registrations.requestRegistration({
username: 'request2',
registrationReason: 'tt'
})
}) })
describe('When asking a password reset', function () { describe('When asking a password reset', function () {
@ -50,6 +62,39 @@ describe('Test users API validators', function () {
await makePostBodyRequest({ url: server.url, path, fields }) await makePostBodyRequest({ url: server.url, path, fields })
}) })
it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
const fields = { email: 'USER@example.com' }
await makePostBodyRequest({
url: server.url,
path,
fields,
expectedStatus: HttpStatusCode.NO_CONTENT_204
})
})
it('Should success with correct capitalization when multiple users with similar email exists', async function () {
const fields = { email: 'User@example.com' }
await makePostBodyRequest({
url: server.url,
path,
fields,
expectedStatus: HttpStatusCode.NO_CONTENT_204
})
})
it('Should success with wrong capitalization when no similar emails exists', async function () {
const fields = { email: 'USER2@example.com' }
await makePostBodyRequest({
url: server.url,
path,
fields,
expectedStatus: HttpStatusCode.NO_CONTENT_204
})
})
it('Should success with the correct params', async function () { it('Should success with the correct params', async function () {
const fields = { email: 'admin@example.com' } const fields = { email: 'admin@example.com' }
@ -104,7 +149,29 @@ describe('Test users API validators', function () {
await makePostBodyRequest({ url: server.url, path, fields }) await makePostBodyRequest({ url: server.url, path, fields })
}) })
it('Should succeed with the correct params', async function () { it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
const fields = { email: 'REQUEST1@example.com' }
await makePostBodyRequest({
url: server.url,
path,
fields,
expectedStatus: HttpStatusCode.NO_CONTENT_204
})
})
it('Should success with wrong capitalization when no similar emails exists', async function () {
const fields = { email: 'REQUEST2@example.com' }
await makePostBodyRequest({
url: server.url,
path,
fields,
expectedStatus: HttpStatusCode.NO_CONTENT_204
})
})
it('Should success with correct capitalization when multiple users with similar email exists', async function () {
const fields = { email: 'request1@example.com' } const fields = { email: 'request1@example.com' }
await makePostBodyRequest({ await makePostBodyRequest({

View File

@ -237,6 +237,12 @@ async function isUserQuotaValid (options: {
return true return true
} }
function getUserByEmailPermissive <T extends { email: string }> (users: T[], email: string): T {
if (users.length === 1) return users[0]
return users.find(r => r.email === email)
}
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------
export { export {
@ -250,7 +256,8 @@ export {
sendVerifyRegistrationEmail, sendVerifyRegistrationEmail,
isUserQuotaValid, isUserQuotaValid,
buildUser buildUser,
getUserByEmailPermissive
} }
// --------------------------------------------------------------------------- // ---------------------------------------------------------------------------

View File

@ -1,5 +1,6 @@
import { forceNumber } from '@peertube/peertube-core-utils' import { forceNumber } from '@peertube/peertube-core-utils'
import { HttpStatusCode, UserRightType } from '@peertube/peertube-models' import { HttpStatusCode, UserRightType } from '@peertube/peertube-models'
import { getUserByEmailPermissive } from '@server/lib/user.js'
import { ActorModel } from '@server/models/actor/actor.js' import { ActorModel } from '@server/models/actor/actor.js'
import { UserModel } from '@server/models/user/user.js' import { UserModel } from '@server/models/user/user.js'
import { MAccountId, MUserAccountId, MUserDefault } from '@server/types/models/index.js' import { MAccountId, MUserAccountId, MUserDefault } from '@server/types/models/index.js'
@ -10,8 +11,12 @@ export function checkUserIdExist (idArg: number | string, res: express.Response,
return checkUserExist(() => UserModel.loadByIdWithChannels(id, withStats), res) return checkUserExist(() => UserModel.loadByIdWithChannels(id, withStats), res)
} }
export function checkUserEmailExist (email: string, res: express.Response, abortResponse = true) { export function checkUserEmailExistPermissive (email: string, res: express.Response, abortResponse = true) {
return checkUserExist(() => UserModel.loadByEmail(email), res, abortResponse) return checkUserExist(async () => {
const users = await UserModel.loadByEmailCaseInsensitive(email)
return getUserByEmailPermissive(users, email)
}, res, abortResponse)
} }
export async function checkUserNameOrEmailDoNotAlreadyExist (username: string, email: string, res: express.Response) { export async function checkUserNameOrEmailDoNotAlreadyExist (username: string, email: string, res: express.Response) {

View File

@ -3,14 +3,19 @@ import { UserRegistrationModel } from '@server/models/user/user-registration.js'
import { MRegistration } from '@server/types/models/index.js' import { MRegistration } from '@server/types/models/index.js'
import { forceNumber, pick } from '@peertube/peertube-core-utils' import { forceNumber, pick } from '@peertube/peertube-core-utils'
import { HttpStatusCode } from '@peertube/peertube-models' import { HttpStatusCode } from '@peertube/peertube-models'
import { getUserByEmailPermissive } from '@server/lib/user.js'
function checkRegistrationIdExist (idArg: number | string, res: express.Response) { function checkRegistrationIdExist (idArg: number | string, res: express.Response) {
const id = forceNumber(idArg) const id = forceNumber(idArg)
return checkRegistrationExist(() => UserRegistrationModel.load(id), res) return checkRegistrationExist(() => UserRegistrationModel.load(id), res)
} }
function checkRegistrationEmailExist (email: string, res: express.Response, abortResponse = true) { function checkRegistrationEmailExistPermissive (email: string, res: express.Response, abortResponse = true) {
return checkRegistrationExist(() => UserRegistrationModel.loadByEmail(email), res, abortResponse) return checkRegistrationExist(async () => {
const registrations = await UserRegistrationModel.loadByEmailCaseInsensitive(email)
return getUserByEmailPermissive(registrations, email)
}, res, abortResponse)
} }
async function checkRegistrationHandlesDoNotAlreadyExist (options: { async function checkRegistrationHandlesDoNotAlreadyExist (options: {
@ -54,7 +59,7 @@ async function checkRegistrationExist (finder: () => Promise<MRegistration>, res
export { export {
checkRegistrationIdExist, checkRegistrationIdExist,
checkRegistrationEmailExist, checkRegistrationEmailExistPermissive,
checkRegistrationHandlesDoNotAlreadyExist, checkRegistrationHandlesDoNotAlreadyExist,
checkRegistrationExist checkRegistrationExist
} }

View File

@ -4,8 +4,8 @@ import { toBooleanOrNull } from '@server/helpers/custom-validators/misc.js'
import { HttpStatusCode } from '@peertube/peertube-models' import { HttpStatusCode } from '@peertube/peertube-models'
import { logger } from '../../../helpers/logger.js' import { logger } from '../../../helpers/logger.js'
import { Redis } from '../../../lib/redis.js' import { Redis } from '../../../lib/redis.js'
import { areValidationErrors, checkUserEmailExist, checkUserIdExist } from '../shared/index.js' import { areValidationErrors, checkUserEmailExistPermissive, checkUserIdExist } from '../shared/index.js'
import { checkRegistrationEmailExist, checkRegistrationIdExist } from './shared/user-registrations.js' import { checkRegistrationEmailExistPermissive, checkRegistrationIdExist } from './shared/user-registrations.js'
const usersAskSendVerifyEmailValidator = [ const usersAskSendVerifyEmailValidator = [
body('email').isEmail().not().isEmpty().withMessage('Should have a valid email'), body('email').isEmail().not().isEmpty().withMessage('Should have a valid email'),
@ -14,8 +14,8 @@ const usersAskSendVerifyEmailValidator = [
if (areValidationErrors(req, res)) return if (areValidationErrors(req, res)) return
const [ userExists, registrationExists ] = await Promise.all([ const [ userExists, registrationExists ] = await Promise.all([
checkUserEmailExist(req.body.email, res, false), checkUserEmailExistPermissive(req.body.email, res, false),
checkRegistrationEmailExist(req.body.email, res, false) checkRegistrationEmailExistPermissive(req.body.email, res, false)
]) ])
if (!userExists && !registrationExists) { if (!userExists && !registrationExists) {

View File

@ -32,7 +32,7 @@ import { ActorModel } from '../../../models/actor/actor.js'
import { import {
areValidationErrors, areValidationErrors,
checkUserCanManageAccount, checkUserCanManageAccount,
checkUserEmailExist, checkUserEmailExistPermissive,
checkUserIdExist, checkUserIdExist,
checkUserNameOrEmailDoNotAlreadyExist, checkUserNameOrEmailDoNotAlreadyExist,
doesVideoChannelIdExist, doesVideoChannelIdExist,
@ -334,7 +334,7 @@ export const usersAskResetPasswordValidator = [
async (req: express.Request, res: express.Response, next: express.NextFunction) => { async (req: express.Request, res: express.Response, next: express.NextFunction) => {
if (areValidationErrors(req, res)) return if (areValidationErrors(req, res)) return
const exists = await checkUserEmailExist(req.body.email, res, false) const exists = await checkUserEmailExistPermissive(req.body.email, res, false)
if (!exists) { if (!exists) {
logger.debug('User with email %s does not exist (asking reset password).', req.body.email) logger.debug('User with email %s does not exist (asking reset password).', req.body.email)
// Do not leak our emails // Do not leak our emails

View File

@ -8,7 +8,7 @@ import { isVideoChannelDisplayNameValid } from '@server/helpers/custom-validator
import { cryptPassword } from '@server/helpers/peertube-crypto.js' import { cryptPassword } from '@server/helpers/peertube-crypto.js'
import { USER_REGISTRATION_STATES } from '@server/initializers/constants.js' import { USER_REGISTRATION_STATES } from '@server/initializers/constants.js'
import { MRegistration, MRegistrationFormattable } from '@server/types/models/index.js' import { MRegistration, MRegistrationFormattable } from '@server/types/models/index.js'
import { FindOptions, Op, QueryTypes, WhereOptions } from 'sequelize' import { col, FindOptions, fn, Op, QueryTypes, where, WhereOptions } from 'sequelize'
import { import {
AllowNull, AllowNull,
BeforeCreate, BeforeCreate,
@ -129,12 +129,16 @@ export class UserRegistrationModel extends SequelizeModel<UserRegistrationModel>
return UserRegistrationModel.findByPk(id) return UserRegistrationModel.findByPk(id)
} }
static loadByEmail (email: string): Promise<MRegistration> { static loadByEmailCaseInsensitive (email: string): Promise<MRegistration[]> {
const query = { const query = {
where: { email } where: where(
fn('LOWER', col('email')),
'=',
email.toLowerCase()
)
} }
return UserRegistrationModel.findOne(query) return UserRegistrationModel.findAll(query)
} }
static loadByEmailOrUsername (emailOrUsername: string): Promise<MRegistration> { static loadByEmailOrUsername (emailOrUsername: string): Promise<MRegistration> {

View File

@ -673,6 +673,18 @@ export class UserModel extends SequelizeModel<UserModel> {
return UserModel.findOne(query) return UserModel.findOne(query)
} }
static loadByEmailCaseInsensitive (email: string): Promise<MUserDefault[]> {
const query = {
where: where(
fn('LOWER', col('email')),
'=',
email.toLowerCase()
)
}
return UserModel.findAll(query)
}
static loadByUsernameOrEmail (username: string, email?: string): Promise<MUserDefault> { static loadByUsernameOrEmail (username: string, email?: string): Promise<MUserDefault> {
if (!email) email = username if (!email) email = username