Skip to content
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 improvement : refresh-token are now saved if not repeated by the backend refresh endpoint #593

Merged
merged 60 commits into from
Aug 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
cef7a1d
Added password grant_type option
alain-charles Jun 29, 2018
9f0dbab
Added password grant_type option implementation
alain-charles Jun 29, 2018
44d7d2e
Playground sample for oAuth2 password grant-type
alain-charles Jun 29, 2018
f57b737
Added route to oAuth2 password grant-type sample
alain-charles Jun 29, 2018
ed4628e
Addes angular-jwt for decoding jwt token (used in playground only)
alain-charles Jun 29, 2018
d7f01df
Added /api/auth/token endpoint (oAuth2 password grant-type playground)
alain-charles Jun 29, 2018
af52e88
Patched code for passing ci
alain-charles Jun 29, 2018
554f7dc
code optimization for passing ci
alain-charles Jun 29, 2018
fa0548f
Merge branch 'master' into master
nnixaa Jun 29, 2018
b72adae
Changes request by Dmitry (first review)
alain-charles Jun 29, 2018
733efc0
Generalized code in oauth2-strategy
alain-charles Jun 29, 2018
57fab69
Merge remote-tracking branch 'origin/master'
alain-charles Jun 29, 2018
c50df3a
Generalized code in oauth2-strategy
alain-charles Jun 29, 2018
dda5436
Added unit tests for oAuth-strategy password authentication
alain-charles Jul 2, 2018
ef0be25
Merge branch 'master' into master
alain-charles Jul 3, 2018
e180dc3
Merge branch 'master' into master
nnixaa Jul 4, 2018
6aa1fdb
Cleaned code according to nnixaa second review
alain-charles Jul 4, 2018
96c7344
Merge branch 'master' of https://github.com/alain-charles/nebular
alain-charles Jul 4, 2018
cf82c42
Removed package-lock.json from git repo
alain-charles Jul 4, 2018
dcff930
package-lock.json to revert
alain-charles Jul 4, 2018
315662f
reverted package-lock.json
alain-charles Jul 4, 2018
57a0230
Added new grant_type 'PASSWORD' in the block comment for it to be ins…
alain-charles Jul 4, 2018
d69d633
Merge remote-tracking branch 'upstream/master'
alain-charles Jul 5, 2018
1be8d91
Add the refreshtoken request management in NbJwtInterceptor and NbAut…
alain-charles Jul 13, 2018
4b67825
Merge branch 'master' of https://github.com/alain-charles/nebular
alain-charles Jul 19, 2018
deea3e2
Merge branch 'master' into temp
alain-charles Jul 19, 2018
acd0241
The token now contains ownerStrategyName, with is a back link to the …
alain-charles Jul 19, 2018
25250f0
feature/auth:
alain-charles Jul 19, 2018
c0fdd31
feature/auth:
alain-charles Jul 19, 2018
bd564b3
feature/auth:
alain-charles Jul 20, 2018
44ed61a
Merge branch 'master' into master
nnixaa Jul 20, 2018
9c16387
feature/auth
alain-charles Jul 24, 2018
2dd26c4
Merge remote-tracking branch 'origin/master'
alain-charles Jul 24, 2018
b284377
Merge branch 'master' into master
alain-charles Jul 24, 2018
e462c7d
feature/auth
alain-charles Jul 24, 2018
a3ee10b
Merge branch 'master' into master
nnixaa Jul 24, 2018
b38ae55
Merge branch 'master' into master
nnixaa Jul 25, 2018
62a2aff
feature/auth
alain-charles Jul 25, 2018
66ca493
Merge remote-tracking branch 'origin/master'
alain-charles Jul 25, 2018
3bd5a50
feature/auth
alain-charles Jul 25, 2018
398c058
feature/auth
alain-charles Jul 25, 2018
ee706d3
Merge branch 'master' into master
alain-charles Jul 26, 2018
5eb5821
feature/auth
alain-charles Jul 26, 2018
1a08225
Merge remote-tracking branch 'origin/master'
alain-charles Jul 26, 2018
1d697bc
feature/auth
alain-charles Jul 27, 2018
1c25956
Merge remote-tracking branch 'upstream/master'
alain-charles Jul 27, 2018
ff0e8d3
Merge remote-tracking branch 'upstream/master'
alain-charles Jul 27, 2018
e7af99e
Optimized getAccessTokenPayload() calls
alain-charles Jul 28, 2018
95632b1
Optimized getAccessTokenPayload() calls
alain-charles Jul 28, 2018
bc4001f
Merge branch 'master' into master
nnixaa Jul 30, 2018
e06a45c
Feat/auth
alain-charles Jul 31, 2018
7c6fc43
Merge remote-tracking branch 'upstream/master' into feat/auth-Automat…
alain-charles Jul 31, 2018
88c7106
Moved code from tokenservice to strategy.
alain-charles Jul 31, 2018
76dda02
Added setRefreshToken(token)
alain-charles Jul 31, 2018
10d0b31
oAuth2Strategy : Move refreshToken creation into independant method
alain-charles Aug 1, 2018
f6ba089
Merge branch 'master' into feat/auth-AutomaticRefreshToken
nnixaa Aug 6, 2018
c88b96e
Merge branch 'master' into feat/auth-AutomaticRefreshToken
nnixaa Aug 7, 2018
b3eb264
refactor(token): generic type for `createToken` method
nnixaa Aug 7, 2018
a491f21
Merge pull request #1 from nnixaa/alain-charles-feat/auth-AutomaticRe…
alain-charles Aug 7, 2018
0ae51f6
added parameter type = NbAuthToken in exported const NB_AUTH_TOKENS.
alain-charles Aug 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/framework/auth/auth.options.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { InjectionToken } from '@angular/core';
import { NbAuthStrategy, NbAuthStrategyOptions } from './strategies';
import { NbAuthTokenClass } from './services';
import { NbAuthToken, NbAuthTokenClass } from './services/token/token';

export type NbAuthStrategyClass = new (...params: any[]) => NbAuthStrategy;

Expand Down Expand Up @@ -87,5 +87,5 @@ export const defaultAuthOptions: any = {
export const NB_AUTH_OPTIONS = new InjectionToken<NbAuthOptions>('Nebular Auth Options');
export const NB_AUTH_USER_OPTIONS = new InjectionToken<NbAuthOptions>('Nebular User Auth Options');
export const NB_AUTH_STRATEGIES = new InjectionToken<NbAuthStrategies>('Nebular Auth Strategies');
export const NB_AUTH_TOKENS = new InjectionToken<NbAuthTokenClass[]>('Nebular Auth Tokens');
export const NB_AUTH_TOKENS = new InjectionToken<NbAuthTokenClass<NbAuthToken>[]>('Nebular Auth Tokens');
export const NB_AUTH_INTERCEPTOR_HEADER = new InjectionToken<NbAuthStrategies>('Nebular Simple Interceptor Header');
2 changes: 1 addition & 1 deletion src/framework/auth/services/auth.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ export class NbAuthService {
* @returns {Observable<NbAuthResult>}
*/
refreshToken(strategyName: string, data?: any): Observable<NbAuthResult> {
return this.getStrategy(strategyName).refreshToken()
return this.getStrategy(strategyName).refreshToken(data)
.pipe(
switchMap((result: NbAuthResult) => {
return this.processResultToken(result);
Expand Down
15 changes: 12 additions & 3 deletions src/framework/auth/services/token/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ export abstract class NbAuthToken {

export interface NbAuthRefreshableToken {
getRefreshToken(): string;
setRefreshToken(refreshToken: string);
}

export interface NbAuthTokenClass {
export interface NbAuthTokenClass<T = NbAuthToken> {
NAME: string;
new (raw: any, ownerStrategyName: string, createdAt: Date): NbAuthToken;
new (raw: any, strategyName: string, expDate?: Date): T;
}

export function nbAuthCreateToken(tokenClass: NbAuthTokenClass,
export function nbAuthCreateToken<T extends NbAuthToken>(tokenClass: NbAuthTokenClass<T>,
token: any,
ownerStrategyName: string,
createdAt?: Date) {
Expand Down Expand Up @@ -206,6 +207,14 @@ export class NbAuthOAuth2Token extends NbAuthSimpleToken {
return this.token.refresh_token;
}

/**
* put refreshToken in the token payload
* @param refreshToken
*/
setRefreshToken(refreshToken: string) {
this.token.refresh_token = refreshToken;
}

/**
* Returns token payload
* @returns any
Expand Down
4 changes: 2 additions & 2 deletions src/framework/auth/strategies/auth-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ export abstract class NbAuthStrategy {
return getDeepFromObject(this.options, key, null);
}

createToken(value: any): NbAuthToken {
return nbAuthCreateToken(this.getOption('token.class'), value, this.getName());
createToken<T extends NbAuthToken>(value: any): T {
return nbAuthCreateToken<T>(this.getOption('token.class'), value, this.getName());
}

getName(): string {
Expand Down
70 changes: 70 additions & 0 deletions src/framework/auth/strategies/oauth2/oauth2-strategy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,36 @@ describe('oauth2-auth-strategy', () => {
example_parameter: 'example_value',
};

const tokenWithoutRefreshTokenResponse = {
access_token: '8uoloUIg765fHGF9jknjksdn9',
expires_in: 3600,
example_parameter: 'example_refresh_value',
}

const refreshedTokenPayload = {
access_token: '8uoloUIg765fHGF9jknjksdn9',
expires_in: 3600,
refresh_token: 'tGzv3JOkF0XG5Qx2TlKWIA',
example_parameter: 'example_refresh_value',
}

const refreshedTokenResponse = {
access_token: '8uoloUIg765fHGF9jknjksdn9',
expires_in: 3600,
refresh_token: 'dfsjkgkdh989JHJHJDSHJns',
example_parameter: 'example_refresh_value',
}

const tokenErrorResponse = {
error: 'unauthorized_client',
error_description: 'unauthorized',
error_uri: 'some',
};

const successToken = nbAuthCreateToken(NbAuthOAuth2Token, tokenSuccessResponse, 'strategy') as NbAuthOAuth2Token;
// tslint:disable-next-line
const refreshedToken = nbAuthCreateToken(NbAuthOAuth2Token, refreshedTokenPayload, 'strategy') as NbAuthOAuth2Token;
const refreshedTokenWithRefreshToken = nbAuthCreateToken(NbAuthOAuth2Token, refreshedTokenResponse, 'strategy') as NbAuthOAuth2Token;


beforeEach(() => {
Expand Down Expand Up @@ -241,6 +264,53 @@ describe('oauth2-auth-strategy', () => {
).flush(tokenSuccessResponse);
});

it('handle refresh token and inserts existing refresh_token if needed', (done: DoneFn) => {
strategy.setOptions(basicOptions);
strategy.refreshToken(successToken)
.subscribe((result: NbAuthResult) => {
expect(result).toBeTruthy();
expect(result.isSuccess()).toBe(true);
expect(result.isFailure()).toBe(false);
expect(result.getToken().getValue()).toEqual(refreshedToken.getValue());
expect(result.getToken().getOwnerStrategyName()).toEqual(refreshedToken.getOwnerStrategyName());
expect(result.getMessages()).toEqual(successMessages);
expect(result.getErrors()).toEqual([]); // no error message, response is success
expect(result.getRedirect()).toEqual('/');
done();
});

httpMock.expectOne(
req => req.url === 'http://example.com/token'
&& req.body['grant_type'] === NbOAuth2GrantType.REFRESH_TOKEN
&& req.body['refresh_token'] === successToken.getRefreshToken()
&& !req.body['scope'],
).flush(tokenWithoutRefreshTokenResponse);
});

it('Handle refresh-token and leaves refresh_token unchanged if present', (done: DoneFn) => {
strategy.setOptions(basicOptions);
strategy.refreshToken(successToken)
.subscribe((result: NbAuthResult) => {
expect(result).toBeTruthy();
expect(result.isSuccess()).toBe(true);
expect(result.isFailure()).toBe(false);
expect(result.getToken().getValue()).toEqual(refreshedTokenWithRefreshToken.getValue());
expect(result.getToken().getOwnerStrategyName()).
toEqual(refreshedTokenWithRefreshToken.getOwnerStrategyName());
expect(result.getMessages()).toEqual(successMessages);
expect(result.getErrors()).toEqual([]); // no error message, response is success
expect(result.getRedirect()).toEqual('/');
done();
});

httpMock.expectOne(
req => req.url === 'http://example.com/token'
&& req.body['grant_type'] === NbOAuth2GrantType.REFRESH_TOKEN
&& req.body['refresh_token'] === successToken.getRefreshToken()
&& !req.body['scope'],
).flush(refreshedTokenResponse);
});

it('handle error token refresh response', (done: DoneFn) => {

strategy.refreshToken(successToken)
Expand Down
18 changes: 16 additions & 2 deletions src/framework/auth/strategies/oauth2/oauth2-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@ import { switchMap, map, catchError } from 'rxjs/operators';
import { NB_WINDOW } from '@nebular/theme';

import { NbAuthStrategy } from '../auth-strategy';
import { NbAuthRefreshableToken, NbAuthResult } from '../../services/';
import {
NbAuthRefreshableToken,
NbAuthResult,
NbAuthToken,
} from '../../services/';
import {
NbOAuth2AuthStrategyOptions,
NbOAuth2ResponseType,
Expand Down Expand Up @@ -205,7 +209,7 @@ export class NbOAuth2AuthStrategy extends NbAuthStrategy {
this.getOption('redirect.success'),
[],
this.getOption('defaultMessages'),
this.createToken(res));
this.createRefreshedToken(res, token));
}),
catchError((res) => this.handleResponseError(res)),
);
Expand Down Expand Up @@ -372,6 +376,16 @@ export class NbOAuth2AuthStrategy extends NbAuthStrategy {
}, {}) : {};
}

protected createRefreshedToken(res, existingToken: NbAuthRefreshableToken): NbAuthToken {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we can leave this check as:

if (!refreshedToken.getRefreshToken() && existingToken.getRefreshToken()) {

as we can only have the NbAuthRefreshableToken token here, which presumes that both required get and set method exist because this method could only be called from the refreshToken one, which cannot be called for a non-refreshable token.
In case when the developer forces a call of refreshToken method for a non-refreshble token it will be a typescript level error, which is completely fine in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!refreshedToken.getRefreshToken() does not compile as NbAuthToken (the return type) does not implement this method.
That is the reason why i added the isNbAuthRefreshableToken(refreshedToken) test

So what do you prefer :

  • should we return NbAuthOauth2Token ?
  • or just cast in the body of the method the refreshedToken to NbAuth0Auth2Token ?
  • or leave the code like in #10d0b31 commit ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alain-charles can we do this:

protected createRefreshedToken(res, existingToken: NbAuthToken & NbAuthRefreshableToken)

Sorry not able to check it myself.
If we can't, let's leave it as your current version.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alain-charles sorry for the delay, wrote the comment last Thursday but forgot to submit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nnixaa with the intersection type you propose above, the problem remains on refreshedToken

const refreshedToken = this.createToken(res);

this.createToken(res) returns NbAuthToken that does not implement getRefreshToken().

Any other idea ?

Alain

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

type AuthRefreshToken = NbAuthRefreshableToken & NbAuthToken;

const refreshedToken: AuthRefreshToken = this.createToken<AuthRefreshToken>(res);
if (!refreshedToken.getRefreshToken() && existingToken.getRefreshToken()) {
refreshedToken.setRefreshToken(existingToken.getRefreshToken());
}
return refreshedToken;
}

register(data?: any): Observable<NbAuthResult> {
throw new Error('`register` is not supported by `NbOAuth2AuthStrategy`, use `authenticate`.');
}
Expand Down