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

Make Response Mode Optional (Oracle IDCS) #1010

Closed
dzirg44 opened this issue May 15, 2023 · 5 comments · Fixed by #1248
Closed

Make Response Mode Optional (Oracle IDCS) #1010

dzirg44 opened this issue May 15, 2023 · 5 comments · Fixed by #1248
Assignees
Labels
breaking change bug Something isn't working Oracle IDCS Oracle Identity Cloud Services
Milestone

Comments

@dzirg44
Copy link

dzirg44 commented May 15, 2023

In Oracle IDCS (Identity Cloud Services) both options
(according to https://openid.net/specs/oauth-v2-multiple-response-types-1_0.html)

  • Response Mode for the OAuth 2.0 code Response Type is the query encoding
  • Response Mode for the OAuth 2.0 token Response Type is the fragment encoding

are invalid.
If I use
query => code
I have

Oops... Client 111111 provided an invalid response mode: query.

in case
fragment => token

Unhandled Rejection (Error): Only the Authorization Code flow (with PKCE) is supported

Only when I remove the default value

https://github.com/authts/oidc-client-ts/blob/main/src/OidcClient.ts#L102

I can make it work.

In my opinion , if it is not a mistake caused by Oracle developers there are 2 ways to fix.
1 - simply delete the default value
2 - add 'none' option to response_mode

the fix I found out exploring
https://auth0.com/docs/authenticate/login/oidc-conformant-authentication/oidc-adoption-auth-code-flow#code-exchange-request-authorization-code-flow-with-pkce

@pamapa
Copy link
Member

pamapa commented May 16, 2023

I agree, when reading the spec (https://openid.net/specs/openid-connect-core-1_0.html#AuthRequest):

  • "response_mode
    OPTIONAL. Informs the Authorization Server of the mechanism to be used for returning parameters from the Authorization Endpoint. This use of this parameter is NOT RECOMMENDED when the Response Mode that would be requested is the default mode specified for the Response Type. "

Means when response_type is code, which is always the case in this library anyway, we should not set response_mode by default.

I will need to test this first on other real IDPs first...

@pamapa pamapa added the bug Something isn't working label May 16, 2023
@pamapa
Copy link
Member

pamapa commented May 16, 2023

Tested: Works on my IDP: response_type=code without response_mode

Tried:

index c10152ed..f89f04f5 100644
--- a/oidc-client-ts/OidcClientSettings.ts
+++ b/oidc-client-ts/OidcClientSettings.ts
@@ -162,7 +162,7 @@ export class OidcClientSettingsStore {
     public readonly ui_locales: string | undefined;
     public readonly acr_values: string | undefined;
     public readonly resource: string | string[] | undefined;
-    public readonly response_mode: "query" | "fragment";
+    public readonly response_mode: "query" | "fragment" | undefined;
 
     // behavior flags
     public readonly filterProtocolClaims: boolean | string[];
@@ -191,7 +191,7 @@ export class OidcClientSettingsStore {
         redirect_uri, post_logout_redirect_uri,
         client_authentication = DefaultClientAuthentication,
         // optional protocol
-        prompt, display, max_age, ui_locales, acr_values, resource, response_mode = DefaultResponseMode,
+        prompt, display, max_age, ui_locales, acr_values, resource, response_mode,// = DefaultResponseMode,
         // behavior flags
         filterProtocolClaims = true,
         loadUserInfo = false,

@pamapa
Copy link
Member

pamapa commented May 16, 2023

Should we fixed that in a major or minor version? I tend to do it in a minor, as it fixes a bug and the chance that existing IDPs still work seems high. Will add a release notice for that...

@pamapa pamapa added this to the 2.3.0 milestone May 16, 2023
@dzirg44
Copy link
Author

dzirg44 commented May 17, 2023

@pamapa the chance that it can break something is low, i agree, but if you don't specify it in your config it means that the response_mode will be query , and after the upgrade it will be absent and it can lead to changing the behavior for the app. I would like to see it in a minor version update, but if you don't want to struggle with issues like "after the upgrade the auth is broken" - major is more suitable. And thank you for your work.

@pamapa
Copy link
Member

pamapa commented May 17, 2023

@dzirg44 You are right lets do this on the planned 3.0.0, which comes later this year.

@pamapa pamapa modified the milestones: 2.3.0, 3.0.0 May 17, 2023
@pamapa pamapa added the Oracle IDCS Oracle Identity Cloud Services label May 24, 2023
@pamapa pamapa self-assigned this Nov 15, 2023
pamapa added a commit that referenced this issue Nov 15, 2023
@pamapa pamapa mentioned this issue Nov 15, 2023
2 tasks
pamapa added a commit that referenced this issue Nov 15, 2023
pamapa added a commit that referenced this issue Nov 15, 2023
pamapa added a commit that referenced this issue Nov 15, 2023
pamapa added a commit that referenced this issue Nov 16, 2023
pamapa added a commit that referenced this issue Nov 16, 2023
dbfr3qs pushed a commit to dbfr3qs/oidc-client-ts that referenced this issue Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug Something isn't working Oracle IDCS Oracle Identity Cloud Services
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants