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

client_id missing from token request with broken provider & no client secret #220

Closed
mmp opened this issue Feb 18, 2017 · 2 comments
Closed
Assignees

Comments

@mmp
Copy link

mmp commented Feb 18, 2017

When requesting a token from a broken provider (e.g. www.googleapis.com), if there is a client_id but not a client_secret, the client_id isn't included in the request generated by the oauth2 library, leading to a 400 response. This seems to have been introduced in 4464e78, and is contrary to the change description, so was presumably unintended.

The attached test program illustrates the bug, giving

Error: oauth2: cannot fetch token: 400 Bad Request
Response: {
  "error" : "invalid_request",
  "error_description" : "Could not determine client ID from request."
}

with top of tree.

The patch below fixes it for me, but I'm not an oauth expert, so don't know if this is the appropriate fix.

diff --git a/internal/token.go b/internal/token.go
index ba90a34..6477379 100644
--- a/internal/token.go
+++ b/internal/token.go
@@ -155,9 +155,11 @@ func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string,
 		return nil, err
 	}
 	bustedAuth := !providerAuthHeaderWorks(tokenURL)
-	if bustedAuth && clientSecret != "" {
+	if bustedAuth {
 		v.Set("client_id", clientID)
-		v.Set("client_secret", clientSecret)
+		if clientSecret != "" {
+			v.Set("client_secret", clientSecret)
+		}
 	}
 	req, err := http.NewRequest("POST", tokenURL, strings.NewReader(v.Encode()))
 	if err != nil {

Test case:
oauth.go.txt

@mmp
Copy link
Author

mmp commented Mar 13, 2017

Ping. Any chance of this being fixed?

@rakyll
Copy link
Contributor

rakyll commented Mar 13, 2017

I am going to create a CL right now.

@rakyll rakyll self-assigned this Mar 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants