redirect_uri should be URL Encoded when redirecting to Authorization server URL

[aravinda-b-juego]

What feature do you want to see added?

I've noticed that the redirect_uri parameter in the plugin is not URL encoded when redirecting to the Authorization server URL. While I couldn't find a hard rule explicitly stating that the redirect_uri must be URL encoded, I have observed that all the examples and best practices include the redirect_uri as URL encoded.

Suggestion:

I propose that the "oic-auth-plugin" should encode the redirect_uri parameter before appending and redirecting to the Authorization server URL. This adjustment would align the plugin with common best practices and increase compatibility with a wider range of clients.

Additional Information:

We are facing issues with Gitea on login, and we suspect that this is because Gitea expects the redirect_uri to be URL encoded. This issue is reflected in the following Gitea GitHub issues:
go-gitea/gitea#26097
go-gitea/gitea#26945

I'd appreciate your thoughts on this matter and any feedback you might have regarding the suggested change. Thank you for your attention to this issue.

Upstream changes

No response

[wxiaoguang]

Hi Jenkins team & OIDC team, I submitted a PR on Gitea side to "fix" the URL problem.

Actually, according to the standard, the URL parameter should be encoded.

Suppose there is redirect_uri = "https://host/" in code, then the URL with it as parameter should be: https://example.com/?redirect_uri=https%3A%2F%2Fhost%2F&other_key=other_value.

But now Jenkins generates: https://example.com/?redirect_uri=https://host/&other_key=other_value , although it works in most cases and many apps could tolerate it, I think it's better to encoded it as the standard, otherwise it might break the URL again in the future if the redirect_uri needs to add parameters, eg: when redirect_uri = "https://host/?a=b&c=d", then without encoding, https://example.com/?redirect_uri=https://host/?a=b&c=d&other_key=other_value won't work as expected.

[michael-doubez]

Hello.

This is actually a gray area of the standard and usual practice is to place it at the end to avoid issues.
Some providers don't handle url encoded redirect. That would break the configuration of people.

People that need a reditrect with parameter can encode it themselves in config. It is not ideal but better than yet another config parameter.

That said, I wonder if this is still possible after #261.

[wxiaoguang]

I disagree.

1. It is not "a gray area of the standard". It has been clearly defined in many standards. For example:
   • https://datatracker.ietf.org/doc/html/rfc2396#section-3.4
   • Within a query component, the characters ";", "/", "?", ":", "@", "&", "=", "+", ",", and "$" are reserved.
   • https://datatracker.ietf.org/doc/html/rfc3986#section-2.2
   • If data for a URI component would conflict with a reserved character's purpose as a delimiter, then the conflicting data must be percent-encoded before the URI is formed.
   • So the / must be encoded to %2F
2. It isn't true for "Some providers don't handle url encoded redirect. That would break the configuration of people." If an provider reads the HTTP query parameter, the parameter is always automatically decoded by all frameworks, no exception.
3. It isn't true for "People that need a reditrect with parameter can encode it themselves in config". End users should only provide the "real" URL to be used in browser address bar. The encoding work should be done by the http client caller.

[wxiaoguang]

That said, I wonder if this is still possible after #261.

261 is not directly related. Actually, by using 261, end users must provide the real URL like https://host/, but not the encoded one https%3A%2F%2Fhost%2F (not like you said "encode it themselves in config"). It is the http client caller's responsibility to encode the URL correctly.

[michael-doubez]

I agree with you on the normative aspect, it should be url encoded but in practice, it isn't. Proof is that it works with current implementation on all major OIDC.

I don't know all client library but I have encountered 2 providers where url encoding the redirect caused the redirect verification to fail.

Regarding #261, I mean that the implementation verifies that the URL configured is well formed and if someone urlencodes the "https://" part in config, this will fail. I guess, I 'll just wait for defects to stream in

[michael-doubez]

@wxiaoguang looking into the matter more deeply, it may be a choice from google oauth library for security reason or it may be a bug (to avoid query parameter injection) or (as I assumed) a way to handle provider that do not interpret parameters in redirect.

You can raise the issue with them.