diff --git a/client/src/app/+signup/+register/register.component.ts b/client/src/app/+signup/+register/register.component.ts index 9ffdcb21b0b..33967d6ced8 100644 --- a/client/src/app/+signup/+register/register.component.ts +++ b/client/src/app/+signup/+register/register.component.ts @@ -7,7 +7,7 @@ import { AuthService, ServerService } from '@app/core' import { HooksService } from '@app/core/plugins/hooks.service' import { InstanceAboutAccordionComponent } from '@app/shared/shared-instance/instance-about-accordion.component' import { AlertComponent } from '@app/shared/shared-main/common/alert.component' -import { PeerTubeProblemDocument, ServerConfig, ServerStats, UserRegister } from '@peertube/peertube-models' +import { UserRegistrationState, PeerTubeProblemDocument, ServerConfig, ServerStats, UserRegister } from '@peertube/peertube-models' import { LoaderComponent } from '../../shared/shared-main/common/loader.component' import { SignupLabelComponent } from '../../shared/shared-main/users/signup-label.component' import { SignupStepTitleComponent } from '../shared/signup-step-title.component' @@ -78,6 +78,7 @@ export class RegisterComponent implements OnInit { serverStats: ServerStats private serverConfig: ServerConfig + private _requiresApproval: boolean constructor ( private route: ActivatedRoute, @@ -92,7 +93,11 @@ export class RegisterComponent implements OnInit { } get requiresApproval () { - return this.serverConfig.signup.requiresApproval + return this._requiresApproval ?? this.serverConfig.signup.requiresApproval + } + + set requiresApproval (value: boolean) { + this._requiresApproval = value } get minimumAge () { @@ -197,12 +202,13 @@ export class RegisterComponent implements OnInit { 'filter:api.signup.registration.create.params' ) - const obs = this.requiresApproval - ? this.signupService.requestSignup(body) - : this.signupService.directSignup(body) + const obs = this.signupService.signup(body) obs.subscribe({ - next: () => { + next: (registration) => { + const { state } = registration + this.requiresApproval = state.id === UserRegistrationState.PENDING + if (this.requiresEmailVerification || this.requiresApproval) { this.signupSuccess = true return diff --git a/client/src/app/+signup/shared/signup.service.ts b/client/src/app/+signup/shared/signup.service.ts index 7c331437e00..9edafb279cb 100644 --- a/client/src/app/+signup/shared/signup.service.ts +++ b/client/src/app/+signup/shared/signup.service.ts @@ -2,7 +2,7 @@ import { catchError, tap } from 'rxjs/operators' import { HttpClient } from '@angular/common/http' import { Injectable } from '@angular/core' import { RestExtractor, UserService } from '@app/core' -import { UserRegister, UserRegistrationRequest } from '@peertube/peertube-models' +import { UserRegister, UserRegistration as UserRegistrationServerModel } from '@peertube/peertube-models' @Injectable() export class SignupService { @@ -13,19 +13,14 @@ export class SignupService { private userService: UserService ) { } - directSignup (userCreate: UserRegister) { - return this.authHttp.post(UserService.BASE_USERS_URL + 'register', userCreate) + signup (userCreate: UserRegister) { + return this.authHttp.post(UserService.BASE_USERS_URL + 'register', userCreate) .pipe( tap(() => this.userService.setSignupInThisSession(true)), catchError(err => this.restExtractor.handleError(err)) ) } - requestSignup (userCreate: UserRegistrationRequest) { - return this.authHttp.post(UserService.BASE_USERS_URL + 'registrations/request', userCreate) - .pipe(catchError(err => this.restExtractor.handleError(err))) - } - // --------------------------------------------------------------------------- verifyUserEmail (options: { diff --git a/packages/models/src/plugins/server/server-hook.model.ts b/packages/models/src/plugins/server/server-hook.model.ts index 1d4f7917aba..ac069fcde1f 100644 --- a/packages/models/src/plugins/server/server-hook.model.ts +++ b/packages/models/src/plugins/server/server-hook.model.ts @@ -100,6 +100,9 @@ export const serverFilterHookObject = { // Filter result used to check if a user can register on the instance 'filter:api.user.signup.allowed.result': true, + // Filter result used to check if signup requires approval on the instance + 'filter:api.user.signup.requires-approval.result': true, + // Filter result used to check if a user can send a registration request on the instance // PeerTube >= 5.1 'filter:api.user.request-signup.allowed.result': true, diff --git a/packages/models/src/users/registration/user-registration.model.ts b/packages/models/src/users/registration/user-registration.model.ts index 0d01add36de..aa76f85fc9e 100644 --- a/packages/models/src/users/registration/user-registration.model.ts +++ b/packages/models/src/users/registration/user-registration.model.ts @@ -1,5 +1,12 @@ import { UserRegistrationStateType } from './user-registration-state.model.js' +export interface UserRegistrationResponse { + state: { + id: UserRegistrationStateType + label: string + } +} + export interface UserRegistration { id: number diff --git a/packages/server-commands/src/users/registrations-command.ts b/packages/server-commands/src/users/registrations-command.ts index 2111fbd39bd..8e8e1aaefd8 100644 --- a/packages/server-commands/src/users/registrations-command.ts +++ b/packages/server-commands/src/users/registrations-command.ts @@ -20,13 +20,13 @@ export class RegistrationsCommand extends AbstractCommand { path, fields: { - ...pick(options, [ 'username', 'displayName', 'channel' ]), + ...pick(options, [ 'username', 'displayName', 'channel', 'registrationReason' ]), password, email }, implicitToken: false, - defaultExpectedStatus: HttpStatusCode.NO_CONTENT_204 + defaultExpectedStatus: HttpStatusCode.OK_200 }) } diff --git a/packages/tests/fixtures/peertube-plugin-test/main.js b/packages/tests/fixtures/peertube-plugin-test/main.js index e031d2b1cc4..e4a58f30fc9 100644 --- a/packages/tests/fixtures/peertube-plugin-test/main.js +++ b/packages/tests/fixtures/peertube-plugin-test/main.js @@ -320,6 +320,16 @@ async function register ({ registerHook, registerSetting, settingsManager, stora } }) + registerHook({ + target: 'filter:api.user.signup.requires-approval.result', + handler: ({ requiresApproval, registrationReason }, { body, headers }) => { + return { + requiresApproval: body.username !== 'welcome_john', + registrationReason: 'Marked as spam' + } + } + }) + { registerHook({ target: 'filter:api.user.signup.allowed.result', diff --git a/packages/tests/src/api/check-params/registrations.ts b/packages/tests/src/api/check-params/registrations.ts index 1e78bdae34f..04e3628cb84 100644 --- a/packages/tests/src/api/check-params/registrations.ts +++ b/packages/tests/src/api/check-params/registrations.ts @@ -190,10 +190,10 @@ describe('Test registrations API validators', function () { const { total } = await server.users.list() await server.config.enableSignup(false, total + 1) - await server.registrations.register({ username: 'user43', expectedStatus: HttpStatusCode.NO_CONTENT_204 }) + await server.registrations.register({ username: 'user43', expectedStatus: HttpStatusCode.OK_200 }) await server.config.enableSignup(true, total + 2) - await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user44', registrationReason: 'reason', expectedStatus: HttpStatusCode.OK_200 @@ -214,14 +214,14 @@ describe('Test registrations API validators', function () { channel: { name: 'super_user_direct_1_channel', displayName: 'super user direct 1 channel' } } - await makePostBodyRequest({ url: server.url, path: registrationPath, fields, expectedStatus: HttpStatusCode.NO_CONTENT_204 }) + await makePostBodyRequest({ url: server.url, path: registrationPath, fields, expectedStatus: HttpStatusCode.OK_200 }) }) - it('Should fail if the instance requires approval', async function () { + it('Should fail if registration reason isnt provided', async function () { this.timeout(60000) await server.config.enableSignup(true) - await server.registrations.register({ username: 'user42', expectedStatus: HttpStatusCode.FORBIDDEN_403 }) + await server.registrations.register({ username: 'user42', expectedStatus: HttpStatusCode.BAD_REQUEST_400 }) }) }) @@ -312,10 +312,15 @@ describe('Test registrations API validators', function () { before(async function () { this.timeout(60000) - await server.config.enableSignup(true); + await server.config.enableSignup(true) + + await server.registrations.requestRegistration({ username: 'request_2', registrationReason: 'toto' }) + await server.registrations.requestRegistration({ username: 'request_3', registrationReason: 'toto' }) - ({ id: id1 } = await server.registrations.requestRegistration({ username: 'request_2', registrationReason: 'toto' })); - ({ id: id2 } = await server.registrations.requestRegistration({ username: 'request_3', registrationReason: 'toto' })) + const registrations = await server.registrations.list() + + id1 = registrations.data[0].id + id2 = registrations.data[1].id }) it('Should fail to accept/reject registration without token', async function () { @@ -375,9 +380,15 @@ describe('Test registrations API validators', function () { let id3: number before(async function () { - ({ id: id1 } = await server.registrations.requestRegistration({ username: 'request_4', registrationReason: 'toto' })); - ({ id: id2 } = await server.registrations.requestRegistration({ username: 'request_5', registrationReason: 'toto' })); - ({ id: id3 } = await server.registrations.requestRegistration({ username: 'request_6', registrationReason: 'toto' })) + await server.registrations.requestRegistration({ username: 'request_4', registrationReason: 'toto' }) + await server.registrations.requestRegistration({ username: 'request_5', registrationReason: 'toto' }) + await server.registrations.requestRegistration({ username: 'request_6', registrationReason: 'toto' }) + + const registrations = await server.registrations.list() + + id1 = registrations.data[0].id + id2 = registrations.data[1].id + id3 = registrations.data[2].id await server.registrations.accept({ id: id2, moderationResponse: 'tt' }) await server.registrations.reject({ id: id3, moderationResponse: 'tt' }) diff --git a/packages/tests/src/api/users/registrations.ts b/packages/tests/src/api/users/registrations.ts index dcf09cade6e..d73d8b67f53 100644 --- a/packages/tests/src/api/users/registrations.ts +++ b/packages/tests/src/api/users/registrations.ts @@ -76,6 +76,56 @@ describe('Test registrations', function () { }) }) + describe('Deprecated registration requests', function () { + before(async function () { + this.timeout(60000) + + await server.config.enableSignup(true) + + { + await server.registrations.requestRegistration({ + username: 'deprecated_user4', + email: 'deprecated_user4@example.com', + registrationReason: 'registration reason 4' + }) + } + }) + + it('Should request a registration without a channel', async function () { + { + await server.registrations.requestRegistration({ + username: 'deprecated_user2', + displayName: 'my super deprecated_user 2', + email: 'deprecated_user2@example.com', + password: 'deprecated_user2password', + registrationReason: 'registration reason 2' + }) + } + }) + + it('Should request a registration with a channel', async function () { + await server.registrations.requestRegistration({ + username: 'deprecated_user3', + displayName: 'my super deprecated_user 3', + channel: { + displayName: 'my deprecated_user 3 channel', + name: 'super_deprecated_user3_channel' + }, + email: 'deprecated_user3@example.com', + password: 'deprecated_user3password', + registrationReason: 'registration reason 3' + }) + }) + + after(async function () { + const registrations = await server.registrations.list() + + for (const reg of registrations.data) { + await server.registrations.delete({ id: reg.id }) + } + }) + }) + describe('Registration requests', function () { let id2: number let id3: number @@ -90,31 +140,33 @@ describe('Test registrations', function () { await server.config.enableSignup(true) { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user4', registrationReason: 'registration reason 4' }) + const registrations = await server.registrations.list() - id4 = id + id4 = registrations.data[0].id } }) it('Should request a registration without a channel', async function () { { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user2', displayName: 'my super user 2', email: 'user2@example.com', password: 'user2password', registrationReason: 'registration reason 2' }) + const registrations = await server.registrations.list() - id2 = id + id2 = registrations.data[0].id } }) it('Should request a registration with a channel', async function () { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user3', displayName: 'my super user 3', channel: { @@ -126,7 +178,8 @@ describe('Test registrations', function () { registrationReason: 'registration reason 3' }) - id3 = id + const registrations = await server.registrations.list() + id3 = registrations.data[0].id }) it('Should list these registration requests', async function () { @@ -336,20 +389,22 @@ describe('Test registrations', function () { let id2: number { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user7', email: 'user7@example.com', registrationReason: 'tt' }) - id1 = id + const registrations = await server.registrations.list() + id1 = registrations.data[0].id } { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ username: 'user8', email: 'user8@example.com', registrationReason: 'tt' }) - id2 = id + const registrations = await server.registrations.list() + id2 = registrations.data[0].id } await server.registrations.accept({ id: id1, moderationResponse: 'tt', preventEmailDelivery: true }) @@ -370,7 +425,7 @@ describe('Test registrations', function () { let id2: number { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ registrationReason: 'tt', username: 'user5', password: 'user5password', @@ -380,17 +435,19 @@ describe('Test registrations', function () { } }) - id1 = id + const registrations = await server.registrations.list() + id1 = registrations.data[0].id } { - const { id } = await server.registrations.requestRegistration({ + await server.registrations.register({ registrationReason: 'tt', username: 'user6', password: 'user6password' }) - id2 = id + const registrations = await server.registrations.list() + id2 = registrations.data[0].id } await server.registrations.accept({ id: id1, moderationResponse: 'tt' }) diff --git a/packages/tests/src/plugins/filter-hooks.ts b/packages/tests/src/plugins/filter-hooks.ts index 497fbcc9fb9..ab2e1942fd7 100644 --- a/packages/tests/src/plugins/filter-hooks.ts +++ b/packages/tests/src/plugins/filter-hooks.ts @@ -478,6 +478,21 @@ describe('Test plugin filter hooks', function () { }) }) + describe('Should run filter:api.user.signup.requires-approval.result', function () { + + before(async function () { + await servers[0].config.updateExistingConfig({ newConfig: { signup: { requiresApproval: false } } }) + }) + + it('Should allow a signup', async function () { + await servers[0].registrations.register({ username: 'welcome_john' }) + }) + + it('Should not allow a signup', async function () { + await servers[0].registrations.register({ username: 'anybody' }) + }) + }) + describe('Should run filter:api.user.signup.allowed.result', function () { before(async function () { diff --git a/server/core/controllers/api/users/registrations.ts b/server/core/controllers/api/users/registrations.ts index e38b215a754..83e7e480a3c 100644 --- a/server/core/controllers/api/users/registrations.ts +++ b/server/core/controllers/api/users/registrations.ts @@ -3,6 +3,7 @@ import { Emailer } from '@server/lib/emailer.js' import { Hooks } from '@server/lib/plugins/hooks.js' import { UserRegistrationModel } from '@server/models/user/user-registration.js' import { pick } from '@peertube/peertube-core-utils' +import { USER_REGISTRATION_STATES } from '@server/initializers/constants.js' import { HttpStatusCode, UserRegister, @@ -22,6 +23,7 @@ import { asyncRetryTransactionMiddleware, authenticate, buildRateLimiter, + determineSignupMode, ensureUserHasRight, ensureUserRegistrationAllowedFactory, ensureUserRegistrationAllowedForIP, @@ -31,7 +33,7 @@ import { setDefaultPagination, setDefaultSort, userRegistrationsSortValidator, - usersDirectRegistrationValidator, + usersRegistrationValidator, usersRequestRegistrationValidator } from '../../../middlewares/index.js' @@ -86,9 +88,10 @@ registrationsRouter.get('/registrations', registrationsRouter.post('/register', registrationRateLimiter, - asyncMiddleware(ensureUserRegistrationAllowedFactory('direct-registration')), + determineSignupMode, + asyncMiddleware(ensureUserRegistrationAllowedFactory()), ensureUserRegistrationAllowedForIP, - asyncMiddleware(usersDirectRegistrationValidator), + usersRegistrationValidator, asyncRetryTransactionMiddleware(registerUser) ) @@ -125,7 +128,12 @@ async function requestRegistration (req: express.Request, res: express.Response) Hooks.runAction('action:api.user.requested-registration', { body, registration, req, res }) - return res.json(registration.toFormattedJSON()) + return res.json({ + state: { + id: UserRegistrationState.PENDING, + label: USER_REGISTRATION_STATES[UserRegistrationState.PENDING] + } + }) } // --------------------------------------------------------------------------- @@ -224,6 +232,10 @@ async function listRegistrations (req: express.Request, res: express.Response) { // --------------------------------------------------------------------------- async function registerUser (req: express.Request, res: express.Response) { + if (res.locals.signupMode === 'request-registration') { + return requestRegistration(req, res) + } + const body: UserRegister = req.body const userToCreate = buildUser({ @@ -249,5 +261,10 @@ async function registerUser (req: express.Request, res: express.Response) { Hooks.runAction('action:api.user.registered', { body, user, account, videoChannel, req, res }) - return res.sendStatus(HttpStatusCode.NO_CONTENT_204) + return res.json({ + state: { + id: UserRegistrationState.ACCEPTED, + label: USER_REGISTRATION_STATES[UserRegistrationState.ACCEPTED] + } + }) } diff --git a/server/core/middlewares/validators/users/user-registrations.ts b/server/core/middlewares/validators/users/user-registrations.ts index c9d06ba992c..6cc3329440a 100644 --- a/server/core/middlewares/validators/users/user-registrations.ts +++ b/server/core/middlewares/validators/users/user-registrations.ts @@ -11,8 +11,16 @@ import { isSignupAllowed, isSignupAllowedForCurrentIP, SignupMode } from '../../ import { ActorModel } from '../../../models/actor/actor.js' import { areValidationErrors, checkUserNameOrEmailDoNotAlreadyExist } from '../shared/index.js' import { checkRegistrationHandlesDoNotAlreadyExist, checkRegistrationIdExist } from './shared/user-registrations.js' +import { pick } from '@peertube/peertube-core-utils' +import { asyncMiddleware } from '@server/middlewares/async.js' -const usersDirectRegistrationValidator = usersCommonRegistrationValidatorFactory() +const usersRegistrationValidator = async (req: express.Request, res: express.Response, next: express.NextFunction) => { + if (res.locals.signupMode === 'direct-registration') { + return asyncMiddleware(usersCommonRegistrationValidatorFactory())(req, res, next) + } + + return asyncMiddleware(usersRequestRegistrationValidator)(req, res, next) +} const usersRequestRegistrationValidator = [ ...usersCommonRegistrationValidatorFactory([ @@ -22,8 +30,10 @@ const usersRequestRegistrationValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { const body: UserRegistrationRequest = req.body + // CONFIG.SIGNUP.REQUIRES_APPROVAL is only used for deprecated /registrations/request endpoint + const requiresApproval = (res.locals.signupMode && res.locals.signupMode === 'request-registration') || CONFIG.SIGNUP.REQUIRES_APPROVAL - if (CONFIG.SIGNUP.REQUIRES_APPROVAL !== true) { + if (requiresApproval !== true) { return res.fail({ status: HttpStatusCode.BAD_REQUEST_400, message: 'Signup approval is not enabled on this instance' @@ -39,8 +49,30 @@ const usersRequestRegistrationValidator = [ // --------------------------------------------------------------------------- -function ensureUserRegistrationAllowedFactory (signupMode: SignupMode) { +const determineSignupMode = [ + async (req: express.Request, res: express.Response, next: express.NextFunction) => { + const { registrationReason, requiresApproval } = await Hooks.wrapObject( + { + requiresApproval: CONFIG.SIGNUP.REQUIRES_APPROVAL, + registrationReason: req.body.registrationReason + }, + 'filter:api.user.signup.requires-approval.result', + { + body: pick(req.body, [ 'username', 'email', 'registrationReason' ]), + headers: req.headers + } + ) + + req.body.registrationReason = registrationReason + res.locals.signupMode = requiresApproval ? 'request-registration' : 'direct-registration' + + return next() + } +] + +function ensureUserRegistrationAllowedFactory (sm?: SignupMode) { return async (req: express.Request, res: express.Response, next: express.NextFunction) => { + const signupMode = sm || res.locals.signupMode // sm is provided by deprecated /registrations/request endpoint const allowedParams = { body: req.body, ip: req.ip, @@ -142,7 +174,9 @@ const listRegistrationsValidator = [ // --------------------------------------------------------------------------- export { - usersDirectRegistrationValidator, + determineSignupMode, + + usersRegistrationValidator, usersRequestRegistrationValidator, ensureUserRegistrationAllowedFactory, diff --git a/server/core/types/express.d.ts b/server/core/types/express.d.ts index a1455c4a04b..6546bca9c82 100644 --- a/server/core/types/express.d.ts +++ b/server/core/types/express.d.ts @@ -50,6 +50,7 @@ import { } from './models/index.js' import { MRunner, MRunnerJobRunner, MRunnerRegistrationToken } from './models/runners/index.js' import { MVideoSource } from './models/video/video-source.js' +import { SignupMode } from '@server/lib/signup.ts' declare module 'express' { export interface Request { @@ -116,6 +117,8 @@ declare module 'express' { }) => void locals: { + signupMode?: SignupMode + requestStart: number apicacheGroups: string[]