fix: automatically set prompt to consent for oauth providers

forge-go/conf/oauth-providers.yaml

@@ -13,6 +13,39 @@
 # use_pkce: controls PKCE (S256 code challenge). Defaults to true. Set to
 # false for providers that do not support it.
 #
+# prompt: consent is sent on every provider's authorization request by default.
+# Servers that follow OIDC Core §11 strictly need it to issue a refresh token;
+# the rest ignore it. Override or disable it via auth_params below.
+#
+# auth_params: extra parameters added to the authorization request only (never
+# the token exchange or refresh). Needed because providers disagree on how to
+# ask for a refresh token, and getting it wrong fails silently — you connect
+# successfully and get an empty refresh_token:
+#
+#   Authorization servers that follow OIDC Core §11 strictly
+#     scopes must include offline_access, and the request must carry
+#     prompt: consent (the default). Without the prompt the scope is stripped
+#     from the grant with no error and no refresh token is issued.
+#   Google
+#     ignores offline_access; wants access_type: offline. It also returns a
+#     refresh token only on the FIRST grant per (account, client_id) — a grant
+#     Google remembers long after Forge's stored token is gone — so the
+#     prompt: consent default is what makes reconnects work.
+#   Microsoft Entra v2.0, Okta, Auth0
+#     offline_access in scopes is enough; the prompt is harmless.
+#   GitHub, Slack
+#     neither applies. See the entries below.
+#
+# A parameter with an empty value is omitted, which is how you switch off a
+# default — set prompt: "" for a provider that rejects the parameter.
+# Parameters the flow generates itself (state, code_challenge, redirect_uri,
+# scope, resource, ...) are rejected at startup.
+#
+# Note the prompt default only earns you a refresh token in combination with
+# offline_access in scopes, which stays explicit per provider: a server may
+# reject an unknown scope outright (RFC 6749 §3.3), and turning "connects but
+# cannot refresh" into "cannot connect" would be the worst failure.
+#
 # resource_url: the canonical URI of the protected resource behind this
 # provider (e.g. an MCP server endpoint). When set, Forge sends it as the
 # RFC 8707 `resource` parameter on the authorization, token and refresh
@@ -32,14 +65,24 @@
 # editing this file.
 
 providers:
+#  # Classic GitHub OAuth apps issue a non-expiring access token and no refresh
+#  # token at all, so there is nothing to configure here. Refresh tokens require
+#  # a GitHub App with "expiring user tokens" enabled.
 #  github:
 #    display_name: GitHub
 #    description: Access GitHub repositories and user profile for code reading and pull request automation.
 #
 #  google-drive:
 #    display_name: Google Drive
 #    description: Read and write files in Google Drive for document-based workflows.
+#    scopes: [https://www.googleapis.com/auth/drive]
+#    # Google ignores offline_access and needs access_type for a refresh token.
+#    # The prompt: consent default is what makes reconnects get one too.
+#    auth_params:
+#      access_type: offline
 #
+#  # Slack has no offline_access and no prompt. Refresh tokens arrive only if
+#  # token rotation is enabled on the Slack app itself.
 #  slack:
 #    display_name: Slack
 #    description: Send messages and read channel history to integrate agents with your Slack workspace.
@@ -54,10 +97,18 @@ providers:
 #   use_pkce: false
 #   # Optional: ask for a token restricted to this resource (RFC 8707).
 #   resource_url: https://api.example.com
+#   # Optional: extra authorization-request parameters. Setting prompt here
+#   # overrides the default; prompt: "" sends none.
+#   auth_params:
+#     access_type: offline
 #
 #   Dynamic Client Registration example (e.g. a remote MCP server):
 #  notion-mcp:
 #    display_name: Notion (MCP)
 #    description: Connect to the Notion MCP server.
 #    resource_url: https://mcp.notion.com/mcp
 #    use_dcrp: true
+#    # prompt: consent is applied automatically, but it only earns a refresh
+#    # token in combination with this scope.
+#    scopes: [offline_access]
+

forge-go/oauth/config.go

@@ -2,9 +2,11 @@ package oauth
 
 import (
 	"fmt"
+	"maps"
 	"net/url"
 	"os"
 	"regexp"
+	"slices"
 
 	"gopkg.in/yaml.v3"
 )
@@ -22,6 +24,25 @@ type ProviderConfig struct {
 	// Set to false for providers that do not support it.
 	UsePKCE *bool `yaml:"use_pkce" json:"usePkce,omitempty"`
 
+	// AuthParams are extra parameters appended to the authorization request —
+	// and only that request, never the token exchange or the refresh. They exist
+	// for provider-specific knobs the generic flow does not model, because
+	// providers disagree on how to ask for a refresh token:
+	//
+	//	prompt: consent        // authorization servers that follow OIDC Core §11
+	//	                       // strictly silently drop the offline_access scope
+	//	                       // without it, so no refresh token is ever issued.
+	//	                       // Sent by default; see authParams.
+	//	access_type: offline   // Google's proprietary offline_access equivalent
+	//
+	// A parameter whose value is empty is left out entirely; that is how a
+	// built-in default is switched off (see authParams).
+	//
+	// Parameters the flow generates for itself — state, code_challenge, and the
+	// rest of reservedAuthParams — cannot be set here; they are rejected at
+	// load time.
+	AuthParams map[string]string `yaml:"auth_params" json:"authParams,omitempty"`
+
 	// ResourceURL is the canonical URI of the OAuth2 protected resource this
 	// provider fronts (e.g. an MCP server endpoint).
 	//
@@ -51,15 +72,73 @@ func (p ProviderConfig) RequiresClientCredentials() bool {
 	return !p.UseDCRP
 }
 
+// reservedAuthParams are the authorization-request parameters the flow derives
+// itself, so auth_params is not allowed to supply them.
+//
+// The restriction is load-bearing rather than tidiness:
+// oauth2.Config.AuthCodeURL writes its own parameters first and then applies
+// each AuthCodeOption with url.Values.Set, so a colliding auth_params entry
+// would silently overwrite the generated value — including state, which is the
+// CSRF defence, and code_challenge, which is what binds the code to this
+// client. resource is reserved too: it is derived from ResourceURL and has to
+// match on the exchange and refresh, which auth_params does not reach.
+var reservedAuthParams = map[string]struct{}{
+	"client_id":             {},
+	"client_secret":         {},
+	"code":                  {},
+	"code_challenge":        {},
+	"code_challenge_method": {},
+	"code_verifier":         {},
+	"grant_type":            {},
+	"redirect_uri":          {},
+	"resource":              {},
+	"response_type":         {},
+	"scope":                 {},
+	"state":                 {},
+}
+
+// authParams returns the parameters to append to the authorization request.
+//
+// Every provider gets prompt=consent by default, so the behaviour is the same
+// whether the far side is a static provider or one registered by DCR. It is the
+// safe direction to be wrong in: a server that follows OIDC Core §11 strictly
+// needs it to issue a refresh token at all, and one that does not simply
+// ignores it. An explicit prompt in auth_params takes precedence; set it to the
+// empty string to opt out, for a provider that rejects the parameter.
+//
+// The default only has an effect together with offline_access in Scopes, which
+// stays explicit per provider: an authorization server may reject an unknown
+// scope outright (RFC 6749 §3.3), and turning "connects but cannot refresh" into
+// "cannot connect" would be the worse failure.
+func (p ProviderConfig) authParams() map[string]string {
+	if _, set := p.AuthParams["prompt"]; set {
+		return p.AuthParams
+	}
+	out := make(map[string]string, len(p.AuthParams)+1)
+	maps.Copy(out, p.AuthParams)
+	out["prompt"] = "consent"
+	return out
+}
+
 // Validate reports configuration errors that would otherwise surface only when
 // an auth flow is started: DCR discovers its endpoints from the resource, so it
 // requires resource_url, and resource_url — which is sent as the RFC 8707
 // `resource` parameter — must be a valid resource indicator: an absolute URI
-// with no fragment (RFC 8707, section 2).
+// with no fragment (RFC 8707, section 2). auth_params must not collide with the
+// parameters the flow generates for itself.
 func (p ProviderConfig) Validate(id string) error {
 	if p.UseDCRP && p.ResourceURL == "" {
 		return fmt.Errorf("provider %q: use_dcrp requires resource_url", id)
 	}
+	// Sorted so a config with several bad names always reports the same one.
+	for _, k := range slices.Sorted(maps.Keys(p.AuthParams)) {
+		if k == "" {
+			return fmt.Errorf("provider %q: auth_params has an empty parameter name", id)
+		}
+		if _, reserved := reservedAuthParams[k]; reserved {
+			return fmt.Errorf("provider %q: auth_params must not set %q; the OAuth2 flow generates it", id, k)
+		}
+	}
 	if p.ResourceURL != "" {
 		u, err := url.Parse(p.ResourceURL)
 		if err != nil {
@@ -118,6 +197,15 @@ func LoadProvidersConfig(path string) (*ProvidersConfig, error) {
 		p.TokenURL = interpolateEnv(p.TokenURL)
 		p.RedirectURL = interpolateEnv(p.RedirectURL)
 		p.ResourceURL = interpolateEnv(p.ResourceURL)
+		if len(p.AuthParams) > 0 {
+			// A fresh map rather than an in-place rewrite, so the parsed config
+			// is not aliased by the interpolated one.
+			params := make(map[string]string, len(p.AuthParams))
+			for k, v := range p.AuthParams {
+				params[k] = interpolateEnv(v)
+			}
+			p.AuthParams = params
+		}
 		if err := p.Validate(id); err != nil {
 			return nil, fmt.Errorf("parsing oauth providers config: %w", err)
 		}

forge-go/oauth/manager.go

@@ -254,6 +254,14 @@ func (m *Manager) GetAuthURL(ctx context.Context, orgID, providerID, clientID, c
 	}
 
 	var authOpts []oauth2.AuthCodeOption
+	// Provider-specific extras first, so the generated PKCE and resource options
+	// below still win on any collision that got past validation.
+	for k, v := range cfg.authParams() {
+		if v == "" {
+			continue // an empty value switches a built-in default off
+		}
+		authOpts = append(authOpts, oauth2.SetAuthURLParam(k, v))
+	}
 	if usePKCE {
 		authOpts = append(authOpts, oauth2.S256ChallengeOption(verifier))
 	}
@@ -362,7 +370,8 @@ func (m *Manager) ExchangeCode(ctx context.Context, code, state string) (provide
 }
 
 // GetAccessToken returns a valid access token for the provider, refreshing it
-// if it expires within 60 seconds.
+// if it expires within 60 seconds. A token with no expiry never expires and is
+// returned as-is.
 func (m *Manager) GetAccessToken(ctx context.Context, orgID, providerID string) (string, error) {
 	m.mu.Lock()
 	defer m.mu.Unlock()
@@ -372,7 +381,12 @@ func (m *Manager) GetAccessToken(ctx context.Context, orgID, providerID string)
 		return "", fmt.Errorf("provider %q not connected for org %q: %w", providerID, orgID, ErrNotConnected)
 	}
 
-	if entry.token.Valid() && time.Until(entry.token.Expiry) > 60*time.Second {
+	// A zero Expiry means "never expires" (Token.Valid agrees), but time.Until
+	// on it is hugely negative, so it has to be checked separately or a
+	// non-expiring token — a Slack bot token, a classic GitHub token — would
+	// take the refresh path on every single call and rewrite the keychain entry
+	// for nothing.
+	if entry.token.Valid() && (entry.token.Expiry.IsZero() || time.Until(entry.token.Expiry) > 60*time.Second) {
 		return entry.token.AccessToken, nil
 	}