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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is the response type
id_token
in the first place?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if this is really needed at this place.
I'll test it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After trying, if I remove the id_token from the signIn It will remove the id_token from the session's stored user...
I think this could be an issue...
To not create a side effect, I guess it is better to keep both id_token and access_token in the request ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kbeaugrand is there a reason why your response type is not
code
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiercn , because this method should request for an access token, not an authorization code ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kbeaugrand this method should request whatever is configured in the settings, it's not up to the library to change the response, changing the response type affects the security characteristics of the app.
id_token
,access_token
, what they do, is request for the tokens to be delivered by the authorization endpoint (implicit flows).What tokens you get back is based on the scopes you request. At a minimum an access token to talk to your API, and optionally, in addition to that, an ID token to identify the user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiercn, the issue mentioned, relates that by using the standard documentation, the Authentication is requesting the id_token;
source: https://docs.microsoft.com/en-us/aspnet/core/blazor/security/webassembly/standalone-with-authentication-library?view=aspnetcore-6.0&tabs=visual-studio#access-token-scopes.
I' would be happy if by setting
ProviderOptions.ResponseType
to"id_token token"
my client goes well to the authentication but this is not the case.By using the
BaseAddressAuthorizationMessageHandler
, the program calls thegetAccessToken
method fromAuthenticationService
.As I see in the code, there is condition, if the access token is present and has all the required scopes, it returns the access token. otherwise it will sign in silently.
If the access token is not present (because not requested) and we don't force to request the access token, we never will have that ...
I might not understand correctly the philosophy of this library and the solution might not be this one.
In your opinion, what should be the correct approach ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@javiercn , nevermind, I got why getting the access token as part of the login doesn't work in my case...
In my case, I'm using OpenID connect with Azure AD B2C as the IdP.
AD B2C doesn't provide the user info endpoint and he oidc client used requires it ;(
It fails to perfom the sign in and validates the login but without any error message...
By changing the settings, I got my flow working (after disabling the loadUserInfo (see: https://github.com/IdentityModel/oidc-client-js/wiki#other-optional-settings)
After, that I can have my access token with the id_token.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I close this PR since it doesn't provide what I expected.
However, the documentation makes me some troubles, do you think this could be an opportunity to documentation that if the access token is required to authenticate to APIs, the developer should also change the
reponse_type
to add the token request as part of the sign in process ?