Fix: Route unauthenticated specific STS requests to STS handler correctly (#7920)

* Fix STS Access Denied for AssumeRoleWithWebIdentity (Issue #7917) * Fix logging regression: ensure IAM status is logged even if STS is enabled * Address PR feedback: fix duplicate log, clarify comments, add comprehensive routing tests * Add edge case test: authenticated STS action routes to IAM (auth precedence)
3 days ago · e3db95e0c1
2 changed files with 216 additions and 7 deletions
--- a/weed/s3api/s3api_server.go
+++ b/weed/s3api/s3api_server.go
@ -618,9 +618,8 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {

 	// STS API endpoint for AssumeRoleWithWebIdentity
 	// POST /?Action=AssumeRoleWithWebIdentity&WebIdentityToken=...
-	// This endpoint is unauthenticated - the JWT token in the request is the authentication
-	// IMPORTANT: Register this BEFORE the general IAM route to prevent interception
 	if s3a.stsHandlers != nil {
+		// 1. Explicit query param match (highest priority)
 		apiRouter.Methods(http.MethodPost).Path("/").Queries("Action", "AssumeRoleWithWebIdentity").
 			HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS"))
 		glog.V(0).Infof("STS API enabled on S3 port (AssumeRoleWithWebIdentity)")
@ -628,15 +627,30 @@ func (s3a *S3ApiServer) registerRouter(router *mux.Router) {

 	// Embedded IAM API endpoint
 	// POST / (without specific query parameters)
-	// This must be before ListBuckets since IAM uses POST and ListBuckets uses GET
-	// Uses AuthIam for granular permission checking:
-	// - Self-service operations (own access keys) don't require admin
-	// - Operations on other users require admin privileges
+	// Uses AuthIam for granular permission checking
 	if s3a.embeddedIam != nil {
-		apiRouter.Methods(http.MethodPost).Path("/").HandlerFunc(track(s3a.embeddedIam.AuthIam(s3a.cb.Limit(s3a.embeddedIam.DoActions, ACTION_WRITE)), "IAM"))
+		// 2. Authenticated IAM requests
+		// Only match if the request appears to be authenticated (AWS Signature)
+		// This prevents unauthenticated STS requests (like AssumeRoleWithWebIdentity in body)
+		// from being captured by the IAM handler which would reject them.
+		iamMatcher := func(r *http.Request, rm *mux.RouteMatch) bool {
+			return getRequestAuthType(r) != authTypeAnonymous
+		}
+
+		apiRouter.Methods(http.MethodPost).Path("/").MatcherFunc(iamMatcher).
+			HandlerFunc(track(s3a.embeddedIam.AuthIam(s3a.cb.Limit(s3a.embeddedIam.DoActions, ACTION_WRITE)), "IAM"))
 		glog.V(0).Infof("Embedded IAM API enabled on S3 port")
 	}

+	// 3. Fallback STS handler (lowest priority)
+	// Catches unauthenticated POST / requests that didn't match specific query params.
+	// This primarily handles AssumeRoleWithWebIdentity where parameters are in the POST body.
+	if s3a.stsHandlers != nil {
+		glog.V(1).Infof("Registering fallback STS handler for unauthenticated POST requests")
+		apiRouter.Methods(http.MethodPost).Path("/").
+			HandlerFunc(track(s3a.stsHandlers.HandleSTSRequest, "STS-Fallback"))
+	}
+
 	// ListBuckets
 	apiRouter.Methods(http.MethodGet).Path("/").HandlerFunc(track(s3a.iam.Auth(s3a.ListBucketsHandler, ACTION_LIST), "LIST"))

--- a/weed/s3api/s3api_server_routing_test.go
+++ b/weed/s3api/s3api_server_routing_test.go
@ -0,0 +1,195 @@
+package s3api
+
+import (
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/gorilla/mux"
+	"github.com/seaweedfs/seaweedfs/weed/credential"
+	"github.com/seaweedfs/seaweedfs/weed/util"
+	"github.com/stretchr/testify/assert"
+)
+
+// setupRoutingTestServer creates a minimal S3ApiServer for routing tests
+func setupRoutingTestServer(t *testing.T) *S3ApiServer {
+	opt := &S3ApiServerOption{EnableIam: true}
+	iam := NewIdentityAccessManagementWithStore(opt, "memory")
+	iam.isAuthEnabled = true
+
+	if iam.credentialManager == nil {
+		cm, err := credential.NewCredentialManager("memory", util.GetViper(), "")
+		if err != nil {
+			t.Fatalf("Failed to create credential manager: %v", err)
+		}
+		iam.credentialManager = cm
+	}
+
+	server := &S3ApiServer{
+		option:            opt,
+		iam:               iam,
+		credentialManager: iam.credentialManager,
+		embeddedIam:       NewEmbeddedIamApi(iam.credentialManager, iam),
+		stsHandlers:       &STSHandlers{},
+	}
+
+	return server
+}
+
+// TestRouting_STSWithQueryParams verifies that AssumeRoleWithWebIdentity with query params routes to STS
+func TestRouting_STSWithQueryParams(t *testing.T) {
+	router := mux.NewRouter()
+	s3a := setupRoutingTestServer(t)
+	s3a.registerRouter(router)
+
+	// Create request with Action in query params (no auth header)
+	req, _ := http.NewRequest("POST", "/?Action=AssumeRoleWithWebIdentity&WebIdentityToken=test-token&RoleArn=arn:aws:iam::123:role/test&RoleSessionName=test-session", nil)
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+
+	// Should route to STS handler -> 503 (service not initialized) or 400 (validation error)
+	assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable}, rr.Code, "Should route to STS handler")
+}
+
+// TestRouting_STSWithBodyParams verifies that AssumeRoleWithWebIdentity with body params routes to STS fallback
+func TestRouting_STSWithBodyParams(t *testing.T) {
+	router := mux.NewRouter()
+	s3a := setupRoutingTestServer(t)
+	s3a.registerRouter(router)
+
+	// Create request with Action in POST body (no auth header)
+	data := url.Values{}
+	data.Set("Action", "AssumeRoleWithWebIdentity")
+	data.Set("WebIdentityToken", "test-token")
+	data.Set("RoleArn", "arn:aws:iam::123:role/test")
+	data.Set("RoleSessionName", "test-session")
+
+	req, _ := http.NewRequest("POST", "/", strings.NewReader(data.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+
+	// Should route to STS fallback handler -> 503 (service not initialized in test)
+	assert.Equal(t, http.StatusServiceUnavailable, rr.Code, "Should route to STS fallback handler (503 because STS not initialized)")
+}
+
+// TestRouting_AuthenticatedIAM verifies that authenticated IAM requests route to IAM handler
+func TestRouting_AuthenticatedIAM(t *testing.T) {
+	router := mux.NewRouter()
+	s3a := setupRoutingTestServer(t)
+	s3a.registerRouter(router)
+
+	// Create IAM request with Authorization header
+	data := url.Values{}
+	data.Set("Action", "CreateUser")
+	data.Set("UserName", "testuser")
+
+	req, _ := http.NewRequest("POST", "/", strings.NewReader(data.Encode()))
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Authorization", "AWS4-HMAC-SHA256 Credential=AKIA.../...")
+
+	rr := httptest.NewRecorder()
+	router.ServeHTTP(rr, req)
+
+	// Should route to IAM handler -> 400/403 (invalid signature)
+	// NOT 503 (which would indicate STS handler)
+	assert.NotEqual(t, http.StatusServiceUnavailable, rr.Code, "Should NOT route to STS handler")
+	assert.Contains(t, []int{http.StatusBadRequest, http.StatusForbidden}, rr.Code, "Should route to IAM handler (400/403 due to invalid signature)")
+}
+
+// TestRouting_IAMMatcherLogic verifies the iamMatcher correctly distinguishes auth types
+func TestRouting_IAMMatcherLogic(t *testing.T) {
+	tests := []struct {
+		name        string
+		authHeader  string
+		queryParams string
+		expectsIAM  bool
+		description string
+	}{
+		{
+			name:        "No auth - anonymous",
+			authHeader:  "",
+			queryParams: "",
+			expectsIAM:  false,
+			description: "Request with no auth should NOT match IAM",
+		},
+		{
+			name:        "AWS4 signature",
+			authHeader:  "AWS4-HMAC-SHA256 Credential=AKIA.../...",
+			queryParams: "",
+			expectsIAM:  true,
+			description: "Request with AWS4 signature should match IAM",
+		},
+		{
+			name:        "AWS2 signature",
+			authHeader:  "AWS AKIA...:signature",
+			queryParams: "",
+			expectsIAM:  true,
+			description: "Request with AWS2 signature should match IAM",
+		},
+		{
+			name:        "Presigned V4",
+			authHeader:  "",
+			queryParams: "?X-Amz-Credential=AKIA...",
+			expectsIAM:  true,
+			description: "Request with presigned V4 params should match IAM",
+		},
+		{
+			name:        "Presigned V2",
+			authHeader:  "",
+			queryParams: "?AWSAccessKeyId=AKIA...",
+			expectsIAM:  true,
+			description: "Request with presigned V2 params should match IAM",
+		},
+		{
+			name:        "AWS4 signature with STS action in body",
+			authHeader:  "AWS4-HMAC-SHA256 Credential=AKIA.../...",
+			queryParams: "",
+			expectsIAM:  true,
+			description: "Authenticated STS action should still route to IAM (auth takes precedence)",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			router := mux.NewRouter()
+			s3a := setupRoutingTestServer(t)
+			s3a.registerRouter(router)
+
+			data := url.Values{}
+			// For the authenticated STS action test, set the STS action
+			// For other tests, don't set Action to avoid STS validation errors
+			if tt.name == "AWS4 signature with STS action in body" {
+				data.Set("Action", "AssumeRoleWithWebIdentity")
+				data.Set("WebIdentityToken", "test-token")
+				data.Set("RoleArn", "arn:aws:iam::123:role/test")
+				data.Set("RoleSessionName", "test-session")
+			}
+
+			req, _ := http.NewRequest("POST", "/"+tt.queryParams, strings.NewReader(data.Encode()))
+			req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+			if tt.authHeader != "" {
+				req.Header.Set("Authorization", tt.authHeader)
+			}
+
+			rr := httptest.NewRecorder()
+			router.ServeHTTP(rr, req)
+
+			if tt.expectsIAM {
+				// Should route to IAM (400/403 for invalid sig)
+				// NOT 400 from STS (which would be missing Action parameter)
+				// We distinguish by checking it's NOT a generic 400 with empty body
+				assert.NotEqual(t, http.StatusServiceUnavailable, rr.Code, tt.description)
+			} else {
+				// Should route to STS fallback
+				// Can be 503 (service not initialized) or 400 (missing/invalid Action parameter)
+				assert.Contains(t, []int{http.StatusBadRequest, http.StatusServiceUnavailable}, rr.Code, tt.description)
+			}
+		})
+	}
+}