add filter:api.user.signup.requires-approval.result

closes #6691

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

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<UserRegistrationServerModel>(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: {

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,

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
 

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
     })
   }
 

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',

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' })

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: '[email protected]',
+          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: '[email protected]',
+          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: '[email protected]',
+        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: '[email protected]',
           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: '[email protected]',
           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: '[email protected]',
           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' })

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 () {