Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth2 access request handler doesn't URL-decode basic auth #182

Closed
wyattanderson opened this issue Jun 20, 2017 · 2 comments · Fixed by #184
Closed

OAuth2 access request handler doesn't URL-decode basic auth #182

wyattanderson opened this issue Jun 20, 2017 · 2 comments · Fixed by #184
Labels
bug Something is not working.

Comments

@wyattanderson
Copy link
Contributor

Per RFC 6749 section 2.3.1, the base64-encoded client ID and client secret must also be URL-encoded:

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 code makes a reference to this requirement:

// Decode client_id and client_secret which should be in "application/x-www-form-urlencoded" format.

but does not decode values. Go's http.Request.BasicAuth() does not URL-decode the username and password components; I believe this requirement is specific to RFC 6749 and not part of RFC 2617.

If RFC 6749-compliant clients try to authenticate to Hydra with client IDs and secrets that contain characters that need URL escaping, they will fail.

@aeneasr
Copy link
Member

aeneasr commented Jun 21, 2017

I've tried it here but it caused issues (demo didn't work with postman iirc) which is why the change was reverted.

@aeneasr aeneasr added the bug Something is not working. label Jun 21, 2017
@aeneasr
Copy link
Member

aeneasr commented Jun 21, 2017

I remember now why I reverted, the golang library itself does not seem to do it:

func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values) (*Token, error) {
	hc, err := ContextClient(ctx)
	if err != nil {
		return nil, err
	}
	bustedAuth := !providerAuthHeaderWorks(tokenURL)
	if bustedAuth && clientSecret != "" {
		v.Set("client_id", clientID)
		v.Set("client_secret", clientSecret)
	}
	req, err := http.NewRequest("POST", tokenURL, strings.NewReader(v.Encode()))
	if err != nil {
		return nil, err
	}
	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
	if !bustedAuth {
		req.SetBasicAuth(clientID, clientSecret)
	}
func (r *Request) SetBasicAuth(username, password string) {
	r.Header.Set("Authorization", "Basic "+basicAuth(username, password))
}
func basicAuth(username, password string) string {
	auth := username + ":" + password
	return base64.StdEncoding.EncodeToString([]byte(auth))
}

which could probably considered a bug?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants