-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Feat/auth : Rejects malformed JWTToken and requireValidToken Option #597
Changes from 64 commits
cef7a1d
9f0dbab
44d7d2e
f57b737
ed4628e
d7f01df
af52e88
554f7dc
fa0548f
b72adae
733efc0
57fab69
c50df3a
dda5436
ef0be25
e180dc3
6aa1fdb
96c7344
cf82c42
dcff930
315662f
57a0230
d69d633
1be8d91
4b67825
deea3e2
acd0241
25250f0
c0fdd31
bd564b3
44ed61a
9c16387
2dd26c4
b284377
e462c7d
a3ee10b
b38ae55
62a2aff
66ca493
3bd5a50
398c058
ee706d3
5eb5821
1a08225
1d697bc
1c25956
ff0e8d3
e7af99e
95632b1
bc4001f
e06a45c
7c6fc43
88c7106
76dda02
a75f75b
ab75598
2ad04ba
cfe6b87
ad90f20
66cf701
9d35278
432b2cc
01e2c2f
a22d3ef
ffef510
03e1fae
cb1bfed
b6a9db6
4b57287
27d74a7
9a8d41f
ae46531
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 |
---|---|---|
|
@@ -14,6 +14,14 @@ export abstract class NbAuthToken { | |
} | ||
} | ||
|
||
export class InvalidJWTTokenError extends Error { | ||
constructor(message: string) { | ||
super(message); | ||
const actualPrototype = new.target.prototype; | ||
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. no need in additional variable here, just |
||
Object.setPrototypeOf(this, actualPrototype); | ||
} | ||
} | ||
|
||
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. we also need another error then, as we cannot use |
||
export interface NbAuthRefreshableToken { | ||
getRefreshToken(): string; | ||
setRefreshToken(refreshToken: string); | ||
|
@@ -34,24 +42,27 @@ export function nbAuthCreateToken<T extends NbAuthToken>(tokenClass: NbAuthToken | |
export function decodeJwtPayload(payload: string): string { | ||
|
||
if (!payload) { | ||
throw new Error('Cannot extract payload from an empty token.'); | ||
throw new InvalidJWTTokenError('Cannot extract from an empty payload.'); | ||
} | ||
|
||
const parts = payload.split('.'); | ||
|
||
if (parts.length !== 3) { | ||
throw new Error(`The payload ${payload} is not valid JWT payload and must consist of three parts.`); | ||
throw new InvalidJWTTokenError( | ||
`The payload ${payload} is not valid JWT payload and must consist of three parts.`); | ||
} | ||
|
||
let decoded; | ||
try { | ||
decoded = urlBase64Decode(parts[1]); | ||
} catch (e) { | ||
throw new Error(`The payload ${payload} is not valid JWT payload and cannot be parsed.`); | ||
throw new InvalidJWTTokenError( | ||
`The payload ${payload} is not valid JWT payload and cannot be parsed.`); | ||
} | ||
|
||
if (!decoded) { | ||
throw new Error(`The payload ${payload} is not valid JWT payload and cannot be decoded.`); | ||
throw new InvalidJWTTokenError( | ||
`The payload ${payload} is not valid JWT payload and cannot be decoded.`); | ||
} | ||
|
||
return JSON.parse(decoded); | ||
|
@@ -127,12 +138,17 @@ export class NbAuthJWTToken extends NbAuthSimpleToken { | |
* for JWT token, the iat (issued at) field of the token payload contains the creation Date | ||
*/ | ||
protected prepareCreatedAt(date: Date) { | ||
let decoded; | ||
// We do not want prepareCreatedAt to fail if token is empty because we have to accept it | ||
// if requireValidToken is set to false | ||
// We want it to fail only if payload is present but malformed. | ||
try { | ||
decoded = this.getPayload(); | ||
} | ||
finally { | ||
const decoded = this.getPayload(); | ||
return decoded && decoded.iat ? new Date(Number(decoded.iat) * 1000) : super.prepareCreatedAt(date); | ||
} catch (err) { | ||
if (err instanceof InvalidJWTTokenError) { | ||
return super.prepareCreatedAt(date); | ||
} | ||
throw err; | ||
} | ||
} | ||
|
||
|
@@ -221,7 +237,7 @@ export class NbAuthOAuth2Token extends NbAuthSimpleToken { | |
*/ | ||
getPayload(): any { | ||
if (!this.token || !Object.keys(this.token).length) { | ||
throw new Error('Cannot extract payload from an empty token.'); | ||
throw new InvalidJWTTokenError('Cannot extract payload from an empty token.'); | ||
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. this isn't a JWT token error I suppose as we are in |
||
} | ||
|
||
return this.token; | ||
|
@@ -274,12 +290,17 @@ export class NbAuthOAuth2JWTToken extends NbAuthOAuth2Token { | |
* for Oauth2 JWT token, the iat (issued at) field of the access_token payload | ||
*/ | ||
protected prepareCreatedAt(date: Date) { | ||
let decoded; | ||
// We do not want prepareCreatedAt to fail if token is empty because we have to accept it | ||
// if requireValidToken is set to false | ||
// We want it to fail is payload is present but malformed. | ||
try { | ||
decoded = this.getAccessTokenPayload(); | ||
} | ||
finally { | ||
const decoded = this.getAccessTokenPayload(); | ||
return decoded && decoded.iat ? new Date(Number(decoded.iat) * 1000) : super.prepareCreatedAt(date); | ||
} catch (err) { | ||
if (err instanceof InvalidJWTTokenError) { | ||
return super.prepareCreatedAt(date); | ||
} | ||
throw err; | ||
} | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,7 @@ import { Observable } from 'rxjs'; | |
import { NbAuthResult } from '../services/auth-result'; | ||
import { NbAuthStrategyOptions } from './auth-strategy-options'; | ||
import { deepExtend, getDeepFromObject } from '../helpers'; | ||
import { NbAuthToken, nbAuthCreateToken } from '../services/token/token'; | ||
import { NbAuthToken, nbAuthCreateToken, InvalidJWTTokenError } from '../services/token/token'; | ||
|
||
export abstract class NbAuthStrategy { | ||
|
||
|
@@ -21,7 +21,12 @@ export abstract class NbAuthStrategy { | |
} | ||
|
||
createToken<T extends NbAuthToken>(value: any): T { | ||
return nbAuthCreateToken<T>(this.getOption('token.class'), value, this.getName()); | ||
const requireValidToken: boolean = this.getOption('requireValidToken'); | ||
const token = nbAuthCreateToken<T>(this.getOption('token.class'), value, this.getName()); | ||
if (requireValidToken && !token.isValid()) { | ||
throw new InvalidJWTTokenError('Token is empty or invalid.'); | ||
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. same here, we can only throw a more generic error here |
||
} | ||
return token; | ||
} | ||
|
||
getName(): string { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,6 @@ import { NbAuthStrategyOptions } from '../auth-strategy-options'; | |
import { NbAuthSimpleToken } from '../../services/'; | ||
|
||
export class NbDummyAuthStrategyOptions extends NbAuthStrategyOptions { | ||
name: string; | ||
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. why do we need to remove this name? 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. @nnixaa In this case What do you think ? 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. @alain-charles I'm bit confused, as far as I can see, all strategies' options (oauth2, dummy, password) are extended from 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. @nnixaa Have a look at |
||
token? = { | ||
class: NbAuthSimpleToken, | ||
}; | ||
|
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.
InvalidJWTTokenError
->NbAuthInvalidJWTTokenError
would be better