pow-auth / assent

Multi-provider framework in Elixir
https://powauth.com
MIT License
406 stars 47 forks source link

Add response_type configuration option to Azure AD #29

Closed Ivor closed 4 years ago

Ivor commented 4 years ago

The default value for response_type of id_token code requires that the app in the Azure AD portal be configured to allow the implici flow for id_token. If the id_token is not requested in the :response_typeconfiguration the token is still returned and the authentication works without requiring the implicit flow configuration in the portal.

This change allows the developer to override the response type. The default is currently to still have the id_token code value.

Ivor commented 4 years ago

I tried setting up the simplest authentication and found that when using the Azure AD module out of the box I needed to add the ID token "implicit flow" configuration. If I remove the id_token response type the ID token is still returned and the user is authenticated.

Ivor commented 4 years ago

Something related that I am not sure about is that the config for the Azure AD module does not mention the client_secret at all, however, this is the first thing that Azure complains about when I start from scratch. What I am not sure is if the client_secret is perhaps only required if you are trying to authenticate for a specific tenant? Or if this is just something that can be added to the documentation.

My configuration currently looks like this:

      client_id: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
      nonce: true,
      tenant_id: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
      response_type: "code",
      strategy: Assent.Strategy.AzureAD,
      client_secret: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"

If I remove the client_secret I get a 401 response from Azure.

Ivor commented 4 years ago

Further investigating this issue by reading here: https://docs.microsoft.com/en-us/azure/active-directory/develop/v1-protocols-openid-connect-code it does seem like they specify that the response_type must include id_token which is confusing in that it doesn't seem to be required.

Based on that post explicitly saying that ID token is required I am not sure that this PR has any value. I'll leave it open incase you disagree.

Does the code value that is returned actually get used? Or do we just rely on id_token.

danschultzer commented 4 years ago

Does the code value that is returned actually get used? Or do we just rely on id_token.

Yeah, it gets used to enable auth code flow as Assent doesn't support implicit flow. That you got the strategy to work without id_token is surprising since per their docs it's required:

Parameter response_type

Required Yes

Description Must include an ID token for OpenID Connect.

I followed the instructions in https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-protocols-oidc which further states:

This request is similar to the first leg of the OAuth 2.0 authorization code flow, with these important distinctions:

  • The request must include the openid scope in the scope parameter.
  • The response_type parameter must include id_token.
  • The request must include the nonce parameter.

This goes agains the OIDC specs:

scope REQUIRED. OpenID Connect requests MUST contain the openid scope value. OPTIONAL scope values of profile, email, address, phone, and offline_access are also defined. See Section 2.4 for more about the scope values defined by this document.

nonce OPTIONAL. String value used to associate a Client session with an ID Token, and to mitigate replay attacks. The value is passed through unmodified from the Authentication Request to the ID Token. Sufficient entropy MUST be present in the nonce values used to prevent attackers from guessing values. One method to achieve this is to store a cryptographically random value as an HttpOnly a session cookie and use a cryptographic hash of the value as the nonce parameter. In that case, the nonce in the returned ID Token is compared to the hash of the session cookie to detect ID Token replay by third parties. Use of the nonce is OPTIONAL when using the code flow.

response_type REQUIRED. This value MUST be code. This requests that both an Access Token and an ID Token be returned from the Token Endpoint in exchange for the code value returned from the Authorization Endpoint.

So there are two options here:

  1. Remove the custom :response_type, which sounds like it won't use OIDC, but it really should since openid is in the scope. Nonce is not necessary here.
  2. Update the docs to instruct users to enable implicit flow.

I much prefer the first option.

danschultzer commented 4 years ago

I've opened #30 to resolve this. Let me know if that branch works as expected (you no longer need to enable implicit flow, and have the custom config). The only config necessary should be:

[
  client_id: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
  client_secret: "xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx",
  tenant_id: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx",
  strategy: Assent.Strategy.AzureAD
]

Edit: The above also assumes :nonce is optional as per the specs, so no longer necessary to set it.