Skip to content

Commit 0ff2448

Browse files
committed
profile: Allow dashes in display name.
1 parent 6eb5ad8 commit 0ff2448

File tree

7 files changed

+60
-50
lines changed

7 files changed

+60
-50
lines changed

src/app/authentication/authentication.api.spec.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ describe('The authentication engine of the VPDB API', () => {
474474
profile: {
475475
provider: 'github',
476476
id: '11234',
477-
displayName: 'Motörhead Dude-23',
477+
displayName: 'Motörhead-Dude&23',
478478
username: 'motorhead',
479479
profileUrl: 'https://github.com/mockuser',
480480
emails: [
@@ -484,7 +484,7 @@ describe('The authentication engine of the VPDB API', () => {
484484
}
485485
}).then(res => api.retrieveUserProfile(res));
486486
api.tearDownUser(profile.id);
487-
expect(profile.name).to.be('Motorhead Dude23');
487+
expect(profile.name).to.be('Motorhead-Dude23');
488488
});
489489

490490
it('should match the same already registered Github user even though email and name are different', async () => {

src/app/authentication/authentication.api.ts

Lines changed: 5 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -477,7 +477,7 @@ export class AuthenticationApi extends Api {
477477
user.providers = user.providers || {};
478478
user.providers[provider] = {
479479
id: String(profile.id),
480-
name: this.getNameFromProfile(profile),
480+
name: UserUtil.getNameFromProfile(profile),
481481
emails,
482482
created_at: new Date(),
483483
modified_at: new Date(),
@@ -492,7 +492,7 @@ export class AuthenticationApi extends Api {
492492
} else {
493493
user.providers[provider].id = String(profile.id);
494494
user.providers[provider].emails = emails;
495-
user.providers[provider].name = this.getNameFromProfile(profile);
495+
user.providers[provider].name = UserUtil.getNameFromProfile(profile);
496496
user.providers[provider].modified_at = new Date();
497497
user.providers[provider].profile = profile._json;
498498
await LogUserUtil.success(ctx, user, 'authenticate', { provider });
@@ -523,30 +523,16 @@ export class AuthenticationApi extends Api {
523523
*/
524524
private async createOAuthUser(ctx: Context, provider: string, profile: OAuthProfile, emails: string[]): Promise<UserDocument> {
525525
// compute username
526-
let name: string;
527-
if (!profile.displayName && !profile.username) {
528-
logger.warn(ctx.state, '[AuthenticationApi.createOAuthUser] Profile data does contain neither display name nor username: %s', JSON.stringify(profile));
529-
name = profile.emails[0].value.substr(0, profile.emails[0].value.indexOf('@'));
530-
} else {
531-
name = profile.displayName || profile.username;
532-
}
533-
name = UserUtil.removeDiacritics(name).replace(/[^0-9a-z ]+/gi, '');
534-
535-
// check if username doesn't conflict
536-
let newUser: UserDocument;
537-
const dupeNameUser = await state.models.User.findOne({ name }).exec();
538-
if (dupeNameUser) {
539-
name += Math.floor(Math.random() * 1000);
540-
}
526+
const name = await UserUtil.makeValidName(UserUtil.getNameFromProfile(profile));
541527
const now = new Date();
542-
newUser = {
528+
let newUser = {
543529
is_local: false,
544530
name,
545531
email: emails[0],
546532
providers: {
547533
[provider]: {
548534
id: String(profile.id),
549-
name: this.getNameFromProfile(profile),
535+
name: UserUtil.getNameFromProfile(profile),
550536
emails,
551537
created_at: now,
552538
modified_at: now,
@@ -580,19 +566,6 @@ export class AuthenticationApi extends Api {
580566

581567
return newUser;
582568
}
583-
584-
/**
585-
* Retrieves the username from the received OAuth profile. Falls back to
586-
* email prefix if none found.
587-
* @param profile
588-
* @return {string}
589-
*/
590-
private getNameFromProfile(profile: any) {
591-
return profile.displayName
592-
|| profile.username
593-
|| (profile.name ? profile.name.givenName || profile.name.familyName : '')
594-
|| profile.emails[0].value.substr(0, profile.emails[0].value.indexOf('@'));
595-
}
596569
}
597570

598571
/**

src/app/authentication/strategies/ips.strategy.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,7 @@ export class IpsStrategy extends Strategy {
109109
break;
110110
default: throw new ApiError('Unsupported IPS version %s', this.config.version);
111111
}
112+
logger.debug(ctx.state, '[IpsStrategy.getProfile] Retrieving profile at: %s (%s)', this.config.baseURL + path, JSON.stringify(postData));
112113

113114
// get access token
114115
let res = await this.client.post(path, postData, config) as AxiosResponse;

src/app/authentication/strategies/strategy.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ export abstract class Strategy extends AuthenticationApi {
4848
/**
4949
* Where the redirection from the provider lands.
5050
*
51-
* @see GET /auth/:provider
51+
* @see GET /v1/authenticate/:provider
5252
* @param {Context} ctx Koa context coming from redirection, i.e. it should contain the access code or an error in the query.
5353
*/
5454
public async authenticateOAuth(ctx: Context): Promise<boolean> {

src/app/users/user.api.ts

Lines changed: 6 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -432,29 +432,22 @@ export class UserApi extends Api {
432432
}
433433

434434
private async createProviderUser(ctx: Context, provider: string): Promise<UserDocument> {
435-
// check if username doesn't conflict
436-
let newUser;
437-
let name = UserUtil.removeDiacritics(ctx.request.body.username).replace(/[^0-9a-z ]+/gi, '');
438-
const originalName = name;
439-
const dupeNameUser = await state.models.User.findOne({ name }).exec();
440-
if (dupeNameUser) {
441-
name += Math.floor(Math.random() * 1000);
442-
}
443-
newUser = {
444-
is_local: false,
435+
const name = await UserUtil.makeValidName(ctx.request.body.username);
436+
const newUser = {
445437
name,
438+
is_local: false,
446439
email: ctx.request.body.email,
447440
emails: [ctx.request.body.email],
448441
providers: {
449442
[provider]: {
450443
id: String(ctx.request.body.provider_id),
451-
name: originalName,
444+
name: UserUtil.stripToValidName(ctx.request.body.username),
452445
emails: [ctx.request.body.email],
453446
created_at: new Date(),
454447
profile: ctx.request.body.provider_profile,
455448
},
456449
},
457-
};
458-
return UserUtil.createUser(ctx, newUser as UserDocument, false);
450+
} as UserDocument;
451+
return UserUtil.createUser(ctx, newUser, false);
459452
}
460453
}

src/app/users/user.schema.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import { config } from '../common/settings';
3030
import { flavors } from '../releases/release.flavors';
3131
import { state } from '../state';
3232
import { UserDocument } from './user.document';
33+
import { UserUtil } from './user.util';
3334

3435
const shortId = require('shortid32');
3536

@@ -151,7 +152,6 @@ userSchema.virtual('provider')
151152
//-----------------------------------------------------------------------------
152153
// VALIDATIONS
153154
//-----------------------------------------------------------------------------
154-
const validNameRegex = /^[0-9a-z ]{3,}$/i;
155155
userSchema.path('name').validate(function(name: string) {
156156
// this gets default from username if not set anyway.
157157
if (this.isNew) {
@@ -165,7 +165,7 @@ userSchema.path('name').validate(function(name: string) {
165165
if (this.isNew) {
166166
return true;
167167
}
168-
return validNameRegex.test(name);
168+
return UserUtil.isValidName(name);
169169
}, 'Name can only contain letters, numbers and spaces.');
170170

171171
userSchema.path('email').validate(function(email: string) {

src/app/users/user.util.ts

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
import { assign, keys, sum, uniq, values } from 'lodash';
2121
import randomString from 'randomstring';
2222

23+
import { OAuthProfile } from '../authentication/authentication.api';
2324
import { BackglassDocument } from '../backglasses/backglass.document';
2425
import { acl } from '../common/acl';
2526
import { ApiError } from '../common/api.error';
@@ -312,6 +313,48 @@ export class UserUtil {
312313
return { permissions };
313314
}
314315

316+
/**
317+
* Checks if a given display name is valid.
318+
* @param name Display name to check
319+
*/
320+
public static isValidName(name: string) {
321+
return /^[0-9a-z -]{3,}$/i.test(name);
322+
}
323+
324+
/**
325+
* Strips or replaces invalid characters from the display name.
326+
* @param name Display name to strip
327+
*/
328+
public static stripToValidName(name: string) {
329+
return UserUtil.removeDiacritics(name).replace(/[^0-9a-z -]+/gi, '');
330+
}
331+
332+
/**
333+
* Returns a valid display name, if necessary suffixed by a random number in case the name already exists.
334+
* @param originalName Display name to convert to a valid name
335+
*/
336+
public static async makeValidName(originalName: string) {
337+
let name = UserUtil.stripToValidName(originalName);
338+
const dupeNameUser = await state.models.User.findOne({ name }).exec();
339+
if (dupeNameUser) {
340+
name += Math.floor(Math.random() * 1000);
341+
}
342+
return name;
343+
}
344+
345+
/**
346+
* Retrieves the username from the received OAuth profile. Falls back to
347+
* email prefix if none found.
348+
* @param profile
349+
* @return {string}
350+
*/
351+
public static getNameFromProfile(profile: OAuthProfile) {
352+
return profile.displayName
353+
|| profile.username
354+
|| (profile.name ? profile.name.givenName || profile.name.familyName : '')
355+
|| profile.emails[0].value.substr(0, profile.emails[0].value.indexOf('@'));
356+
}
357+
315358
/**
316359
* Replaces umlauts with standard ascii sequence, e.g. "börk" becomes "boerk".
317360
* @param {string} str Input with diacritics

0 commit comments

Comments
 (0)