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

NbOAuth2AuthStrategy token request body now provides username instead of email parameter #659

Merged
merged 61 commits into from
Aug 23, 2018
Merged

Conversation

alain-charles
Copy link
Contributor

@alain-charles alain-charles commented Aug 22, 2018

According to RFC6749 section 4.3.2, the oAuth2 request body with grant-type='password' must provide username to the auth server's token endpoint instead of email.

According to RFC6749 section 4.3.2, the oAuth2 request body with grant-type='password' may provide scope to the auth server's token endpoint.

=> Corrects issue #653

alain-charles and others added 30 commits June 29, 2018 08:33
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.
nnixaa and others added 26 commits July 24, 2018 15:47
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
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
…h grant-type='password' must provide username to the auth server and not email.

Corrects issue #653
…h grant-type='password' MAY provide the scope parameter

=>Corrects completely issue #653
@codecov
Copy link

codecov bot commented Aug 22, 2018

Codecov Report

Merging #659 into master will not change coverage.
The diff coverage is 100%.

@@           Coverage Diff           @@
##           master     #659   +/-   ##
=======================================
  Coverage   72.74%   72.74%           
=======================================
  Files         154      154           
  Lines        4296     4296           
  Branches      333      333           
=======================================
  Hits         3125     3125           
  Misses       1107     1107           
  Partials       64       64
Impacted Files Coverage Δ
.../auth/strategies/oauth2/oauth2-strategy.options.ts 100% <ø> (ø) ⬆️
...ramework/auth/strategies/oauth2/oauth2-strategy.ts 83.8% <100%> (ø) ⬆️

@nnixaa
Copy link
Collaborator

nnixaa commented Aug 23, 2018

@alain-charles ah, sorry I missed it during the review.

@alain-charles
Copy link
Contributor Author

And I missed it before 🤔

@nnixaa nnixaa merged commit 3a708dd into akveo:master Aug 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants