From 1f20622f2b087eaf8738d60fae00a44b9c558ca3 Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 7 Jun 2019 16:59:53 +0200 Subject: Improve registration * Add ability to set the user display name * Use display name to guess the username/channel name * Add explanations about what is the purpose of a username/channel name * Add a loader at the "done" step --- server/controllers/api/users/index.ts | 8 +++-- server/initializers/installer.ts | 2 +- server/initializers/migrations/0100-activitypub.ts | 9 +++-- server/lib/user.ts | 41 ++++++++++++++++------ server/middlewares/validators/users.ts | 12 +++++-- server/tests/api/check-params/users.ts | 7 ++++ server/tests/api/users/users.ts | 15 +++++--- server/tests/api/videos/video-playlists.ts | 1 - 8 files changed, 73 insertions(+), 22 deletions(-) (limited to 'server') diff --git a/server/controllers/api/users/index.ts b/server/controllers/api/users/index.ts index 48a6c63b8..99f51a648 100644 --- a/server/controllers/api/users/index.ts +++ b/server/controllers/api/users/index.ts @@ -184,7 +184,7 @@ async function createUser (req: express.Request, res: express.Response) { adminFlags: body.adminFlags || UserAdminFlag.NONE }) - const { user, account } = await createUserAccountAndChannelAndPlaylist(userToCreate) + const { user, account } = await createUserAccountAndChannelAndPlaylist({ userToCreate: userToCreate }) auditLogger.create(getAuditIdFromRes(res), new UserAuditView(user.toFormattedJSON())) logger.info('User %s with its channel and account created.', body.username) @@ -214,7 +214,11 @@ async function registerUser (req: express.Request, res: express.Response) { emailVerified: CONFIG.SIGNUP.REQUIRES_EMAIL_VERIFICATION ? false : null }) - const { user } = await createUserAccountAndChannelAndPlaylist(userToCreate, body.channel) + const { user } = await createUserAccountAndChannelAndPlaylist({ + userToCreate: userToCreate, + userDisplayName: body.displayName || undefined, + channelNames: body.channel + }) auditLogger.create(body.username, new UserAuditView(user.toFormattedJSON())) logger.info('User %s with its channel and account registered.', body.username) diff --git a/server/initializers/installer.ts b/server/initializers/installer.ts index e14554ede..cb58454cb 100644 --- a/server/initializers/installer.ts +++ b/server/initializers/installer.ts @@ -146,7 +146,7 @@ async function createOAuthAdminIfNotExist () { } const user = new UserModel(userData) - await createUserAccountAndChannelAndPlaylist(user, undefined, validatePassword) + await createUserAccountAndChannelAndPlaylist({ userToCreate: user, channelNames: undefined, validateUser: validatePassword }) logger.info('Username: ' + username) logger.info('User password: ' + password) } diff --git a/server/initializers/migrations/0100-activitypub.ts b/server/initializers/migrations/0100-activitypub.ts index 2880a97d9..96d44a7ce 100644 --- a/server/initializers/migrations/0100-activitypub.ts +++ b/server/initializers/migrations/0100-activitypub.ts @@ -65,7 +65,12 @@ async function up (utils: { // Create application account { const applicationInstance = await ApplicationModel.findOne() - const accountCreated = await createLocalAccountWithoutKeys(SERVER_ACTOR_NAME, null, applicationInstance.id, undefined) + const accountCreated = await createLocalAccountWithoutKeys({ + name: SERVER_ACTOR_NAME, + userId: null, + applicationId: applicationInstance.id, + t: undefined + }) const { publicKey, privateKey } = await createPrivateAndPublicKeys() accountCreated.Actor.publicKey = publicKey @@ -83,7 +88,7 @@ async function up (utils: { // Recreate accounts for each user const users = await db.User.findAll() for (const user of users) { - const account = await createLocalAccountWithoutKeys(user.username, user.id, null, undefined) + const account = await createLocalAccountWithoutKeys({ name: user.username, userId: user.id, applicationId: null, t: undefined }) const { publicKey, privateKey } = await createPrivateAndPublicKeys() account.Actor.publicKey = publicKey diff --git a/server/lib/user.ts b/server/lib/user.ts index d9fd89e15..b50b09d72 100644 --- a/server/lib/user.ts +++ b/server/lib/user.ts @@ -1,4 +1,3 @@ -import * as Sequelize from 'sequelize' import * as uuidv4 from 'uuid/v4' import { ActivityPubActorType } from '../../shared/models/activitypub' import { SERVER_ACTOR_NAME } from '../initializers/constants' @@ -12,9 +11,17 @@ import { UserNotificationSettingModel } from '../models/account/user-notificatio import { UserNotificationSetting, UserNotificationSettingValue } from '../../shared/models/users' import { createWatchLaterPlaylist } from './video-playlist' import { sequelizeTypescript } from '../initializers/database' +import { Transaction } from 'sequelize/types' type ChannelNames = { name: string, displayName: string } -async function createUserAccountAndChannelAndPlaylist (userToCreate: UserModel, channelNames?: ChannelNames, validateUser = true) { +async function createUserAccountAndChannelAndPlaylist (parameters: { + userToCreate: UserModel, + userDisplayName?: string, + channelNames?: ChannelNames, + validateUser?: boolean +}) { + const { userToCreate, userDisplayName, channelNames, validateUser = true } = parameters + const { user, account, videoChannel } = await sequelizeTypescript.transaction(async t => { const userOptions = { transaction: t, @@ -24,7 +31,13 @@ async function createUserAccountAndChannelAndPlaylist (userToCreate: UserModel, const userCreated = await userToCreate.save(userOptions) userCreated.NotificationSetting = await createDefaultUserNotificationSettings(userCreated, t) - const accountCreated = await createLocalAccountWithoutKeys(userCreated.username, userCreated.id, null, t) + const accountCreated = await createLocalAccountWithoutKeys({ + name: userCreated.username, + displayName: userDisplayName, + userId: userCreated.id, + applicationId: null, + t: t + }) userCreated.Account = accountCreated const channelAttributes = await buildChannelAttributes(userCreated, channelNames) @@ -46,20 +59,22 @@ async function createUserAccountAndChannelAndPlaylist (userToCreate: UserModel, return { user, account, videoChannel } as { user: UserModel, account: AccountModel, videoChannel: VideoChannelModel } } -async function createLocalAccountWithoutKeys ( +async function createLocalAccountWithoutKeys (parameters: { name: string, + displayName?: string, userId: number | null, applicationId: number | null, - t: Sequelize.Transaction | undefined, - type: ActivityPubActorType= 'Person' -) { + t: Transaction | undefined, + type?: ActivityPubActorType +}) { + const { name, displayName, userId, applicationId, t, type = 'Person' } = parameters const url = getAccountActivityPubUrl(name) const actorInstance = buildActorInstance(type, url, name) const actorInstanceCreated = await actorInstance.save({ transaction: t }) const accountInstance = new AccountModel({ - name, + name: displayName || name, userId, applicationId, actorId: actorInstanceCreated.id @@ -72,7 +87,13 @@ async function createLocalAccountWithoutKeys ( } async function createApplicationActor (applicationId: number) { - const accountCreated = await createLocalAccountWithoutKeys(SERVER_ACTOR_NAME, null, applicationId, undefined, 'Application') + const accountCreated = await createLocalAccountWithoutKeys({ + name: SERVER_ACTOR_NAME, + userId: null, + applicationId: applicationId, + t: undefined, + type: 'Application' + }) accountCreated.Actor = await setAsyncActorKeys(accountCreated.Actor) @@ -89,7 +110,7 @@ export { // --------------------------------------------------------------------------- -function createDefaultUserNotificationSettings (user: UserModel, t: Sequelize.Transaction | undefined) { +function createDefaultUserNotificationSettings (user: UserModel, t: Transaction | undefined) { const values: UserNotificationSetting & { userId: number } = { userId: user.id, newVideoFromSubscription: UserNotificationSettingValue.WEB, diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index 7a081af33..b4e09c9b7 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -53,8 +53,16 @@ const usersRegisterValidator = [ body('username').custom(isUserUsernameValid).withMessage('Should have a valid username'), body('password').custom(isUserPasswordValid).withMessage('Should have a valid password'), body('email').isEmail().withMessage('Should have a valid email'), - body('channel.name').optional().custom(isActorPreferredUsernameValid).withMessage('Should have a valid channel name'), - body('channel.displayName').optional().custom(isVideoChannelNameValid).withMessage('Should have a valid display name'), + body('displayName') + .optional() + .custom(isUserDisplayNameValid).withMessage('Should have a valid display name'), + + body('channel.name') + .optional() + .custom(isActorPreferredUsernameValid).withMessage('Should have a valid channel name'), + body('channel.displayName') + .optional() + .custom(isVideoChannelNameValid).withMessage('Should have a valid display name'), async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking usersRegister parameters', { parameters: omit(req.body, 'password') }) diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 95097817b..3268f8c90 100644 --- a/server/tests/api/check-params/users.ts +++ b/server/tests/api/check-params/users.ts @@ -643,6 +643,7 @@ describe('Test users API validators', function () { const registrationPath = path + '/register' const baseCorrectParams = { username: 'user3', + displayName: 'super user', email: 'test3@example.com', password: 'my super password' } @@ -725,6 +726,12 @@ describe('Test users API validators', function () { }) }) + it('Should fail with a bad display name', async function () { + const fields = immutableAssign(baseCorrectParams, { displayName: 'a'.repeat(150) }) + + await makePostBodyRequest({ url: server.url, path: registrationPath, token: server.accessToken, fields }) + }) + it('Should fail with a bad channel name', async function () { const fields = immutableAssign(baseCorrectParams, { channel: { name: '[]azf', displayName: 'toto' } }) diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index 9d2ef786f..b1f214fe2 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -17,11 +17,12 @@ import { getUserInformation, getUsersList, getUsersListPaginationAndSort, + getVideoChannel, getVideosList, login, makePutBodyRequest, rateVideo, - registerUser, + registerUserWithChannel, removeUser, removeVideo, ServerInfo, @@ -31,8 +32,7 @@ import { updateMyUser, updateUser, uploadVideo, - userLogin, - registerUserWithChannel, getVideoChannel + userLogin } from '../../../../shared/extra-utils' import { follow } from '../../../../shared/extra-utils/server/follows' import { setAccessTokensToServers } from '../../../../shared/extra-utils/users/login' @@ -618,7 +618,7 @@ describe('Test users', function () { describe('Registering a new user', function () { it('Should register a new user', async function () { - const user = { username: 'user_15', password: 'my super password' } + const user = { displayName: 'super user 15', username: 'user_15', password: 'my super password' } const channel = { name: 'my_user_15_channel', displayName: 'my channel rocks' } await registerUserWithChannel({ url: server.url, user, channel }) @@ -633,6 +633,13 @@ describe('Test users', function () { accessToken = await userLogin(server, user15) }) + it('Should have the correct display name', async function () { + const res = await getMyUserInformation(server.url, accessToken) + const user: User = res.body + + expect(user.account.displayName).to.equal('super user 15') + }) + it('Should have the correct video quota', async function () { const res = await getMyUserInformation(server.url, accessToken) const user = res.body diff --git a/server/tests/api/videos/video-playlists.ts b/server/tests/api/videos/video-playlists.ts index 8690327c4..f82c8cbce 100644 --- a/server/tests/api/videos/video-playlists.ts +++ b/server/tests/api/videos/video-playlists.ts @@ -754,7 +754,6 @@ describe('Test video playlists', function () { } }) - it('Should be able to create a public playlist, and set it to private', async function () { this.timeout(30000) -- cgit v1.2.3