Feat/Auth : refreshToken request automatically sent by NbAuthService if isAuthenticatedOrRefresh is called #649
Conversation
Now : If existing, NbAuthResult contains backend error description other Changes requested by Dmitry (first review)
Now : If existing, NbAuthResult contains backend error description other Changes requested by Dmitry (first review) +tslint missing trailing comma arghhh
…strategy used to create the token (future use)
The token now contains ownerStrategyName, with is a back link to the strategy used to create the token (future use). BREAKING CHANGE : NbAuthCreateToken (token.ts) now takes a third parameter, which is the ownerStrategyName Tokens are created by strategies that are called from services, so it is *potentially* a breaking change.
The token now contains ownerStrategyName, with is a back link to the strategy used to create the token (future use). updated unit tests files and oauth2-password-login.component (breaking change below) BREAKING CHANGE : NbAuthCreateToken (token.ts) now takes a third parameter, which is the ownerStrategyName Tokens are created by strategies that are called from services, so it is *potentially* a breaking change.
removed useless code and cleaned one unit test file BREAKING CHANGE : NbAuthCreateToken (token.ts) now takes a third parameter, which is the ownerStrategyName Tokens are created by strategies that are called from services, so it is *potentially* a breaking change.
Tokens can now be asked for their creation date via createdAt attribute or getCreatedAt() NbAuthOAuth2Token.getTokenExpDate() now returns a correct expDate (Issue #564) Unit tests updated nnixaa first review
Tokens can now be asked for their creation date via createdAt attribute or getCreatedAt() NbAuthOAuth2Token.getTokenExpDate() now returns a correct expDate (Issue #564) Unit tests updated nnixaa first review nnixaa second review
Tokens can now be asked for their creation date via createdAt attribute or getCreatedAt() NbAuthOAuth2Token.getTokenExpDate() now returns a correct expDate (Issue #564) Unit tests updated nnixaa first review nnixaa second review removed attributes declaration overriding in NbAuthOauth2Token constructor
Tokens can now be asked for their creation date via createdAt attribute or getCreatedAt() NbAuthOAuth2Token.getTokenExpDate() now returns a correct expDate (Issue #564) Unit tests updated nnixaa first review nnixaa second review nnixaa third review (am i so bad ? :-p)
A new class of token has been defined. NbAuthOAuth2JWTToken : class to use when your auth backend sends Oauth tokens encapsulating a jwt access token.
# Conflicts: # src/framework/auth/services/token/token.ts Optimized token.ts code
# Conflicts: # src/framework/auth/services/token/token.ts Optimized token.ts code
…thJWTInterceptor to NbAuthService
Codecov Report
@@ Coverage Diff @@
## master #649 +/- ##
==========================================
+ Coverage 72.44% 72.74% +0.29%
==========================================
Files 153 154 +1
Lines 4272 4296 +24
Branches 330 333 +3
==========================================
+ Hits 3095 3125 +30
+ Misses 1113 1107 -6
Partials 64 64
|
| ) | ||
| } else { | ||
| return observableOf(token.isValid()); | ||
| } |
| @@ -24,7 +24,7 @@ describe('auth-service', () => { | |||
| let tokenService: NbTokenService; | |||
| let dummyAuthStrategy: NbDummyAuthStrategy; | |||
| const testTokenValue = 'test-token'; | |||
| const ownerStrategyName = 'strategy'; | |||
| const ownerStrategyName = 'dummy'; | |||
There was a problem hiding this comment.
do we need this change? looks unrelated
There was a problem hiding this comment.
Yes we do since i added isAuthenticatedOrRefresh() calls in the unit tests
The strategy provided in test module is named 'dummy'.
So i modified the ownerStrategyName to dummy and replaced all 'dummy' occurrences by ownerStrategyName.
I know everything is not related to this PR, but is was necessary.
src/framework/auth/auth.module.ts
Outdated
| @@ -60,7 +60,6 @@ export function nbStrategiesFactory(options: NbAuthOptions, injector: Injector): | |||
| .forEach(([strategyClass, strategyOptions]: [NbAuthStrategyClass, NbAuthStrategyOptions]) => { | |||
| const strategy: NbAuthStrategy = injector.get(strategyClass); | |||
| strategy.setOptions(strategyOptions); | |||
|
|
|||
There was a problem hiding this comment.
do we need this change? looks unrelated
There was a problem hiding this comment.
OK i put on back the empty line :)
| import { NbAuthService } from '../auth.service'; | ||
| import { NbAuthJWTToken } from '../token/token'; | ||
| import { NB_AUTH_TOKEN_INTERCEPTOR_FILTER} from '../../auth.options'; |
| switchMap(authenticated => { | ||
| if (authenticated) { | ||
| return this.authService.getToken().pipe( | ||
| switchMap( (token: NbAuthToken) => { |
There was a problem hiding this comment.
@alain-charles just one last formatting issue here :)
switchMap( (token: NbAuthToken) => { -> switchMap((token: NbAuthToken) => {
There was a problem hiding this comment.
I run npm lint before I push but it seems my config is not as restrictive as yours .
Which one do you use ?
There was a problem hiding this comment.
It looks like the project lint config isn't the best one. Will look into it.
| } | ||
|
|
||
| protected get authService(): NbAuthService { | ||
| return this.injector.get(NbAuthService); | ||
| } | ||
|
|
||
| protected get filter(): (req: HttpRequest<any>) => boolean { | ||
| return this.injector.get(NB_AUTH_TOKEN_INTERCEPTOR_FILTER); |
There was a problem hiding this comment.
I guess the token can be injected using constructor (except for AuthService)
| } | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
couple of unnecessary new lines
| switchMap(authenticated => { | ||
| if (authenticated) { | ||
| return this.authService.getToken().pipe( | ||
| switchMap( (token: NbAuthToken) => { |
There was a problem hiding this comment.
@alain-charles just one last formatting issue here :)
switchMap( (token: NbAuthToken) => { -> switchMap((token: NbAuthToken) => {
|
@alain-charles I just noticed that we don't provide a default value for the filter right? Meaning that if we try to use the interceptor without providing a filter it will fail by calling We can provide a |
|
No we don’t. |
Returns always true so that NO url is intercepted => the user writes the filter according to the doc (Auth urls MUST be filtered) and injects it in his own auth_module
| .pipe( | ||
| switchMap(token => { | ||
| if (!token.isValid()) { | ||
| return this.refreshToken(token.getOwnerStrategyName(), token) |
There was a problem hiding this comment.
Does this work with any kind of token type? Is the refreshToken endpoint always expecting the same token that was stored?
I am using an API that during authentication returns an object consisting of a refresh token, an access token and other properties. However, on refreshing the token I am expected to only supply the refresh token.
To get it working, I had to convert the token to an NbAuthOAuth2Token and explicitly call "oAuth2Token.getRefreshToken()" on this line instead of just "token".
There was a problem hiding this comment.
Hi @cloakedch ,
The refresh_token is implemented according to RFC6749 section 1.5.
This RFC specifies the OAuth2 authentication scheme. So the code works with auths server returning tokens according to the RFC section 4.2
The refreshToken endpoint is waiting for the refresh_token that was sent during login or during the latest refresh_token call. Both grant_type=refresh_token and refresh_token=theRefreshToken are sent to the refreshToken endpoint for getting a new access-token, according to the RFC.
I would say that you have to use the NbOAuth2AuthStrategy with password grantType(if you send login/password at login) and NbAuthOAuth2Tokenclass in your case.
Someting like
NbOAuth2AuthStrategy.setup({
name: 'password',
clientId: 'test',
clientSecret: 'secret',
baseEndpoint: 'http:https://your.BaseEnd/Point',
token: {
endpoint: 'token',
grantType: NbOAuth2GrantType.PASSWORD,
class: NbAuthOAuth2Token,
},
refresh: {
endpoint: 'refresh-token',
grantType: NbOAuth2GrantType.REFRESH_TOKEN,
},
}),
Hope it helps
There was a problem hiding this comment.
OK, I guess the API I am using is then not working according to the RFC.
Thanks for clearing that up!
|
@alain-charles congrats! |
Feat/Auth :
NbAuthServiceifisAuthenticatedOrRefresh()is calledNbAuthJWTInterceptornow callsNbAuthService.isAuthenticatedOrRefresh()POTENTIAL BREAKING CHANGE if
NbAuthJWTInterceptoris used :The developper has now to provide in his auth_module a function
filterInterceptorRequest(req: HttpRequest<any>):booleanthat returns true if req.url must not be intercepted for bearer token injection.The auth urls MUST be included there.
Playground sample follows.