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

Fix/auth oauth2 Password grant_type now sends credentials in x-www-form-urlencoded form #832

Merged
merged 77 commits into from
Oct 17, 2018
Merged

Fix/auth oauth2 Password grant_type now sends credentials in x-www-form-urlencoded form #832

merged 77 commits into from
Oct 17, 2018

Conversation

alain-charles
Copy link
Contributor

Please read and mark the following check list before creating a pull request:

Short description of what this resolves:

Fixes #716

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.
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
…*() methods of authService

Updated unit tests
# Conflicts:
#	src/framework/auth/services/auth.spec.ts
# Conflicts:
#	src/framework/auth/services/auth.spec.ts
…form-urlencoded string

Unit tests updated for corresponding strategy
Resolves #716
@alain-charles
Copy link
Contributor Author

@nnixaa did you have a look at that ?

@nnixaa
Copy link
Collaborator

nnixaa commented Oct 3, 2018

Hi @alain-charles, sorry, will have a look later this week.

Copy link
Collaborator

@nnixaa nnixaa left a comment

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!

src/framework/auth/strategies/oauth2/oauth2-strategy.ts Outdated Show resolved Hide resolved
src/framework/auth/strategies/oauth2/oauth2-strategy.ts Outdated Show resolved Hide resolved
headers = headers.append('Content-Type', 'application/x-www-form-urlencoded');

return this.http.post(url, this.buildPasswordRequestData(username, password),
{headers: headers})
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is no need to a new line here and also the spaces { headers: this.buildAuthHeader() }

src/framework/auth/strategies/oauth2/oauth2-strategy.ts Outdated Show resolved Hide resolved
@alain-charles
Copy link
Contributor Author

@nnixaa i'ms sorry, i had corrected that but it must have been corrupted by some undo command :)

@codecov
Copy link

codecov bot commented Oct 17, 2018

Codecov Report

Merging #832 into master will decrease coverage by 0.19%.
The diff coverage is 100%.

@@           Coverage Diff            @@
##           master    #832     +/-   ##
========================================
- Coverage   76.99%   76.8%   -0.2%     
========================================
  Files         200     201      +1     
  Lines        5947    5966     +19     
  Branches      451     455      +4     
========================================
+ Hits         4579    4582      +3     
- Misses       1262    1278     +16     
  Partials      106     106
Impacted Files Coverage Δ
...ramework/auth/strategies/oauth2/oauth2-strategy.ts 84.13% <100%> (+0.33%) ⬆️
...heme/components/datepicker/datepicker.directive.ts 63.2% <0%> (-10.38%) ⬇️
...heme/components/datepicker/datepicker.component.ts 82.85% <0%> (-3.58%) ⬇️
.../theme/components/datepicker/datepicker-adapter.ts 64.7% <0%> (-2.95%) ⬇️
...rc/framework/theme/components/menu/menu.service.ts 32.6% <0%> (-0.24%) ⬇️
...eme/components/calendar-kit/calendar-kit.module.ts 100% <0%> (ø) ⬆️
.../theme/components/calendar-kit/components/index.ts 100% <0%> (ø) ⬆️
...kit/components/calendar-date/calendar-date.pipe.ts 100% <0%> (ø)
...mework/theme/components/select/select.component.ts 84.65% <0%> (+0.47%) ⬆️
...rk/theme/components/cdk/overlay/overlay-trigger.ts 97.64% <0%> (+1.49%) ⬆️

@nnixaa nnixaa merged commit 57fda28 into akveo:master Oct 17, 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.

NbOAuth2AuthStrategy sends parameters in body instead of using 'x-www-form-urlencoded'
2 participants