diff options
author | Chocobozzz <me@florianbigard.com> | 2020-05-20 10:04:44 +0200 |
---|---|---|
committer | Chocobozzz <me@florianbigard.com> | 2020-05-20 10:17:27 +0200 |
commit | 9a7fd9600bf513adffbf2127be7c3a8b4d31073f (patch) | |
tree | a2ac8e321f57f5c7add15ec8166a6a2e7bdf989a | |
parent | 51539e95d954867d5c4561ac56843105253db79c (diff) | |
download | PeerTube-9a7fd9600bf513adffbf2127be7c3a8b4d31073f.tar.gz PeerTube-9a7fd9600bf513adffbf2127be7c3a8b4d31073f.tar.zst PeerTube-9a7fd9600bf513adffbf2127be7c3a8b4d31073f.zip |
Fix external auth email/password update
Also check if an actor does not already exist when creating the user
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 @@ | |||
9 | <span class="email">{{ user.pendingEmail }}</span> is awaiting email verification | 9 | <span class="email">{{ user.pendingEmail }}</span> is awaiting email verification |
10 | </div> | 10 | </div> |
11 | 11 | ||
12 | <form role="form" class="change-email" (ngSubmit)="changeEmail()" [formGroup]="form"> | 12 | <form role="form" class="change-email" (ngSubmit)="changeEmail()" [formGroup]="form" *ngIf="user.pluginAuth === null"> |
13 | 13 | ||
14 | <div class="form-group"> | 14 | <div class="form-group"> |
15 | <label i18n for="new-email">New email</label> | 15 | <label i18n for="new-email">New email</label> |
@@ -23,6 +23,7 @@ | |||
23 | </div> | 23 | </div> |
24 | 24 | ||
25 | <div class="form-group"> | 25 | <div class="form-group"> |
26 | <label i18n for="new-email">Your current password</label> | ||
26 | <input | 27 | <input |
27 | type="password" id="password" i18n-placeholder placeholder="Your password" autocomplete="off" | 28 | type="password" id="password" i18n-placeholder placeholder="Your password" autocomplete="off" |
28 | formControlName="password" [ngClass]="{ 'input-error': formErrors['password'] }" class="form-control" | 29 | formControlName="password" [ngClass]="{ 'input-error': formErrors['password'] }" class="form-control" |
diff --git a/client/src/app/+my-account/my-account-settings/my-account-settings.component.html b/client/src/app/+my-account/my-account-settings/my-account-settings.component.html index f1c466545..b4e4d29f0 100644 --- a/client/src/app/+my-account/my-account-settings/my-account-settings.component.html +++ b/client/src/app/+my-account/my-account-settings/my-account-settings.component.html | |||
@@ -58,7 +58,7 @@ | |||
58 | </div> | 58 | </div> |
59 | </div> | 59 | </div> |
60 | 60 | ||
61 | <div class="form-row mt-5"> <!-- password grid --> | 61 | <div class="form-row mt-5" *ngIf="user.pluginAuth === null"> <!-- password grid --> |
62 | <div class="form-group col-12 col-lg-4 col-xl-3"> | 62 | <div class="form-group col-12 col-lg-4 col-xl-3"> |
63 | <div i18n class="account-title">PASSWORD</div> | 63 | <div i18n class="account-title">PASSWORD</div> |
64 | </div> | 64 | </div> |
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' | |||
14 | import { createUserAccountAndChannelAndPlaylist } from './user' | 14 | import { createUserAccountAndChannelAndPlaylist } from './user' |
15 | import { UserRole } from '@shared/models/users/user-role' | 15 | import { UserRole } from '@shared/models/users/user-role' |
16 | import { PluginManager } from '@server/lib/plugins/plugin-manager' | 16 | import { PluginManager } from '@server/lib/plugins/plugin-manager' |
17 | import { ActorModel } from '@server/models/activitypub/actor' | ||
17 | 18 | ||
18 | type TokenInfo = { accessToken: string, refreshToken: string, accessTokenExpiresAt: Date, refreshTokenExpiresAt: Date } | 19 | type TokenInfo = { accessToken: string, refreshToken: string, accessTokenExpiresAt: Date, refreshTokenExpiresAt: Date } |
19 | 20 | ||
@@ -109,6 +110,9 @@ async function getUser (usernameOrEmail?: string, password?: string) { | |||
109 | let user = await UserModel.loadByEmail(obj.user.email) | 110 | let user = await UserModel.loadByEmail(obj.user.email) |
110 | if (!user) user = await createUserFromExternal(obj.pluginName, obj.user) | 111 | if (!user) user = await createUserFromExternal(obj.pluginName, obj.user) |
111 | 112 | ||
113 | // Cannot create a user | ||
114 | if (!user) throw new AccessDeniedError('Cannot create such user: an actor with that name already exists.') | ||
115 | |||
112 | // If the user does not belongs to a plugin, it was created before its installation | 116 | // If the user does not belongs to a plugin, it was created before its installation |
113 | // Then we just go through a regular login process | 117 | // Then we just go through a regular login process |
114 | if (user.pluginAuth !== null) { | 118 | if (user.pluginAuth !== null) { |
@@ -208,6 +212,10 @@ async function createUserFromExternal (pluginAuth: string, options: { | |||
208 | role: UserRole | 212 | role: UserRole |
209 | displayName: string | 213 | displayName: string |
210 | }) { | 214 | }) { |
215 | // Check an actor does not already exists with that name (removed user) | ||
216 | const actor = await ActorModel.loadLocalByName(options.username) | ||
217 | if (actor) return null | ||
218 | |||
211 | const userToCreate = new UserModel({ | 219 | const userToCreate = new UserModel({ |
212 | username: options.username, | 220 | username: options.username, |
213 | password: null, | 221 | 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 = [ | |||
234 | async (req: express.Request, res: express.Response, next: express.NextFunction) => { | 234 | async (req: express.Request, res: express.Response, next: express.NextFunction) => { |
235 | logger.debug('Checking usersUpdateMe parameters', { parameters: omit(req.body, 'password') }) | 235 | logger.debug('Checking usersUpdateMe parameters', { parameters: omit(req.body, 'password') }) |
236 | 236 | ||
237 | const user = res.locals.oauth.token.User | ||
238 | |||
237 | if (req.body.password || req.body.email) { | 239 | if (req.body.password || req.body.email) { |
240 | if (user.pluginAuth !== null) { | ||
241 | return res.status(400) | ||
242 | .json({ error: 'You cannot update your email or password that is associated with an external auth system.' }) | ||
243 | } | ||
244 | |||
238 | if (!req.body.currentPassword) { | 245 | if (!req.body.currentPassword) { |
239 | return res.status(400) | 246 | return res.status(400) |
240 | .json({ error: 'currentPassword parameter is missing.' }) | 247 | .json({ error: 'currentPassword parameter is missing.' }) |
241 | .end() | ||
242 | } | 248 | } |
243 | 249 | ||
244 | const user = res.locals.oauth.token.User | ||
245 | if (await user.isPasswordMatch(req.body.currentPassword) !== true) { | 250 | if (await user.isPasswordMatch(req.body.currentPassword) !== true) { |
246 | return res.status(401) | 251 | return res.status(401) |
247 | .json({ error: 'currentPassword is invalid.' }) | 252 | .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 () { | |||
1044 | } | 1044 | } |
1045 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { targetUrl: getYoutubeVideoUrl() })) | 1045 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { targetUrl: getYoutubeVideoUrl() })) |
1046 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { magnetUri: getMagnetURI() })) | 1046 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { magnetUri: getMagnetURI() })) |
1047 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { torrentfile: 'video-720p.torrent' })) | 1047 | await importVideo(server.url, server.accessToken, immutableAssign(baseAttributes, { torrentfile: 'video-720p.torrent' as any })) |
1048 | 1048 | ||
1049 | await waitJobs([ server ]) | 1049 | await waitJobs([ server ]) |
1050 | 1050 | ||
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`) | |||
175 | 175 | ||
176 | { | 176 | { |
177 | const attributes = immutableAssign(baseAttributes, { | 177 | const attributes = immutableAssign(baseAttributes, { |
178 | torrentfile: 'video-720p.torrent', | 178 | torrentfile: 'video-720p.torrent' as any, |
179 | description: 'this is a super torrent description', | 179 | description: 'this is a super torrent description', |
180 | tags: [ 'tag_torrent1', 'tag_torrent2' ] | 180 | tags: [ 'tag_torrent1', 'tag_torrent2' ] |
181 | }) | 181 | }) |
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 () { | |||
255 | expect(body.role).to.equal(UserRole.USER) | 255 | expect(body.role).to.equal(UserRole.USER) |
256 | }) | 256 | }) |
257 | 257 | ||
258 | it('Should not update an external auth email', async function () { | ||
259 | await updateMyUser({ | ||
260 | url: server.url, | ||
261 | accessToken: cyanAccessToken, | ||
262 | email: 'toto@example.com', | ||
263 | currentPassword: 'toto', | ||
264 | statusCodeExpected: 400 | ||
265 | }) | ||
266 | }) | ||
267 | |||
258 | it('Should reject token of Kefka by the plugin hook', async function () { | 268 | it('Should reject token of Kefka by the plugin hook', async function () { |
259 | this.timeout(10000) | 269 | this.timeout(10000) |
260 | 270 | ||
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, | |||
216 | .expect(expectedStatus) | 216 | .expect(expectedStatus) |
217 | } | 217 | } |
218 | 218 | ||
219 | function updateMyUser (options: { url: string, accessToken: string } & UserUpdateMe) { | 219 | function updateMyUser (options: { url: string, accessToken: string, statusCodeExpected?: number } & UserUpdateMe) { |
220 | const path = '/api/v1/users/me' | 220 | const path = '/api/v1/users/me' |
221 | 221 | ||
222 | const toSend: UserUpdateMe = omit(options, 'url', 'accessToken') | 222 | const toSend: UserUpdateMe = omit(options, 'url', 'accessToken') |
@@ -226,7 +226,7 @@ function updateMyUser (options: { url: string, accessToken: string } & UserUpdat | |||
226 | path, | 226 | path, |
227 | token: options.accessToken, | 227 | token: options.accessToken, |
228 | fields: toSend, | 228 | fields: toSend, |
229 | statusCodeExpected: 204 | 229 | statusCodeExpected: options.statusCodeExpected || 204 |
230 | }) | 230 | }) |
231 | } | 231 | } |
232 | 232 | ||