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

Remove fragment response type and repost view #13

Merged
merged 1 commit into from
Sep 30, 2019
Merged

Remove fragment response type and repost view #13

merged 1 commit into from
Sep 30, 2019

Conversation

joshcanhelp
Copy link
Contributor

Description

Remove the fragment response mode and supporting components. This means that the library assumes form_post capability by the authorization server. If that capability is not present, then the authorization code grant should be used.

References

Closes #4

Testing

  • This change adds test coverage for new/changed/fixed functionality

@panva
Copy link
Contributor

panva commented Sep 19, 2019

What's the point of removing the support for it? It's fine to prefer form_post over fragment but it still has its use.

@joshcanhelp
Copy link
Contributor Author

@panva - I've never heard the fragment response_type recommended for a web application with a back end, like what would use this library. Main point was to reduce the number of features we're supporting here. Maybe I'm missing something ... what scenario would a web application need to use fragment?

@panva
Copy link
Contributor

panva commented Sep 19, 2019

one that wishes to restrict its session and/or authorization state binding cookies to sameSite=lax and only cares about sign-in (response_type=id_token).

Such app cannot use form_post since it does not send the RP cookies with the POST from another origin (OP).

@joshcanhelp
Copy link
Contributor Author

A little more color here thanks to @panva:

The state or nonce cookies set by the client (RP) won't be read when posting from an external site (in this case, the authorization server response) when sameSite is set to lax or strict (see OWASP description here) on the client. The fragment response_type allows this library to receive the request on an HTML page, then re-post the request back to the site so the cookies are received.

@panva
Copy link
Contributor

panva commented Sep 26, 2019

@joshcanhelp after having discussion with @vibronet this is probably still something we should do. We should work around the browser cookie behaviour changes on the cookie level, not change our protocol setup.

@joshcanhelp
Copy link
Contributor Author

browser cookie behaviour changes on the cookie level, not change our protocol setup

As in, the application should not use sameSite cookies or is there a different configuration there that we could recommend?

@joshcanhelp joshcanhelp reopened this Sep 26, 2019
@joshcanhelp joshcanhelp requested a review from a team September 26, 2019 13:31
@joshcanhelp joshcanhelp merged commit b5e9500 into auth0:master Sep 30, 2019
@joshcanhelp joshcanhelp deleted the remove-fragment-response-mode branch September 30, 2019 15:36
@Pizzacus
Copy link

What's your opinion on the recent issue over at ory/hydra#1591, another OpenID Connect implementation, in which they argue that they will not support form_post.

Perhaps expecting the provider to support form_post is a little too much? Maybe fragment should be kept for compatibility reasons?

@joshcanhelp
Copy link
Contributor Author

@Pizzacus - Thanks for the link, it was interesting reading overall. I don't think I want to go too deep on my opinion on what that repo is doing but ...

The response mode form_post is a perfectly valid response from an authorization server, Auth0 or otherwise. It's a simple way to just get back an ID token to be used for logging a user into your application.

That said ... this library doesn't require a provider to support form_post, even though it's the default. As the maintainer said in there, getting an ID token back using an Authorization Code grant is a perfectly legitimate way to do sign in with OIDC. In this library, you would initialize like so:

app.use(auth({
  authorizationParams: {
    response_type: "code",
    scope: "openid profile email"
  }
}));

@Pizzacus
Copy link

@joshcanhelp I see, thanks for the insight! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use form_post instead of fragment if supported by the provider
4 participants