fix: Remove urlencode from Basic Authentication header construction

[ovsep404]

Fixes #465

Description

This PR fixes the Basic Authentication header construction by removing the unnecessary and incorrect urlencode() calls. According to RFC 2617 Section 2 and RFC 7617, Basic Authentication should use base64 encoding of the raw credentials without URL encoding.

Basic Authentication header construction in OpenIDConnectClient incorrectly URL-encodes the client credentials before base64 encoding them. This causes issues when client_secret contains special characters (like '/'), as they get double-encoded , they get URL-encoded (e.g., '%2F') before base64 encoding, resulting in incorrect authentication headers

Changes

• Removed urlencode() calls from Basic Authentication header construction in:
  • requestResourceOwnerToken
  • requestTokens
  • requestTokenExchange
  • refreshToken
  • introspectToken
  • revokeToken

Compliance

This change brings the implementation into compliance with:

• RFC 2617 Section 2
• RFC 7617

Testing Done

• Tested with client credentials containing special characters
• Verified correct Basic Authentication header construction

Backwards Compatibility

This change may affect clients that were relying on the URL-encoded behavior, although such clients were technically not compliant with the RFC specifications.

[DeepDiver1975]

Reasonable - THX - please try to add a unit test - thx

[ovsep404]

Reasonable - THX - please try to add a unit test - thx

Thanks @DeepDiver1975 ! I've added unit test for the Basic Authentication header construction. The tests cover:

• Simple credentials
• Credentials with forward slashes (original issue)
• Special characters
• Unicode characters

[SamuelWei]

This urlencode was added in #192 as RFC 6749 (OAuth 2.0, https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1) mentions this encoding.

The client identifier is encoded using the "application/x-www-form-urlencoded" encoding algorithm per Appendix B, and the encoded value is used as the username; the client password is encoded using the same algorithm and used as the password.

[ovsep404]

This urlencode was added in #192 as RFC 6749 (OAuth 2.0, https://datatracker.ietf.org/doc/html/rfc6749#section-2.3.1) mentions this encoding.

The client identifier is encoded using the "application/x-www-form-urlencoded" encoding algorithm per Appendix B, and the encoded value is used as the username; the client password is encoded using the same algorithm and used as the password.

Thank you for raising this! There is indeed confusion around this point, so let’s clarify:

Summary

RFC 6749 (OAuth 2.0) §2.3.1 describes two separate ways to send client credentials:

1. As body parameters:
   Here, both client_id and client_secret must be URL-encoded (application/x-www-form-urlencoded).

2. In the Authorization header using HTTP Basic:
   Here, credentials are not URL-encoded. The spec says to follow RFC 2617/7617, which means:

   Authorization: Basic base64(client_id:client_secret)

   See RFC 6749 Errata #3914 for confirmation.

---

Therefore

• When constructing the Authorization: Basic ... header, do not URL-encode the credentials—just concatenate with a colon and base64-encode.
• The current PR correctly implements the RFCs and errata.
• The previous addition of urlencode() was only necessary for form-encoded body parameters, not for the header.

---

References

• RFC 6749 Section 2.3.1: Client Authentication
• RFC 6749 Errata #3914
• RFC 7617: The 'Basic' HTTP Authentication Scheme

[SamuelWei]

Thanks for your detailed explanation @ovsep404

I just had a look how the apache module handles it. I just looked it up on the phone and not good a C , however at first glance it looks to me like they also url encode before base64 ?
https://github.com/OpenIDC/mod_auth_openidc/blob/8a1dc8aac202f123fa8021cc04dd7279729afd22/src/proto/auth.c#L67

[SamuelWei]

Also this commit for the apache module OpenIDC/mod_auth_openidc@aa20bd9

and this issue for the nginx plugin zmartzone/lua-resty-openidc#204

[ovsep404]

Also this commit for the apache module OpenIDC/mod_auth_openidc@aa20bd9

and this issue for the nginx plugin zmartzone/lua-resty-openidc#204

Thanks for the follow-up!

It’s true that some libraries like mod_auth_openidc URL-encode credentials before base64 in the Basic Auth header, but this is for compatibility with certain non-standard servers—not because the RFC requires it.

Per the standards (RFC 6749, RFC 7617, and OAuth 2.0 Errata #3914), credentials in the Authorization header should NOT be URL-encoded.
The correct, interoperable approach is to send base64(client_id:client_secret) as the header value.

If there’s demand, a compatibility flag could be added, but the default should remain standards-compliant.

[SamuelWei]

How does it work if the client_id contains a :, like a url as the client id e.g. https://op.example.org?
According to https://datatracker.ietf.org/doc/html/rfc2617#section-2 the user id cannot contains a ":" char. If it would, how would the server know where to split to extract user and pass.

userid = *<TEXT excluding ":">

That could be a reason why RFC 6749 states:

The client identifier is encoded using the "application/x-www-form-urlencoded" encoding algorithm

[SamuelWei]

The current draft of OAuth2.1 mentions this whole drama: https://datatracker.ietf.org/doc/html/draft-ietf-oauth-v2-1-13#section-2.4.1

Note: This method of initially form-encoding the client identifier and secret, and then using the encoded values as the HTTP Basic authentication username and password, has led to many interoperability problems in the past. Some implementations have missed the encoding step, or decided to only encode certain characters, or ignored the encoding requirement when validating the credentials, leading to clients having to special-case how they present the credentials to individual authorization servers. Including the credentials in the request body content avoids the encoding issues and leads to more interoperable implementations.

You can follow their debate about this here: oauth-wg/oauth-v2-1#128

[SamuelWei]

I guess we will not be able to find a solution that works for everyone. My understanding of the specs is that it should be encoded, however reality shows that is is not always done, leading to compatibilty issues. We can either extract this to a protected method anyone can overwrite, or add flags to control how to encode.

[ovsep404]

I guess we will not be able to find a solution that works for everyone. My understanding of the specs is that it should be encoded, however reality shows that is is not always done, leading to compatibilty issues. We can either extract this to a protected method anyone can overwrite, or add flags to control how to encode.

Thank you for highlighting this edge case and referencing the OAuth 2.1 draft!

It’s true that putting a colon in the client_id (e.g., a URL) creates ambiguity for HTTP Basic Auth, since per RFC 2617 the username cannot contain a colon and splitting becomes ambiguous. However, this is an unfortunate mismatch between OAuth’s flexibility for client_id and Basic Auth’s ancient constraints.

Let’s be clear:

• OAuth 2.0’s guidance to “form-encode” the credentials for Basic Auth has led to widespread confusion and interoperability failures, as documented in the OAuth 2.1 draft §2.4.1 and oauth-wg/oauth-v2-1#128.
• Many major OAuth and OIDC providers (including Google, Microsoft, Auth0, Okta, etc.) expect raw, unencoded credentials in the Authorization header, per RFC 7617.
• Introducing URL-encoding into the Basic Auth header breaks compatibility with all standards-compliant servers and introduces security and maintenance risk by deviating from a decades-old HTTP norm.

Relying on server-specific quirks is not sustainable or secure for a widely-used library.
If a provider requires a URL-encoded header, that provider is violating the HTTP Basic Auth and OAuth standards, and should be reported to their maintainers.

The right approach is:

• Default to strict RFC 7617 compliance: No encoding. If a client_id contains a colon, that is an architectural mistake; use body parameters instead, as recommended by the OAuth 2.1 draft and working group.
• Optional escape hatch: For legacy or non-standard providers, allow users to opt-in to encoding via a documented flag or overridable method—but make it clear this is a non-standard workaround, not a recommended usage.

Bottom line:

• Standards compliance must be the default for security, predictability, and interoperability.
• There is no justification to make non-standard encoding the default, even if some reference libraries offer it as an option for broken servers.

My proposal:

I strongly recommend to keep the default strictly standards-compliant, for the long-term health and reliability of the project , but i agree with you , we will not be able to find a solution that works for everyone.
So my proposals:
- Default to strict RFC 7617 (no encoding in the header) for the widest compatibility with providers.
- Expose an option (e.g., a property or protected method) to allow URL-encoding for environments that require it.
- or your proposition,  to extract this to a protected method anyone can overwrite, or add flags to control how to encode

This gives users flexibility while keeping standards compliance as the default.

---

References:

• RFC 2617 Section 2
• RFC 7617
• RFC 6749 Section 2.3.1
• OAuth 2.1 Draft §2.4.1
• oauth-wg/oauth-v2-1#128 debate

[SamuelWei]

Hi @ovsep404,

I agree with you in most points, however I come to the oposite conlusion.
If you look at the draft for OpenID-Federation (https://openid.net/specs/openid-federation-1_0.html#name-authorization-request-with-) you can see client ids containing "https://" this would break basic auth. without encoding.

The section in OAuth 2.1 draft §2.4.1 also mentions "Some implementations have missed the encoding step" underlining that this non RFC 7617 complianced behaivor is the OpenID-Connect complianced behaivor. Therefore the default for this libary should be with encoding, with an option to opt out, so users can be RFC 7617 compliant.