swagger-api / swagger-ui

Swagger UI is a collection of HTML, JavaScript, and CSS assets that dynamically generate beautiful documentation from a Swagger-compliant API.
https://swagger.io
Apache License 2.0
26.48k stars 8.96k forks source link

Basic authentication header does not encode client id and client secret according to RFC6749 #5123

Open marcelion opened 5 years ago

marcelion commented 5 years ago

When using the Authorization header in combination with "Content-Type": "application/x-www-form-urlencoded" does not work when the client id or client secret contains any of the following characters:

According to RFC6749 Appendix B. these characters should be encoded before base 64 encoding the auth string.

Wrong: Raw: us€r:+password Authorization: Basic dXPigqxyOitwYXNzd29yZA==

Correct: Raw: us€r:+password Encoded: us%E2%82%ACr:%2Bpassword Authorization: Basic dXMlRTIlODIlQUNyOiUyQnBhc3N3b3Jk

In the code:

https://github.com/swagger-api/swagger-ui/blob/a5568f9e1642f5ce286cd2b4927a4ce3fa663ba2/src/core/plugins/auth/actions.js#L90

https://github.com/swagger-api/swagger-ui/blob/a5568f9e1642f5ce286cd2b4927a4ce3fa663ba2/src/core/plugins/auth/actions.js#L112

https://github.com/swagger-api/swagger-ui/blob/a5568f9e1642f5ce286cd2b4927a4ce3fa663ba2/src/core/plugins/auth/actions.js#L138

Thank you for reviewing this issue.

michael-o commented 4 years ago

No, your understanding is wrong. The mentioned RFC solely applies to the request body, not to the headers. This needs to be implemented according to RFC 7617. See here.

hmorato commented 4 years ago

It applies to the header as well, @michael-o.

2.3.1. Client Password

Clients in possession of a client password MAY use the HTTP Basic authentication scheme as defined in [RFC2617] to authenticate with the authorization server. 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.

https://tools.ietf.org/html/rfc6749#section-2.3.1

To sum it up:

  1. According to RFC 2617 (obsoleted by RFC 7617) you have to send the header: "Authorization: Basic " base64(userid ":" password)

  2. RFC6749 builds on top of RFC 2617 / RFC 7617, it doesn't contradict them, but says:

    • The Basic authentication userid is the OAuth client identifier urlencoded
    • The Basic authentication password is the OAuth client password urlencoded
michael-o commented 4 years ago

@hmorato, I concur because RFC 6749 was written before RFC 7617. The latter does not mention a single word about URI encoded non-ASCII characters. So one must make explicit difference between real Basis auth and the broken one for OAuth 2.0. Reading further 2.3.1, I such cases I would completely avoid Basic and use the client_id/client_secret approach. Or better schemes like SPNEGO or client certs.

michael-o commented 4 years ago

@hmorato I have sent an errata to RFC 6749. It in review and will hopefully published.

hmorato commented 4 years ago

That would be a breaking change and probably it will be rejected: https://mailarchive.ietf.org/arch/msg/oauth/Rr494-RU92VF6Wwo7bZaFcd29OQ/

This bug should be fixed.

bbeda commented 4 years ago

When using this with IdentityServer4, if the token contains one of the characters, the authentication will not work because IdentityServer4 follows rfc6749 so I am also interested in how this will be sorted.

notaphplover commented 5 months ago
  1. RFC 6749 is not updated nor obsoleted by RFC 7617
  2. As @hmorato said, RFC 6749 clearly states:

Clients in possession of a client password MAY use the HTTP Basic authentication scheme as defined in [RFC2617] to authenticate with the authorization server. 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. The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password.

  1. This statement is not an errata.
  2. Certified oidc servers (like Ory Hydra or Auth0) are expecting this encoding when sending basic auth.

Basically, credentials fetching through swagger ui is broken when targeting oidc services.

Solving this issue should be sort of easy, I would gladly submit a PR if I'm allowed to do so.