diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..433d310 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,27 @@ +# Changelog + +## Unreleased + +### Added: opt-in support for expiring access tokens and refresh tokens + +GitHub OAuth apps can issue access tokens that expire after 8 hours along with a refresh token valid +for 6 months. This release adds support for that flow. It is **opt-in and fully backwards +compatible** — existing applications continue to receive non-expiring tokens with no code changes. + +- `Flow.RequestRefreshToken` opts an authorization into expiring tokens by requesting the + `offline_access` scope, in both Device flow and Web application flow. Also available as + `device.WithRefreshToken()` and `webapp.WithRefreshToken()` for callers using those packages + directly. +- `api.AccessToken` now records `ExpiresIn`, `ExpiresAt`, `RefreshTokenExpiresIn`, and + `RefreshTokenExpiresAt`, with `IsExpired()` and `CanRefresh()` helpers. +- `api.Refresh` exchanges a refresh token for a new token. A rejected refresh token is reported as + `api.ErrRefreshTokenInvalid`. +- `oauth.TokenSource` holds a token and refreshes it on demand, with an `OnRefresh` callback for + persisting rotated credentials. +- `oauth.NewHTTPClient` returns an `http.Client` that attaches the token and, on a rejected request, + refreshes once and retries once. + +Servers that do not support expiring tokens ignore the request and return a non-expiring token with +no refresh token, so applications must not assume a refresh token was issued. + +See the "Expiring access tokens" section of the README for an adoption guide. diff --git a/README.md b/README.md index 41cb35c..021ab55 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ To transparently enable OAuth authorization on _any GitHub host_ (e.g. GHES inst ## Usage - [OAuth Device flow with fallback](./examples_test.go) +- [OAuth flow with refresh token support](./examples_test.go) - [manual OAuth Device flow](./device/examples_test.go) - [manual OAuth web application flow](./webapp/examples_test.go) @@ -23,6 +24,90 @@ Applications that need more control over the user experience around authenticati In theory, these packages would enable authorization on any OAuth-enabled host. In practice, however, this was only tested for authorizing with GitHub. +## Expiring access tokens + +GitHub OAuth apps can issue access tokens that expire after 8 hours, accompanied by a refresh token that is valid for 6 months. Rotating tokens limits the damage a leaked token can do. See [Expiring access tokens][gh-expiring]. + +Support in this library is **opt-in and off by default**: existing code continues to receive non-expiring tokens and needs no changes. + +### 1. Opt in + +Set `RequestRefreshToken`, which requests the `offline_access` scope so that GitHub issues an expiring token even if your app isn't globally configured for them: + +```go +flow := &oauth.Flow{ + Host: host, + ClientID: clientID, + Scopes: []string{"repo", "read:org"}, + + RequestRefreshToken: true, // <- the only change needed to opt in +} + +accessToken, err := flow.DetectFlow() +``` + +`offline_access` is not a normal scope: it doesn't widen the token's access and doesn't add anything to the authorization prompt. + +### 2. Persist the new fields + +Apps typically store only `accessToken.Token`. That is no longer enough — you must persist the refresh token and both expiration times, or the user will have to re-authorize every 8 hours: + +```go +type storedCredentials struct { + Token string `json:"token"` + RefreshToken string `json:"refresh_token,omitempty"` + ExpiresAt time.Time `json:"expires_at,omitempty"` + RefreshTokenExpiresAt time.Time `json:"refresh_token_expires_at,omitempty"` +} +``` + +If `RefreshToken` is empty, the server does not support expiring tokens. This is expected on GitHub Enterprise Server. Store the token as you always have and skip the rest of this section — **never assume a refresh token was returned**. + +### 3. Use a `TokenSource` instead of setting the header yourself + +Wrap the token in a `TokenSource` and build an HTTP client from it. The client attaches the `Authorization` header, refreshes the token when it expires, and — if the server rejects a request anyway — refreshes once and retries the request exactly once: + +```go +src := oauth.NewTokenSource(accessToken, clientID, clientSecret, host.TokenURL) +httpClient := oauth.NewHTTPClient(src) + +resp, err := httpClient.Get("https://api.github.com/user") +``` + +Replace any code that sets `Authorization` manually. Share one `TokenSource` across your app so a refresh performed for one request is seen by all the others. + +### 4. Save rotated tokens with `OnRefresh` + +**Refresh tokens are single-use.** A successful refresh immediately invalidates both the old access token and the old refresh token, so a rotated token that you fail to save is a token you have lost: + +```go +src.OnRefresh = func(token *api.AccessToken) error { + return saveCredentials(token) +} +``` + +If `OnRefresh` returns an error, that error is returned to the caller so the failure is visible, but the `TokenSource` keeps the refreshed token — discarding it would not bring back the old one. The callback runs without the `TokenSource` lock held, so it is safe for it to use a client built from the same source. + +### 5. Handle the failure cases + +| Situation | How to detect it | What to do | +| --- | --- | --- | +| Refresh token expired or already used | `errors.Is(err, api.ErrRefreshTokenInvalid)` | Send the user through the flow again | +| Token expired, no refresh token available | `errors.Is(err, oauth.ErrNotRefreshable)` | Send the user through the flow again | +| Server doesn't support expiring tokens | `accessToken.RefreshToken == ""` | Nothing — behaves exactly as before | + +### Adoption checklist + +1. Set `RequestRefreshToken: true` on your `Flow`. +2. Extend your credential storage with `RefreshToken`, `ExpiresAt`, and `RefreshTokenExpiresAt`. +3. Build a `TokenSource` from the stored token and replace manual `Authorization` headers with `oauth.NewHTTPClient`. +4. Set `OnRefresh` to persist rotated tokens. +5. Handle `api.ErrRefreshTokenInvalid` and `oauth.ErrNotRefreshable` by restarting the authorization flow. +6. Confirm your app still works against a server that returns no refresh token. + +See [the complete example](./examples_test.go). + [oauth-device]: https://oauth.net/2/device-flow/ [gh-device]: https://docs.github.com/en/free-pro-team@latest/developers/apps/authorizing-oauth-apps#device-flow +[gh-expiring]: https://docs.github.com/en/apps/oauth-apps/building-oauth-apps/authorizing-oauth-apps#expiring-access-tokens diff --git a/api/access_token.go b/api/access_token.go index 718d69d..52fb682 100644 --- a/api/access_token.go +++ b/api/access_token.go @@ -1,5 +1,17 @@ package api +import ( + "strconv" + "time" +) + +// timeNow is swappable in tests. +var timeNow = time.Now + +// expiryLeeway is subtracted from a token's expiration time when determining whether it is expired, +// so that a token is not considered valid moments before the server would reject it. +const expiryLeeway = 60 * time.Second + // AccessToken is an OAuth access token. type AccessToken struct { // The token value, typically a 40-character random string. @@ -10,18 +22,67 @@ type AccessToken struct { Type string // Space-separated list of OAuth scopes that this token grants. Scope string + + // The number of seconds from the time of issue until Token expires. Zero if the server issued a + // non-expiring token. + ExpiresIn int + // The number of seconds from the time of issue until RefreshToken expires. Zero if the server did + // not issue a refresh token. + RefreshTokenExpiresIn int + // The absolute time at which Token expires. Zero if the server issued a non-expiring token. + ExpiresAt time.Time + // The absolute time at which RefreshToken expires. Zero if the server did not issue a refresh token. + RefreshTokenExpiresAt time.Time +} + +// IsExpired reports whether the access token has expired. Tokens that never expire are never +// reported as expired. A small leeway is applied to guard against clock skew. +func (t *AccessToken) IsExpired() bool { + if t == nil || t.ExpiresAt.IsZero() { + return false + } + return !timeNow().Before(t.ExpiresAt.Add(-expiryLeeway)) +} + +// CanRefresh reports whether the token carries a refresh token that has not itself expired. If it +// returns false, obtaining a new token requires sending the user through an authorization flow again. +func (t *AccessToken) CanRefresh() bool { + if t == nil || t.RefreshToken == "" { + return false + } + if t.RefreshTokenExpiresAt.IsZero() { + return true + } + return timeNow().Before(t.RefreshTokenExpiresAt.Add(-expiryLeeway)) } // AccessToken extracts the access token information from a server response. func (f FormResponse) AccessToken() (*AccessToken, error) { - if accessToken := f.Get("access_token"); accessToken != "" { - return &AccessToken{ - Token: accessToken, - RefreshToken: f.Get("refresh_token"), - Type: f.Get("token_type"), - Scope: f.Get("scope"), - }, nil + accessToken := f.Get("access_token") + if accessToken == "" { + return nil, f.Err() + } + + now := timeNow() + token := &AccessToken{ + Token: accessToken, + RefreshToken: f.Get("refresh_token"), + Type: f.Get("token_type"), + Scope: f.Get("scope"), + } + + // Servers that do not support expiring tokens omit these values entirely. Unparseable values are + // treated the same as missing ones so that a usable token is never discarded over metadata. + if expiresIn, err := strconv.Atoi(f.Get("expires_in")); err == nil && expiresIn > 0 { + token.ExpiresIn = expiresIn + token.ExpiresAt = now.Add(time.Duration(expiresIn) * time.Second) + } + if token.RefreshToken != "" { + if expiresIn, err := strconv.Atoi(f.Get("refresh_token_expires_in")); err == nil && expiresIn > 0 { + token.RefreshTokenExpiresIn = expiresIn + token.RefreshTokenExpiresAt = now.Add(time.Duration(expiresIn) * time.Second) + } } - return nil, f.Err() + return token, nil } diff --git a/api/access_token_expiry_test.go b/api/access_token_expiry_test.go new file mode 100644 index 0000000..6305e2e --- /dev/null +++ b/api/access_token_expiry_test.go @@ -0,0 +1,168 @@ +package api + +import ( + "net/url" + "testing" + "time" +) + +func TestFormResponse_AccessToken_expiry(t *testing.T) { + now := time.Date(2026, 8, 18, 12, 0, 0, 0, time.UTC) + timeNow = func() time.Time { return now } + t.Cleanup(func() { timeNow = time.Now }) + + tests := []struct { + name string + values url.Values + wantExpiresIn int + wantExpiresAt time.Time + wantRefreshExpiresIn int + wantRefreshTokenExpiresAt time.Time + }{ + { + name: "expiring token with refresh token", + values: url.Values{ + "access_token": []string{"ATOKEN"}, + "refresh_token": []string{"RTOKEN"}, + "expires_in": []string{"28800"}, + "refresh_token_expires_in": []string{"15897600"}, + }, + wantExpiresIn: 28800, + wantExpiresAt: now.Add(28800 * time.Second), + wantRefreshExpiresIn: 15897600, + wantRefreshTokenExpiresAt: now.Add(15897600 * time.Second), + }, + { + name: "server without expiring token support", + values: url.Values{ + "access_token": []string{"ATOKEN"}, + "token_type": []string{"bearer"}, + }, + }, + { + name: "unparseable expiry is ignored", + values: url.Values{ + "access_token": []string{"ATOKEN"}, + "expires_in": []string{"soon"}, + }, + }, + { + name: "refresh expiry ignored without refresh token", + values: url.Values{ + "access_token": []string{"ATOKEN"}, + "refresh_token_expires_in": []string{"15897600"}, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := FormResponse{values: tt.values}.AccessToken() + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got.ExpiresIn != tt.wantExpiresIn { + t.Errorf("ExpiresIn = %d, want %d", got.ExpiresIn, tt.wantExpiresIn) + } + if !got.ExpiresAt.Equal(tt.wantExpiresAt) { + t.Errorf("ExpiresAt = %v, want %v", got.ExpiresAt, tt.wantExpiresAt) + } + if got.RefreshTokenExpiresIn != tt.wantRefreshExpiresIn { + t.Errorf("RefreshTokenExpiresIn = %d, want %d", got.RefreshTokenExpiresIn, tt.wantRefreshExpiresIn) + } + if !got.RefreshTokenExpiresAt.Equal(tt.wantRefreshTokenExpiresAt) { + t.Errorf("RefreshTokenExpiresAt = %v, want %v", got.RefreshTokenExpiresAt, tt.wantRefreshTokenExpiresAt) + } + }) + } +} + +func TestAccessToken_IsExpired(t *testing.T) { + now := time.Date(2026, 8, 18, 12, 0, 0, 0, time.UTC) + timeNow = func() time.Time { return now } + t.Cleanup(func() { timeNow = time.Now }) + + tests := []struct { + name string + token *AccessToken + want bool + }{ + {name: "nil token", token: nil, want: false}, + {name: "non-expiring token", token: &AccessToken{Token: "A"}, want: false}, + {name: "valid token", token: &AccessToken{ExpiresAt: now.Add(time.Hour)}, want: false}, + {name: "expired token", token: &AccessToken{ExpiresAt: now.Add(-time.Second)}, want: true}, + {name: "within leeway counts as expired", token: &AccessToken{ExpiresAt: now.Add(30 * time.Second)}, want: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.token.IsExpired(); got != tt.want { + t.Errorf("IsExpired() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestAccessToken_CanRefresh(t *testing.T) { + now := time.Date(2026, 8, 18, 12, 0, 0, 0, time.UTC) + timeNow = func() time.Time { return now } + t.Cleanup(func() { timeNow = time.Now }) + + tests := []struct { + name string + token *AccessToken + want bool + }{ + {name: "nil token", token: nil, want: false}, + {name: "no refresh token", token: &AccessToken{Token: "A"}, want: false}, + {name: "refresh token without expiry", token: &AccessToken{RefreshToken: "R"}, want: true}, + {name: "unexpired refresh token", token: &AccessToken{RefreshToken: "R", RefreshTokenExpiresAt: now.Add(time.Hour)}, want: true}, + {name: "expired refresh token", token: &AccessToken{RefreshToken: "R", RefreshTokenExpiresAt: now.Add(-time.Hour)}, want: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.token.CanRefresh(); got != tt.want { + t.Errorf("CanRefresh() = %v, want %v", got, tt.want) + } + }) + } +} + +func TestAppendOfflineAccess(t *testing.T) { + tests := []struct { + name string + input []string + want []string + }{ + {name: "empty", input: nil, want: []string{"offline_access"}}, + {name: "appends", input: []string{"repo"}, want: []string{"repo", "offline_access"}}, + {name: "dedupes", input: []string{"repo", "offline_access"}, want: []string{"repo", "offline_access"}}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := AppendOfflineAccess(tt.input) + if len(got) != len(tt.want) { + t.Fatalf("got %v, want %v", got, tt.want) + } + for i := range got { + if got[i] != tt.want[i] { + t.Fatalf("got %v, want %v", got, tt.want) + } + } + }) + } +} + +func TestAppendOfflineAccess_doesNotMutateInput(t *testing.T) { + // A caller's Scopes slice with spare capacity must not be written through. + input := make([]string, 1, 4) + input[0] = "repo" + + _ = AppendOfflineAccess(input) + + if got := input[:cap(input)]; got[1] != "" { + t.Errorf("input slice was mutated: %q", got[1]) + } +} diff --git a/api/refresh.go b/api/refresh.go new file mode 100644 index 0000000..c95ea9f --- /dev/null +++ b/api/refresh.go @@ -0,0 +1,60 @@ +package api + +import ( + "errors" + "net/url" +) + +// ErrRefreshTokenInvalid is returned when the server rejects the refresh token, either because it +// has expired or because it has already been used. Recovering requires sending the user through an +// authorization flow again. +var ErrRefreshTokenInvalid = errors.New("refresh token is invalid or expired") + +const refreshGrantType = "refresh_token" + +// RefreshOptions specifies parameters to exchange a refresh token for a new access token. +type RefreshOptions struct { + // ClientID is the app client ID value. + ClientID string + // ClientSecret is the app client secret value. Required for tokens obtained via web application + // flow; not needed for tokens obtained via device flow. + ClientSecret string + // RefreshToken is the refresh token issued alongside the expiring access token. + RefreshToken string +} + +// Refresh exchanges a refresh token for a new access token at tokenURL. +// +// Refresh tokens are single-use: on success, both the refresh token passed in and its associated +// access token are immediately invalidated by the server, and the returned AccessToken carries their +// replacements. Callers must persist the result before making further requests. +func Refresh(c httpClient, tokenURL string, opts RefreshOptions) (*AccessToken, error) { + if opts.RefreshToken == "" { + return nil, ErrRefreshTokenInvalid + } + + values := url.Values{ + "client_id": {opts.ClientID}, + "refresh_token": {opts.RefreshToken}, + "grant_type": {refreshGrantType}, + } + if opts.ClientSecret != "" { + values.Set("client_secret", opts.ClientSecret) + } + + resp, err := PostForm(c, tokenURL, values) + if err != nil { + return nil, err + } + + token, err := resp.AccessToken() + if err != nil { + var apiError *Error + if errors.As(err, &apiError) && apiError.Code == "bad_refresh_token" { + return nil, ErrRefreshTokenInvalid + } + return nil, err + } + + return token, nil +} diff --git a/api/refresh_test.go b/api/refresh_test.go new file mode 100644 index 0000000..cf0bed8 --- /dev/null +++ b/api/refresh_test.go @@ -0,0 +1,156 @@ +package api + +import ( + "bytes" + "errors" + "io" + "net/http" + "net/url" + "testing" +) + +type recordingClient struct { + status int + body string + contentType string + err error + + postCount int + lastURL string + lastForm url.Values +} + +func (c *recordingClient) PostForm(u string, params url.Values) (*http.Response, error) { + c.postCount++ + c.lastURL = u + c.lastForm = params + if c.err != nil { + return nil, c.err + } + return &http.Response{ + Body: io.NopCloser(bytes.NewBufferString(c.body)), + Header: http.Header{"Content-Type": {c.contentType}}, + StatusCode: c.status, + }, nil +} + +func TestRefresh(t *testing.T) { + client := &recordingClient{ + status: 200, + contentType: "application/x-www-form-urlencoded", + body: "access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800&refresh_token_expires_in=15897600&token_type=bearer&scope=repo", + } + + token, err := Refresh(client, "https://github.com/login/oauth/access_token", RefreshOptions{ + ClientID: "CLIENTID", + ClientSecret: "CLIENTSECRET", + RefreshToken: "OLDREFRESH", + }) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if token.Token != "NEWTOKEN" { + t.Errorf("Token = %q, want NEWTOKEN", token.Token) + } + if token.RefreshToken != "NEWREFRESH" { + t.Errorf("RefreshToken = %q, want NEWREFRESH", token.RefreshToken) + } + if token.ExpiresIn != 28800 { + t.Errorf("ExpiresIn = %d, want 28800", token.ExpiresIn) + } + if token.ExpiresAt.IsZero() { + t.Error("ExpiresAt was not set") + } + + wantForm := url.Values{ + "client_id": {"CLIENTID"}, + "client_secret": {"CLIENTSECRET"}, + "refresh_token": {"OLDREFRESH"}, + "grant_type": {"refresh_token"}, + } + if client.lastForm.Encode() != wantForm.Encode() { + t.Errorf("form = %v, want %v", client.lastForm, wantForm) + } +} + +func TestRefresh_omitsEmptyClientSecret(t *testing.T) { + // Tokens obtained via device flow are refreshed without a client secret. + client := &recordingClient{ + status: 200, + contentType: "application/x-www-form-urlencoded", + body: "access_token=NEWTOKEN", + } + + if _, err := Refresh(client, "https://example.com/token", RefreshOptions{ + ClientID: "CLIENTID", + RefreshToken: "OLDREFRESH", + }); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if _, ok := client.lastForm["client_secret"]; ok { + t.Error("client_secret was sent despite being empty") + } +} + +func TestRefresh_badRefreshToken(t *testing.T) { + client := &recordingClient{ + status: 400, + contentType: "application/x-www-form-urlencoded", + body: "error=bad_refresh_token&error_description=The+refresh+token+passed+is+incorrect+or+expired.", + } + + _, err := Refresh(client, "https://example.com/token", RefreshOptions{ + ClientID: "CLIENTID", + RefreshToken: "EXPIRED", + }) + if !errors.Is(err, ErrRefreshTokenInvalid) { + t.Fatalf("error = %v, want ErrRefreshTokenInvalid", err) + } +} + +func TestRefresh_otherAPIError(t *testing.T) { + client := &recordingClient{ + status: 400, + contentType: "application/x-www-form-urlencoded", + body: "error=incorrect_client_credentials", + } + + _, err := Refresh(client, "https://example.com/token", RefreshOptions{ + ClientID: "CLIENTID", + RefreshToken: "AREFRESH", + }) + if errors.Is(err, ErrRefreshTokenInvalid) { + t.Fatal("unrelated API error was reported as an invalid refresh token") + } + var apiError *Error + if !errors.As(err, &apiError) || apiError.Code != "incorrect_client_credentials" { + t.Fatalf("error = %v, want incorrect_client_credentials", err) + } +} + +func TestRefresh_withoutRefreshToken(t *testing.T) { + client := &recordingClient{status: 200} + + _, err := Refresh(client, "https://example.com/token", RefreshOptions{ClientID: "CLIENTID"}) + if !errors.Is(err, ErrRefreshTokenInvalid) { + t.Fatalf("error = %v, want ErrRefreshTokenInvalid", err) + } + if client.postCount != 0 { + t.Errorf("made %d requests, want 0", client.postCount) + } +} + +func TestRefresh_transportError(t *testing.T) { + wantErr := errors.New("network is unreachable") + client := &recordingClient{err: wantErr} + + _, err := Refresh(client, "https://example.com/token", RefreshOptions{ + ClientID: "CLIENTID", + RefreshToken: "AREFRESH", + }) + if !errors.Is(err, wantErr) { + t.Fatalf("error = %v, want %v", err, wantErr) + } +} diff --git a/api/scopes.go b/api/scopes.go new file mode 100644 index 0000000..63f30f2 --- /dev/null +++ b/api/scopes.go @@ -0,0 +1,23 @@ +package api + +// ScopeOfflineAccess is the scope that opts an individual authorization into receiving an expiring +// access token and a refresh token, even when the OAuth app is not globally configured to use +// expiring tokens. +// +// Servers that do not support expiring tokens, such as older GitHub Enterprise Server instances, +// ignore this scope and issue a non-expiring token with no refresh token. It is not tracked as a +// normal scope and does not affect the scopes granted to the resulting token. +const ScopeOfflineAccess = "offline_access" + +// AppendOfflineAccess returns scopes with ScopeOfflineAccess appended, unless it is already present. +// The input slice is never modified. +func AppendOfflineAccess(scopes []string) []string { + for _, s := range scopes { + if s == ScopeOfflineAccess { + return scopes + } + } + result := make([]string, len(scopes), len(scopes)+1) + copy(result, scopes) + return append(result, ScopeOfflineAccess) +} diff --git a/device/device_flow.go b/device/device_flow.go index 5a5722b..3f67459 100644 --- a/device/device_flow.go +++ b/device/device_flow.go @@ -66,6 +66,15 @@ func WithAudience(audience string) AuthRequestEditorFn { } } +// WithRefreshToken requests an expiring access token and a refresh token. Servers that do not +// support expiring tokens ignore this and issue a non-expiring token with no refresh token. +func WithRefreshToken() AuthRequestEditorFn { + return func(values *url.Values) { + scopes := strings.Fields(values.Get("scope")) + values.Set("scope", strings.Join(api.AppendOfflineAccess(scopes), " ")) + } +} + // RequestCode initiates the authorization flow by requesting a code from uri. func RequestCode(c httpClient, uri string, clientID string, scopes []string, optionalRequestParams ...AuthRequestEditorFn) (*CodeResponse, error) { diff --git a/device/examples_test.go b/device/examples_test.go index 3e2dcb7..89b7d15 100644 --- a/device/examples_test.go +++ b/device/examples_test.go @@ -6,6 +6,7 @@ import ( "net/http" "os" + "github.com/cli/oauth/api" "github.com/cli/oauth/device" ) @@ -35,3 +36,47 @@ func ExampleRequestCode() { fmt.Printf("Access token: %s\n", accessToken.Token) } + +// Request an expiring access token and a refresh token. +// Servers that do not support expiring tokens ignore the request and return a non-expiring token instead. +func ExampleWithRefreshToken() { + clientID := os.Getenv("OAUTH_CLIENT_ID") + scopes := []string{"repo", "read:org"} + httpClient := http.DefaultClient + + code, err := device.RequestCode(httpClient, "https://github.com/login/device/code", + clientID, scopes, device.WithRefreshToken()) + if err != nil { + panic(err) + } + + fmt.Printf("Copy code: %s\n", code.UserCode) + fmt.Printf("then open: %s\n", code.VerificationURI) + + accessToken, err := device.Wait(context.TODO(), httpClient, "https://github.com/login/oauth/access_token", device.WaitOptions{ + ClientID: clientID, + DeviceCode: code, + }) + if err != nil { + panic(err) + } + + if accessToken.RefreshToken == "" { + // The server does not support expiring tokens; the access token does not expire. + fmt.Println("no refresh token issued") + return + } + + // Store the refresh token and both expiration times alongside the access token. Refreshing a + // token invalidates the previous access token and refresh token, so the replacements returned by + // api.Refresh must be persisted. The client secret is not needed for device flow tokens. + newToken, err := api.Refresh(httpClient, "https://github.com/login/oauth/access_token", api.RefreshOptions{ + ClientID: clientID, + RefreshToken: accessToken.RefreshToken, + }) + if err != nil { + panic(err) + } + + fmt.Printf("Access token expires in %d seconds\n", newToken.ExpiresIn) +} diff --git a/device/offline_access_test.go b/device/offline_access_test.go new file mode 100644 index 0000000..06eb24d --- /dev/null +++ b/device/offline_access_test.go @@ -0,0 +1,49 @@ +package device + +import ( + "net/url" + "testing" +) + +func TestWithRefreshToken(t *testing.T) { + tests := []struct { + name string + scope string + want string + }{ + {name: "adds to existing scopes", scope: "repo read:org", want: "repo read:org offline_access"}, + {name: "adds to empty scope", scope: "", want: "offline_access"}, + {name: "does not duplicate", scope: "repo offline_access", want: "repo offline_access"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + values := url.Values{"scope": {tt.scope}} + WithRefreshToken()(&values) + if got := values.Get("scope"); got != tt.want { + t.Errorf("scope = %q, want %q", got, tt.want) + } + }) + } +} + +func TestRequestCode_withRefreshToken(t *testing.T) { + client := &apiClient{ + stubs: []apiStub{ + { + status: 200, + contentType: "application/x-www-form-urlencoded", + body: "verification_uri=http://verify.me&interval=5&expires_in=99&device_code=DEVIC&user_code=123-abc", + }, + }, + } + + if _, err := RequestCode(client, "https://example.com/device/code", "CLIENTID", + []string{"repo"}, WithRefreshToken()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + if got := client.calls[0].params.Get("scope"); got != "repo offline_access" { + t.Errorf("scope = %q, want %q", got, "repo offline_access") + } +} diff --git a/examples_test.go b/examples_test.go index 975c8c9..348da6c 100644 --- a/examples_test.go +++ b/examples_test.go @@ -1,10 +1,12 @@ package oauth_test import ( + "errors" "fmt" "os" "github.com/cli/oauth" + "github.com/cli/oauth/api" ) // DetectFlow attempts to initiate OAuth Device flow with the server and falls back to OAuth Web @@ -31,3 +33,72 @@ func ExampleFlow_DetectFlow() { fmt.Printf("Access token: %s\n", accessToken.Token) } + +// Opt in to expiring access tokens and use the resulting token with an HTTP client that refreshes it +// automatically. Refresh tokens are single-use, so the OnRefresh callback must persist the new token. +func ExampleFlow_requestRefreshToken() { + host, err := oauth.NewGitHubHost("https://github.com") + if err != nil { + panic(err) + } + clientID := os.Getenv("OAUTH_CLIENT_ID") + clientSecret := os.Getenv("OAUTH_CLIENT_SECRET") + + flow := &oauth.Flow{ + Host: host, + ClientID: clientID, + ClientSecret: clientSecret, + CallbackURI: "http://127.0.0.1/callback", + Scopes: []string{"repo", "read:org"}, + + // Request an expiring access token along with a refresh token. + RequestRefreshToken: true, + } + + accessToken, err := flow.DetectFlow() + if err != nil { + panic(err) + } + + // Servers without support for expiring tokens ignore the request and return a non-expiring token + // with no refresh token, so never assume that one was issued. + if accessToken.RefreshToken == "" { + fmt.Println("received a non-expiring token") + } + + src := oauth.NewTokenSource(accessToken, clientID, clientSecret, host.TokenURL) + + // Refreshing invalidates both the previous access token and the previous refresh token, so the + // new credentials must be stored before they are used. + src.OnRefresh = func(token *api.AccessToken) error { + return saveCredentials(token) + } + + // This client attaches the token, refreshes it when it expires, and retries a rejected request + // once with a freshly refreshed token. + httpClient := oauth.NewHTTPClient(src) + + resp, err := httpClient.Get("https://api.github.com/user") + if err != nil { + if errors.Is(err, api.ErrRefreshTokenInvalid) || errors.Is(err, oauth.ErrNotRefreshable) { + // The token can no longer be renewed; the user has to authorize the app again. + panic("re-authorization required") + } + panic(err) + } + defer func() { + _ = resp.Body.Close() + }() + + fmt.Printf("Status: %d\n", resp.StatusCode) +} + +// saveCredentials stands in for writing the token to the application's credential store. Along with +// the token value itself, the refresh token and both expiration times must be persisted. +func saveCredentials(token *api.AccessToken) error { + _ = token.Token + _ = token.RefreshToken + _ = token.ExpiresAt + _ = token.RefreshTokenExpiresAt + return nil +} diff --git a/oauth.go b/oauth.go index 5b98c38..627d024 100644 --- a/oauth.go +++ b/oauth.go @@ -1,5 +1,13 @@ // Package oauth is a library for Go client applications that need to perform OAuth authorization // against a server, typically GitHub.com. +// +// Flow performs the authorization itself, via either Device flow or Web application flow, and +// returns an access token. By default that token does not expire. +// +// Applications may instead opt in to expiring tokens by setting Flow.RequestRefreshToken, in which +// case the server issues a short-lived access token together with a refresh token. Pass the result +// to a TokenSource and build an http.Client with NewHTTPClient to have the token attached to +// outgoing requests and refreshed automatically as it expires. package oauth import ( @@ -77,6 +85,15 @@ type Flow struct { // The localhost URI for web application flow callback, e.g. "http://127.0.0.1/callback". CallbackURI string + // RequestRefreshToken opts this authorization into receiving an expiring access token along with + // a refresh token, by requesting the "offline_access" scope. Defaults to false, which preserves + // the traditional behavior of receiving a non-expiring token. + // + // Servers that do not support expiring tokens ignore the request and issue a non-expiring token + // with no refresh token, so callers must not assume that a refresh token was returned. Use + // TokenSource to manage refreshing the resulting token. + RequestRefreshToken bool + // Display a one-time code to the user. Receives the code and the browser URL as arguments. Defaults to printing the // code to the user on Stdout with instructions to copy the code and to press Enter to continue in their browser. DisplayCode func(string, string) error diff --git a/oauth_device.go b/oauth_device.go index f993eaa..b852132 100644 --- a/oauth_device.go +++ b/oauth_device.go @@ -39,8 +39,13 @@ func (oa *Flow) DeviceFlow() (*api.AccessToken, error) { host = parsedHost } + requestOptions := []device.AuthRequestEditorFn{device.WithAudience(oa.Audience)} + if oa.RequestRefreshToken { + requestOptions = append(requestOptions, device.WithRefreshToken()) + } + code, err := device.RequestCode(httpClient, host.DeviceCodeURL, - oa.ClientID, oa.Scopes, device.WithAudience(oa.Audience)) + oa.ClientID, oa.Scopes, requestOptions...) if err != nil { return nil, err } diff --git a/oauth_webapp.go b/oauth_webapp.go index 360c53b..a7def78 100644 --- a/oauth_webapp.go +++ b/oauth_webapp.go @@ -35,7 +35,12 @@ func (oa *Flow) WebAppFlow() (*api.AccessToken, error) { Audience: oa.Audience, AllowSignup: true, } - browserURL, err := flow.BrowserURL(host.AuthorizeURL, params) + browserOptions := []webapp.BrowserURLOption{} + if oa.RequestRefreshToken { + browserOptions = append(browserOptions, webapp.WithRefreshToken()) + } + + browserURL, err := flow.BrowserURL(host.AuthorizeURL, params, browserOptions...) if err != nil { return nil, err } diff --git a/token_source.go b/token_source.go new file mode 100644 index 0000000..6e93d82 --- /dev/null +++ b/token_source.go @@ -0,0 +1,155 @@ +package oauth + +import ( + "context" + "errors" + "net/http" + "sync" + + "github.com/cli/oauth/api" +) + +// ErrNotRefreshable is returned when a refresh is attempted for a token that has no usable refresh +// token, either because the server never issued one or because the refresh token itself expired. +// Recovering requires sending the user through an authorization flow again. +var ErrNotRefreshable = errors.New("token cannot be refreshed") + +// TokenSource holds an access token and knows how to refresh it. +// +// It is safe for concurrent use. A single TokenSource should be shared by everything in the +// application that needs the token, so that a refresh performed on behalf of one caller is observed +// by all the others. +type TokenSource struct { + // ClientID is the app client ID value. + ClientID string + // ClientSecret is the app client secret value. Required to refresh tokens obtained via web + // application flow; not needed for tokens obtained via device flow. + ClientSecret string + // TokenURL is the URL to exchange the refresh token at, e.g. Host.TokenURL. + TokenURL string + + // OnRefresh is invoked with the new token whenever a refresh succeeds. Use it to persist the new + // credentials. + // + // Refresh tokens are single-use: once a refresh succeeds, the previous access token and refresh + // token are both dead, and the token passed here is the only usable credential. If OnRefresh + // returns an error, that error is returned to the caller alongside the new token, which is + // retained by the TokenSource regardless so that a failure to persist does not also destroy the + // only working token. + // + // The callback is invoked without the TokenSource's lock held, so it may safely call back into + // this TokenSource, including through an http.Client built from it. + OnRefresh func(*api.AccessToken) error + + // HTTPClient is the client used for the refresh request. Defaults to http.DefaultClient. + HTTPClient httpClient + + mu sync.Mutex + token *api.AccessToken +} + +// NewTokenSource creates a TokenSource for an existing token, which may have been loaded from +// storage or just obtained from a Flow. +func NewTokenSource(token *api.AccessToken, clientID, clientSecret, tokenURL string) *TokenSource { + return &TokenSource{ + ClientID: clientID, + ClientSecret: clientSecret, + TokenURL: tokenURL, + token: token, + } +} + +// SetToken replaces the current token, e.g. after the user has re-authorized the app. +func (ts *TokenSource) SetToken(token *api.AccessToken) { + ts.mu.Lock() + defer ts.mu.Unlock() + ts.token = token +} + +// Token returns a usable access token, refreshing it first if it has expired and can be refreshed. +// +// A non-expiring token is returned as-is. An expired token that cannot be refreshed is also returned +// as-is, along with ErrNotRefreshable, so that callers which prefer to try the token anyway may do so. +func (ts *TokenSource) Token(ctx context.Context) (*api.AccessToken, error) { + ts.mu.Lock() + defer ts.mu.Unlock() + + if ts.token == nil { + return nil, ErrNotRefreshable + } + if !ts.token.IsExpired() { + return ts.token, nil + } + if !ts.token.CanRefresh() { + return ts.token, ErrNotRefreshable + } + return ts.refreshLocked(ctx) +} + +// Refresh unconditionally exchanges the current refresh token for a new token, invokes OnRefresh, +// and returns the new token. +// +// If another goroutine refreshed the token in the meantime, that newer token is returned instead and +// no request is made, so that a single expired token does not cause a stampede of refreshes. +func (ts *TokenSource) Refresh(ctx context.Context) (*api.AccessToken, error) { + ts.mu.Lock() + defer ts.mu.Unlock() + return ts.refreshLocked(ctx) +} + +// refreshStale exchanges the token only if it is still the one the caller saw. It is used by the +// transport so that a burst of concurrent 401s results in exactly one refresh. +func (ts *TokenSource) refreshStale(ctx context.Context, seen *api.AccessToken) (*api.AccessToken, error) { + ts.mu.Lock() + defer ts.mu.Unlock() + + if seen != nil && ts.token != nil && ts.token != seen { + return ts.token, nil + } + return ts.refreshLocked(ctx) +} + +func (ts *TokenSource) refreshLocked(ctx context.Context) (*api.AccessToken, error) { + if ts.token == nil || !ts.token.CanRefresh() { + return nil, ErrNotRefreshable + } + + // The api package does not yet accept a context; respect cancellation at the boundary. + if err := ctx.Err(); err != nil { + return nil, err + } + + client := ts.HTTPClient + if client == nil { + client = http.DefaultClient + } + + newToken, err := api.Refresh(client, ts.TokenURL, api.RefreshOptions{ + ClientID: ts.ClientID, + ClientSecret: ts.ClientSecret, + RefreshToken: ts.token.RefreshToken, + }) + if err != nil { + return nil, err + } + + // The exchange succeeded, so the server has already invalidated the previous token pair and + // newToken is now the only usable credential. Adopt it before doing anything that can fail, so + // that it cannot be lost. + ts.token = newToken + + if ts.OnRefresh != nil { + // Release the lock around the callback: it is caller-supplied code that may legitimately + // re-enter this TokenSource, and sync.Mutex is not reentrant. + ts.mu.Unlock() + err := ts.OnRefresh(newToken) + ts.mu.Lock() + if err != nil { + // Report the failure to persist, but still hand back the live token: discarding it would + // not bring back the old one, it would only strand the user. + return newToken, err + } + } + + return newToken, nil +} diff --git a/token_source_test.go b/token_source_test.go new file mode 100644 index 0000000..ef81835 --- /dev/null +++ b/token_source_test.go @@ -0,0 +1,345 @@ +package oauth + +import ( + "bytes" + "context" + "errors" + "io" + "net/http" + "net/url" + "sync" + "testing" + "time" + + "github.com/cli/oauth/api" +) + +type stubTokenClient struct { + mu sync.Mutex + responses []string + postCount int + lastForm url.Values + err error +} + +func (c *stubTokenClient) PostForm(_ string, params url.Values) (*http.Response, error) { + c.mu.Lock() + defer c.mu.Unlock() + + c.lastForm = params + if c.err != nil { + c.postCount++ + return nil, c.err + } + + body := "error=bad_refresh_token" + if c.postCount < len(c.responses) { + body = c.responses[c.postCount] + } + c.postCount++ + + return &http.Response{ + Body: io.NopCloser(bytes.NewBufferString(body)), + Header: http.Header{"Content-Type": {"application/x-www-form-urlencoded"}}, + StatusCode: 200, + }, nil +} + +func (c *stubTokenClient) count() int { + c.mu.Lock() + defer c.mu.Unlock() + return c.postCount +} + +func expiredToken() *api.AccessToken { + return &api.AccessToken{ + Token: "OLDTOKEN", + RefreshToken: "OLDREFRESH", + Type: "bearer", + ExpiresAt: time.Now().Add(-time.Hour), + } +} + +func TestTokenSource_TokenRefreshesExpiredToken(t *testing.T) { + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + src := &TokenSource{ + ClientID: "CLIENTID", + ClientSecret: "SECRET", + TokenURL: "https://example.com/token", + HTTPClient: client, + token: expiredToken(), + } + + token, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if token.Token != "NEWTOKEN" { + t.Errorf("Token = %q, want NEWTOKEN", token.Token) + } + + // A second call must reuse the fresh token rather than refresh again. + if _, err := src.Token(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } +} + +func TestTokenSource_TokenLeavesValidTokenAlone(t *testing.T) { + client := &stubTokenClient{} + src := &TokenSource{ + HTTPClient: client, + token: &api.AccessToken{Token: "ATOKEN", RefreshToken: "R", ExpiresAt: time.Now().Add(time.Hour)}, + } + + token, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if token.Token != "ATOKEN" { + t.Errorf("Token = %q, want ATOKEN", token.Token) + } + if client.count() != 0 { + t.Errorf("made %d refresh requests, want 0", client.count()) + } +} + +func TestTokenSource_TokenNonExpiringToken(t *testing.T) { + // A server without expiring-token support returns no expiry and no refresh token; that token + // must keep working exactly as before. + client := &stubTokenClient{} + src := &TokenSource{HTTPClient: client, token: &api.AccessToken{Token: "ATOKEN"}} + + token, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if token.Token != "ATOKEN" { + t.Errorf("Token = %q, want ATOKEN", token.Token) + } + if client.count() != 0 { + t.Errorf("made %d refresh requests, want 0", client.count()) + } +} + +func TestTokenSource_TokenExpiredWithoutRefreshToken(t *testing.T) { + src := &TokenSource{ + HTTPClient: &stubTokenClient{}, + token: &api.AccessToken{Token: "ATOKEN", ExpiresAt: time.Now().Add(-time.Hour)}, + } + + token, err := src.Token(context.Background()) + if !errors.Is(err, ErrNotRefreshable) { + t.Fatalf("error = %v, want ErrNotRefreshable", err) + } + if token == nil || token.Token != "ATOKEN" { + t.Error("expired token should still be returned for the caller to try") + } +} + +func TestTokenSource_RefreshInvokesOnRefresh(t *testing.T) { + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + + var persisted *api.AccessToken + src := &TokenSource{ + ClientID: "CLIENTID", + HTTPClient: client, + token: expiredToken(), + OnRefresh: func(tok *api.AccessToken) error { + persisted = tok + return nil + }, + } + + token, err := src.Refresh(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if persisted == nil || persisted.Token != "NEWTOKEN" { + t.Fatal("OnRefresh was not called with the new token") + } + if persisted != token { + t.Error("OnRefresh received a different token than the caller") + } + if persisted.RefreshToken != "NEWREFRESH" { + t.Errorf("RefreshToken = %q, want NEWREFRESH", persisted.RefreshToken) + } +} + +func TestTokenSource_RefreshPropagatesOnRefreshError(t *testing.T) { + // Persisting may fail transiently. The error must reach the caller, but the refreshed token must + // survive, since the server already invalidated the previous one. + wantErr := errors.New("disk full") + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + src := &TokenSource{ + HTTPClient: client, + token: expiredToken(), + OnRefresh: func(*api.AccessToken) error { return wantErr }, + } + + token, err := src.Refresh(context.Background()) + if !errors.Is(err, wantErr) { + t.Fatalf("error = %v, want %v", err, wantErr) + } + if token == nil || token.Token != "NEWTOKEN" { + t.Fatal("the refreshed token was not returned to the caller") + } + + // The source must now hold the new token, not the dead one. + src.OnRefresh = nil + current, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if current.Token != "NEWTOKEN" { + t.Errorf("Token = %q, want NEWTOKEN; the refreshed token was lost", current.Token) + } + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } +} + +func TestTokenSource_OnRefreshMayReenter(t *testing.T) { + // A callback that persists via a client built on the same TokenSource must not deadlock. + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + src := &TokenSource{HTTPClient: client, token: expiredToken()} + + var seen string + src.OnRefresh = func(*api.AccessToken) error { + token, err := src.Token(context.Background()) + if err != nil { + return err + } + seen = token.Token + return nil + } + + done := make(chan struct{}) + go func() { + defer close(done) + if _, err := src.Refresh(context.Background()); err != nil { + t.Errorf("unexpected error: %v", err) + } + }() + + select { + case <-done: + case <-time.After(5 * time.Second): + t.Fatal("OnRefresh deadlocked against the TokenSource lock") + } + + if seen != "NEWTOKEN" { + t.Errorf("re-entrant Token() saw %q, want NEWTOKEN", seen) + } +} + +func TestTokenSource_RefreshWithoutNewRefreshToken(t *testing.T) { + // The consumed refresh token must not be carried forward: it is dead by definition, and keeping + // it would make the token claim to be refreshable when it is not. + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&expires_in=28800"}} + src := &TokenSource{HTTPClient: client, token: expiredToken()} + + token, err := src.Refresh(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if token.RefreshToken != "" { + t.Errorf("RefreshToken = %q, want empty", token.RefreshToken) + } + if token.CanRefresh() { + t.Error("token reports itself refreshable with a spent refresh token") + } +} + +func TestTokenSource_RefreshNotRefreshable(t *testing.T) { + client := &stubTokenClient{} + src := &TokenSource{HTTPClient: client, token: &api.AccessToken{Token: "ATOKEN"}} + + if _, err := src.Refresh(context.Background()); !errors.Is(err, ErrNotRefreshable) { + t.Fatalf("error = %v, want ErrNotRefreshable", err) + } + if client.count() != 0 { + t.Errorf("made %d requests, want 0", client.count()) + } +} + +func TestTokenSource_RefreshBadRefreshToken(t *testing.T) { + client := &stubTokenClient{responses: []string{"error=bad_refresh_token"}} + src := &TokenSource{HTTPClient: client, token: expiredToken()} + + if _, err := src.Refresh(context.Background()); !errors.Is(err, api.ErrRefreshTokenInvalid) { + t.Fatalf("error = %v, want ErrRefreshTokenInvalid", err) + } +} + +func TestTokenSource_ConcurrentTokenCalls(t *testing.T) { + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + src := &TokenSource{HTTPClient: client, token: expiredToken()} + + var wg sync.WaitGroup + results := make([]string, 20) + for i := 0; i < 20; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + token, err := src.Token(context.Background()) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + results[i] = token.Token + }(i) + } + wg.Wait() + + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } + for i, got := range results { + if got != "NEWTOKEN" { + t.Errorf("result %d = %q, want NEWTOKEN", i, got) + } + } +} + +func TestTokenSource_SetToken(t *testing.T) { + src := NewTokenSource(&api.AccessToken{Token: "OLD"}, "CLIENTID", "SECRET", "https://example.com/token") + src.SetToken(&api.AccessToken{Token: "NEW"}) + + token, err := src.Token(context.Background()) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if token.Token != "NEW" { + t.Errorf("Token = %q, want NEW", token.Token) + } +} + +func TestTokenSource_RefreshOmitsEmptyClientSecret(t *testing.T) { + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH"}} + src := &TokenSource{ClientID: "CLIENTID", HTTPClient: client, token: expiredToken()} + + if _, err := src.Refresh(context.Background()); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if _, ok := client.lastForm["client_secret"]; ok { + t.Error("client_secret was sent for a device flow token") + } +} + +func TestTokenSource_RefreshRespectsCanceledContext(t *testing.T) { + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN"}} + src := &TokenSource{HTTPClient: client, token: expiredToken()} + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + if _, err := src.Refresh(ctx); !errors.Is(err, context.Canceled) { + t.Fatalf("error = %v, want context.Canceled", err) + } + if client.count() != 0 { + t.Errorf("made %d requests, want 0", client.count()) + } +} diff --git a/transport.go b/transport.go new file mode 100644 index 0000000..fa35660 --- /dev/null +++ b/transport.go @@ -0,0 +1,135 @@ +package oauth + +import ( + "io" + "net/http" + "strings" + + "github.com/cli/oauth/api" +) + +// Transport is an http.RoundTripper that authenticates requests with a token from a TokenSource and +// transparently recovers from an expired access token. +// +// Before each request it attaches the current token, refreshing it first if it is known to have +// expired. If the server rejects the request anyway, the token is refreshed and the request is +// retried exactly once; a second rejection is returned to the caller unmodified. Requests are +// therefore never attempted more than twice, so a persistently rejected token cannot produce a +// refresh loop. +type Transport struct { + // Source supplies the access token. Required. + Source *TokenSource + // Base is the underlying transport. Defaults to http.DefaultTransport. + Base http.RoundTripper +} + +// NewHTTPClient returns an http.Client that authenticates requests with tokens from src and refreshes +// them as needed. +func NewHTTPClient(src *TokenSource) *http.Client { + return &http.Client{Transport: &Transport{Source: src}} +} + +// RoundTrip implements http.RoundTripper. +func (t *Transport) RoundTrip(req *http.Request) (*http.Response, error) { + base := t.Base + if base == nil { + base = http.DefaultTransport + } + + ctx := req.Context() + + // An ErrNotRefreshable here means the token is expired but cannot be renewed. Send it anyway: + // the server is the authority on whether it is still accepted, and failing here would turn a + // working request into an error whenever expiry metadata is wrong. + token, err := t.Source.Token(ctx) + if err != nil && token == nil { + return nil, err + } + + resp, err := base.RoundTrip(setAuth(cloneRequest(req), token)) + if err != nil { + return resp, err + } + + if !isTokenRejected(resp) || !token.CanRefresh() { + return resp, nil + } + + // The request must be replayable to retry it. If the body cannot be rewound, return the original + // response rather than sending a request with a consumed body. + retryReq, ok := rewind(req) + if !ok { + return resp, nil + } + + newToken, err := t.Source.refreshStale(ctx, token) + if err != nil { + // Refreshing failed; the caller gets the original rejection, which is the more actionable + // error, and can inspect the token source themselves. + return resp, nil + } + + // This is the one and only retry: its response is returned regardless of status. + drain(resp) + return base.RoundTrip(setAuth(retryReq, newToken)) +} + +func isTokenRejected(resp *http.Response) bool { + if resp.StatusCode == http.StatusUnauthorized { + return true + } + // GitHub answers an expired token with 401, but some endpoints answer 403. Only treat a 403 as a + // token problem when the server says so, to avoid refreshing on ordinary permission errors. + if resp.StatusCode == http.StatusForbidden { + return strings.Contains(strings.ToLower(resp.Header.Get("WWW-Authenticate")), "expired") + } + return false +} + +func setAuth(req *http.Request, token *api.AccessToken) *http.Request { + if token == nil { + return req + } + tokenType := token.Type + if tokenType == "" { + tokenType = "Bearer" + } + req.Header.Set("Authorization", tokenType+" "+token.Token) + return req +} + +func cloneRequest(req *http.Request) *http.Request { + r := req.Clone(req.Context()) + r.Header = req.Header.Clone() + if r.Header == nil { + r.Header = make(http.Header) + } + return r +} + +// rewind returns a copy of req with a fresh body, reporting whether it can be safely replayed. +func rewind(req *http.Request) (*http.Request, bool) { + r := cloneRequest(req) + if req.Body == nil || req.Body == http.NoBody { + return r, true + } + if req.GetBody == nil { + return nil, false + } + body, err := req.GetBody() + if err != nil { + return nil, false + } + r.Body = body + return r, true +} + +// drain discards and closes a response body that is being replaced by a retry, so the underlying +// connection can be reused. +func drain(resp *http.Response) { + if resp == nil || resp.Body == nil { + return + } + _, _ = io.Copy(io.Discard, io.LimitReader(resp.Body, 64*1024)) + _ = resp.Body.Close() +} diff --git a/transport_test.go b/transport_test.go new file mode 100644 index 0000000..9604717 --- /dev/null +++ b/transport_test.go @@ -0,0 +1,381 @@ +package oauth + +import ( + "bytes" + "io" + "net/http" + "strings" + "sync" + "testing" + "time" + + "github.com/cli/oauth/api" +) + +type stubTransport struct { + mu sync.Mutex + // statuses are returned in order; the last one repeats. + statuses []int + headers []http.Header + + authHeaders []string + bodies []string +} + +func (t *stubTransport) RoundTrip(req *http.Request) (*http.Response, error) { + t.mu.Lock() + defer t.mu.Unlock() + + i := len(t.authHeaders) + t.authHeaders = append(t.authHeaders, req.Header.Get("Authorization")) + + body := "" + if req.Body != nil { + b, err := io.ReadAll(req.Body) + if err != nil { + return nil, err + } + body = string(b) + } + t.bodies = append(t.bodies, body) + + status := t.statuses[len(t.statuses)-1] + if i < len(t.statuses) { + status = t.statuses[i] + } + + header := http.Header{} + if i < len(t.headers) && t.headers[i] != nil { + header = t.headers[i] + } + + return &http.Response{ + StatusCode: status, + Header: header, + Body: io.NopCloser(strings.NewReader("response body")), + }, nil +} + +func (t *stubTransport) calls() []string { + t.mu.Lock() + defer t.mu.Unlock() + return append([]string(nil), t.authHeaders...) +} + +func newTestTransport(base http.RoundTripper, client *stubTokenClient, token *api.AccessToken) *Transport { + return &Transport{ + Source: &TokenSource{ + ClientID: "CLIENTID", + TokenURL: "https://example.com/token", + HTTPClient: client, + token: token, + }, + Base: base, + } +} + +func validToken() *api.AccessToken { + return &api.AccessToken{ + Token: "OLDTOKEN", + RefreshToken: "OLDREFRESH", + Type: "bearer", + ExpiresAt: time.Now().Add(time.Hour), + } +} + +func TestTransport_AttachesToken(t *testing.T) { + base := &stubTransport{statuses: []int{200}} + tr := newTestTransport(base, &stubTokenClient{}, validToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 200 { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + + calls := base.calls() + if len(calls) != 1 { + t.Fatalf("made %d requests, want 1", len(calls)) + } + if calls[0] != "bearer OLDTOKEN" { + t.Errorf("Authorization = %q, want %q", calls[0], "bearer OLDTOKEN") + } + if got := req.Header.Get("Authorization"); got != "" { + t.Errorf("caller's request was mutated: Authorization = %q", got) + } +} + +func TestTransport_RefreshesOnceAndRetries(t *testing.T) { + base := &stubTransport{statuses: []int{401, 200}} + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + tr := newTestTransport(base, client, validToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 200 { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + + calls := base.calls() + if len(calls) != 2 { + t.Fatalf("made %d requests, want 2", len(calls)) + } + if calls[0] != "bearer OLDTOKEN" { + t.Errorf("first Authorization = %q, want bearer OLDTOKEN", calls[0]) + } + if calls[1] != "Bearer NEWTOKEN" { + t.Errorf("retry Authorization = %q, want Bearer NEWTOKEN", calls[1]) + } + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } +} + +func TestTransport_DoesNotLoopWhenRetryAlsoFails(t *testing.T) { + // The whole point of refresh-once: a persistently rejected token must not spin. + base := &stubTransport{statuses: []int{401}} + client := &stubTokenClient{responses: []string{ + "access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800", + "access_token=NEWERTOKEN&refresh_token=NEWERREFRESH&expires_in=28800", + }} + tr := newTestTransport(base, client, validToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 401 { + t.Errorf("status = %d, want 401", resp.StatusCode) + } + if got := len(base.calls()); got != 2 { + t.Errorf("made %d requests, want exactly 2", got) + } + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } +} + +func TestTransport_NoRefreshWithoutRefreshToken(t *testing.T) { + // A non-expiring token from a server without refresh support: a 401 is just a 401. + base := &stubTransport{statuses: []int{401}} + client := &stubTokenClient{} + tr := newTestTransport(base, client, &api.AccessToken{Token: "ATOKEN"}) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 401 { + t.Errorf("status = %d, want 401", resp.StatusCode) + } + if got := len(base.calls()); got != 1 { + t.Errorf("made %d requests, want 1", got) + } + if client.count() != 0 { + t.Errorf("made %d refresh requests, want 0", client.count()) + } +} + +func TestTransport_ReturnsOriginalResponseWhenRefreshFails(t *testing.T) { + base := &stubTransport{statuses: []int{401}} + client := &stubTokenClient{responses: []string{"error=bad_refresh_token"}} + tr := newTestTransport(base, client, validToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 401 { + t.Errorf("status = %d, want 401", resp.StatusCode) + } + if got := len(base.calls()); got != 1 { + t.Errorf("made %d requests, want 1", got) + } +} + +func TestTransport_RetriesWithReplayedBody(t *testing.T) { + base := &stubTransport{statuses: []int{401, 200}} + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + tr := newTestTransport(base, client, validToken()) + + req, _ := http.NewRequest("POST", "https://api.github.com/user/repos", bytes.NewBufferString(`{"name":"x"}`)) + if _, err := tr.RoundTrip(req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + base.mu.Lock() + defer base.mu.Unlock() + if len(base.bodies) != 2 { + t.Fatalf("made %d requests, want 2", len(base.bodies)) + } + if base.bodies[0] != `{"name":"x"}` || base.bodies[1] != `{"name":"x"}` { + t.Errorf("bodies = %q, want both to be the request body", base.bodies) + } +} + +func TestTransport_DoesNotRetryUnreplayableBody(t *testing.T) { + base := &stubTransport{statuses: []int{401, 200}} + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH"}} + tr := newTestTransport(base, client, validToken()) + + // A request built with an opaque reader has no GetBody, so it cannot be safely replayed. + req, _ := http.NewRequest("POST", "https://api.github.com/user/repos", io.NopCloser(strings.NewReader("data"))) + req.GetBody = nil + + resp, err := tr.RoundTrip(req) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if resp.StatusCode != 401 { + t.Errorf("status = %d, want 401", resp.StatusCode) + } + if got := len(base.calls()); got != 1 { + t.Errorf("made %d requests, want 1", got) + } +} + +func TestTransport_RefreshesProactivelyForExpiredToken(t *testing.T) { + base := &stubTransport{statuses: []int{200}} + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + tr := newTestTransport(base, client, expiredToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + if _, err := tr.RoundTrip(req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + calls := base.calls() + if len(calls) != 1 { + t.Fatalf("made %d requests, want 1", len(calls)) + } + if calls[0] != "Bearer NEWTOKEN" { + t.Errorf("Authorization = %q, want Bearer NEWTOKEN", calls[0]) + } +} + +func TestTransport_Forbidden(t *testing.T) { + tests := []struct { + name string + header http.Header + wantCalls int + wantRefresh int + }{ + { + name: "expired token indicated", + header: http.Header{"Www-Authenticate": {`Bearer error="invalid_token", error_description="token expired"`}}, + wantCalls: 2, + wantRefresh: 1, + }, + { + name: "ordinary permission error", + header: http.Header{}, + wantCalls: 1, + wantRefresh: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + base := &stubTransport{statuses: []int{403, 200}, headers: []http.Header{tt.header}} + client := &stubTokenClient{responses: []string{"access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800"}} + tr := newTestTransport(base, client, validToken()) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + if _, err := tr.RoundTrip(req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := len(base.calls()); got != tt.wantCalls { + t.Errorf("made %d requests, want %d", got, tt.wantCalls) + } + if client.count() != tt.wantRefresh { + t.Errorf("made %d refresh requests, want %d", client.count(), tt.wantRefresh) + } + }) + } +} + +func TestTransport_DefaultsTokenTypeToBearer(t *testing.T) { + base := &stubTransport{statuses: []int{200}} + tr := newTestTransport(base, &stubTokenClient{}, &api.AccessToken{Token: "ATOKEN"}) + + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + if _, err := tr.RoundTrip(req); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := base.calls()[0]; got != "Bearer ATOKEN" { + t.Errorf("Authorization = %q, want %q", got, "Bearer ATOKEN") + } +} + +func TestTransport_ConcurrentRequestsRefreshOnce(t *testing.T) { + // Several in-flight requests all rejected for the same stale token must collapse into a single + // refresh, rather than each performing its own and invalidating the others'. + base := &rejectStaleTransport{staleToken: "OLDTOKEN"} + client := &stubTokenClient{responses: []string{ + "access_token=NEWTOKEN&refresh_token=NEWREFRESH&expires_in=28800", + "access_token=NEWERTOKEN&refresh_token=NEWERREFRESH&expires_in=28800", + }} + tr := newTestTransport(base, client, validToken()) + + var wg sync.WaitGroup + for i := 0; i < 5; i++ { + wg.Add(1) + go func() { + defer wg.Done() + req, _ := http.NewRequest("GET", "https://api.github.com/user", nil) + resp, err := tr.RoundTrip(req) + if err != nil { + t.Errorf("unexpected error: %v", err) + return + } + if resp.StatusCode != 200 { + t.Errorf("status = %d, want 200", resp.StatusCode) + } + }() + } + wg.Wait() + + if client.count() != 1 { + t.Errorf("made %d refresh requests, want 1", client.count()) + } +} + +// rejectStaleTransport answers 401 for one specific token and 200 for anything else, modeling a +// server that has expired a token but accepts its replacement. +type rejectStaleTransport struct { + staleToken string +} + +func (t *rejectStaleTransport) RoundTrip(req *http.Request) (*http.Response, error) { + status := 200 + if strings.HasSuffix(req.Header.Get("Authorization"), " "+t.staleToken) { + status = 401 + } + return &http.Response{ + StatusCode: status, + Header: http.Header{}, + Body: io.NopCloser(strings.NewReader("response body")), + }, nil +} + +func TestNewHTTPClient(t *testing.T) { + src := NewTokenSource(&api.AccessToken{Token: "ATOKEN"}, "CLIENTID", "SECRET", "https://example.com/token") + client := NewHTTPClient(src) + + tr, ok := client.Transport.(*Transport) + if !ok { + t.Fatalf("Transport = %T, want *Transport", client.Transport) + } + if tr.Source != src { + t.Error("Transport was not wired to the given TokenSource") + } +} diff --git a/webapp/examples_test.go b/webapp/examples_test.go index 547bdec..5f6c9ea 100644 --- a/webapp/examples_test.go +++ b/webapp/examples_test.go @@ -27,7 +27,9 @@ func ExampleInitFlow() { Scopes: []string{"repo", "read:org"}, AllowSignup: true, } - browserURL, err := flow.BrowserURL("https://github.com/login/oauth/authorize", params) + // WithRefreshToken requests an expiring access token and a refresh token. Servers without + // support for expiring tokens ignore it and return a non-expiring token with no refresh token. + browserURL, err := flow.BrowserURL("https://github.com/login/oauth/authorize", params, webapp.WithRefreshToken()) if err != nil { panic(err) } @@ -52,4 +54,10 @@ func ExampleInitFlow() { } fmt.Printf("Access token: %s\n", accessToken.Token) + + if accessToken.RefreshToken != "" { + // Persist the refresh token and both expiration times along with the access token; they are + // required to renew it once it expires. + fmt.Printf("Expires in %d seconds\n", accessToken.ExpiresIn) + } } diff --git a/webapp/offline_access_test.go b/webapp/offline_access_test.go new file mode 100644 index 0000000..97e2077 --- /dev/null +++ b/webapp/offline_access_test.go @@ -0,0 +1,88 @@ +package webapp + +import ( + "net/url" + "testing" +) + +func TestFlow_BrowserURL_withRefreshToken(t *testing.T) { + tests := []struct { + name string + scopes []string + requestRefreshToken bool + wantScope string + }{ + { + name: "not requested", + scopes: []string{"repo", "read:org"}, + wantScope: "repo read:org", + }, + { + name: "requested", + scopes: []string{"repo", "read:org"}, + requestRefreshToken: true, + wantScope: "repo read:org offline_access", + }, + { + name: "already present", + scopes: []string{"repo", "offline_access"}, + requestRefreshToken: true, + wantScope: "repo offline_access", + }, + { + name: "no other scopes", + requestRefreshToken: true, + wantScope: "offline_access", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + flow, err := InitFlow() + if err != nil { + t.Fatalf("InitFlow: %v", err) + } + + options := []BrowserURLOption{} + if tt.requestRefreshToken { + options = append(options, WithRefreshToken()) + } + browserURL, err := flow.BrowserURL("https://github.com/login/oauth/authorize", BrowserParams{ + ClientID: "CLIENTID", + RedirectURI: "http://127.0.0.1/callback", + Scopes: tt.scopes, + }, options...) + if err != nil { + t.Fatalf("BrowserURL: %v", err) + } + + u, err := url.Parse(browserURL) + if err != nil { + t.Fatalf("parsing %q: %v", browserURL, err) + } + if got := u.Query().Get("scope"); got != tt.wantScope { + t.Errorf("scope = %q, want %q", got, tt.wantScope) + } + }) + } +} + +func TestFlow_BrowserURL_doesNotMutateCallerScopes(t *testing.T) { + flow, err := InitFlow() + if err != nil { + t.Fatalf("InitFlow: %v", err) + } + + scopes := []string{"repo"} + if _, err := flow.BrowserURL("https://github.com/login/oauth/authorize", BrowserParams{ + ClientID: "CLIENTID", + RedirectURI: "http://127.0.0.1/callback", + Scopes: scopes, + }, WithRefreshToken()); err != nil { + t.Fatalf("BrowserURL: %v", err) + } + + if len(scopes) != 1 || scopes[0] != "repo" { + t.Errorf("caller's scopes were modified: %v", scopes) + } +} diff --git a/webapp/webapp_flow.go b/webapp/webapp_flow.go index a6c96ae..b767218 100644 --- a/webapp/webapp_flow.go +++ b/webapp/webapp_flow.go @@ -52,14 +52,34 @@ type BrowserParams struct { AllowSignup bool } +// BrowserURLOption configures an authorization request. +type BrowserURLOption func(*browserURLOptions) + +type browserURLOptions struct { + requestRefreshToken bool +} + +// WithRefreshToken requests an expiring access token and a refresh token. Servers that do not +// support expiring tokens ignore this and issue a non-expiring token with no refresh token. +func WithRefreshToken() BrowserURLOption { + return func(options *browserURLOptions) { + options.requestRefreshToken = true + } +} + // BrowserURL appends GET query parameters to baseURL and returns the url that the user should // navigate to in their web browser. -func (flow *Flow) BrowserURL(baseURL string, params BrowserParams) (string, error) { +func (flow *Flow) BrowserURL(baseURL string, params BrowserParams, options ...BrowserURLOption) (string, error) { ru, err := url.Parse(params.RedirectURI) if err != nil { return "", err } + requestOptions := browserURLOptions{} + for _, option := range options { + option(&requestOptions) + } + ru.Host = fmt.Sprintf("%s:%d", ru.Hostname(), flow.server.Port()) flow.server.CallbackPath = ru.Path flow.clientID = params.ClientID @@ -67,7 +87,11 @@ func (flow *Flow) BrowserURL(baseURL string, params BrowserParams) (string, erro q := url.Values{} q.Set("client_id", params.ClientID) q.Set("redirect_uri", ru.String()) - q.Set("scope", strings.Join(params.Scopes, " ")) + scopes := params.Scopes + if requestOptions.requestRefreshToken { + scopes = api.AppendOfflineAccess(scopes) + } + q.Set("scope", strings.Join(scopes, " ")) q.Set("state", flow.state) if params.Audience != "" {