Disable CORS on /login/oauth/authorize

[apata]

Changes

This PR disables CORS at /login/oauth/authorize.

   Cross-Origin Resource Sharing [WHATWG.CORS] MUST NOT be supported at
   the Authorization Endpoint as the client does not access this
   endpoint directly, instead the client redirects the user agent to it.

From https://datatracker.ietf.org/doc/draft-ietf-oauth-v2-1/15/ (section 3.1)

Tests

• Not needed

Changelog

• This PR does not make a user-facing change

Documentation

• This change does not need a documentation update

Dark mode

• This PR does not change the UI

[zoldar]

Ugh, it's not ideal, but given the alternative is sprinkling option routes all over the router, that's probably the lesser bad out of the two...

[apata]

Ugh, it's not ideal, but given the alternative is sprinkling option routes all over the router, that's probably the lesser bad out of the two...

Agreed that it's not ideal. The alternative to sprinkling option everywhere is to take the time to figure out where CORS is necessary. I seriously doubt it's needed on everything.

[aerosol]

is it possible to use a path helper here, instead of hardcoding the literal?

Verified route maybe even? https://phoenix.hexdocs.pm/routing.html#verified-routes

[ukutaht]

Last time I checked, verified routes are useless for us because our router has a catch-all entry for dashboards. This means any route like ~p"/completely-wrong" will pass verification because it would technically match the dashboard route handler. So as far as I could tell it provides no benefit over using literals.

I've thought that it would be nice to be able to tell the verified routes in Phoenix to ignore the catch-all. We don't really have a need to supply dynamic dashboard URLs as literals in code. Verified routes would be useful if they only verified all the remaining static routes. When I checked I didn't find a way to achieve that.

[zoldar]

Maybe we could use verified routes now?

https://phoenix.hexdocs.pm/Phoenix.Router.html#match/5-options

:warn_on_verify - the boolean for whether matches to this route in verified routes should emit a warning, rather than being accepted as verified. It is useful to ignore an otherwise catch-all route definition from being matched when verifying routes. Defaults false

[apata]

Thanks all for commenting! I looked those up in the context of the other PR as well, but didn't want to introduce the new way of doing things just yet.

[zoldar]

JFTR I took a brief stab at a sweeping conversion to verified paths in #6660. The starting point was https://gist.github.com/andreaseriksson/e454b9244a734310d4ab74d8595f98cd, which converted a major chunk, fixing stuff manually afterwards. Stuff is still broken there, but it looks like wrapping it up would be doable.