From 3dcc74e93d98e575274d0d6f9352fcb6b692d82f Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Tue, 6 Oct 2026 08:54:04 -0700 Subject: [PATCH] iam: correct LimitExceeded, InvalidAction and ServiceFailure mappings LimitExceeded is a conflict (409), an unknown Action is InvalidAction (404) rather than NotImplemented, and internal failures report the IAM receiver fault type. Applies to both the embedded IAM endpoint and the standalone iamapi server. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- weed/iamapi/iamapi_handlers.go | 1 + weed/iamapi/iamapi_management_handlers.go | 5 +-- weed/s3api/s3api_embedded_iam.go | 11 +++-- weed/s3api/s3api_embedded_iam_test.go | 45 ++++++++++++++++++- .../s3api_embedded_iam_user_tags_test.go | 2 +- 5 files changed, 55 insertions(+), 9 deletions(-) diff --git a/weed/iamapi/iamapi_handlers.go b/weed/iamapi/iamapi_handlers.go index 695ce293d7..54c15ef2aa 100644 --- a/weed/iamapi/iamapi_handlers.go +++ b/weed/iamapi/iamapi_handlers.go @@ -31,6 +31,7 @@ func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, reqID string, errorResp := newErrorResponse(errCode, errMsg, reqID) internalErrorResponse := newErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error", reqID) + internalErrorResponse.Error.Type = "Receiver" switch errCode { case iam.ErrCodeNoSuchEntityException: diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index f81a8d76c3..592c886e3c 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -1618,9 +1618,8 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { } changed = false default: - errNotImplemented := s3err.GetAPIError(s3err.ErrNotImplemented) - errorResponse := newErrorResponse(errNotImplemented.Code, errNotImplemented.Description, reqID) - s3err.WriteXMLResponse(w, r, errNotImplemented.HTTPStatusCode, errorResponse) + errorResponse := newErrorResponse("InvalidAction", fmt.Sprintf("The action %s is not valid for this endpoint", values.Get("Action")), reqID) + s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResponse) return } if changed { diff --git a/weed/s3api/s3api_embedded_iam.go b/weed/s3api/s3api_embedded_iam.go index bd1ea53354..c7f395a400 100644 --- a/weed/s3api/s3api_embedded_iam.go +++ b/weed/s3api/s3api_embedded_iam.go @@ -232,20 +232,25 @@ func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, reqID string, errorResp := newIamErrorResponse(errCode, errMsg, reqID) internalErrorResponse := newIamErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error", reqID) + internalErrorResponse.Error.Type = "Receiver" switch errCode { case iam.ErrCodeNoSuchEntityException: s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResp) case iam.ErrCodeEntityAlreadyExistsException: s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) - case iam.ErrCodeMalformedPolicyDocumentException, iam.ErrCodeInvalidInputException, "ValidationError": + case iam.ErrCodeMalformedPolicyDocumentException, iam.ErrCodeInvalidInputException, "ValidationError", "IncompleteSignature", "ExpiredToken": s3err.WriteXMLResponse(w, r, http.StatusBadRequest, errorResp) - case "AccessDenied", iam.ErrCodeLimitExceededException: + case "AccessDenied", "InvalidClientTokenId", "SignatureDoesNotMatch", "RequestTimeTooSkewed": s3err.WriteXMLResponse(w, r, http.StatusForbidden, errorResp) + case iam.ErrCodeLimitExceededException: + s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) case iam.ErrCodeServiceFailureException: s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalErrorResponse) case "NotImplemented": s3err.WriteXMLResponse(w, r, http.StatusNotImplemented, errorResp) + case "InvalidAction": + s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResp) case iam.ErrCodeDeleteConflictException: s3err.WriteXMLResponse(w, r, http.StatusConflict, errorResp) case iam.ErrCodeUnmodifiableEntityException: @@ -3003,7 +3008,7 @@ func (e *EmbeddedIamApi) ExecuteAction(ctx context.Context, values url.Values, s } changed = false default: - return nil, &iamError{Code: s3err.GetAPIError(s3err.ErrNotImplemented).Code, Error: errors.New(s3err.GetAPIError(s3err.ErrNotImplemented).Description)} + return nil, &iamError{Code: "InvalidAction", Error: fmt.Errorf("The action %s is not valid for this endpoint", values.Get("Action"))} } if changed { if !skipPersist { diff --git a/weed/s3api/s3api_embedded_iam_test.go b/weed/s3api/s3api_embedded_iam_test.go index 4e82baaa33..091d562be7 100644 --- a/weed/s3api/s3api_embedded_iam_test.go +++ b/weed/s3api/s3api_embedded_iam_test.go @@ -1043,7 +1043,7 @@ func TestEmbeddedIamAttachPolicyLimitExceeded(t *testing.T) { response, err := executeEmbeddedIamRequest(api, req.HTTPRequest, nil) assert.NoError(t, err) - assert.Equal(t, http.StatusForbidden, response.Code) + assert.Equal(t, http.StatusConflict, response.Code) code, _ := extractEmbeddedIamErrorCodeAndMessage(response) assert.Equal(t, iam.ErrCodeLimitExceededException, code) assert.Len(t, api.mockConfig.Identities[0].PolicyNames, MaxManagedPoliciesPerUser) @@ -2039,7 +2039,8 @@ func TestEmbeddedIamNotImplementedAction(t *testing.T) { apiRouter.Path("/").Methods(http.MethodPost).HandlerFunc(api.DoActions) apiRouter.ServeHTTP(rr, req) - assert.Equal(t, http.StatusNotImplemented, rr.Code) + assert.Equal(t, http.StatusNotFound, rr.Code) + assert.Contains(t, rr.Body.String(), "InvalidAction") assert.Contains(t, rr.Body.String(), "") assert.NotContains(t, rr.Body.String(), "") assert.Equal(t, rr.Header().Get(request_id.AmzRequestIDHeader), extractEmbeddedIamRequestID(rr)) @@ -2831,3 +2832,43 @@ func TestEmbeddedIamReadOnly(t *testing.T) { assert.NoError(t, err) assert.Equal(t, http.StatusOK, responseList.Code) } + +func TestWriteIamErrorResponseMapping(t *testing.T) { + tests := []struct { + name string + code string + wantStatus int + }{ + {"NoSuchEntity", iam.ErrCodeNoSuchEntityException, http.StatusNotFound}, + {"EntityAlreadyExists", iam.ErrCodeEntityAlreadyExistsException, http.StatusConflict}, + {"InvalidInput", iam.ErrCodeInvalidInputException, http.StatusBadRequest}, + {"LimitExceeded", iam.ErrCodeLimitExceededException, http.StatusConflict}, + {"DeleteConflict", iam.ErrCodeDeleteConflictException, http.StatusConflict}, + {"InvalidAction", "InvalidAction", http.StatusNotFound}, + {"AccessDenied", "AccessDenied", http.StatusForbidden}, + {"InvalidClientTokenId", "InvalidClientTokenId", http.StatusForbidden}, + {"SignatureDoesNotMatch", "SignatureDoesNotMatch", http.StatusForbidden}, + {"IncompleteSignature", "IncompleteSignature", http.StatusBadRequest}, + {"ServiceFailure", iam.ErrCodeServiceFailureException, http.StatusInternalServerError}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodPost, "/", nil) + writeIamErrorResponse(rec, r, "req-1", &iamError{Code: tt.code, Error: fmt.Errorf("boom")}) + assert.Equal(t, tt.wantStatus, rec.Code) + body := rec.Body.String() + assert.True(t, strings.HasPrefix(strings.TrimSpace(body), ""+tt.code+"") + }) + } +} + +func TestIamAuthErrorCode(t *testing.T) { + assert.Equal(t, "InvalidClientTokenId", iamAuthErrorCode(s3err.ErrInvalidAccessKeyID)) + assert.Equal(t, "SignatureDoesNotMatch", iamAuthErrorCode(s3err.ErrSignatureDoesNotMatch)) + assert.Equal(t, "AccessDenied", iamAuthErrorCode(s3err.ErrAccessDenied)) + assert.Equal(t, "RequestTimeTooSkewed", iamAuthErrorCode(s3err.ErrRequestTimeTooSkewed)) + assert.Equal(t, "IncompleteSignature", iamAuthErrorCode(s3err.ErrMissingFields)) +} diff --git a/weed/s3api/s3api_embedded_iam_user_tags_test.go b/weed/s3api/s3api_embedded_iam_user_tags_test.go index 07c2b4e25c..ba9a5f389a 100644 --- a/weed/s3api/s3api_embedded_iam_user_tags_test.go +++ b/weed/s3api/s3api_embedded_iam_user_tags_test.go @@ -200,7 +200,7 @@ func TestEmbeddedIamTagUserLimitExceeded(t *testing.T) { form.Set("Tags.member.1.Value", "v") rr := postTagAction(t, api, form) - assert.Equal(t, http.StatusForbidden, rr.Code) + assert.Equal(t, http.StatusConflict, rr.Code) code, _ := extractEmbeddedIamErrorCodeAndMessage(rr) assert.Equal(t, iam.ErrCodeLimitExceededException, code)