swagger-api / swagger-api/swagger-ui

OAuth2 Password Flow modes do not match RFC

Open
#3,227 11 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

cat: auth type: bug
Dominant language
JavaScript
Stars
29k
Forks
9.3k
Avg merge
2d 23h
Merged PRs (30d)
25

Description

Currently (Swagger UI master branch), only one out of three implemented OAuth2 Password Flow modes match RFC 6749 section 2.3.1, which explicitly states:

The authorization server MUST support the HTTP Basic authentication scheme for authenticating clients that were issued a client password.

For example (with extra line breaks for display purposes only):

      Authorization: Basic czZCaGRSa3F0Mzo3RmpmcDBaQnIxS3REUmJuZlZkbUl3

Alternatively, the authorization server MAY support including the client credentials in the request-body using the following parameters:

client_id

REQUIRED. The client identifier issued to the client during the registration process described by Section 2.2.

client_secret

REQUIRED. The client secret. The client MAY omit the parameter if the client secret is an empty string.

Including the client credentials in the request-body using the two parameters is NOT RECOMMENDED and SHOULD be limited to clients unable to directly utilize the HTTP Basic authentication scheme (or other password-based HTTP authentication schemes). The parameters can only be transmitted in the request-body and MUST NOT be included in the request URI.

There are 3 modes implemented in Swagger UI for OAuth2 Password flow:

  1. "Request body" - Pass username with password in request-body and client_id with client_secret in "Authorization" header (Basic) - this is the CORRECT and RECOMMENDED way of doing this according to RFC.
  2. "Basic auth" - Pass username with password in "Authorization" header (Basic) with no client_id/client_secret - this has NOTHING TO DO with OAuth2 as far as I can tell, this is just someone's simplified authentication method. If there is no other place to implement this, well let it be there, but this might be a bit confusing.
  3. "Query parameters" - Pass username with password in request-body and client_id with client_secret in URI query encoded parameters - this is EXPLICITLY DISCOURAGED in the RFC since URLs are usually logged on the web servers and this would expose the plaintext secrets to anyone reading the logs.

The names in Swagger UI are completely misleading, by the way.

Here is my proposal:

  1. Rename "Basic auth" to "Basic username & password auth" to be more explicit, and move to the end of the list, though the only reason to leave it there is that there is no other place for it :(
  2. Rename "Request body" to "Basic auth" and leave it as default just as it is now.
  3. Replace "Query parameters" with new "Request body" and implement it so it passes client_id and client_secret via request-body.

/cc @bodnia @Naid405 @benoj @webron

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by locating Swagger UI's OAuth2 Password Flow mode implementation and any related tests; compare the three modes with RFC 6749 section 2.3.1 and the proposal in this issue. Done means the labels and default are corrected, credentials use the intended header or request-body placements, and the discouraged query-parameter mode is replaced, with coverage for each mode.

Written by the indexing model from the issue text.

Assessment

Tech stack
javascript
Domain
authentication
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.