Skip to content

Commit

Permalink
oauth2: auto-detect auth style by default, add Endpoint.AuthStyle
Browse files Browse the repository at this point in the history 
Instead of maintaining a global map of which OAuth2 servers do which
auth style and/or requiring the user to tell us, just try both ways
and remember which way worked. But if users want to tell us in the
Endpoint, this CL also add Endpoint.AuthStyle.

Fixes #111
Fixes #365
Fixes #362
Fixes #357
Fixes #353
Fixes #345
Fixes #326
Fixes #352
Fixes #268
Fixes https://go-review.googlesource.com/c/oauth2/+/58510
(... and surely many more ...)

Change-Id: I7b4d98ba1900ee2d3e11e629316b0bf867f7d237
Reviewed-on: https://go-review.googlesource.com/c/157820
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Ross Light <[email protected]>
  • Loading branch information
@bradfitz
bradfitz committed Feb 11, 2019
1 parent 99b60b7 commit 80673b4
Show file tree
Hide file tree
Showing 9 changed files with 200 additions and 183 deletions.
8 changes: 7 additions & 1 deletion clientcredentials/clientcredentials.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,11 @@ type Config struct {

// EndpointParams specifies additional parameters for requests to the token endpoint.
EndpointParams url.Values

// AuthStyle optionally specifies how the endpoint wants the
// client ID & client secret sent. The zero value means to
// auto-detect.
AuthStyle oauth2.AuthStyle
}

// Token uses client credentials to retrieve a token.
Expand Down Expand Up @@ -97,7 +102,8 @@ func (c *tokenSource) Token() (*oauth2.Token, error) {
}
v[k] = p
}
tk, err := internal.RetrieveToken(c.ctx, c.conf.ClientID, c.conf.ClientSecret, c.conf.TokenURL, v)

tk, err := internal.RetrieveToken(c.ctx, c.conf.ClientID, c.conf.ClientSecret, c.conf.TokenURL, v, internal.AuthStyle(c.conf.AuthStyle))
if err != nil {
if rErr, ok := err.(*internal.RetrieveError); ok {
return nil, (*oauth2.RetrieveError)(rErr)
Expand Down
17 changes: 12 additions & 5 deletions clientcredentials/clientcredentials_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,14 @@ package clientcredentials

import (
"context"
"io"
"io/ioutil"
"net/http"
"net/http/httptest"
"net/url"
"testing"

"golang.org/x/oauth2/internal"
)

func newConf(serverURL string) *Config {
Expand Down Expand Up @@ -111,21 +114,25 @@ func TestTokenRequest(t *testing.T) {
}

func TestTokenRefreshRequest(t *testing.T) {
internal.ResetAuthCache()
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.String() == "/somethingelse" {
return
}
if r.URL.String() != "/token" {
t.Errorf("Unexpected token refresh request URL, %v is found.", r.URL)
t.Errorf("Unexpected token refresh request URL: %q", r.URL)
}
headerContentType := r.Header.Get("Content-Type")
if headerContentType != "application/x-www-form-urlencoded" {
t.Errorf("Unexpected Content-Type header, %v is found.", headerContentType)
if got, want := headerContentType, "application/x-www-form-urlencoded"; got != want {
t.Errorf("Content-Type = %q; want %q", got, want)
}
body, _ := ioutil.ReadAll(r.Body)
if string(body) != "audience=audience1&grant_type=client_credentials&scope=scope1+scope2" {
t.Errorf("Unexpected refresh token payload, %v is found.", string(body))
const want = "audience=audience1&grant_type=client_credentials&scope=scope1+scope2"
if string(body) != want {
t.Errorf("Unexpected refresh token payload.\n got: %s\nwant: %s\n", body, want)
}
w.Header().Set("Content-Type", "application/json")
io.WriteString(w, `{"access_token": "foo", "refresh_token": "bar"}`)
}))
defer ts.Close()
conf := newConf(ts.URL)
Expand Down
5 changes: 3 additions & 2 deletions google/google.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,9 @@ import (

// Endpoint is Google's OAuth 2.0 endpoint.
var Endpoint = oauth2.Endpoint{
AuthURL: "https://accounts.google.com/o/oauth2/auth",
TokenURL: "https://accounts.google.com/o/oauth2/token",
AuthURL: "https://accounts.google.com/o/oauth2/auth",
TokenURL: "https://accounts.google.com/o/oauth2/token",
AuthStyle: oauth2.AuthStyleInParams,
}

// JWTTokenURL is Google's OAuth 2.0 token URL to use with the JWT flow.
Expand Down
210 changes: 115 additions & 95 deletions internal/token.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
"net/url"
"strconv"
"strings"
"sync"
"time"

"golang.org/x/net/context/ctxhttp"
Expand Down Expand Up @@ -90,102 +91,71 @@ func (e *expirationTime) UnmarshalJSON(b []byte) error {
return nil
}

var brokenAuthHeaderProviders = []string{
"https://accounts.google.com/",
"https://api.codeswholesale.com/oauth/token",
"https://api.dropbox.com/",
"https://api.dropboxapi.com/",
"https://api.instagram.com/",
"https://api.netatmo.net/",
"https://api.odnoklassniki.ru/",
"https://api.pushbullet.com/",
"https://api.soundcloud.com/",
"https://api.twitch.tv/",
"https://id.twitch.tv/",
"https://app.box.com/",
"https://api.box.com/",
"https://connect.stripe.com/",
"https://login.mailchimp.com/",
"https://login.microsoftonline.com/",
"https://login.salesforce.com/",
"https://login.windows.net",
"https://login.live.com/",
"https://login.live-int.com/",
"https://oauth.sandbox.trainingpeaks.com/",
"https://oauth.trainingpeaks.com/",
"https://oauth.vk.com/",
"https://openapi.baidu.com/",
"https://slack.com/",
"https://test-sandbox.auth.corp.google.com",
"https://test.salesforce.com/",
"https://user.gini.net/",
"https://www.douban.com/",
"https://www.googleapis.com/",
"https://www.linkedin.com/",
"https://www.strava.com/oauth/",
"https://www.wunderlist.com/oauth/",
"https://api.patreon.com/",
"https://sandbox.codeswholesale.com/oauth/token",
"https://api.sipgate.com/v1/authorization/oauth",
"https://api.medium.com/v1/tokens",
"https://log.finalsurge.com/oauth/token",
"https://multisport.todaysplan.com.au/rest/oauth/access_token",
"https://whats.todaysplan.com.au/rest/oauth/access_token",
"https://stackoverflow.com/oauth/access_token",
"https://account.health.nokia.com",
"https://accounts.zoho.com",
"https://gitter.im/login/oauth/token",
"https://openid-connect.onelogin.com/oidc",
"https://api.dailymotion.com/oauth/token",
// RegisterBrokenAuthHeaderProvider previously did something. It is now a no-op.
//
// Deprecated: this function no longer does anything. Caller code that
// wants to avoid potential extra HTTP requests made during
// auto-probing of the provider's auth style should set
// Endpoint.AuthStyle.
func RegisterBrokenAuthHeaderProvider(tokenURL string) {}

// AuthStyle is a copy of the golang.org/x/oauth2 package's AuthStyle type.
type AuthStyle int

const (
AuthStyleUnknown AuthStyle = 0
AuthStyleInParams AuthStyle = 1

This comment has been minimized.

Sign in to view

Copy link
@mjhaller

mjhaller Jan 19, 2022

This seems like it should be AuthStyleInBody. the word "params" implies query parameters.
AuthStyleInHeader AuthStyle = 2
)

// authStyleCache is the set of tokenURLs we've successfully used via
// RetrieveToken and which style auth we ended up using.
// It's called a cache, but it doesn't (yet?) shrink. It's expected that
// the set of OAuth2 servers a program contacts over time is fixed and
// small.
var authStyleCache struct {
sync.Mutex
m map[string]AuthStyle // keyed by tokenURL
}

// brokenAuthHeaderDomains lists broken providers that issue dynamic endpoints.
var brokenAuthHeaderDomains = []string{
".auth0.com",
".force.com",
".myshopify.com",
".okta.com",
".oktapreview.com",
// ResetAuthCache resets the global authentication style cache used
// for AuthStyleUnknown token requests.
func ResetAuthCache() {
authStyleCache.Lock()
defer authStyleCache.Unlock()
authStyleCache.m = nil
}

func RegisterBrokenAuthHeaderProvider(tokenURL string) {
brokenAuthHeaderProviders = append(brokenAuthHeaderProviders, tokenURL)
// lookupAuthStyle reports which auth style we last used with tokenURL
// when calling RetrieveToken and whether we have ever done so.
func lookupAuthStyle(tokenURL string) (style AuthStyle, ok bool) {
authStyleCache.Lock()
defer authStyleCache.Unlock()
style, ok = authStyleCache.m[tokenURL]
return
}

// providerAuthHeaderWorks reports whether the OAuth2 server identified by the tokenURL
// implements the OAuth2 spec correctly
// See https://code.google.com/p/goauth2/issues/detail?id=31 for background.
// In summary:
// - Reddit only accepts client secret in the Authorization header
// - Dropbox accepts either it in URL param or Auth header, but not both.
// - Google only accepts URL param (not spec compliant?), not Auth header
// - Stripe only accepts client secret in Auth header with Bearer method, not Basic
func providerAuthHeaderWorks(tokenURL string) bool {
for _, s := range brokenAuthHeaderProviders {
if strings.HasPrefix(tokenURL, s) {
// Some sites fail to implement the OAuth2 spec fully.
return false
}
// setAuthStyle adds an entry to authStyleCache, documented above.
func setAuthStyle(tokenURL string, v AuthStyle) {
authStyleCache.Lock()
defer authStyleCache.Unlock()
if authStyleCache.m == nil {
authStyleCache.m = make(map[string]AuthStyle)
}

if u, err := url.Parse(tokenURL); err == nil {
for _, s := range brokenAuthHeaderDomains {
if strings.HasSuffix(u.Host, s) {
return false
}
}
}

// Assume the provider implements the spec properly
// otherwise. We can add more exceptions as they're
// discovered. We will _not_ be adding configurable hooks
// to this package to let users select server bugs.
return true
authStyleCache.m[tokenURL] = v
}

func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values) (*Token, error) {
bustedAuth := !providerAuthHeaderWorks(tokenURL)
if bustedAuth {
// newTokenRequest returns a new *http.Request to retrieve a new token
// from tokenURL using the provided clientID, clientSecret, and POST
// body parameters.
//
// inParams is whether the clientID & clientSecret should be encoded
// as the POST body. An 'inParams' value of true means to send it in
// the POST body (along with any values in v); false means to send it
// in the Authorization header.
func newTokenRequest(tokenURL, clientID, clientSecret string, v url.Values, authStyle AuthStyle) (*http.Request, error) {
if authStyle == AuthStyleInParams {
v = cloneURLValues(v)
if clientID != "" {
v.Set("client_id", clientID)
}
Expand All @@ -198,15 +168,70 @@ func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string,
return nil, err
}
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
if !bustedAuth {
if authStyle == AuthStyleInHeader {
req.SetBasicAuth(url.QueryEscape(clientID), url.QueryEscape(clientSecret))
}
return req, nil
}

func cloneURLValues(v url.Values) url.Values {
v2 := make(url.Values, len(v))
for k, vv := range v {
v2[k] = append([]string(nil), vv...)
}
return v2
}

func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string, v url.Values, authStyle AuthStyle) (*Token, error) {
needsAuthStyleProbe := authStyle == 0
if needsAuthStyleProbe {
if style, ok := lookupAuthStyle(tokenURL); ok {
authStyle = style
needsAuthStyleProbe = false
} else {
authStyle = AuthStyleInHeader // the first way we'll try
}
}
req, err := newTokenRequest(tokenURL, clientID, clientSecret, v, authStyle)
if err != nil {
return nil, err
}
token, err := doTokenRoundTrip(ctx, req)
if err != nil && needsAuthStyleProbe {
// If we get an error, assume the server wants the
// clientID & clientSecret in a different form.
// See https://code.google.com/p/goauth2/issues/detail?id=31 for background.
// In summary:
// - Reddit only accepts client secret in the Authorization header
// - Dropbox accepts either it in URL param or Auth header, but not both.
// - Google only accepts URL param (not spec compliant?), not Auth header
// - Stripe only accepts client secret in Auth header with Bearer method, not Basic
//
// We used to maintain a big table in this code of all the sites and which way
// they went, but maintaining it didn't scale & got annoying.
// So just try both ways.
authStyle = AuthStyleInParams // the second way we'll try
req, _ = newTokenRequest(tokenURL, clientID, clientSecret, v, authStyle)
token, err = doTokenRoundTrip(ctx, req)
}
if needsAuthStyleProbe && err == nil {
setAuthStyle(tokenURL, authStyle)
}
// Don't overwrite `RefreshToken` with an empty value
// if this was a token refreshing request.
if token != nil && token.RefreshToken == "" {
token.RefreshToken = v.Get("refresh_token")
}
return token, err
}

func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
r, err := ctxhttp.Do(ctx, ContextClient(ctx), req)
if err != nil {
return nil, err
}
defer r.Body.Close()
body, err := ioutil.ReadAll(io.LimitReader(r.Body, 1<<20))
r.Body.Close()
if err != nil {
return nil, fmt.Errorf("oauth2: cannot fetch token: %v", err)
}
Expand Down Expand Up @@ -256,13 +281,8 @@ func RetrieveToken(ctx context.Context, clientID, clientSecret, tokenURL string,
}
json.Unmarshal(body, &token.Raw) // no error checks for optional fields
}
// Don't overwrite `RefreshToken` with an empty value
// if this was a token refreshing request.
if token.RefreshToken == "" {
token.RefreshToken = v.Get("refresh_token")
}
if token.AccessToken == "" {
return token, errors.New("oauth2: server response missing access_token")
return nil, errors.New("oauth2: server response missing access_token")
}
return token, nil
}
Expand Down
Loading

0 comments on commit 80673b4

Please sign in to comment.