Skip to content

Commit c85d3e9

Browse files
ggiccibradfitz
authored andcommitted
internal: remove fallback parsing for expires_in
Facebook has correctted its OAuth2 implementation. The code as a fallback can be removed now. Updates golang#51, golang#239 Change-Id: Ib5f84bc35c0c4ecbdd25d4169f950410d4ae79a2 Reviewed-on: https://go-review.googlesource.com/c/oauth2/+/168017 Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-by: JBD <[email protected]> Run-TryBot: Brad Fitzpatrick <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent e64efc7 commit c85d3e9

File tree

3 files changed

+21
-27
lines changed

3 files changed

+21
-27
lines changed

facebook/facebook.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,6 @@ import (
1111

1212
// Endpoint is Facebook's OAuth 2.0 endpoint.
1313
var Endpoint = oauth2.Endpoint{
14-
AuthURL: "https://www.facebook.com/v3.1/dialog/oauth",
15-
TokenURL: "https://graph.facebook.com/v3.1/oauth/access_token",
14+
AuthURL: "https://www.facebook.com/v3.2/dialog/oauth",
15+
TokenURL: "https://graph.facebook.com/v3.2/oauth/access_token",
1616
}

internal/token.go

-10
Original file line numberDiff line numberDiff line change
@@ -63,16 +63,12 @@ type tokenJSON struct {
6363
TokenType string `json:"token_type"`
6464
RefreshToken string `json:"refresh_token"`
6565
ExpiresIn expirationTime `json:"expires_in"` // at least PayPal returns string, while most return number
66-
Expires expirationTime `json:"expires"` // broken Facebook spelling of expires_in
6766
}
6867

6968
func (e *tokenJSON) expiry() (t time.Time) {
7069
if v := e.ExpiresIn; v != 0 {
7170
return time.Now().Add(time.Duration(v) * time.Second)
7271
}
73-
if v := e.Expires; v != 0 {
74-
return time.Now().Add(time.Duration(v) * time.Second)
75-
}
7672
return
7773
}
7874

@@ -264,12 +260,6 @@ func doTokenRoundTrip(ctx context.Context, req *http.Request) (*Token, error) {
264260
Raw: vals,
265261
}
266262
e := vals.Get("expires_in")
267-
if e == "" || e == "null" {
268-
// TODO(jbd): Facebook's OAuth2 implementation is broken and
269-
// returns expires_in field in expires. Remove the fallback to expires,
270-
// when Facebook fixes their implementation.
271-
e = vals.Get("expires")
272-
}
273263
expires, _ := strconv.Atoi(e)
274264
if expires != 0 {
275265
token.Expiry = time.Now().Add(time.Duration(expires) * time.Second)

oauth2_test.go

+19-15
Original file line numberDiff line numberDiff line change
@@ -275,26 +275,26 @@ const day = 24 * time.Hour
275275
func TestExchangeRequest_JSONResponse_Expiry(t *testing.T) {
276276
seconds := int32(day.Seconds())
277277
for _, c := range []struct {
278-
name string
279-
expires string
280-
want bool
278+
name string
279+
expires string
280+
want bool
281+
nullExpires bool
281282
}{
282-
{"normal", fmt.Sprintf(`"expires_in": %d`, seconds), true},
283-
{"paypal", fmt.Sprintf(`"expires_in": "%d"`, seconds), true},
284-
{"facebook", fmt.Sprintf(`"expires": %d`, seconds), true},
285-
{"issue_239", fmt.Sprintf(`"expires_in": null, "expires": %d`, seconds), true},
286-
287-
{"wrong_type", `"expires": false`, false},
288-
{"wrong_type2", `"expires": {}`, false},
289-
{"wrong_value", `"expires": "zzz"`, false},
283+
{"normal", fmt.Sprintf(`"expires_in": %d`, seconds), true, false},
284+
{"paypal", fmt.Sprintf(`"expires_in": "%d"`, seconds), true, false},
285+
{"issue_239", fmt.Sprintf(`"expires_in": null`), true, true},
286+
287+
{"wrong_type", `"expires_in": false`, false, false},
288+
{"wrong_type2", `"expires_in": {}`, false, false},
289+
{"wrong_value", `"expires_in": "zzz"`, false, false},
290290
} {
291291
t.Run(c.name, func(t *testing.T) {
292-
testExchangeRequest_JSONResponse_expiry(t, c.expires, c.want)
292+
testExchangeRequest_JSONResponse_expiry(t, c.expires, c.want, c.nullExpires)
293293
})
294294
}
295295
}
296296

297-
func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want bool) {
297+
func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want, nullExpires bool) {
298298
ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
299299
w.Header().Set("Content-Type", "application/json")
300300
w.Write([]byte(fmt.Sprintf(`{"access_token": "90d", "scope": "user", "token_type": "bearer", %s}`, exp)))
@@ -303,7 +303,7 @@ func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want bool
303303
conf := newConf(ts.URL)
304304
t1 := time.Now().Add(day)
305305
tok, err := conf.Exchange(context.Background(), "exchange-code")
306-
t2 := time.Now().Add(day)
306+
t2 := t1.Add(day)
307307

308308
if got := (err == nil); got != want {
309309
if want {
@@ -319,8 +319,12 @@ func testExchangeRequest_JSONResponse_expiry(t *testing.T, exp string, want bool
319319
t.Fatalf("Token invalid. Got: %#v", tok)
320320
}
321321
expiry := tok.Expiry
322+
323+
if nullExpires && expiry.IsZero() {
324+
return
325+
}
322326
if expiry.Before(t1) || expiry.After(t2) {
323-
t.Errorf("Unexpected value for Expiry: %v (shold be between %v and %v)", expiry, t1, t2)
327+
t.Errorf("Unexpected value for Expiry: %v (should be between %v and %v)", expiry, t1, t2)
324328
}
325329
}
326330

0 commit comments

Comments
 (0)