fix(error-redirect): use query-safe fragment as recommended in the latest OAuth 2 security best practices

[aomader]

This PR adjusts the fragment that is added to error redirect responses, which is supposed to prevent browsers from reattaching existing ones.

The previous fragment #_=_ is an invalid query selector and can not be used as is on the receiving end, hence it has been adjusted to the recommended #_.

Additionally, I adjusted the link to the respective latest active internet draft.

This looks like a minor thing, but one can run into issues caused by the browsers behavior of carrying along fragments.

[josephdecock]

The BCP uses #=_ as a non-normative example, so I don't think it is saying that we need to change this. I'm surprised that this is causing a problem for you, as normally authorize redirects are going to be handled by your client's OAuth implementation. At least in asp.net, the middleware that listens for the redirect processes the response and then redirects on to some UI endpoint within the host. So the browser wouldn't end up parsing the fragment and trying to use it for navigation.

[aomader]

I guess it's a combination of the behavior of the browser to keep the fragment until it reaches the final address and that we do not know what the final address will be. The chain of redirects can be fairly long, with the fragment being added to some URL that eventually assumes something about fragments. Or shortly, the identity server is here eventually causing a final URL that is "invalid".

More precisely, we noticed that a client reacts on errors by performing redirects that end up on a page where fragments are assumed to be valid query selectors. In context of text/html as media type, I guess that this is a valid assumption, since they are document fragment identifiers.

Given that the recommendation was changed from a non-query-selector-safe fragment to a query-selector-safe one, and that following that recommendation is a non-breaking change, I would argue that this would be a generally beneficial adjustment.

[aomader]

@josephdecock Any thoughts? We are seeing users being affected by this.

[josephdecock]

Thanks for the reminder about this. Looking at it again, I think we should do this. The old draft said to do _=_ as a MUST, but that isn't the case anymore.

I'd like to see test automation around this code path - it's too bad there isn't an existing test. Would you be interested in adding a test?

[aomader]

In the open source context generally yes, but we are a paying customer (Enterprise License) and I am currently not having enough capacities to do this in my free time.

[josephdecock]

That's absolutely fair - we'll add the test coverage and get this included in the 7.2 release.