Fix authorization code flow to work with openiddict library #3951
Conversation
Co-authored-by: Tanel Kuhi <tanel.kuhi@helmes.com>
| @@ -48,23 +48,25 @@ public async Task<TokenResponse> GetToken(Secret secret, string? authCode, strin | |||
| { "grant_type", secret.GetProperty("GrantType") }, | |||
| { "client_id", clientId }, | |||
| { "client_secret", clientSecret }, | |||
| { "scope", secret.GetProperty("Scope") } | |||
| { "offline_access ", "true" } | |||
There was a problem hiding this comment.
Hey 👋🏻
FYI, offline_access is not a standard parameter (and not used by OpenIddict unless you add custom logic to handle it). It also seems there's an extra space at the end of the parameter name so it's likely something you'll want to fix/remove 😃
There was a problem hiding this comment.
Damn, you are right. "offline_access" is probably not required indeed, but was something I added when trying to get refresh tokens to work. I will create a new PR to remove this.
There was a problem hiding this comment.
@tanelkuhi If you create a PR with the required corrections we may be able to include it in a 2.11.2 release soon
| try | ||
| { | ||
| var httpClient = _httpClientFactory.CreateClient(nameof(OAuth2TokenService)); | ||
| httpClient.DefaultRequestHeaders.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); | ||
| httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Basic", Base64Encode(clientId + ":" + clientSecret)); |
There was a problem hiding this comment.
Note: OpenIddict supports sending the client credentials in the Authorization header (basic authentication) or in the request form, but as per the spec, it's illegal to send credentials using both methods (which is likely why you were getting an error).
That said, if this client is expected to be used with other vendors that don't support sending the credentials in the request form, this change might break them. You may want to add an option to control how the credentials are sent 😃
There was a problem hiding this comment.
Yep, I indeed got an error, because the credentials were specified in 2 ways. Do you think there are vendors, that support only credentials in request form and not in headers?
There was a problem hiding this comment.
Do you think there are vendors, that support only credentials in request form and not in headers?
Yeah, a lot, sadly (even if supporting basic authentication is - in theory - mandatory for an OAuth 2.0 server: https://www.rfc-editor.org/rfc/rfc6749#section-2.3.1)
When I tried to make authorization code flow with one of our apps that uses OpenIddict, there were some validation errors. So these changes should fix that.