Error response appends extra ? to redirect_uri

[geoff-va]

When the redirect_uri already contains a query string, if there is an error with the authorization request then an additional ? is appended to the redirect_uri with the error query params.

Example:
redirect_uri=https://some.domain.com/index?action=authorize

becomes:
redirect_uri=https://some.domain.com/index?action=authorize?error=login_required&error_description=The Authorization Server requires End-User authentication&...

I couldn't find anywhere in the spec that says you can't already have a query string as part of the redirect_uri. Perhaps the solution would be to check the redirect_uri and only append the ? if a query string doesn't already exist on the redirect_uri? If it does we append & instead of ?.

Open to suggestions if anyone has experience with this situation.

[toti1212]

Hi @geoff-va ! Good to know this! I have no experience with that bug, but, I realized that the function create_response_uri could be better to fix that.

I have a problem with the same function when the redirect_uri is a deep link like slack://test
That function returns me something like slack:?code=xxx

[geoff-va]

@toti1212 I think the error (in my case at least) is coming from create_uri in errors.AuthorizeError. I'm going to try to make that a little more robust.

I think the ultimate goal there is to leave the redirect_uri intact and append our parameters to either the query string or the query fragment (if implicit).

Good point about deep linking and non-http schemas, too! I'll consider that as well. I haven't looked into create_response_uri yet.