Skip to content

Conversation

@m0rt3nlund
Copy link

No description provided.

@danschultzer
Copy link
Member

Why is it necessary with this conditional?

@m0rt3nlund
Copy link
Author

Hi!

This was to allow this line to be triggered if the method was set to "none"

defp parse_client_auth_method("none"), do: {:ok, nil}

Since none is not a valid token_endpoint_auth_methods_supported method as I can see from the spec I cannot add this in the serverside openid specification.

If I understand this correctly we should never expect to get this method as a valid option from the server, so to be able to use it the client should specify it.
But being able to specify this "clientside" in combination with code_verifier is very beneficial when you have an application that is not "Confidential" and the server does not support "Client secret"

@danschultzer
Copy link
Member

What OIDC provider are you using? AFAIK none is valid, and providers I've used returns that value in the token_endpoint_auth_methods_supported list. It's not clear from the core RFC whether the client should validate or not, closest thing I found was this draft: https://openid.net/specs/openid-connect-rp-metadata-choices-1_0.html#section-2-7.22

If it's not a good expectation that all auth methods will exist in token_endpoint_auth_methods_supported then I think this validation should be removed rather than having it as a conditional.

@m0rt3nlund
Copy link
Author

Hi @danschultzer ! I have done some more research and I see that Azure B2C actually never include none in this list. Even if "Allow public client flows" are enabled.

Therefor, checking of the presence of the method should maybe be optional, or removed?

This is a common list returned by the discovery file at Azure:

"token_endpoint_auth_methods_supported": [
    "client_secret_post",
    "client_secret_basic"
  ]

They support none implicitly, but does not list it.

Is there any examples of this working for Azure AD OIDC when not using a client_secret?

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.

2 participants