From 9a7fd9600bf513adffbf2127be7c3a8b4d31073f Mon Sep 17 00:00:00 2001 From: Chocobozzz Date: Wed, 20 May 2020 10:04:44 +0200 Subject: Fix external auth email/password update Also check if an actor does not already exist when creating the user --- .../my-account-change-email.component.html | 3 ++- .../my-account-settings/my-account-settings.component.html | 2 +- server/lib/oauth-model.ts | 8 ++++++++ server/middlewares/validators/users.ts | 9 +++++++-- server/tests/api/check-params/users.ts | 2 +- server/tests/api/videos/video-imports.ts | 2 +- server/tests/plugins/external-auth.ts | 10 ++++++++++ shared/extra-utils/users/users.ts | 4 ++-- 8 files changed, 32 insertions(+), 8 deletions(-) diff --git a/client/src/app/+my-account/my-account-settings/my-account-change-email/my-account-change-email.component.html b/client/src/app/+my-account/my-account-settings/my-account-change-email/my-account-change-email.component.html index f39f66696..ce176d682 100644 --- a/client/src/app/+my-account/my-account-settings/my-account-change-email/my-account-change-email.component.html +++ b/client/src/app/+my-account/my-account-settings/my-account-change-email/my-account-change-email.component.html @@ -9,7 +9,7 @@ {{ user.pendingEmail }} is awaiting email verification -
+
@@ -23,6 +23,7 @@
+
-
+
diff --git a/server/lib/oauth-model.ts b/server/lib/oauth-model.ts index e5ea4636e..db546efb1 100644 --- a/server/lib/oauth-model.ts +++ b/server/lib/oauth-model.ts @@ -14,6 +14,7 @@ import { UserAdminFlag } from '@shared/models/users/user-flag.model' import { createUserAccountAndChannelAndPlaylist } from './user' import { UserRole } from '@shared/models/users/user-role' import { PluginManager } from '@server/lib/plugins/plugin-manager' +import { ActorModel } from '@server/models/activitypub/actor' type TokenInfo = { accessToken: string, refreshToken: string, accessTokenExpiresAt: Date, refreshTokenExpiresAt: Date } @@ -109,6 +110,9 @@ async function getUser (usernameOrEmail?: string, password?: string) { let user = await UserModel.loadByEmail(obj.user.email) if (!user) user = await createUserFromExternal(obj.pluginName, obj.user) + // Cannot create a user + if (!user) throw new AccessDeniedError('Cannot create such user: an actor with that name already exists.') + // If the user does not belongs to a plugin, it was created before its installation // Then we just go through a regular login process if (user.pluginAuth !== null) { @@ -208,6 +212,10 @@ async function createUserFromExternal (pluginAuth: string, options: { role: UserRole displayName: string }) { + // Check an actor does not already exists with that name (removed user) + const actor = await ActorModel.loadLocalByName(options.username) + if (actor) return null + const userToCreate = new UserModel({ username: options.username, password: null, diff --git a/server/middlewares/validators/users.ts b/server/middlewares/validators/users.ts index 840b9fc74..3bdbcdf6a 100644 --- a/server/middlewares/validators/users.ts +++ b/server/middlewares/validators/users.ts @@ -234,14 +234,19 @@ const usersUpdateMeValidator = [ async (req: express.Request, res: express.Response, next: express.NextFunction) => { logger.debug('Checking usersUpdateMe parameters', { parameters: omit(req.body, 'password') }) + const user = res.locals.oauth.token.User + if (req.body.password || req.body.email) { + if (user.pluginAuth !== null) { + return res.status(400) + .json({ error: 'You cannot update your email or password that is associated with an external auth system.' }) + } + if (!req.body.currentPassword) { return res.status(400) .json({ error: 'currentPassword parameter is missing.' }) - .end() } - const user = res.locals.oauth.token.User if (await user.isPasswordMatch(req.body.currentPassword) !== true) { return res.status(401) .json({ error: 'currentPassword is invalid.' }) diff --git a/server/tests/api/check-params/users.ts b/server/tests/api/check-params/users.ts index 4d597f0a3..6e737af15 100644 --- a/server/tests/api/check-params/users.ts +++ b/server/tests/api/check-params/users.ts @@ -1044,7 +1044,7 @@ describe('Test users API validators', function () { } await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { targetUrl: getYoutubeVideoUrl() })) await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { magnetUri: getMagnetURI() })) - await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { torrentfile: 'video-720p.torrent' })) + await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { torrentfile: 'video-720p.torrent' as any })) await waitJobs([ server ]) diff --git a/server/tests/api/videos/video-imports.ts b/server/tests/api/videos/video-imports.ts index 4d5989f43..d211859e4 100644 --- a/server/tests/api/videos/video-imports.ts +++ b/server/tests/api/videos/video-imports.ts @@ -175,7 +175,7 @@ Ajouter un sous-titre est vraiment facile`) { const attributes = immutableAssign(baseAttributes, { - torrentfile: 'video-720p.torrent', + torrentfile: 'video-720p.torrent' as any, description: 'this is a super torrent description', tags: [ 'tag_torrent1', 'tag_torrent2' ] }) diff --git a/server/tests/plugins/external-auth.ts b/server/tests/plugins/external-auth.ts index a85672782..57361be05 100644 --- a/server/tests/plugins/external-auth.ts +++ b/server/tests/plugins/external-auth.ts @@ -255,6 +255,16 @@ describe('Test external auth plugins', function () { expect(body.role).to.equal(UserRole.USER) }) + it('Should not update an external auth email', async function () { + await updateMyUser({ + url: server.url, + accessToken: cyanAccessToken, + email: 'toto@example.com', + currentPassword: 'toto', + statusCodeExpected: 400 + }) + }) + it('Should reject token of Kefka by the plugin hook', async function () { this.timeout(10000) diff --git a/shared/extra-utils/users/users.ts b/shared/extra-utils/users/users.ts index 54b506bce..08b7743a6 100644 --- a/shared/extra-utils/users/users.ts +++ b/shared/extra-utils/users/users.ts @@ -216,7 +216,7 @@ function unblockUser (url: string, userId: number | string, accessToken: string, .expect(expectedStatus) } -function updateMyUser (options: { url: string, accessToken: string } & UserUpdateMe) { +function updateMyUser (options: { url: string, accessToken: string, statusCodeExpected?: number } & UserUpdateMe) { const path = '/api/v1/users/me' const toSend: UserUpdateMe = omit(options, 'url', 'accessToken') @@ -226,7 +226,7 @@ function updateMyUser (options: { url: string, accessToken: string } & UserUpdat path, token: options.accessToken, fields: toSend, - statusCodeExpected: 204 + statusCodeExpected: options.statusCodeExpected || 204 }) } -- cgit v1.2.3