feat/auth : created NbAuthOauth2JWTToken class for handling Oauth2 tokens encapsulating jwt access_token #583

alain-charles · 2018-07-27T08:11:49Z

What it does

The class NbAuthOauth2JWTToken has been created for handling oAuth2 backends whose access-token are jwt-token

To do

Implements some methods on the oAuth2 refreshtoken when it is in jwt format too ?
(may be useful in the future when refreshToken expired : useless in this case to send request to the resource backend, redirect to login)

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

…erted in the doc

…hService

…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

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 missing semicolon after conflict resolution

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

nnixaa · 2018-07-27T08:47:55Z

src/framework/auth/services/token/token.ts

+ * Returns access token payload
+ * @returns any
+ */
+ getAccessTokenPayload(): any {


so the getValue()) will return an encoded token and this method returns the decoded value, right?

so the complete token is encoded twice - the whole payload and the access token itself is also a jwt token, right?

As for every token class, the getValue() method returns the string you have to put in the authorization header (Bearer). So yes, in this case, it returns the encoded payload of the access token

I do not know if we can say "encoded twice" since the Oauth2 token has a payload that is not 'encoded'. But yes, the complete token has got a payload with an access-token, that is a jwt token, i.e. the access-token has got a encoded payload with 'iss', 'iat', 'exp'.

I'm sorry, so sorry, but here i am not an IT head, so we have microsoft ADFS OAuth2 servers 😨 , and they put jwt access tokens in their Oauth2 token payload.

See here

Tokens used
These scenarios make use of three token types:

id_token: A JWT token used to represent the identity of the user. The 'aud' or audience claim of the id_token matches the client ID of the native or server application.

access_token: A JWT token used in Oauth and OpenID connect scenarios and intended to be consumed by the resource. The 'aud' or audience claim of this token must match the identifier of the resource or Web API.

refresh_token: This token is submitted in place of collecting user credentials to provide a single sign on experience. This token is both issued and consumed by AD FS, and is not readable by clients or resources.

Maybe here we will find other people who needs to athenticate against Microsoft Oauth2 ADFS servers...

but probably this part should be in the app code already, not library? So that the getPayload will return a complete token payload and then you have to deal with the other parts of the token on your side?

or otherwise we need to also have a getIdTokenPayload() method at least.

Well, just thought of it one more time, and probably this isn't that bad to put these methods inside of the JWTOAuth token, but then I propose the following:

getValue() - token string to put into request, access_token string in case of OAuth2 getRefreshToken() - refresh token string to put into request getIdToken() - id token string to put into request getAccessTokenPayload() getIdTokenPayload() getRefreshTokenPayload()

i think you helped me in my reflexion. If i had to make the choice, i would send this PR to the recycle bin 🤣

The framework do not need to know what github's owner put in their damn Oauth2 tokens :)

Sorry it seems we had reflexion at the same time...
So do you want me do do that ?

getValue() - token string to put into request, access_token string in case of OAuth2 getRefreshToken() - refresh token string to put into request getIdToken() - id token string to put into request getAccessTokenPayload() getIdTokenPayload() getRefreshTokenPayload()

Well, as we still have the default OAuth2 token implementation which is according to the spec, we can move forward with a complete implementation of OAuth2JWT token.

@nnixaa
As far as i know, id_token JWT token is not part of the RFC 6749 oAuth2 spec.
But Section 8.4 of RFC 6749 allows extensions.

=> So id_token is defined by an extension of Oauth2 called OpenId Connect (implemented by Microsoft ADFS 2016, Azure, Google...)

If we want to introduce id_token, we have to introduce a new strategy (that probably extends NbOAuth2Strategy, let us call it NbOpenConnectIdStrategy), with the new response_type id_token, and any of the next combination :
id_token token
code id_token
code token
code id_token token
none

See the openid connect spec
See this excellent paper

We will then have to introduce a new token class for handling that, extending NbAuthOAuth2JWTToken, let's call it NbConnectIdToken.

It would be wonderful to succeed in doing that, but i think we have to make it in multiple PR (maybe a branch for working quiet ?). I would love to help writing that.

So maybe for the moment we can only merge the NbAuthOauth2JWT token as it is, fully compliant with rfc 6749, what do you think ?

@alain-charles yeah, that makes sense. Just a couple of related questions bellow.

codecov · 2018-07-27T08:52:06Z

Codecov Report

Merging #583 into master will increase coverage by 0.14%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
+ Coverage   68.82%   68.97%   +0.14%     
==========================================
  Files         120      120              
  Lines        3394     3410      +16     
  Branches      241      243       +2     
==========================================
+ Hits         2336     2352      +16     
  Misses       1014     1014              
  Partials       44       44

Impacted Files	Coverage Δ
src/framework/auth/services/token/token.ts	`95.14% <100%> (+0.89%)`	⬆️

nnixaa · 2018-07-27T13:48:25Z

src/framework/auth/services/token/token.ts

+ * Returns access token payload
+ * @returns any
+ */
+ getAccessTokenPayload(): any {


@alain-charles yeah, that makes sense. Just a couple of related questions bellow.

nnixaa · 2018-07-27T13:49:44Z

src/framework/auth/services/token/token.ts

+ protected prepareCreatedAt(date: Date) {
+ let decoded;
+ try {
+ decoded = this.getAccessTokenPayload();


so in case when OAuth2 token is JWT, the iat fields is a part of JWT access token, right?

Yes exactly. Redundant with expires_in of the 'containing' token

nnixaa · 2018-07-27T13:51:25Z

src/framework/auth/services/token/token.ts

+ * @returns Date
+ */
+ getTokenExpDate(): Date {
+ if (this.getAccessTokenPayload() && this.getAccessTokenPayload().exp) {


and same here, exp is moved into the token payload?
if so, we need to make this getAccessTokenPayload call once and probably not cover it with a null check same way as for the other tokens

Yes exp is present in the token.access-token payload
OK i'll see that

nnixaa · 2018-07-27T13:53:13Z

src/framework/auth/services/token/token.ts

- date = super.prepareCreatedAt(date);
- let decoded = null;
- try { // needed as getPayload() throws error and we want the token to be created in any case
+ let decoded


alain-charles and others added 30 commits June 29, 2018 08:33

Added password grant_type option

cef7a1d

Added password grant_type option implementation

9f0dbab

Playground sample for oAuth2 password grant-type

44d7d2e

Added route to oAuth2 password grant-type sample

f57b737

Addes angular-jwt for decoding jwt token (used in playground only)

ed4628e

Added /api/auth/token endpoint (oAuth2 password grant-type playground)

d7f01df

Patched code for passing ci

af52e88

code optimization for passing ci

554f7dc

Merge branch 'master' into master

fa0548f

Changes request by Dmitry (first review)

b72adae

Generalized code in oauth2-strategy

733efc0

Now : If existing, NbAuthResult contains backend error description other Changes requested by Dmitry (first review)

Merge remote-tracking branch 'origin/master'

57fab69

Generalized code in oauth2-strategy

c50df3a

Now : If existing, NbAuthResult contains backend error description other Changes requested by Dmitry (first review) +tslint missing trailing comma arghhh

Added unit tests for oAuth-strategy password authentication

dda5436

Merge branch 'master' into master

ef0be25

Merge branch 'master' into master

e180dc3

Cleaned code according to nnixaa second review

6aa1fdb

Merge branch 'master' of https://github.com/alain-charles/nebular

96c7344

Removed package-lock.json from git repo

cf82c42

package-lock.json to revert

dcff930

reverted package-lock.json

315662f

Added new grant_type 'PASSWORD' in the block comment for it to be ins…

57a0230

…erted in the doc

Merge remote-tracking branch 'upstream/master'

d69d633

Add the refreshtoken request management in NbJwtInterceptor and NbAut…

1be8d91

…hService

Merge branch 'master' of https://github.com/alain-charles/nebular

4b67825

Merge branch 'master' into temp

deea3e2

The token now contains ownerStrategyName, with is a back link to the …

acd0241

…strategy used to create the token (future use)

feature/auth:

bd564b3

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.

nnixaa and others added 17 commits July 20, 2018 19:28

Merge branch 'master' into master

44ed61a

feature/auth

9c16387

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

Merge remote-tracking branch 'origin/master'

2dd26c4

Merge branch 'master' into master

b284377

feature/auth

e462c7d

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 missing semicolon after conflict resolution

Merge branch 'master' into master

a3ee10b

Merge branch 'master' into master

b38ae55

feature/auth

62a2aff

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

Merge remote-tracking branch 'origin/master'

66ca493

feature/auth

3bd5a50

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

Merge branch 'master' into master

ee706d3

feature/auth

5eb5821

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)

Merge remote-tracking branch 'origin/master'

1a08225

feature/auth

1d697bc

A new class of token has been defined. NbAuthOAuth2JWTToken : class to use when your auth backend sends Oauth tokens encapsulating a jwt access token.

Merge remote-tracking branch 'upstream/master'

1c25956

# Conflicts: # src/framework/auth/services/token/token.ts Optimized token.ts code

Merge remote-tracking branch 'upstream/master'

ff0e8d3

# Conflicts: # src/framework/auth/services/token/token.ts Optimized token.ts code

nnixaa reviewed Jul 27, 2018

View reviewed changes

alain-charles added 2 commits July 28, 2018 16:04

Optimized getAccessTokenPayload() calls

e7af99e

Optimized getAccessTokenPayload() calls

95632b1

nnixaa approved these changes Jul 30, 2018

View reviewed changes

Merge branch 'master' into master

bc4001f

nnixaa merged commit aed2099 into akveo:master Jul 30, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat/auth : created NbAuthOauth2JWTToken class for handling Oauth2 tokens encapsulating jwt access_token #583

feat/auth : created NbAuthOauth2JWTToken class for handling Oauth2 tokens encapsulating jwt access_token #583

alain-charles commented Jul 27, 2018

nnixaa Jul 27, 2018

alain-charles Jul 27, 2018 •

edited

Loading

nnixaa Jul 27, 2018

nnixaa Jul 27, 2018

nnixaa Jul 27, 2018

alain-charles Jul 27, 2018 •

edited

Loading

alain-charles Jul 27, 2018

nnixaa Jul 27, 2018

alain-charles Jul 27, 2018 •

edited

Loading

nnixaa Jul 27, 2018

codecov bot commented Jul 27, 2018 •

edited

Loading

nnixaa Jul 27, 2018

nnixaa Jul 27, 2018

alain-charles Jul 27, 2018

nnixaa Jul 27, 2018

alain-charles Jul 27, 2018

nnixaa Jul 27, 2018

feat/auth : created NbAuthOauth2JWTToken class for handling Oauth2 tokens encapsulating jwt access_token #583

feat/auth : created NbAuthOauth2JWTToken class for handling Oauth2 tokens encapsulating jwt access_token #583

Conversation

alain-charles commented Jul 27, 2018

What it does

To do

Choose a reason for hiding this comment

alain-charles Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alain-charles Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alain-charles Jul 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 27, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alain-charles Jul 27, 2018 •

edited

Loading

alain-charles Jul 27, 2018 •

edited

Loading

alain-charles Jul 27, 2018 •

edited

Loading

codecov bot commented Jul 27, 2018 •

edited

Loading