8000 Support for identity providers that don't use client secrets · Issue #420 · golang/oauth2 · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Support for identity providers that don't use client secrets #420

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.

Sign up for GitHub

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

Jump to bottom
Closed
juicemia opened this issue May 6, 2020 · 3 comments
Closed

Support for identity providers that don't use client secrets #420

juicemia opened this issue May 6, 2020 · 3 comments

Comments

@juicemia
Copy link
juicemia commented May 6, 2020 •
edited
Loading

I'm working with the TD Ameritrade API and it looks like, somehow, they don't use client secrets with their oauth identity.

Here is the documentation for their token URL. All that gets passed in is the client ID. It seems like passing in the client secret, even though it's empty, gives an error for some reason.

I figured the package might have a way to turn off sending the client secret, but it doesn't look like that's one of the supported AuthStyles. After digging through the code it looks like the client secret will get added to the request no matter what.

For now I'm working around this with the following:

// conf.Exchange() can't be used here because it will always send a client secret and the TD API's oauth doesn't use it.
authBody := url.Values{}
authBody.Add("grant_type", "authorization_code")
authBody.Add("access_type", "offline")
authBody.Add("client_id", CLIENT_ID)
authBody.Add("redirect_uri", "https://localhost:3000/auth/callback")
authBody.Add("code", code)

body := strings.NewReader(authBody.Encode())
resp, err := http.Post("https://api.tdameritrade.com/v1/oauth2/token", "application/x-www-form-urlencoded", body)
if err != nil {
	log.Fatal(err)
}
defer resp.Body.Close()

buf, err := ioutil.ReadAll(resp.Body)
if err != nil {
	log.Fatal(err)
}

if resp.StatusCode != http.StatusOK {
	e := struct {
		Error string `json:"error"`
	}{}
	err := json.Unmarshal(buf, &e)
	if err != nil {
		log.Fatalf("error unmarshaling errro response: %v", err)
	}

	log.Fatal(e)
}

var authResponse AuthResponse
err = json.Unmarshal(buf, &authResponse)
if err != nil {
	log.Fatal(err)
}

fmt.Printf("using token %v\n", authResponse.AccessToken)

tok := &oauth2.Token{
	AccessToken:  authResponse.AccessToken,
	RefreshToken: authResponse.RefreshToken,
	Expiry:       time.Now().Add(time.Duration(authResponse.ExpiresIn) * time.Second),
}

client := conf.Client(ctx, tok)

So, this isn't a breaking issue for me. I still get to use the configured http.Client.

I wasn't even going to file an issue, but I'm doing it just in case you all didn't know that it was a thing. I've never seen an oauth identity provider that doesn't require a client secret.

I think it would be possible to add an AuthStyle so that this code looks something like the following:

conf := &oauth2.Config{
	ClientID: "BJWRJX1NESVCZGQNHJZAN9LFIQ8A0GJ9@AMER.OAUTHAP",
	Endpoint: oauth2.Endpoint{
		AuthURL:  "https://auth.tdameritrade.com/auth",
		TokenURL: "https://api.tdameritrade.com/v1/oauth2/token",
		AuthStyle: oauth2.AuthStyleInsecure, // this will cause conf.Exchange to not use the secret at all
	},
	RedirectURL: "https://localhost:3000/auth/callback",
}

Using this config, the same call can be made to Exchange.

EDIT: The workaround code is messy because I'm still just playing with the API. I haven't started building anything yet.
@bobthepeanut90
Copy link

I think I'm hitting the same issue (different provider), however it seems that this commit: 1611bb4 should mean that if client_secret is empty string it should omit the param from the request.

I need to dig a bit deeper, but @juicemia I assume you already tried setting the client_secret to ""?

@juicemia
Copy link
Author
juicemia commented Jun 3, 2020 •
edited
Loading

Actually I didn't. I assumed that it would always just try to use the client secret, even if it's blank.

Would you guys mind a PR from me updating the docs on the Config type to mention this?

EDIT: Maybe assume is the wrong word. I gave up on reading the code too early 🤣

7485
@adeinega
Copy link

It's fine to omit the client_secret parameter for certain use cases

  1. public and/or native clients, for details, see the Client Authentification section in https://datatracker.ietf.org/doc/html/rfc8252#section-8.5 for more details
  2. when client_assertion is used in order to authenticate the client, take a look at https://datatracker.ietf.org/doc/html/rfc7523

I don't know what would be a legit reason to omit client_id though. That seems to be a deviation from standards to me.

@seankhliao seankhliao closed this as not planned Won't fix, can't repro, duplicate, stale Apr 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@juicemia @adeinega @seankhliao @bobthepeanut90
0