diff --git a/weed/iamapi/iamapi_handlers.go b/weed/iamapi/iamapi_handlers.go index 5007c34c6..c8eac8ef6 100644 --- a/weed/iamapi/iamapi_handlers.go +++ b/weed/iamapi/iamapi_handlers.go @@ -1,31 +1,45 @@ package iamapi import ( - "fmt" + "net/http" + "github.com/aws/aws-sdk-go/service/iam" "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/s3api/s3err" - "net/http" ) -func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, err error, object string, value string, msg error) { - errCode := err.Error() +func newErrorResponse(errCode string, errMsg string) ErrorResponse { errorResp := ErrorResponse{} errorResp.Error.Type = "Sender" errorResp.Error.Code = &errCode - if msg != nil { - errMsg := msg.Error() - errorResp.Error.Message = &errMsg + errorResp.Error.Message = &errMsg + return errorResp +} + +func writeIamErrorResponse(w http.ResponseWriter, r *http.Request, iamError *IamError) { + + if iamError == nil { + // Do nothing if there is no error + glog.Errorf("No error found") + return } - glog.Errorf("Response %+v", err) + + errCode := iamError.Code + errMsg := iamError.Error.Error() + glog.Errorf("Response %+v", errMsg) + + errorResp := newErrorResponse(errCode, errMsg) + internalErrorResponse := newErrorResponse(iam.ErrCodeServiceFailureException, "Internal server error") + switch errCode { case iam.ErrCodeNoSuchEntityException: - msg := fmt.Sprintf("The %s with name %s cannot be found.", object, value) - errorResp.Error.Message = &msg s3err.WriteXMLResponse(w, r, http.StatusNotFound, errorResp) + case iam.ErrCodeMalformedPolicyDocumentException: + s3err.WriteXMLResponse(w, r, http.StatusBadRequest, errorResp) case iam.ErrCodeServiceFailureException: - s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, errorResp) + // We do not want to expose internal server error to the client + s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalErrorResponse) default: - s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, errorResp) + s3err.WriteXMLResponse(w, r, http.StatusInternalServerError, internalErrorResponse) } } diff --git a/weed/iamapi/iamapi_management_handlers.go b/weed/iamapi/iamapi_management_handlers.go index 6b0f9bbfc..e5c533e27 100644 --- a/weed/iamapi/iamapi_management_handlers.go +++ b/weed/iamapi/iamapi_management_handlers.go @@ -89,6 +89,10 @@ func MapToIdentitiesAction(action string) string { } } +const ( + USER_DOES_NOT_EXIST = "the user with name %s cannot be found." +) + type Statement struct { Effect string `json:"Effect"` Action []string `json:"Action"` @@ -153,27 +157,27 @@ func (iama *IamApiServer) CreateUser(s3cfg *iam_pb.S3ApiConfiguration, values ur return resp } -func (iama *IamApiServer) DeleteUser(s3cfg *iam_pb.S3ApiConfiguration, userName string) (resp DeleteUserResponse, err error) { +func (iama *IamApiServer) DeleteUser(s3cfg *iam_pb.S3ApiConfiguration, userName string) (resp DeleteUserResponse, err *IamError) { for i, ident := range s3cfg.Identities { if userName == ident.Name { s3cfg.Identities = append(s3cfg.Identities[:i], s3cfg.Identities[i+1:]...) return resp, nil } } - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf(USER_DOES_NOT_EXIST, userName)} } -func (iama *IamApiServer) GetUser(s3cfg *iam_pb.S3ApiConfiguration, userName string) (resp GetUserResponse, err error) { +func (iama *IamApiServer) GetUser(s3cfg *iam_pb.S3ApiConfiguration, userName string) (resp GetUserResponse, err *IamError) { for _, ident := range s3cfg.Identities { if userName == ident.Name { resp.GetUserResult.User = iam.User{UserName: &ident.Name} return resp, nil } } - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf(USER_DOES_NOT_EXIST, userName)} } -func (iama *IamApiServer) UpdateUser(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp UpdateUserResponse, err error) { +func (iama *IamApiServer) UpdateUser(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp UpdateUserResponse, err *IamError) { userName := values.Get("UserName") newUserName := values.Get("NewUserName") if newUserName != "" { @@ -186,7 +190,7 @@ func (iama *IamApiServer) UpdateUser(s3cfg *iam_pb.S3ApiConfiguration, values ur } else { return resp, nil } - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf(USER_DOES_NOT_EXIST, userName)} } func GetPolicyDocument(policy *string) (policyDocument PolicyDocument, err error) { @@ -196,12 +200,12 @@ func GetPolicyDocument(policy *string) (policyDocument PolicyDocument, err error return policyDocument, err } -func (iama *IamApiServer) CreatePolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp CreatePolicyResponse, err error) { +func (iama *IamApiServer) CreatePolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp CreatePolicyResponse, iamError *IamError) { policyName := values.Get("PolicyName") policyDocumentString := values.Get("PolicyDocument") policyDocument, err := GetPolicyDocument(&policyDocumentString) if err != nil { - return CreatePolicyResponse{}, err + return CreatePolicyResponse{}, &IamError{Code: iam.ErrCodeMalformedPolicyDocumentException, Error: err} } policyId := Hash(&policyDocumentString) arn := fmt.Sprintf("arn:aws:iam:::policy/%s", policyName) @@ -212,26 +216,36 @@ func (iama *IamApiServer) CreatePolicy(s3cfg *iam_pb.S3ApiConfiguration, values policyLock.Lock() defer policyLock.Unlock() if err = iama.s3ApiConfig.GetPolicies(&policies); err != nil { - return resp, err + return resp, &IamError{Code: iam.ErrCodeServiceFailureException, Error: err} } policies.Policies[policyName] = policyDocument if err = iama.s3ApiConfig.PutPolicies(&policies); err != nil { - return resp, err + return resp, &IamError{Code: iam.ErrCodeServiceFailureException, Error: err} } return resp, nil } +type IamError struct { + Code string + Error error +} + // https://docs.aws.amazon.com/IAM/latest/APIReference/API_PutUserPolicy.html -func (iama *IamApiServer) PutUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp PutUserPolicyResponse, err error) { +func (iama *IamApiServer) PutUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp PutUserPolicyResponse, iamError *IamError) { userName := values.Get("UserName") policyName := values.Get("PolicyName") policyDocumentString := values.Get("PolicyDocument") policyDocument, err := GetPolicyDocument(&policyDocumentString) if err != nil { - return PutUserPolicyResponse{}, err + return PutUserPolicyResponse{}, &IamError{Code: iam.ErrCodeMalformedPolicyDocumentException, Error: err} } policyDocuments[policyName] = &policyDocument - actions := GetActions(&policyDocument) + actions, err := GetActions(&policyDocument) + if err != nil { + return PutUserPolicyResponse{}, &IamError{Code: iam.ErrCodeMalformedPolicyDocumentException, Error: err} + } + // Log the actions + glog.V(3).Infof("PutUserPolicy: actions=%v", actions) for _, ident := range s3cfg.Identities { if userName != ident.Name { continue @@ -239,10 +253,10 @@ func (iama *IamApiServer) PutUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values ident.Actions = actions return resp, nil } - return resp, fmt.Errorf("%s: the user with name %s cannot be found", iam.ErrCodeNoSuchEntityException, userName) + return PutUserPolicyResponse{}, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf("the user with name %s cannot be found", userName)} } -func (iama *IamApiServer) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp GetUserPolicyResponse, err error) { +func (iama *IamApiServer) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp GetUserPolicyResponse, err *IamError) { userName := values.Get("UserName") policyName := values.Get("PolicyName") for _, ident := range s3cfg.Identities { @@ -253,7 +267,7 @@ func (iama *IamApiServer) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values resp.GetUserPolicyResult.UserName = userName resp.GetUserPolicyResult.PolicyName = policyName if len(ident.Actions) == 0 { - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: errors.New("no actions found")} } policyDocument := PolicyDocument{Version: policyDocumentVersion} @@ -293,10 +307,10 @@ func (iama *IamApiServer) GetUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values resp.GetUserPolicyResult.PolicyDocument = policyDocument.String() return resp, nil } - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf(USER_DOES_NOT_EXIST, userName)} } -func (iama *IamApiServer) DeleteUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp PutUserPolicyResponse, err error) { +func (iama *IamApiServer) DeleteUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp PutUserPolicyResponse, err *IamError) { userName := values.Get("UserName") for i, ident := range s3cfg.Identities { if ident.Name == userName { @@ -304,27 +318,27 @@ func (iama *IamApiServer) DeleteUserPolicy(s3cfg *iam_pb.S3ApiConfiguration, val return resp, nil } } - return resp, fmt.Errorf(iam.ErrCodeNoSuchEntityException) + return resp, &IamError{Code: iam.ErrCodeNoSuchEntityException, Error: fmt.Errorf(USER_DOES_NOT_EXIST, userName)} } -func GetActions(policy *PolicyDocument) (actions []string) { +func GetActions(policy *PolicyDocument) ([]string, error) { + var actions []string + for _, statement := range policy.Statement { if statement.Effect != "Allow" { - continue + return nil, fmt.Errorf("not a valid effect: '%s'. Only 'Allow' is possible", statement.Effect) } for _, resource := range statement.Resource { // Parse "arn:aws:s3:::my-bucket/shared/*" res := strings.Split(resource, ":") if len(res) != 6 || res[0] != "arn" || res[1] != "aws" || res[2] != "s3" { - glog.Infof("not match resource: %s", res) - continue + return nil, fmt.Errorf("not a valid resource: '%s'. Expected prefix 'arn:aws:s3'", res) } for _, action := range statement.Action { // Parse "s3:Get*" act := strings.Split(action, ":") if len(act) != 2 || act[0] != "s3" { - glog.Infof("not match action: %s", act) - continue + return nil, fmt.Errorf("not a valid action: '%s'. Expected prefix 's3:'", act) } statementAction := MapToStatementAction(act[1]) if res[5] == "*" { @@ -341,7 +355,7 @@ func GetActions(policy *PolicyDocument) (actions []string) { } } } - return actions + return actions, nil } func (iama *IamApiServer) CreateAccessKey(s3cfg *iam_pb.S3ApiConfiguration, values url.Values) (resp CreateAccessKeyResponse) { @@ -438,7 +452,7 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { glog.V(4).Infof("DoActions: %+v", values) var response interface{} - var err error + var iamError *IamError changed := true switch r.Form.Get("Action") { case "ListUsers": @@ -452,24 +466,24 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { response = iama.CreateUser(s3cfg, values) case "GetUser": userName := values.Get("UserName") - response, err = iama.GetUser(s3cfg, userName) - if err != nil { - writeIamErrorResponse(w, r, err, "user", userName, nil) + response, iamError = iama.GetUser(s3cfg, userName) + if iamError != nil { + writeIamErrorResponse(w, r, iamError) return } changed = false case "UpdateUser": - response, err = iama.UpdateUser(s3cfg, values) - if err != nil { - glog.Errorf("UpdateUser: %+v", err) + response, iamError = iama.UpdateUser(s3cfg, values) + if iamError != nil { + glog.Errorf("UpdateUser: %+v", iamError.Error) s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) return } case "DeleteUser": userName := values.Get("UserName") - response, err = iama.DeleteUser(s3cfg, userName) - if err != nil { - writeIamErrorResponse(w, r, err, "user", userName, nil) + response, iamError = iama.DeleteUser(s3cfg, userName) + if iamError != nil { + writeIamErrorResponse(w, r, iamError) return } case "CreateAccessKey": @@ -479,29 +493,31 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { handleImplicitUsername(r, values) response = iama.DeleteAccessKey(s3cfg, values) case "CreatePolicy": - response, err = iama.CreatePolicy(s3cfg, values) - if err != nil { - glog.Errorf("CreatePolicy: %+v", err) + response, iamError = iama.CreatePolicy(s3cfg, values) + if iamError != nil { + glog.Errorf("CreatePolicy: %+v", iamError.Error) s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) return } case "PutUserPolicy": - response, err = iama.PutUserPolicy(s3cfg, values) - if err != nil { - glog.Errorf("PutUserPolicy: %+v", err) - s3err.WriteErrorResponse(w, r, s3err.ErrInvalidRequest) + var iamError *IamError + response, iamError = iama.PutUserPolicy(s3cfg, values) + if iamError != nil { + glog.Errorf("PutUserPolicy: %+v", iamError.Error) + + writeIamErrorResponse(w, r, iamError) return } case "GetUserPolicy": - response, err = iama.GetUserPolicy(s3cfg, values) - if err != nil { - writeIamErrorResponse(w, r, err, "user", values.Get("UserName"), nil) + response, iamError = iama.GetUserPolicy(s3cfg, values) + if iamError != nil { + writeIamErrorResponse(w, r, iamError) return } changed = false case "DeleteUserPolicy": - if response, err = iama.DeleteUserPolicy(s3cfg, values); err != nil { - writeIamErrorResponse(w, r, err, "user", values.Get("UserName"), nil) + if response, iamError = iama.DeleteUserPolicy(s3cfg, values); iamError != nil { + writeIamErrorResponse(w, r, iamError) return } default: @@ -515,7 +531,8 @@ func (iama *IamApiServer) DoActions(w http.ResponseWriter, r *http.Request) { if changed { err := iama.s3ApiConfig.PutS3ApiConfiguration(s3cfg) if err != nil { - writeIamErrorResponse(w, r, fmt.Errorf(iam.ErrCodeServiceFailureException), "", "", err) + var iamError = IamError{Code: iam.ErrCodeServiceFailureException, Error: err} + writeIamErrorResponse(w, r, &iamError) return } } diff --git a/weed/iamapi/iamapi_test.go b/weed/iamapi/iamapi_test.go index f32e1ac51..067209eb5 100644 --- a/weed/iamapi/iamapi_test.go +++ b/weed/iamapi/iamapi_test.go @@ -5,6 +5,7 @@ import ( "net/http" "net/http/httptest" "net/url" + "regexp" "testing" "github.com/aws/aws-sdk-go/aws" @@ -152,6 +153,53 @@ func TestPutUserPolicy(t *testing.T) { assert.Equal(t, http.StatusOK, response.Code) } +func TestPutUserPolicyError(t *testing.T) { + userName := aws.String("InvalidUser") + params := &iam.PutUserPolicyInput{ + UserName: userName, + PolicyName: aws.String("S3-read-only-example-bucket"), + PolicyDocument: aws.String( + `{ + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Action": [ + "s3:Get*", + "s3:List*" + ], + "Resource": [ + "arn:aws:s3:::EXAMPLE-BUCKET", + "arn:aws:s3:::EXAMPLE-BUCKET/*" + ] + } + ] + }`), + } + req, _ := iam.New(session.New()).PutUserPolicyRequest(params) + _ = req.Build() + response, err := executeRequest(req.HTTPRequest, nil) + assert.Equal(t, nil, err) + assert.Equal(t, http.StatusNotFound, response.Code) + + expectedMessage := "the user with name InvalidUser cannot be found" + expectedCode := "NoSuchEntity" + + code, message := extractErrorCodeAndMessage(response) + + assert.Equal(t, expectedMessage, message) + assert.Equal(t, expectedCode, code) +} + +func extractErrorCodeAndMessage(response *httptest.ResponseRecorder) (string, string) { + pattern := `(.*)(.*)(.*)` + re := regexp.MustCompile(pattern) + + code := re.FindStringSubmatch(response.Body.String())[1] + message := re.FindStringSubmatch(response.Body.String())[2] + return code, message +} + func TestGetUserPolicy(t *testing.T) { userName := aws.String("Test") params := &iam.GetUserPolicyInput{UserName: userName, PolicyName: aws.String("S3-read-only-example-bucket")} diff --git a/weed/s3api/auth_credentials_test.go b/weed/s3api/auth_credentials_test.go index 1d9f1a95f..dbc431332 100644 --- a/weed/s3api/auth_credentials_test.go +++ b/weed/s3api/auth_credentials_test.go @@ -80,8 +80,10 @@ func TestCanDo(t *testing.T) { } // object specific assert.Equal(t, true, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d.txt")) + assert.Equal(t, true, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/c/d/e.txt")) assert.Equal(t, false, ident1.canDo(ACTION_DELETE_BUCKET, "bucket1", "")) assert.Equal(t, false, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/other/some"), "action without *") + assert.Equal(t, false, ident1.canDo(ACTION_WRITE, "bucket1", "/a/b/*"), "action on parent directory") // bucket specific ident2 := &Identity{