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

kontrollanten · kontrollanten · commit f8259c536e87 · 2025-01-24T06:41:38.000+01:00
closes Chocobozzz#6691
diff --git 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
@@ -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: {
diff --git 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
@@ -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
@@ -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
@@ -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 === 'waiting_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
@@ -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/server/email.ts b/packages/tests/src/api/server/email.ts
@@ -314,12 +314,13 @@ describe('Test emails', function () {
     let registrationIdEmail: number
 
     before(async function () {
-      const { id } = await server.registrations.requestRegistration({
+      await server.registrations.requestRegistration({
         username: 'request_1',
         email: 'request_1@example.com',
         registrationReason: 'tt'
       })
-      registrationId = id
+      const registrations = await server.registrations.list()
+      registrationId = registrations.data[0].id
     })
 
     it('Should ask to send the verification email', async function () {
diff --git a/packages/tests/src/api/server/reverse-proxy.ts b/packages/tests/src/api/server/reverse-proxy.ts
@@ -124,7 +124,7 @@ describe('Test application behind a reverse proxy', function () {
       await server.registrations.register({ username: 'test' + i, expectedStatus: HttpStatusCode.CONFLICT_409 })
     }
 
-    await server.registrations.register({ username: 'test43', expectedStatus: HttpStatusCode.NO_CONTENT_204 })
+    await server.registrations.register({ username: 'test43', expectedStatus: HttpStatusCode.OK_200 })
 
   })
 
diff --git a/packages/tests/src/api/server/stats.ts b/packages/tests/src/api/server/stats.ts
@@ -316,9 +316,12 @@ describe('Test stats (excluding redundancy)', function () {
     it('Should create registration requests, accept one and have correct stats', async function () {
       beforeTimestamp = new Date().getTime()
 
-      const { id: id1 } = await servers[0].registrations.requestRegistration({ username: 'user2', registrationReason: 'reason 1' });
-      ({ id: id2 } = await servers[0].registrations.requestRegistration({ username: 'user3', registrationReason: 'reason 2' }))
-      await servers[0].registrations.requestRegistration({ username: 'user4', registrationReason: 'reason 3' })
+      await servers[0].registrations.register({ username: 'user2', registrationReason: 'reason 1' })
+      await servers[0].registrations.register({ username: 'user3', registrationReason: 'reason 2' })
+      const registrations = await servers[0].registrations.list()
+      const id1 = registrations.data[1].id
+      id2 = registrations.data[0].id
+      await servers[0].registrations.register({ username: 'user4', registrationReason: 'reason 3' })
 
       await wait(1500)
 
diff --git a/packages/tests/src/api/users/registrations.ts b/packages/tests/src/api/users/registrations.ts
diff --git a/packages/tests/src/plugins/filter-hooks.ts b/packages/tests/src/plugins/filter-hooks.ts
diff --git a/server/core/controllers/api/users/registrations.ts b/server/core/controllers/api/users/registrations.ts
diff --git a/server/core/middlewares/validators/users/user-registrations.ts b/server/core/middlewares/validators/users/user-registrations.ts
diff --git a/server/core/types/express.d.ts b/server/core/types/express.d.ts

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ describe('Test application behind a reverse proxy', function () {`
`124`	`124`	`await server.registrations.register({ username: 'test' + i, expectedStatus: HttpStatusCode.CONFLICT_409 })`
`125`	`125`	`}`
`126`	`126`
`127`		`- await server.registrations.register({ username: 'test43', expectedStatus: HttpStatusCode.NO_CONTENT_204 })`
	`127`	`+ await server.registrations.register({ username: 'test43', expectedStatus: HttpStatusCode.OK_200 })`
`128`	`128`
`129`	`129`	`})`
`130`	`130`