fix(api): reject invalid external owner inputs

Author: onutc

api/create_request_normalization.go

@@ -94,7 +94,7 @@ func (s *server) normalizeCreateRequest(_ context.Context, principal principal,
 		return nil, newCreateRequestError(http.StatusBadRequest, err)
 	}
 	if body.OwnerRef != nil && strings.EqualFold(strings.TrimSpace(body.OwnerRef.Type), "external") {
-		if !principalCanUseProvisionerFlow(principal) {
+		if !principal.isService() {
 			return nil, newCreateRequestError(http.StatusForbidden, errForbidden)
 		}
 	} else {

api/external_owner_resolution.go

@@ -247,7 +247,17 @@ func (c externalOwnerConfig) resolve(ctx context.Context, principal principal, r
 			subject:  normalized.Subject,
 		}
 	}
-	if len(policy.AllowedTenants) > 0 && normalized.Tenant != "" {
+	if len(policy.AllowedTenants) > 0 {
+		if normalized.Tenant == "" {
+			return externalOwnerResolution{}, externalOwnerResolutionError{
+				status:   http.StatusForbidden,
+				code:     "external_identity_forbidden",
+				message:  "tenant is required for this principal",
+				provider: normalized.Provider,
+				tenant:   normalized.Tenant,
+				subject:  normalized.Subject,
+			}
+		}
 		if _, ok := policy.AllowedTenants[normalized.Tenant]; !ok {
 			return externalOwnerResolution{}, externalOwnerResolutionError{
 				status:   http.StatusForbidden,

api/main_create_external_owner_test.go

@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"strings"
@@ -294,6 +295,79 @@ func TestCreateSpritzRejectsExternalOwnerResolutionWithoutCreateScopes(t *testin
 	}
 }
 
+func TestCreateSpritzRejectsExternalOwnerForAdminCallers(t *testing.T) {
+	s := newCreateSpritzTestServer(t)
+	configureProvisionerTestServer(s)
+	resolverCalls := 0
+	configureExternalOwnerTestServer(s, fakeExternalOwnerResolver{
+		resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) {
+			resolverCalls++
+			return externalOwnerResolution{
+				Status:  externalOwnerResolved,
+				OwnerID: "user-123",
+			}, nil
+		},
+	})
+	e := newCreateSpritzAPI(t, s)
+
+	body := []byte(`{"presetId":"openclaw","ownerRef":{"type":"external","provider":"msteams","tenant":"72f988bf-86f1-41af-91ab-2d7cd011db47","subject":"6f0f9d4f-9b0e-4d52-8c3a-ef0fd64b9b9f"},"idempotencyKey":"teams-admin"}`)
+	req := httptest.NewRequest(http.MethodPost, "/api/spritzes", bytes.NewReader(body))
+	req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON)
+	req.Header.Set("X-Spritz-User-Id", "admin-1")
+	req.Header.Set("X-Spritz-Principal-Type", "admin")
+	rec := httptest.NewRecorder()
+
+	e.ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusForbidden {
+		t.Fatalf("expected status 403, got %d: %s", rec.Code, rec.Body.String())
+	}
+	if resolverCalls != 0 {
+		t.Fatalf("expected resolver to be skipped for admin callers, got %d calls", resolverCalls)
+	}
+}
+
+func TestExternalOwnerResolveRequiresTenantWhenTenantAllowlistIsConfigured(t *testing.T) {
+	config := externalOwnerConfig{
+		subjectHashKey: []byte("test-external-owner-secret"),
+		policies: map[string]externalOwnerPolicy{
+			"zenobot": {
+				PrincipalID: "zenobot",
+				Issuer:      "zenobot",
+				URL:         "http://resolver.example.com/v1/external-owners/resolve",
+				AllowedProviders: map[string]struct{}{
+					"slack": {},
+				},
+				AllowedTenants: map[string]struct{}{
+					"enterprise-1": {},
+				},
+			},
+		},
+		resolver: fakeExternalOwnerResolver{
+			resolve: func(_ context.Context, _ externalOwnerPolicy, _ principal, _ ownerRef, _ string) (externalOwnerResolution, error) {
+				t.Fatal("expected resolver call to be blocked when tenant is missing")
+				return externalOwnerResolution{}, nil
+			},
+		},
+	}
+
+	_, err := config.resolve(context.Background(), principal{ID: "zenobot", Type: principalTypeService}, ownerRef{
+		Type:     "external",
+		Provider: "slack",
+		Subject:  "U123456",
+	}, "")
+	if err == nil {
+		t.Fatal("expected resolve to fail when tenant allowlist is configured but tenant is missing")
+	}
+	var resolutionErr externalOwnerResolutionError
+	if !errors.As(err, &resolutionErr) {
+		t.Fatalf("expected externalOwnerResolutionError, got %T", err)
+	}
+	if resolutionErr.code != "external_identity_forbidden" {
+		t.Fatalf("expected external_identity_forbidden, got %q", resolutionErr.code)
+	}
+}
+
 func TestCreateRequestFingerprintCanonicalizesEquivalentOwnerInputs(t *testing.T) {
 	directFingerprint, err := createRequestFingerprint(createRequest{
 		OwnerID: "user-123",
@@ -372,3 +446,16 @@ func TestNormalizeCreateOwnerSupportsOwnerRefOwner(t *testing.T) {
 		t.Fatalf("expected body ownerId to be populated from ownerRef, got %q", body.OwnerID)
 	}
 }
+
+func TestNormalizeCreateOwnerRejectsOwnerRefWithoutType(t *testing.T) {
+	body := &createRequest{
+		OwnerRef: &ownerRef{ID: "user-123"},
+	}
+	_, err := normalizeCreateOwnerRequest(body, principal{ID: "user-123", Type: principalTypeHuman}, true)
+	if err == nil {
+		t.Fatal("expected normalizeCreateOwnerRequest to reject ownerRef without type")
+	}
+	if !strings.Contains(err.Error(), "ownerRef.type is required") {
+		t.Fatalf("expected ownerRef.type validation error, got %v", err)
+	}
+}

api/provisioning.go

@@ -220,6 +220,7 @@ func normalizeCreateOwnerRequest(body *createRequest, principal principal, authE
 		ref.ID = strings.TrimSpace(ref.ID)
 		switch ref.Type {
 		case "":
+			return owner, fmt.Errorf("ownerRef.type is required")
 		case "owner":
 			if ref.ID == "" {
 				return owner, fmt.Errorf("ownerRef.id is required when ownerRef.type=owner")
@@ -888,8 +889,7 @@ func canonicalOwnerFingerprintPayload(ownerID string, ref *ownerRef, resolvedOwn
 			}
 			return canonicalOwnerRefPayload(normalized), nil
 		case "":
-			// Fall through to the direct owner path below so empty ownerRef does
-			// not create a distinct fingerprint representation.
+			return nil, fmt.Errorf("ownerRef.type is required")
 		default:
 			return nil, fmt.Errorf("ownerRef.type must be owner or external")
 		}