-
Notifications
You must be signed in to change notification settings - Fork 69
[FEATURE] Rendre le refresh token stateless (PIX-24119) #17422
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
46e7668
9c59f08
5d94d7f
6aa86d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -33,6 +33,13 @@ export class RefreshToken { | |||||
| return new RefreshToken({ userId, source, value, audience, sessionId }); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * @param {string} value | ||||||
| */ | ||||||
| static isRefreshToken(value) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Serait-il possible de renommer
Suggested change
|
||||||
| return /^\d+:\p{Hex_Digit}{8}-\p{Hex_Digit}{4}-\p{Hex_Digit}{4}-\p{Hex_Digit}{4}-\p{Hex_Digit}{12}$/u.test(value); | ||||||
| } | ||||||
|
|
||||||
| get expirationDelaySeconds() { | ||||||
| return config.authentication.refreshTokenLifespanMs / 1000; | ||||||
| } | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,49 @@ | ||||||
| import Joi from 'joi'; | ||||||
|
|
||||||
| import { config } from '../../../shared/config.js'; | ||||||
| import { tokenService } from '../../../shared/domain/services/token-service.js'; | ||||||
| import { validateEntity } from '../../../shared/domain/validators/entity-validator.js'; | ||||||
|
|
||||||
| export class UserRefreshToken { | ||||||
| constructor({ userId, audience, sessionId, source }) { | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestion : dans tout ce fichier, par considération d’organisation et de facilité de compréhension du code, veiller à ordonner comme suit les propriétés :
Suggested change
|
||||||
| this.userId = userId; | ||||||
| this.audience = audience; | ||||||
| this.sessionId = sessionId; | ||||||
| this.source = source; | ||||||
|
|
||||||
| validateEntity( | ||||||
| Joi.object({ | ||||||
| userId: Joi.number().required(), | ||||||
| audience: Joi.string().required(), | ||||||
| sessionId: Joi.string().required(), | ||||||
| source: Joi.string().optional(), | ||||||
| }), | ||||||
| this, | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| static generate({ userId, source, audience, sessionId }) { | ||||||
| const expirationDelaySeconds = config.authentication.refreshTokenLifespanMs / 1000; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Est-ce qu’on pourrait définir cette variable à la racine du module pour ne pas la déclarer et la calculer à chaque appel de la méthode ? |
||||||
| return tokenService.encodeToken( | ||||||
| { user_id: userId, source, aud: audience, sid: sessionId }, | ||||||
| config.authentication.secret, | ||||||
| expirationDelaySeconds, | ||||||
| ); | ||||||
| } | ||||||
|
|
||||||
| static decode(encodedRefreshToken) { | ||||||
| const decodedRefreshToken = tokenService.getDecodedToken(encodedRefreshToken, config.authentication.secret); | ||||||
| if (!decodedRefreshToken) return undefined; // FIXME add log like api/src/identity-access-management/infrastructure/server-authentication.js:148 ? | ||||||
|
|
||||||
| return new UserRefreshToken({ | ||||||
| userId: decodedRefreshToken.user_id, | ||||||
| source: decodedRefreshToken.source, | ||||||
| audience: decodedRefreshToken.aud, | ||||||
| sessionId: decodedRefreshToken.sid, | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| hasSameAudience(audience) { | ||||||
| return this.audience === audience; | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Je sais que ce code provient de l’ancienne classe dépréciée
Suggested change
|
||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,93 @@ | ||
| import { expect } from 'chai'; | ||
| import jsonwebtoken from 'jsonwebtoken'; | ||
|
|
||
| import { UserRefreshToken } from '../../../../../src/identity-access-management/domain/models/UserRefreshToken.js'; | ||
| import { config } from '../../../../../src/shared/config.js'; | ||
|
|
||
| describe('Unit | Identity Access Management | Domain | Model | UserRefreshToken', function () { | ||
| describe('UserRefreshToken.decode', function () { | ||
| it('decodes a valid token', function () { | ||
| // given | ||
| const encodedRefreshToken = jsonwebtoken.sign( | ||
| { | ||
| user_id: 123456, | ||
| source: 'source!', | ||
| aud: 'audience!', | ||
| sid: 'ABC-123-321', | ||
| }, | ||
| config.authentication.secret, | ||
| { expiresIn: config.authentication.refreshTokenLifespanMs / 1000 }, | ||
| ); | ||
|
|
||
| // when | ||
| const decoded = UserRefreshToken.decode(encodedRefreshToken); | ||
|
|
||
| // then | ||
| expect(decoded).to.be.instanceOf(UserRefreshToken); | ||
| expect(decoded).to.deep.equal({ | ||
| userId: 123456, | ||
| source: 'source!', | ||
| audience: 'audience!', | ||
| sessionId: 'ABC-123-321', | ||
| }); | ||
| }); | ||
|
|
||
| it('returns undefined for an invalid token', async function () { | ||
| // given | ||
| const invalidToken = 'invalid.token'; | ||
|
|
||
| // when | ||
| const decoded = UserRefreshToken.decode(invalidToken); | ||
|
|
||
| // then | ||
| expect(decoded).to.be.undefined; | ||
| }); | ||
| }); | ||
|
|
||
| describe('UserRefreshToken.generateUserToken', function () { | ||
| it('returns an encoded refresh token', function () { | ||
| // given | ||
| const payload = { | ||
| userId: 123456, | ||
| source: 'source!', | ||
| audience: 'audience!', | ||
| sessionId: 'sessionId!', | ||
| }; | ||
|
|
||
| // when | ||
| const refreshToken = UserRefreshToken.generate(payload); | ||
|
|
||
| // then | ||
| expect(refreshToken).to.be.a('string'); | ||
| const decodedRefreshToken = jsonwebtoken.verify(refreshToken, config.authentication.secret); | ||
| expect(decodedRefreshToken).to.include({ | ||
| user_id: 123456, | ||
| source: 'source!', | ||
| aud: 'audience!', | ||
| sid: 'sessionId!', | ||
| }); | ||
| expect(decodedRefreshToken).to.have.property('iat').which.is.a('number'); | ||
| expect(decodedRefreshToken).to.have.property('exp').which.is.a('number'); | ||
| }); | ||
| }); | ||
|
|
||
| describe('#hasSameAudience', function () { | ||
| it('returns true with same audience otherwise false', function () { | ||
| // given | ||
| const refreshToken = new UserRefreshToken({ | ||
| userId: 123456, | ||
| source: 'source!', | ||
| audience: 'https://app.pix.fr', | ||
| sessionId: 'sessionId!', | ||
| }); | ||
|
|
||
| // when | ||
| const withSameAudience = refreshToken.hasSameAudience('https://app.pix.fr'); | ||
| const withDifferentAudience = refreshToken.hasSameAudience('https://orga.pix.fr'); | ||
|
|
||
| // then | ||
| expect(withSameAudience).to.be.true; | ||
| expect(withDifferentAudience).to.be.false; | ||
| }); | ||
| }); | ||
| }); |
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Serait-il possible de marquer la classe
RefreshToken(et/ou son constructeur) comme@deprecated?