feature/auth : NbOAuth2AuthStrategy implementing basic authentication scheme against token endpoints #582
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
NbOAuth2AuthStrategy now implements basic authentication scheme against token endpoints if both clientId and clientSecret are set. (See https://tools.ietf.org/html/rfc6749#section-2.3) TODO : Alternatively, clientId and clientSecret can be params of the request (RFC 6749) => add a parameter to the strategy indicating if it shall user header or reqsuest params to send credentials
nnixaa
requested changes
Jul 27, 2018
| @@ -282,6 +282,20 @@ export class NbOAuth2AuthStrategy extends NbAuthStrategy { | |||
| return this.cleanParams(params); | |||
| } | |||
|
|
|||
| protected buildRequestsHttpOptions(): any { | |||
There was a problem hiding this comment.
This makes sense since clientId and clientSecret are optional
There was a problem hiding this comment.
Sorry, my comment was a bit misleading. What I meant is that probably we should add an option to the configuration as you proposed in the PR description indicating if we want to enable the base auth, pass clientid/cliensecret as body parameters or none of the above?
There was a problem hiding this comment.
@alain-charles just checking if you saw my comment :)
| @@ -4,7 +4,7 @@ | |||
| * Licensed under the MIT License. See License.txt in the project root for license information. | |||
| */ | |||
| import { Inject, Injectable } from '@angular/core'; | |||
| import { HttpClient, HttpErrorResponse } from '@angular/common/http'; | |||
| import {HttpClient, HttpErrorResponse, HttpHeaders} from '@angular/common/http'; | |||
|
I have seen it in my mailbox
Going to setup the options none / auth / body
Alain
… Le 27 juil. 2018 à 17:36, > Dmitry Nehaychik (par Internet, dépôt ***@***.***) ***@***.***> a écrit :
@nnixaa commented on this pull request.
In src/framework/auth/strategies/oauth2/oauth2-strategy.ts:
> @@ -282,6 +282,20 @@ export class NbOAuth2AuthStrategy extends NbAuthStrategy {
return this.cleanParams(params);
}
+ protected buildRequestsHttpOptions(): any {
@alain-charles just checking if you saw my comment :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
- NONE (default) - BASIC : credentials are sent in the authorization header - REQUEST_BODY: credentials are sent in the request body AuthMethod is used (credentials are sent) when accessing to the authServer for : - Getting token with code grant_type - Getting token with password grant-type - Getting token with refresh_token grant-type RFC6749 says the client must not authenticate when hitting authorize endpoint, even if asking for a token. NO BREAKING CHANGES because of defaults to NONE.
nnixaa
requested changes
Jul 30, 2018
| import { NbOAuth2AuthStrategy } from './oauth2-strategy'; | ||
| import { NbOAuth2GrantType, NbOAuth2ResponseType } from './oauth2-strategy.options'; | ||
| import {NbOAuth2ClientAuthMethod, NbOAuth2GrantType, NbOAuth2ResponseType} from './oauth2-strategy.options'; |
…t/auth_oAuth2_basic
nnixaa
approved these changes
Jul 30, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What it resolves
NbOAuth2Strategy now implements client authentication as specified in RFC 6749 section 2-3
There is a new optional parameter of
NbOAuth2StrategyOption.The parameter is
clientAuthMethod, and is a member ofNbOAuth2ClientAuthMethodenum:NONE(default) : no credentials are sent => No breaking change,BASIC: credentials are sent in the authorization headerREQUEST_BODY: credentials are sent in the request bodyAuthMethod is used (credentials are sent) when accessing to the authServer for :
authorization_codegrant_typepasswordgrant-typerefresh_tokengrant-typeRFC6749 says the client must not authenticate when hitting authorize endpoints, even if asking for a token. So nothing changed here, only clientId is sent in the url.
issue resolved
issue #581