internal: remove fallback parsing for expires_in
Facebook has correctted its OAuth2 implementation. The code as
a fallback can be removed now.
Updates golang/oauth2#51, golang/oauth2#239
Change-Id: Ib5f84bc35c0c4ecbdd25d4169f950410d4ae79a2
Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/168017
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: JBD <jbd@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/facebook/facebook.go b/facebook/facebook.go
index 21c49e7..b0054e3 100644
--- a/facebook/facebook.go
+++ b/facebook/facebook.go
@@ -11,6 +11,6 @@
// Endpoint is Facebook's OAuth 2.0 endpoint.
var Endpoint = oauth2.Endpoint{
- AuthURL: "https://www.facebook.com/v3.1/dialog/oauth",
- TokenURL: "https://graph.facebook.com/v3.1/oauth/access_token",
+ AuthURL: "https://www.facebook.com/v3.2/dialog/oauth",
+ TokenURL: "https://graph.facebook.com/v3.2/oauth/access_token",
}
diff --git a/internal/token.go b/internal/token.go
index 83f7847..355c386 100644
--- a/internal/token.go
+++ b/internal/token.go
@@ -63,16 +63,12 @@
TokenType string `json:"token_type"`
RefreshToken string `json:"refresh_token"`
ExpiresIn expirationTime `json:"expires_in"` // at least PayPal returns string, while most return number
- Expires expirationTime `json:"expires"` // broken Facebook spelling of expires_in
}
func (e *tokenJSON) expiry() (t time.Time) {
if v := e.ExpiresIn; v != 0 {
return time.Now().Add(time.Duration(v) * time.Second)
}
- if v := e.Expires; v != 0 {
- return time.Now().Add(time.Duration(v) * time.Second)
- }
return
}
@@ -264,12 +260,6 @@
Raw: vals,
}
e := vals.Get("expires_in")
- if e == "" || e == "null" {
- // TODO(jbd): Facebook's OAuth2 implementation is broken and
- // returns expires_in field in expires. Remove the fallback to expires,
- // when Facebook fixes their implementation.
- e = vals.Get("expires")
- }
expires, _ := strconv.Atoi(e)
if expires != 0 {
token.Expiry = time.Now().Add(time.Duration(expires) * time.Second)
diff --git a/oauth2_test.go b/oauth2_test.go
index b76eaae..588600b 100644
--- a/oauth2_test.go
+++ b/oauth2_test.go
@@ -275,26 +275,26 @@
func TestExchangeRequest_JSONResponse_Expiry(t *testing.T) {
seconds := int32(day.Seconds())
for _, c := range []struct {
- name string
- expires string
- want bool
+ name string
+ expires string
+ want bool
+ nullExpires bool
}{
- {"normal", fmt.Sprintf(`"expires_in": %d`, seconds), true},
- {"paypal", fmt.Sprintf(`"expires_in": "%d"`, seconds), true},
- {"facebook", fmt.Sprintf(`"expires": %d`, seconds), true},
- {"issue_239", fmt.Sprintf(`"expires_in": null, "expires": %d`, seconds), true},
+ {"normal", fmt.Sprintf(`"expires_in": %d`, seconds), true, false},
+ {"paypal", fmt.Sprintf(`"expires_in": "%d"`, seconds), true, false},
+ {"issue_239", fmt.Sprintf(`"expires_in": null`), true, true},
- {"wrong_type", `"expires": false`, false},
- {"wrong_type2", `"expires": {}`, false},
- {"wrong_value", `"expires": "zzz"`, false},
+ {"wrong_type", `"expires_in": false`, false, false},
+ {"wrong_type2", `"expires_in": {}`, false, false},
+ {"wrong_value", `"expires_in": "zzz"`, false, false},
} {
t.Run(c.name, func(t *testing.T) {
- testExchangeRequest_JSONResponse_expiry(t, c.expires, c.want)
+ testExchangeRequest_JSONResponse_expiry(t, c.expires, c.want, c.nullExpires)
})
}
}
-func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want bool) {
+func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want, nullExpires bool) {
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Set("Content-Type", "application/json")
w.Write([]byte(fmt.Sprintf(`{"access_token": "90d", "scope": "user", "token_type": "bearer", %s}`, exp)))
@@ -303,7 +303,7 @@
conf := newConf(ts.URL)
t1 := time.Now().Add(day)
tok, err := conf.Exchange(context.Background(), "exchange-code")
- t2 := time.Now().Add(day)
+ t2 := t1.Add(day)
if got := (err == nil); got != want {
if want {
@@ -319,8 +319,12 @@
t.Fatalf("Token invalid. Got: %#v", tok)
}
expiry := tok.Expiry
+
+ if nullExpires && expiry.IsZero() {
+ return
+ }
if expiry.Before(t1) || expiry.After(t2) {
- t.Errorf("Unexpected value for Expiry: %v (shold be between %v and %v)", expiry, t1, t2)
+ t.Errorf("Unexpected value for Expiry: %v (should be between %v and %v)", expiry, t1, t2)
}
}