Drop dead OAuth2 TokenExtraParams plumbing → Reject non-numeric ClickUp timestamps via strconv

- Drop dead OAuth2 TokenExtraParams plumbing
- Drop Deel-only x-client-id header from basic-form
- Follow Bitbucket workspace pagination cursor
- Reject non-numeric ClickUp timestamps via strconv

Signed-off-by: Aurélien Sibiril <81782+aureliensibiril@users.noreply.github.com>

pkg/accessreview/drivers/clickup.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"strconv"
 	"time"
 
 	"go.probo.inc/probo/pkg/coredata"
@@ -147,8 +148,10 @@ func parseClickUpTime(raw string) (time.Time, error) {
 		return t, nil
 	}
 
-	var ms int64
+	// strconv.ParseInt rejects trailing non-digit garbage that fmt.Sscanf
-	if _, err := fmt.Sscanf(raw, "%d", &ms); err != nil {
+	// would silently truncate (e.g. "123abc" → 123).
+	ms, err := strconv.ParseInt(raw, 10, 64)
+	if err != nil {
 		return time.Time{}, fmt.Errorf("cannot parse clickup time %q: %w", raw, err)
 	}
 	return time.UnixMilli(ms).UTC(), nil

pkg/accessreview/drivers/organizations.go

@@ -163,59 +163,67 @@ func ListGitLabOrganizations(ctx context.Context, httpClient *http.Client) ([]Or
 // ListBitbucketOrganizations fetches the workspaces the authenticated
 // Bitbucket user belongs to. The legacy /2.0/workspaces endpoint was
 // sunset by CHANGE-2770 (April 2026); /2.0/user/workspaces is the
-// supported cross-workspace replacement (CHANGE-3022).
+// supported cross-workspace replacement (CHANGE-3022). Bitbucket pages
+// via an absolute `next` URL on each response; follow until exhausted.
 func ListBitbucketOrganizations(ctx context.Context, httpClient *http.Client) ([]Organization, error) {
-	req, err := http.NewRequestWithContext(
+	pageURL := "https://api.bitbucket.org/2.0/user/workspaces?pagelen=100"
-		ctx,
+	result := make([]Organization, 0)
-		http.MethodGet,
-		"https://api.bitbucket.org/2.0/user/workspaces?pagelen=100",
-		nil,
-	)
-	if err != nil {
-		return nil, fmt.Errorf("cannot create bitbucket organizations request: %w", err)
-	}
-	req.Header.Set("Accept", "application/json")
 
-	resp, err := httpClient.Do(req)
+	for range maxPaginationPages {
-	if err != nil {
+		req, err := http.NewRequestWithContext(ctx, http.MethodGet, pageURL, nil)
-		return nil, fmt.Errorf("cannot fetch bitbucket organizations: %w", err)
+		if err != nil {
-	}
+			return nil, fmt.Errorf("cannot create bitbucket organizations request: %w", err)
-	defer func() { _ = resp.Body.Close() }()
-
-	if resp.StatusCode != http.StatusOK {
-		return nil, fmt.Errorf("cannot fetch bitbucket organizations: unexpected status %d", resp.StatusCode)
-	}
-
-	// We tolerate both shapes (flat and nested under `workspace`) since
-	// Atlassian has shipped variants of similar endpoints with both.
-	var body struct {
-		Values []struct {
-			Slug      string `json:"slug"`
-			Name      string `json:"name"`
-			Workspace struct {
-				Slug string `json:"slug"`
-				Name string `json:"name"`
-			} `json:"workspace"`
-		} `json:"values"`
-	}
-	if err := json.NewDecoder(resp.Body).Decode(&body); err != nil {
-		return nil, fmt.Errorf("cannot decode bitbucket organizations response: %w", err)
-	}
-
-	result := make([]Organization, 0, len(body.Values))
-	for _, v := range body.Values {
-		slug, name := v.Slug, v.Name
-		if slug == "" {
-			slug = v.Workspace.Slug
-			name = v.Workspace.Name
 		}
-		displayName := name
+		req.Header.Set("Accept", "application/json")
-		if displayName == "" {
+
-			displayName = slug
+		resp, err := httpClient.Do(req)
+		if err != nil {
+			return nil, fmt.Errorf("cannot fetch bitbucket organizations: %w", err)
 		}
-		result = append(result, Organization{Slug: slug, DisplayName: displayName})
+
+		if resp.StatusCode != http.StatusOK {
+			_ = resp.Body.Close()
+			return nil, fmt.Errorf("cannot fetch bitbucket organizations: unexpected status %d", resp.StatusCode)
+		}
+
+		// We tolerate both shapes (flat and nested under `workspace`) since
+		// Atlassian has shipped variants of similar endpoints with both.
+		var body struct {
+			Values []struct {
+				Slug      string `json:"slug"`
+				Name      string `json:"name"`
+				Workspace struct {
+					Slug string `json:"slug"`
+					Name string `json:"name"`
+				} `json:"workspace"`
+			} `json:"values"`
+			Next string `json:"next"`
+		}
+		if err := json.NewDecoder(resp.Body).Decode(&body); err != nil {
+			_ = resp.Body.Close()
+			return nil, fmt.Errorf("cannot decode bitbucket organizations response: %w", err)
+		}
+		_ = resp.Body.Close()
+
+		for _, v := range body.Values {
+			slug, name := v.Slug, v.Name
+			if slug == "" {
+				slug = v.Workspace.Slug
+				name = v.Workspace.Name
+			}
+			displayName := name
+			if displayName == "" {
+				displayName = slug
+			}
+			result = append(result, Organization{Slug: slug, DisplayName: displayName})
+		}
+
+		if body.Next == "" {
+			return result, nil
+		}
+		pageURL = body.Next
 	}
-	return result, nil
+	return nil, fmt.Errorf("cannot list all bitbucket organizations: %w", ErrPaginationLimitReached)
 }
 
 // ListHerokuOrganizations fetches the teams the authenticated Heroku

pkg/connector/oauth2.go

@@ -23,7 +23,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
-	"maps"
 	"net/http"
 	"net/url"
 	"strings"
@@ -58,11 +57,6 @@ type (
 		// to the authorize URL; CompleteWithState replays the verifier
 		// on the token exchange.
 		RequiresPKCE bool
-		// TokenExtraParams are merged into the token-exchange request
-		// body (form-encoded for post-form / basic-form, JSON for
-		// basic-json). Used for provider-specific extras such as
-		// Lever's `audience` parameter.
-		TokenExtraParams map[string]string
 		// AuthURLParams are operator-supplied placeholders substituted
 		// into the static provider AuthURL by ApplyProviderDefaults
 		// (for example Vercel's "{integration_slug}"). Empty for the
@@ -344,8 +338,7 @@ func basicAuthHeader(clientID, clientSecret string) string {
 // buildTokenRequest creates the HTTP request for the token exchange, branching
 // on c.TokenEndpointAuth to support different provider requirements. When
 // codeVerifier is non-empty (PKCE-enabled providers), it is replayed as
-// `code_verifier` in the request body. TokenExtraParams are merged into the
+// `code_verifier` in the request body.
-// body in every branch.
 func (c *OAuth2Connector) buildTokenRequest(ctx context.Context, code, redirectURI, codeVerifier string) (*http.Request, error) {
 	switch c.TokenEndpointAuth {
 	case "basic-json":
@@ -358,7 +351,6 @@ func (c *OAuth2Connector) buildTokenRequest(ctx context.Context, code, redirectU
 		if codeVerifier != "" {
 			body["code_verifier"] = codeVerifier
 		}
-		maps.Copy(body, c.TokenExtraParams)
 		jsonBody, err := json.Marshal(body)
 		if err != nil {
 			return nil, fmt.Errorf("cannot marshal token request body: %w", err)
@@ -389,9 +381,6 @@ func (c *OAuth2Connector) buildTokenRequest(ctx context.Context, code, redirectU
 		if codeVerifier != "" {
 			formData.Set("code_verifier", codeVerifier)
 		}
-		for k, v := range c.TokenExtraParams {
-			formData.Set(k, v)
-		}
 
 		req, err := http.NewRequestWithContext(
 			ctx,
@@ -407,12 +396,6 @@ func (c *OAuth2Connector) buildTokenRequest(ctx context.Context, code, redirectU
 		req.Header.Set("Accept", "application/json")
 		req.Header.Set("User-Agent", "Probo Connector")
 		req.Header.Set("Authorization", basicAuthHeader(c.ClientID, c.ClientSecret))
-		// Deel rejects token-exchange requests that omit the x-client-id
-		// header even when the credentials are correctly Base64-encoded
-		// in the Authorization header. Sending it for every basic-form
-		// provider is harmless — providers that don't expect it ignore
-		// the header.
-		req.Header.Set("x-client-id", c.ClientID)
 		return req, nil
 
 	default:
@@ -426,9 +409,6 @@ func (c *OAuth2Connector) buildTokenRequest(ctx context.Context, code, redirectU
 		if codeVerifier != "" {
 			formData.Set("code_verifier", codeVerifier)
 		}
-		for k, v := range c.TokenExtraParams {
-			formData.Set(k, v)
-		}
 
 		req, err := http.NewRequestWithContext(
 			ctx,

pkg/connector/oauth2_test.go

@@ -694,106 +694,6 @@ func TestInitiateWithState_PKCE(t *testing.T) {
 	})
 }
 
-// TestBuildTokenRequest_TokenExtraParams verifies that TokenExtraParams are
-// merged into the token-exchange body in all three auth branches. This
-// powers Lever's required `audience=https://api.lever.co/v1/` parameter
-// without any per-provider branching in the OAuth2 core.
-func TestBuildTokenRequest_TokenExtraParams(t *testing.T) {
-	t.Parallel()
-
-	t.Run("post-form merges audience into form body", func(t *testing.T) {
-		t.Parallel()
-
-		c := &OAuth2Connector{
-			ClientID:     "lever-client-id",
-			ClientSecret: "lever-client-secret",
-			TokenURL:     "https://auth.lever.co/oauth/token",
-			TokenExtraParams: map[string]string{
-				"audience": "https://api.lever.co/v1/",
-			},
-		}
-
-		req, err := c.buildTokenRequest(
-			context.Background(),
-			"the-code",
-			"https://example.com/cb",
-			"",
-		)
-		require.NoError(t, err)
-
-		body, err := io.ReadAll(req.Body)
-		require.NoError(t, err)
-
-		// Raw body check: the URL-encoded value must be present
-		// verbatim (catches any double-encoding regressions).
-		assert.Contains(t, string(body), "audience=https%3A%2F%2Fapi.lever.co%2Fv1%2F")
-
-		form, err := url.ParseQuery(string(body))
-		require.NoError(t, err)
-		assert.Equal(t, "https://api.lever.co/v1/", form.Get("audience"))
-		assert.Equal(t, "the-code", form.Get("code"))
-		assert.Equal(t, "authorization_code", form.Get("grant_type"))
-	})
-
-	t.Run("basic-form merges extra params into form body", func(t *testing.T) {
-		t.Parallel()
-
-		c := &OAuth2Connector{
-			ClientID:          "id",
-			ClientSecret:      "secret",
-			TokenURL:          "https://provider.example.com/oauth/token",
-			TokenEndpointAuth: "basic-form",
-			TokenExtraParams: map[string]string{
-				"audience": "https://api.lever.co/v1/",
-			},
-		}
-
-		req, err := c.buildTokenRequest(
-			context.Background(),
-			"the-code",
-			"https://example.com/cb",
-			"",
-		)
-		require.NoError(t, err)
-
-		body, err := io.ReadAll(req.Body)
-		require.NoError(t, err)
-
-		form, err := url.ParseQuery(string(body))
-		require.NoError(t, err)
-		assert.Equal(t, "https://api.lever.co/v1/", form.Get("audience"))
-	})
-
-	t.Run("basic-json merges extra params into JSON body", func(t *testing.T) {
-		t.Parallel()
-
-		c := &OAuth2Connector{
-			ClientID:          "id",
-			ClientSecret:      "secret",
-			TokenURL:          "https://provider.example.com/oauth/token",
-			TokenEndpointAuth: "basic-json",
-			TokenExtraParams: map[string]string{
-				"audience": "https://api.lever.co/v1/",
-			},
-		}
-
-		req, err := c.buildTokenRequest(
-			context.Background(),
-			"the-code",
-			"https://example.com/cb",
-			"",
-		)
-		require.NoError(t, err)
-
-		body, err := io.ReadAll(req.Body)
-		require.NoError(t, err)
-
-		var jsonBody map[string]string
-		require.NoError(t, json.Unmarshal(body, &jsonBody))
-		assert.Equal(t, "https://api.lever.co/v1/", jsonBody["audience"])
-	})
-}
-
 // TestApplyProviderDefaults_AuthURLTemplating verifies that operator-supplied
 // AuthURLParams (for example Vercel's "{integration_slug}") are substituted
 // into the static provider AuthURL when the connector is initialized.

pkg/connector/providers.go

@@ -39,10 +39,6 @@ type providerDefinition struct {
 	// request and replays the verifier on the token exchange. Default
 	// false; existing providers are unaffected.
 	RequiresPKCE bool
-	// TokenExtraParams are merged into the token-exchange request body
-	// (form-encoded for "post-form"/"basic-form", JSON for "basic-json").
-	// Used by providers like Lever that require an `audience` parameter.
-	TokenExtraParams map[string]string
 }
 
 // providerDefinitions maps provider names to their static OAuth2 definitions.
@@ -167,19 +163,14 @@ func ApplyProviderDefaults(provider string, redirectURI string, c *OAuth2Connect
 		c.SupportsIncrementalAuth = def.SupportsIncrementalAuth
 		c.RequiresPKCE = def.RequiresPKCE
 
-		// Deep copy ExtraAuthParams and TokenExtraParams so per-connector
+		// Deep copy ExtraAuthParams so per-connector mutations (e.g.
-		// mutations (e.g. incremental auth, scope overrides) cannot alias
+		// incremental auth, scope overrides) cannot alias back into the
-		// back into the shared providerDefinitions map.
+		// shared providerDefinitions map.
 		if len(def.ExtraAuthParams) > 0 {
 			extra := make(map[string]string, len(def.ExtraAuthParams))
 			maps.Copy(extra, def.ExtraAuthParams)
 			c.ExtraAuthParams = extra
 		}
-		if len(def.TokenExtraParams) > 0 {
-			tokenExtra := make(map[string]string, len(def.TokenExtraParams))
-			maps.Copy(tokenExtra, def.TokenExtraParams)
-			c.TokenExtraParams = tokenExtra
-		}
 
 		// Resolve operator-supplied placeholders in the static AuthURL
 		// (for example Vercel's "{integration_slug}"). Providers without