From 9e5cf66be7ad897e106f283bee73a165c72e74de Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Fri, 28 Oct 2022 16:15:04 +0200 Subject: [PATCH] Breaking API: Consistency with role id/label --- .../overview/users/user-edit/user-edit.ts | 2 +- .../users/user-edit/user-update.component.ts | 2 +- .../users/user-list/user-list.component.html | 4 ++-- client/src/app/app.component.ts | 4 ++-- client/src/app/core/auth/auth-user.model.ts | 6 +++--- .../core/users/user-local-storage.service.ts | 11 ++++++++--- client/src/app/core/users/user.model.ts | 10 ++++++---- .../shared/shared-users/user-admin.service.ts | 5 ++++- server/models/user/user.ts | 6 ++++-- server/tests/api/users/users.ts | 6 +++--- server/tests/plugins/external-auth.ts | 6 +++--- server/tests/plugins/id-and-pass-auth.ts | 8 ++++---- shared/models/users/user.model.ts | 6 ++++-- support/doc/api/openapi.yaml | 17 ++++++++++------- 14 files changed, 55 insertions(+), 38 deletions(-) diff --git a/client/src/app/+admin/overview/users/user-edit/user-edit.ts b/client/src/app/+admin/overview/users/user-edit/user-edit.ts index 5428f6ce9..1edca7fbf 100644 --- a/client/src/app/+admin/overview/users/user-edit/user-edit.ts +++ b/client/src/app/+admin/overview/users/user-edit/user-edit.ts @@ -49,7 +49,7 @@ export abstract class UserEdit extends FormReactive implements OnInit { buildRoles () { const authUser = this.auth.getUser() - if (authUser.role === UserRole.ADMINISTRATOR) { + if (authUser.role.id === UserRole.ADMINISTRATOR) { this.roles = Object.keys(USER_ROLE_LABELS) .map(key => ({ value: key.toString(), label: USER_ROLE_LABELS[key] })) return diff --git a/client/src/app/+admin/overview/users/user-edit/user-update.component.ts b/client/src/app/+admin/overview/users/user-edit/user-update.component.ts index 71212b19c..25d02f000 100644 --- a/client/src/app/+admin/overview/users/user-edit/user-update.component.ts +++ b/client/src/app/+admin/overview/users/user-edit/user-update.component.ts @@ -144,7 +144,7 @@ export class UserUpdateComponent extends UserEdit implements OnInit, OnDestroy { this.form.patchValue({ email: userJson.email, - role: userJson.role.toString(), + role: userJson.role.id.toString(), videoQuota: userJson.videoQuota, videoQuotaDaily: userJson.videoQuotaDaily, pluginAuth: userJson.pluginAuth, diff --git a/client/src/app/+admin/overview/users/user-list/user-list.component.html b/client/src/app/+admin/overview/users/user-list/user-list.component.html index c7af7dfae..a96ce561c 100644 --- a/client/src/app/+admin/overview/users/user-list/user-list.component.html +++ b/client/src/app/+admin/overview/users/user-list/user-list.component.html @@ -106,8 +106,8 @@ - {{ user.roleLabel }} - {{ user.roleLabel }} + {{ user.role.label }} + {{ user.role.label }} diff --git a/client/src/app/app.component.ts b/client/src/app/app.component.ts index a2ad4806c..f2488aa59 100644 --- a/client/src/app/app.component.ts +++ b/client/src/app/app.component.ts @@ -247,12 +247,12 @@ export class AppComponent implements OnInit, AfterViewInit { // Admin modal userSub.pipe( - filter(user => user.role === UserRole.ADMINISTRATOR) + filter(user => user.role.id === UserRole.ADMINISTRATOR) ).subscribe(user => this.openAdminModalsIfNeeded(user)) // Account modal userSub.pipe( - filter(user => user.role !== UserRole.ADMINISTRATOR) + filter(user => user.role.id !== UserRole.ADMINISTRATOR) ).subscribe(user => this.openAccountModalsIfNeeded(user)) } diff --git a/client/src/app/core/auth/auth-user.model.ts b/client/src/app/core/auth/auth-user.model.ts index a12325421..226075265 100644 --- a/client/src/app/core/auth/auth-user.model.ts +++ b/client/src/app/core/auth/auth-user.model.ts @@ -43,16 +43,16 @@ export class AuthUser extends User implements ServerMyUserModel { } hasRight (right: UserRight) { - return hasUserRight(this.role, right) + return hasUserRight(this.role.id, right) } canManage (user: ServerUserModel) { - const myRole = this.role + const myRole = this.role.id if (myRole === UserRole.ADMINISTRATOR) return true // I'm a moderator: I can only manage users - return user.role === UserRole.USER + return user.role.id === UserRole.USER } computeCanSeeVideosLink (quotaObservable: Observable): Observable { diff --git a/client/src/app/core/users/user-local-storage.service.ts b/client/src/app/core/users/user-local-storage.service.ts index f1588bdd2..a047efe8e 100644 --- a/client/src/app/core/users/user-local-storage.service.ts +++ b/client/src/app/core/users/user-local-storage.service.ts @@ -59,7 +59,10 @@ export class UserLocalStorageService { id: parseInt(this.localStorageService.getItem(UserLocalStorageKeys.ID), 10), username: this.localStorageService.getItem(UserLocalStorageKeys.USERNAME), email: this.localStorageService.getItem(UserLocalStorageKeys.EMAIL), - role: parseInt(this.localStorageService.getItem(UserLocalStorageKeys.ROLE), 10) as UserRole, + role: { + id: parseInt(this.localStorageService.getItem(UserLocalStorageKeys.ROLE), 10) as UserRole, + label: '' + }, ...this.getUserInfo() } @@ -69,12 +72,14 @@ export class UserLocalStorageService { id: number username: string email: string - role: UserRole + role: { + id: UserRole + } }) { this.localStorageService.setItem(UserLocalStorageKeys.ID, user.id.toString()) this.localStorageService.setItem(UserLocalStorageKeys.USERNAME, user.username) this.localStorageService.setItem(UserLocalStorageKeys.EMAIL, user.email) - this.localStorageService.setItem(UserLocalStorageKeys.ROLE, user.role.toString()) + this.localStorageService.setItem(UserLocalStorageKeys.ROLE, user.role.id.toString()) } flushLoggedInUser () { diff --git a/client/src/app/core/users/user.model.ts b/client/src/app/core/users/user.model.ts index 8385a4012..5534bca33 100644 --- a/client/src/app/core/users/user.model.ts +++ b/client/src/app/core/users/user.model.ts @@ -34,8 +34,10 @@ export class User implements UserServerModel { videosHistoryEnabled: boolean videoLanguages: string[] - role: UserRole - roleLabel: string + role: { + id: UserRole + label: string + } videoQuota: number videoQuotaDaily: number @@ -123,7 +125,7 @@ export class User implements UserServerModel { } hasRight (right: UserRight) { - return hasUserRight(this.role, right) + return hasUserRight(this.role.id, right) } patch (obj: UserServerModel) { @@ -148,6 +150,6 @@ export class User implements UserServerModel { isAutoBlocked (serverConfig: HTMLServerConfig) { if (serverConfig.autoBlacklist.videos.ofUsers.enabled !== true) return false - return this.role === UserRole.USER && this.adminFlags !== UserAdminFlag.BYPASS_VIDEO_AUTO_BLACKLIST + return this.role.id === UserRole.USER && this.adminFlags !== UserAdminFlag.BYPASS_VIDEO_AUTO_BLACKLIST } } diff --git a/client/src/app/shared/shared-users/user-admin.service.ts b/client/src/app/shared/shared-users/user-admin.service.ts index 4128358dc..0b04023a3 100644 --- a/client/src/app/shared/shared-users/user-admin.service.ts +++ b/client/src/app/shared/shared-users/user-admin.service.ts @@ -125,7 +125,10 @@ export class UserAdminService { } return Object.assign(user, { - roleLabel: roleLabels[user.role], + role: { + id: user.role.id, + label: roleLabels[user.role.id] + }, videoQuota, videoQuotaUsed, rawVideoQuota: user.videoQuota, diff --git a/server/models/user/user.ts b/server/models/user/user.ts index 34329580b..f70feed73 100644 --- a/server/models/user/user.ts +++ b/server/models/user/user.ts @@ -891,8 +891,10 @@ export class UserModel extends Model>> { autoPlayNextVideoPlaylist: this.autoPlayNextVideoPlaylist, videoLanguages: this.videoLanguages, - role: this.role, - roleLabel: USER_ROLE_LABELS[this.role], + role: { + id: this.role, + label: USER_ROLE_LABELS[this.role] + }, videoQuota: this.videoQuota, videoQuotaDaily: this.videoQuotaDaily, diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index 3952a7aed..421b3ce16 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -219,7 +219,7 @@ describe('Test users', function () { expect(user.email).to.equal('user_1@example.com') expect(user.nsfwPolicy).to.equal('display') expect(user.videoQuota).to.equal(2 * 1024 * 1024) - expect(user.roleLabel).to.equal('User') + expect(user.role.label).to.equal('User') expect(user.id).to.be.a('number') expect(user.account.displayName).to.equal('user_1') expect(user.account.description).to.be.null @@ -277,7 +277,7 @@ describe('Test users', function () { const user = data[0] expect(user.username).to.equal('root') expect(user.email).to.equal('admin' + server.internalServerNumber + '@example.com') - expect(user.roleLabel).to.equal('Administrator') + expect(user.role.label).to.equal('Administrator') expect(user.nsfwPolicy).to.equal('display') }) @@ -531,7 +531,7 @@ describe('Test users', function () { expect(user.emailVerified).to.be.true expect(user.nsfwPolicy).to.equal('do_not_list') expect(user.videoQuota).to.equal(42) - expect(user.roleLabel).to.equal('Moderator') + expect(user.role.label).to.equal('Moderator') expect(user.id).to.be.a('number') expect(user.adminFlags).to.equal(UserAdminFlag.NONE) expect(user.pluginAuth).to.equal('toto') diff --git a/server/tests/plugins/external-auth.ts b/server/tests/plugins/external-auth.ts index e08b83791..437777e90 100644 --- a/server/tests/plugins/external-auth.ts +++ b/server/tests/plugins/external-auth.ts @@ -155,7 +155,7 @@ describe('Test external auth plugins', function () { expect(body.username).to.equal('cyan') expect(body.account.displayName).to.equal('cyan') expect(body.email).to.equal('cyan@example.com') - expect(body.role).to.equal(UserRole.USER) + expect(body.role.id).to.equal(UserRole.USER) } }) @@ -177,7 +177,7 @@ describe('Test external auth plugins', function () { expect(body.username).to.equal('kefka') expect(body.account.displayName).to.equal('Kefka Palazzo') expect(body.email).to.equal('kefka@example.com') - expect(body.role).to.equal(UserRole.ADMINISTRATOR) + expect(body.role.id).to.equal(UserRole.ADMINISTRATOR) } }) @@ -237,7 +237,7 @@ describe('Test external auth plugins', function () { expect(body.username).to.equal('cyan') expect(body.account.displayName).to.equal('Cyan Garamonde') expect(body.account.description).to.equal('Retainer to the king of Doma') - expect(body.role).to.equal(UserRole.USER) + expect(body.role.id).to.equal(UserRole.USER) }) it('Should not update an external auth email', async function () { diff --git a/server/tests/plugins/id-and-pass-auth.ts b/server/tests/plugins/id-and-pass-auth.ts index 85faac5a8..fc24a5656 100644 --- a/server/tests/plugins/id-and-pass-auth.ts +++ b/server/tests/plugins/id-and-pass-auth.ts @@ -48,7 +48,7 @@ describe('Test id and pass auth plugins', function () { expect(body.username).to.equal('spyro') expect(body.account.displayName).to.equal('Spyro the Dragon') - expect(body.role).to.equal(UserRole.USER) + expect(body.role.id).to.equal(UserRole.USER) }) it('Should login Crash, create the user and use the token', async function () { @@ -63,7 +63,7 @@ describe('Test id and pass auth plugins', function () { expect(body.username).to.equal('crash') expect(body.account.displayName).to.equal('Crash Bandicoot') - expect(body.role).to.equal(UserRole.MODERATOR) + expect(body.role.id).to.equal(UserRole.MODERATOR) } }) @@ -79,7 +79,7 @@ describe('Test id and pass auth plugins', function () { expect(body.username).to.equal('laguna') expect(body.account.displayName).to.equal('laguna') - expect(body.role).to.equal(UserRole.USER) + expect(body.role.id).to.equal(UserRole.USER) } }) @@ -129,7 +129,7 @@ describe('Test id and pass auth plugins', function () { expect(body.username).to.equal('crash') expect(body.account.displayName).to.equal('Beautiful Crash') expect(body.account.description).to.equal('Mutant eastern barred bandicoot') - expect(body.role).to.equal(UserRole.MODERATOR) + expect(body.role.id).to.equal(UserRole.MODERATOR) }) it('Should reject token of laguna by the plugin hook', async function () { diff --git a/shared/models/users/user.model.ts b/shared/models/users/user.model.ts index 7b6494ff8..761a2edba 100644 --- a/shared/models/users/user.model.ts +++ b/shared/models/users/user.model.ts @@ -28,8 +28,10 @@ export interface User { videosHistoryEnabled: boolean videoLanguages: string[] - role: UserRole - roleLabel: string + role: { + id: UserRole + label: string + } videoQuota: number videoQuotaDaily: number diff --git a/support/doc/api/openapi.yaml b/support/doc/api/openapi.yaml index 7ffe8c67b..69a6eff72 100644 --- a/support/doc/api/openapi.yaml +++ b/support/doc/api/openapi.yaml @@ -7522,13 +7522,16 @@ components: nsfwPolicy: $ref: '#/components/schemas/NSFWPolicy' role: - $ref: '#/components/schemas/UserRole' - roleLabel: - type: string - enum: - - User - - Moderator - - Administrator + type: object + properties: + id: + $ref: '#/components/schemas/UserRole' + label: + type: string + enum: + - User + - Moderator + - Administrator theme: type: string description: Theme enabled by this user