fix: close round-3 OAuth strict-proto config-field gaps (v0.2.4)

.github/fixtures/workflow-compat.yaml

@@ -156,6 +156,9 @@ workflows:
           google_oauth_client_id: "google-client"
           google_oauth_client_secret: "google-secret"
           google_oauth_redirect_url: "https://example.test/cb"
+          # return_to added in v0.2.4 (BMW supplies it via config when the
+          # incoming request includes a return_to query param):
+          return_to: "/auth/callback"
 
       - name: oauth_exchange
         type: step.auth_oauth_exchange
@@ -175,6 +178,9 @@ workflows:
           google_oauth_client_id: "google-client"
           google_oauth_client_secret: "google-secret"
           google_oauth_redirect_url: "https://example.test/cb"
+          # access_token added in v0.2.4 (BMW supplies it via config from a
+          # preceding exchange_code step's output):
+          access_token: "compat-access-token"
 
       - name: credential_list
         type: step.auth_credential_list

CHANGELOG.md

@@ -1,5 +1,43 @@
 # Changelog
 
+## v0.2.4 (2026-05-13)
+
+### Strict-proto config-field gaps closed (BMW local smoke vs workflow v0.51.5, round 3)
+
+Round 3 closes two OAuth gaps the v0.2.3 sweep missed. Both surfaced when BMW
+v0.51.5 strict-proto validation rejected fields BMW supplies via the step's
+`config:` block (templates render at runtime, but strict-proto validates Config
+at build-time when templates are still unresolved literals):
+
+- `OAuthProviderConfig`: added `string return_to = 11`. BMW's `step.auth_oauth_start`
+  passes `return_to: '{{ .return_to }}'` in config. The handler now prefers
+  `config.return_to` when non-empty, otherwise falls back to `current.return_to`
+  (OAuthProviderInput).
+- `OAuthProviderConfig`: added `string access_token = 12`. BMW's
+  `step.auth_oauth_userinfo` passes
+  `access_token: '{{ index .steps "exchange_code" "access_token" }}'` in config.
+  The handler now prefers `config.access_token` when non-empty, otherwise falls
+  back to `current.access_token` (OAuthProviderInput).
+
+Both fields remain valid on `OAuthProviderInput` for callers that pass them at
+runtime (the v0.2.3 contract). Config-when-non-empty is the new tie-breaker.
+
+### Tests
+
+- `TestOAuthProviderConfig_AcceptsReturnToAndAccessToken` — strict-proto accepts
+  the new config fields across all four OAuth step types.
+- `TestOAuthStart_UsesReturnToFromConfig`, `TestOAuthStart_ConfigReturnToWinsOverCurrent`,
+  `TestOAuthStart_FallsBackToCurrentReturnTo` — handler precedence for `return_to`.
+- `TestOAuthUserinfo_UsesAccessTokenFromConfig`,
+  `TestOAuthUserinfo_ConfigAccessTokenWinsOverCurrent`,
+  `TestOAuthUserinfo_FallsBackToCurrentAccessToken` — handler precedence for
+  `access_token` (via httptest userinfo server asserting the Bearer header).
+
+### CI fixture
+
+- `.github/fixtures/workflow-compat.yaml` now exercises `config.return_to` on
+  `step.auth_oauth_start` and `config.access_token` on `step.auth_oauth_userinfo`.
+
 ## v0.2.3 (2026-05-13)
 
 ### Strict-proto config-field gaps closed (BMW local smoke vs workflow v0.51.5, round 2)

internal/contracts/auth.proto

@@ -363,6 +363,15 @@ message OAuthProviderConfig {
   repeated string google_oauth_scopes = 8;
   optional bool allow_insecure_test_oauth_endpoints = 9;
   optional bool pkce_required = 10;
+  // return_to and access_token added in v0.2.4: BMW passes both via the
+  // step's config: block (BMW templates `{{ .return_to }}` and
+  // `{{ index .steps "exchange_code" "access_token" }}` respectively).
+  // Under strict-proto these are validated at build-time when templates
+  // are unresolved literals, so they must live on OAuthProviderConfig.
+  // Handlers prefer config values when non-empty, then fall back to
+  // OAuthProviderInput (runtime input).
+  string return_to = 11;
+  string access_token = 12;
 }
 
 message OAuthProviderInput {

internal/step_oauth.go

@@ -66,7 +66,10 @@ func (s *oauthStartStep) Execute(_ context.Context, _ map[string]any, _ map[stri
 		return &sdk.StepResult{Output: map[string]any{"started": false, "provider": provider, "error": disabledReason}}, nil
 	}
 
-	returnTo, err := normalizeOAuthReturnTo(oauthString(current, "return_to"))
+	// return_to may be supplied via OAuthProviderConfig (BMW yaml shape:
+	// `step.auth_oauth_start.config.return_to`) or via OAuthProviderInput
+	// (`current.return_to`). Config wins when non-empty (v0.2.4).
+	returnTo, err := normalizeOAuthReturnTo(oauthConfigOrCurrent(s.config, current, "return_to"))
 	if err != nil {
 		return &sdk.StepResult{Output: map[string]any{"started": false, "provider": provider, "error": err.Error()}}, nil
 	}
@@ -195,7 +198,11 @@ func (s *oauthUserinfoStep) Execute(ctx context.Context, _ map[string]any, _ map
 		return &sdk.StepResult{Output: map[string]any{"fetched": false, "provider": provider, "error": disabledReason}}, nil
 	}
 
-	accessToken := oauthString(current, "access_token")
+	// access_token may be supplied via OAuthProviderConfig (BMW yaml shape:
+	// `step.auth_oauth_userinfo.config.access_token`, templated from a
+	// preceding exchange step) or via OAuthProviderInput (`current.access_token`).
+	// Config wins when non-empty (v0.2.4).
+	accessToken := oauthConfigOrCurrent(s.config, current, "access_token")
 	if accessToken == "" {
 		return &sdk.StepResult{Output: map[string]any{"fetched": false, "provider": provider, "error": "missing access_token"}}, nil
 	}
@@ -380,6 +387,19 @@ func oauthDoJSON(client *http.Client, req *http.Request, label string, target an
 	return nil
 }
 
+// oauthConfigOrCurrent returns config[key] when non-empty, otherwise falls back
+// to current[key]. BMW supplies fields like return_to / access_token via the
+// step's config: block (templated at runtime). Under strict-proto these are
+// validated against OAuthProviderConfig at build-time, so the handler must
+// honor config when it carries a value but still accept runtime input
+// otherwise. Added in v0.2.4.
+func oauthConfigOrCurrent(config, current map[string]any, key string) string {
+	if value := oauthString(config, key); value != "" {
+		return value
+	}
+	return oauthString(current, key)
+}
+
 func oauthString(values map[string]any, key string) string {
 	if values == nil {
 		return ""

internal/strict_proto_fields_test.go

@@ -2,6 +2,9 @@ package internal
 
 import (
 	"context"
+	"encoding/json"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 	"time"
 
@@ -221,3 +224,178 @@ func TestChallengeVerify_FallsBackToConfigSigningSecret(t *testing.T) {
 		t.Fatalf("expected valid=true when signing_secret only supplied via config, got %#v", verifyResult.Output)
 	}
 }
+
+// TestOAuthProviderConfig_AcceptsReturnToAndAccessToken ensures the typed
+// OAuthProviderConfig accepts the return_to and access_token fields BMW
+// supplies via step.auth_oauth_start.config and step.auth_oauth_userinfo.config
+// (round-3 strict-proto gap, v0.2.4).
+func TestOAuthProviderConfig_AcceptsReturnToAndAccessToken(t *testing.T) {
+	cfg := &contracts.OAuthProviderConfig{
+		Provider:                "google",
+		GoogleOauthClientId:     "google-client",
+		GoogleOauthClientSecret: "google-secret",
+		GoogleOauthRedirectUrl:  "https://example.test/cb",
+		ReturnTo:                "/auth/callback",
+		AccessToken:             "access-token-from-exchange",
+	}
+	packed, err := anypb.New(cfg)
+	if err != nil {
+		t.Fatalf("pack config: %v", err)
+	}
+	provider := NewAuthPlugin().(interface {
+		CreateTypedStep(typeName, name string, config *anypb.Any) (sdk.StepInstance, error)
+	})
+	for _, stepType := range []string{
+		"step.auth_oauth_start",
+		"step.auth_oauth_exchange",
+		"step.auth_oauth_userinfo",
+		"step.auth_oauth_provider_config",
+	} {
+		if _, err := provider.CreateTypedStep(stepType, "oauth", packed); err != nil {
+			t.Fatalf("CreateTypedStep(%s) rejected return_to/access_token: %v", stepType, err)
+		}
+	}
+}
+
+// TestOAuthStart_UsesReturnToFromConfig verifies start_oauth honors return_to
+// when supplied via config (BMW yaml shape), not just via input.
+func TestOAuthStart_UsesReturnToFromConfig(t *testing.T) {
+	step := newOAuthStartStep("test", googleOAuthTestConfig(map[string]any{
+		"return_to": "/wishlists",
+	}))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider": "google",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if errStr, _ := result.Output["error"].(string); errStr != "" {
+		t.Fatalf("expected no error, got %q (output=%#v)", errStr, result.Output)
+	}
+	if got, _ := result.Output["return_to"].(string); got != "/wishlists" {
+		t.Fatalf("expected return_to=/wishlists from config, got %v", result.Output["return_to"])
+	}
+}
+
+// TestOAuthStart_ConfigReturnToWinsOverCurrent verifies config.return_to wins
+// when both config and input supply it (Config-when-non-empty rule).
+func TestOAuthStart_ConfigReturnToWinsOverCurrent(t *testing.T) {
+	step := newOAuthStartStep("test", googleOAuthTestConfig(map[string]any{
+		"return_to": "/from-config",
+	}))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider":  "google",
+		"return_to": "/from-input",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if got, _ := result.Output["return_to"].(string); got != "/from-config" {
+		t.Fatalf("expected config.return_to to win, got %v", result.Output["return_to"])
+	}
+}
+
+// TestOAuthStart_FallsBackToCurrentReturnTo verifies start_oauth honors
+// return_to from current/input when config does not supply one.
+func TestOAuthStart_FallsBackToCurrentReturnTo(t *testing.T) {
+	step := newOAuthStartStep("test", googleOAuthTestConfig(nil))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider":  "google",
+		"return_to": "/from-input",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if got, _ := result.Output["return_to"].(string); got != "/from-input" {
+		t.Fatalf("expected return_to from input fallback, got %v", result.Output["return_to"])
+	}
+}
+
+// TestOAuthUserinfo_UsesAccessTokenFromConfig verifies fetch_userinfo honors
+// access_token when supplied via config (BMW yaml shape templates the token
+// from a preceding exchange_code step into config.access_token).
+func TestOAuthUserinfo_UsesAccessTokenFromConfig(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") != "Bearer config-access-token" {
+			t.Fatalf("expected bearer from config.access_token, got %q", r.Header.Get("Authorization"))
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{
+			"sub":   "user-id",
+			"email": "user@example.com",
+		})
+	}))
+	defer server.Close()
+
+	step := newOAuthUserinfoStep("test", googleOAuthTestConfig(map[string]any{
+		"google_oauth_userinfo_url":           server.URL,
+		"allow_insecure_test_oauth_endpoints": true,
+		"access_token":                        "config-access-token",
+	}))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider": "google",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if errStr, _ := result.Output["error"].(string); errStr != "" {
+		t.Fatalf("expected no error, got %q (output=%#v)", errStr, result.Output)
+	}
+	if result.Output["fetched"] != true {
+		t.Fatalf("expected fetched=true, got %v", result.Output["fetched"])
+	}
+}
+
+// TestOAuthUserinfo_ConfigAccessTokenWinsOverCurrent verifies config.access_token
+// wins when both config and input supply it.
+func TestOAuthUserinfo_ConfigAccessTokenWinsOverCurrent(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") != "Bearer config-wins" {
+			t.Fatalf("expected config.access_token to win, got %q", r.Header.Get("Authorization"))
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"sub": "x"})
+	}))
+	defer server.Close()
+
+	step := newOAuthUserinfoStep("test", googleOAuthTestConfig(map[string]any{
+		"google_oauth_userinfo_url":           server.URL,
+		"allow_insecure_test_oauth_endpoints": true,
+		"access_token":                        "config-wins",
+	}))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider":     "google",
+		"access_token": "input-loses",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if result.Output["fetched"] != true {
+		t.Fatalf("expected fetched=true, got %#v", result.Output)
+	}
+}
+
+// TestOAuthUserinfo_FallsBackToCurrentAccessToken verifies fetch_userinfo falls
+// back to current/input access_token when config does not supply one.
+func TestOAuthUserinfo_FallsBackToCurrentAccessToken(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.Header.Get("Authorization") != "Bearer input-access-token" {
+			t.Fatalf("expected bearer from input fallback, got %q", r.Header.Get("Authorization"))
+		}
+		_ = json.NewEncoder(w).Encode(map[string]any{"sub": "user"})
+	}))
+	defer server.Close()
+
+	step := newOAuthUserinfoStep("test", googleOAuthTestConfig(map[string]any{
+		"google_oauth_userinfo_url":           server.URL,
+		"allow_insecure_test_oauth_endpoints": true,
+	}))
+	result, err := step.Execute(context.Background(), nil, nil, map[string]any{
+		"provider":     "google",
+		"access_token": "input-access-token",
+	}, nil, nil)
+	if err != nil {
+		t.Fatalf("execute: %v", err)
+	}
+	if result.Output["fetched"] != true {
+		t.Fatalf("expected fetched=true via input fallback, got %#v", result.Output)
+	}
+}