Unify refresh token opt-in

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: hpsin

CHANGELOG.md

@@ -10,8 +10,8 @@ compatible** — existing applications continue to receive non-expiring tokens w
 
 - `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.WithOfflineAccess()` and `webapp.BrowserParams.RequestRefreshToken` for callers using
-  those packages directly.
+  `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

device/device_flow.go

@@ -66,10 +66,9 @@ func WithAudience(audience string) AuthRequestEditorFn {
 	}
 }
 
-// WithOfflineAccess requests the "offline_access" scope, opting this authorization into receiving an
-// expiring access token and a refresh token. Servers that do not support expiring tokens ignore it
-// and issue a non-expiring token with no refresh token.
-func WithOfflineAccess() 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), " "))

device/examples_test.go

@@ -37,15 +37,15 @@ func ExampleRequestCode() {
 	fmt.Printf("Access token: %s\n", accessToken.Token)
 }
 
-// Request an expiring access token and a refresh token by adding the "offline_access" scope.
-// Servers that do not support expiring tokens ignore it and return a non-expiring token instead.
-func ExampleWithOfflineAccess() {
+// 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.WithOfflineAccess())
+		clientID, scopes, device.WithRefreshToken())
 	if err != nil {
 		panic(err)
 	}

device/offline_access_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 )
 
-func TestWithOfflineAccess(t *testing.T) {
+func TestWithRefreshToken(t *testing.T) {
 	tests := []struct {
 		name  string
 		scope string
@@ -19,15 +19,15 @@ func TestWithOfflineAccess(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			values := url.Values{"scope": {tt.scope}}
-			WithOfflineAccess()(&values)
+			WithRefreshToken()(&values)
 			if got := values.Get("scope"); got != tt.want {
 				t.Errorf("scope = %q, want %q", got, tt.want)
 			}
 		})
 	}
 }
 
-func TestRequestCode_withOfflineAccess(t *testing.T) {
+func TestRequestCode_withRefreshToken(t *testing.T) {
 	client := &apiClient{
 		stubs: []apiStub{
 			{
@@ -39,7 +39,7 @@ func TestRequestCode_withOfflineAccess(t *testing.T) {
 	}
 
 	if _, err := RequestCode(client, "https://example.com/device/code", "CLIENTID",
-		[]string{"repo"}, WithOfflineAccess()); err != nil {
+		[]string{"repo"}, WithRefreshToken()); err != nil {
 		t.Fatalf("unexpected error: %v", err)
 	}
 

oauth_device.go

@@ -39,13 +39,13 @@ func (oa *Flow) DeviceFlow() (*api.AccessToken, error) {
 		host = parsedHost
 	}
 
-	scopes := oa.Scopes
+	requestOptions := []device.AuthRequestEditorFn{device.WithAudience(oa.Audience)}
 	if oa.RequestRefreshToken {
-		scopes = api.AppendOfflineAccess(scopes)
+		requestOptions = append(requestOptions, device.WithRefreshToken())
 	}
 
 	code, err := device.RequestCode(httpClient, host.DeviceCodeURL,
-		oa.ClientID, scopes, device.WithAudience(oa.Audience))
+		oa.ClientID, oa.Scopes, requestOptions...)
 	if err != nil {
 		return nil, err
 	}

oauth_webapp.go

@@ -34,10 +34,13 @@ func (oa *Flow) WebAppFlow() (*api.AccessToken, error) {
 		Scopes:      oa.Scopes,
 		Audience:    oa.Audience,
 		AllowSignup: true,
-
-		RequestRefreshToken: oa.RequestRefreshToken,
 	}
-	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
 	}

webapp/examples_test.go

@@ -26,12 +26,10 @@ func ExampleInitFlow() {
 		RedirectURI: callbackURL,
 		Scopes:      []string{"repo", "read:org"},
 		AllowSignup: true,
-
-		// Opt in to receiving an expiring access token and a refresh token. Servers without support
-		// for expiring tokens ignore this and return a non-expiring token with no refresh token.
-		RequestRefreshToken: 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)
 	}

webapp/offline_access_test.go

@@ -5,7 +5,7 @@ import (
 	"testing"
 )
 
-func TestFlow_BrowserURL_offlineAccess(t *testing.T) {
+func TestFlow_BrowserURL_withRefreshToken(t *testing.T) {
 	tests := []struct {
 		name                string
 		scopes              []string
@@ -43,12 +43,15 @@ func TestFlow_BrowserURL_offlineAccess(t *testing.T) {
 				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,
-				RequestRefreshToken: tt.requestRefreshToken,
-			})
+				ClientID:    "CLIENTID",
+				RedirectURI: "http://127.0.0.1/callback",
+				Scopes:      tt.scopes,
+			}, options...)
 			if err != nil {
 				t.Fatalf("BrowserURL: %v", err)
 			}
@@ -72,11 +75,10 @@ func TestFlow_BrowserURL_doesNotMutateCallerScopes(t *testing.T) {
 
 	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,
-		RequestRefreshToken: true,
-	}); err != nil {
+		ClientID:    "CLIENTID",
+		RedirectURI: "http://127.0.0.1/callback",
+		Scopes:      scopes,
+	}, WithRefreshToken()); err != nil {
 		t.Fatalf("BrowserURL: %v", err)
 	}
 

webapp/webapp_flow.go

@@ -50,32 +50,47 @@ type BrowserParams struct {
 	Audience    string
 	LoginHandle string
 	AllowSignup bool
-	// RequestRefreshToken opts this authorization into receiving an expiring access token along with
-	// a refresh token, by requesting the "offline_access" scope. Servers that do not support
-	// expiring tokens ignore it and issue a non-expiring token with no refresh token.
-	RequestRefreshToken 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
 
-	scopes := params.Scopes
-	if params.RequestRefreshToken {
-		scopes = api.AppendOfflineAccess(scopes)
-	}
-
 	q := url.Values{}
 	q.Set("client_id", params.ClientID)
 	q.Set("redirect_uri", ru.String())
+	scopes := params.Scopes
+	if requestOptions.requestRefreshToken {
+		scopes = api.AppendOfflineAccess(scopes)
+	}
 	q.Set("scope", strings.Join(scopes, " "))
 	q.Set("state", flow.state)