diff --git a/iamapi/authorization_test.go b/iamapi/authorization_test.go index 60730a78..1d5ca44b 100644 --- a/iamapi/authorization_test.go +++ b/iamapi/authorization_test.go @@ -398,6 +398,30 @@ func TestVerifyIAMPolicyGetUserSelfLookupResourceScoped(t *testing.T) { "User: arn:aws:iam::000000000000:user/ivy is not authorized to perform: iam:GetUser because no identity-based policy allows the iam:GetUser action") } +// TestVerifyIAMPolicyAccessKeyImplicitUserNameResourceScoped covers the +// resource-level check for the access-key actions in their omitted-UserName +// form: with no UserName to resolve, the target must still be the caller's +// own user ARN, so a policy scoped to that ARN authorizes the call and one +// scoped to somebody else's does not. +func TestVerifyIAMPolicyAccessKeyImplicitUserNameResourceScoped(t *testing.T) { + server := newIAMControllerTestServer(t) + + rosaAccessKeyID, rosaSecret := createTestUserWithAccessKey(t, server, "rosa", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:ListAccessKeys","Resource":"arn:aws:iam::000000000000:user/rosa"}]}`) + + resp := doSignedIAMActionAs(t, server, rosaAccessKeyID, rosaSecret, "", url.Values{"Action": {"ListAccessKeys"}}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("ListAccessKeys(self) status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + + samAccessKeyID, samSecret := createTestUserWithAccessKey(t, server, "sam", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:ListAccessKeys","Resource":"arn:aws:iam::000000000000:user/rosa"}]}`) + + resp = doSignedIAMActionAs(t, server, samAccessKeyID, samSecret, "", url.Values{"Action": {"ListAccessKeys"}}) + requireIAMError(t, resp, http.StatusForbidden, "Sender", "AccessDenied", + "User: arn:aws:iam::000000000000:user/sam is not authorized to perform: iam:ListAccessKeys because no identity-based policy allows the iam:ListAccessKeys action") +} + // TestVerifyIAMPolicyGetAccessKeyLastUsedResourceScoped guards against // GetAccessKeyLastUsed (which carries only AccessKeyId, never UserName) // falling back to "*" instead of resolving the queried key's owning user: diff --git a/iamapi/controller.go b/iamapi/controller.go index 672ba29f..4b9ed6b1 100644 --- a/iamapi/controller.go +++ b/iamapi/controller.go @@ -133,7 +133,7 @@ func (c IAMApiController) GetUser(ctx fiber.Ctx) (*Response, error) { Result: types.GetUserResult{User: *identity.User}, }}, nil default: - return nil, iamerr.ValidationError("Must specify userName when calling with non-User credentials") + return nil, iamerr.MustSpecifyUserName() } } if err := iamutil.ValidateName("userName", username, iamutil.MaxUserLookupLen); err != nil { @@ -318,7 +318,7 @@ func (c IAMApiController) ListUserTags(ctx fiber.Ctx) (*Response, error) { } func (c IAMApiController) CreateAccessKey(ctx fiber.Ctx) (*Response, error) { - userName, err := iamutil.GetUserName(ctx, "CreateAccessKey", iamutil.MaxUserLookupLen, iamerr.MissingParameter("UserName")) + userName, err := iamutil.GetUserNameOrCaller(ctx, "CreateAccessKey", iamutil.MaxUserLookupLen) if err != nil { return nil, err } @@ -362,7 +362,7 @@ func (c IAMApiController) CreateAccessKey(ctx fiber.Ctx) (*Response, error) { } func (c IAMApiController) UpdateAccessKey(ctx fiber.Ctx) (*Response, error) { - userName, err := iamutil.GetUserName(ctx, "UpdateAccessKey", iamutil.MaxUserLookupLen, iamerr.MissingParameter("UserName")) + userName, err := iamutil.GetUserNameOrCaller(ctx, "UpdateAccessKey", iamutil.MaxUserLookupLen) if err != nil { return nil, err } @@ -398,7 +398,7 @@ func (c IAMApiController) UpdateAccessKey(ctx fiber.Ctx) (*Response, error) { } func (c IAMApiController) DeleteAccessKey(ctx fiber.Ctx) (*Response, error) { - userName, err := iamutil.GetUserName(ctx, "DeleteAccessKey", iamutil.MaxUserLookupLen, iamerr.MissingParameter("UserName")) + userName, err := iamutil.GetUserNameOrCaller(ctx, "DeleteAccessKey", iamutil.MaxUserLookupLen) if err != nil { return nil, err } @@ -463,7 +463,7 @@ func (c IAMApiController) GetAccessKeyLastUsed(ctx fiber.Ctx) (*Response, error) } func (c IAMApiController) ListAccessKeys(ctx fiber.Ctx) (*Response, error) { - userName, err := iamutil.GetUserName(ctx, "ListAccessKeys", iamutil.MaxUserLookupLen, iamerr.MissingParameter("UserName")) + userName, err := iamutil.GetUserNameOrCaller(ctx, "ListAccessKeys", iamutil.MaxUserLookupLen) if err != nil { return nil, err } diff --git a/iamapi/controller_test.go b/iamapi/controller_test.go index 08198c5c..86ceed67 100644 --- a/iamapi/controller_test.go +++ b/iamapi/controller_test.go @@ -236,6 +236,157 @@ func TestIAMApiControllerGetUserSelfLookupSessionRejected(t *testing.T) { "Must specify userName when calling with non-User credentials") } +// TestIAMApiControllerAccessKeyImplicitUserName walks a user through the +// whole access-key lifecycle without ever naming itself: real IAM resolves +// the UserName from the access key signing the request. +func TestIAMApiControllerAccessKeyImplicitUserName(t *testing.T) { + server := newIAMControllerTestServer(t) + accessKeyID, secret := createTestUserWithAccessKey(t, server, "kate", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`) + + resp := doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{"Action": {"CreateAccessKey"}}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("CreateAccessKey status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + var createOut iamtypes.CreateAccessKeyResponse + unmarshalXML(t, readBody(t, resp), &createOut) + created := createOut.Result.AccessKey + if created.UserName != "kate" { + t.Fatalf("CreateAccessKey UserName = %q, want the calling user %q", created.UserName, "kate") + } + + resp = doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{ + "Action": {"UpdateAccessKey"}, + "AccessKeyId": {created.AccessKeyId}, + "Status": {"Inactive"}, + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("UpdateAccessKey status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + + resp = doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{"Action": {"ListAccessKeys"}}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("ListAccessKeys status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + var listOut iamtypes.ListAccessKeysResponse + unmarshalXML(t, readBody(t, resp), &listOut) + keys := listOut.Result.AccessKeyMetadata.Members + if len(keys) != 2 { + t.Fatalf("ListAccessKeys returned %d keys, want the caller's own 2", len(keys)) + } + for _, key := range keys { + if key.UserName != "kate" { + t.Fatalf("ListAccessKeys returned a key owned by %q, want only %q's", key.UserName, "kate") + } + if key.AccessKeyId == created.AccessKeyId && key.Status != iamutil.AccessKeyStatusInactive { + t.Fatalf("UpdateAccessKey left key %s Status = %q, want %q", key.AccessKeyId, key.Status, iamutil.AccessKeyStatusInactive) + } + } + + resp = doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{ + "Action": {"DeleteAccessKey"}, + "AccessKeyId": {created.AccessKeyId}, + }) + if resp.StatusCode != http.StatusOK { + t.Fatalf("DeleteAccessKey status = %d, body=%s", resp.StatusCode, readBody(t, resp)) + } + + resp = doIAMAction(t, server, url.Values{"Action": {"ListAccessKeys"}, "UserName": {"kate"}}) + var remainingOut iamtypes.ListAccessKeysResponse + unmarshalXML(t, readBody(t, resp), &remainingOut) + remaining := remainingOut.Result.AccessKeyMetadata.Members + if len(remaining) != 1 || remaining[0].AccessKeyId == created.AccessKeyId { + t.Fatalf("after implicit DeleteAccessKey kate has %#v, want only her original key", remaining) + } +} + +// TestIAMApiControllerAccessKeyImplicitUserNameScopedToCaller confirms the +// inferred UserName is the caller's own and nothing else: another user's +// access key is simply not found, rather than being updated or deleted. +func TestIAMApiControllerAccessKeyImplicitUserNameScopedToCaller(t *testing.T) { + server := newIAMControllerTestServer(t) + accessKeyID, secret := createTestUserWithAccessKey(t, server, "liam", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`) + otherAccessKeyID, _ := createTestUserWithAccessKey(t, server, "mona", "") + + resp := doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{ + "Action": {"UpdateAccessKey"}, + "AccessKeyId": {otherAccessKeyID}, + "Status": {"Inactive"}, + }) + requireIAMError(t, resp, http.StatusNotFound, "Sender", "NoSuchEntity", + "The Access Key with id "+otherAccessKeyID+" cannot be found") + + resp = doSignedIAMActionAs(t, server, accessKeyID, secret, "", url.Values{ + "Action": {"DeleteAccessKey"}, + "AccessKeyId": {otherAccessKeyID}, + }) + requireIAMError(t, resp, http.StatusNotFound, "Sender", "NoSuchEntity", + "The Access Key with id "+otherAccessKeyID+" cannot be found") + + resp = doIAMAction(t, server, url.Values{"Action": {"ListAccessKeys"}, "UserName": {"mona"}}) + var listOut iamtypes.ListAccessKeysResponse + unmarshalXML(t, readBody(t, resp), &listOut) + if len(listOut.Result.AccessKeyMetadata.Members) != 1 || + listOut.Result.AccessKeyMetadata.Members[0].Status != iamutil.AccessKeyStatusActive { + t.Fatalf("mona's keys = %#v, want her single key left Active", listOut.Result.AccessKeyMetadata.Members) + } +} + +// TestIAMApiControllerAccessKeyImplicitUserNameSessionRejected confirms an +// assumed-role session — which has no IAM user of its own to infer — gets +// IAM's own ValidationError rather than having the action attributed to +// some arbitrary user. +func TestIAMApiControllerAccessKeyImplicitUserNameSessionRejected(t *testing.T) { + server := newIAMControllerTestServer(t) + session := createTestSession(t, server, "role-accesskey", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`, "") + + for _, action := range accessKeyImplicitUserNameActions { + t.Run(action, func(t *testing.T) { + resp := doSignedIAMActionAs(t, server, session.AccessKeyId, session.SecretAccessKey, session.SessionToken, + accessKeyActionParams(action)) + requireIAMError(t, resp, http.StatusBadRequest, "Sender", "ValidationError", + "Must specify userName when calling with non-User credentials") + }) + } +} + +// TestIAMApiControllerAccessKeyImplicitUserNameRootRejected covers the +// gateway's root credential, which is a configured key rather than a stored +// IAM user and so owns no access keys the IAM API could manage: there is +// nothing to infer, so UserName stays required. +func TestIAMApiControllerAccessKeyImplicitUserNameRootRejected(t *testing.T) { + server := newIAMControllerTestServer(t) + + for _, action := range accessKeyImplicitUserNameActions { + t.Run(action, func(t *testing.T) { + resp := doIAMAction(t, server, accessKeyActionParams(action)) + requireIAMError(t, resp, http.StatusBadRequest, "Sender", "ValidationError", + "Must specify userName when calling with non-User credentials") + }) + } +} + +// TestIAMApiControllerAccessKeyEmptyUserNameRejected confirms only an +// entirely absent UserName is inferred: sending the parameter with an empty +// value is a validation failure, not a request to act on the caller. +func TestIAMApiControllerAccessKeyEmptyUserNameRejected(t *testing.T) { + server := newIAMControllerTestServer(t) + accessKeyID, secret := createTestUserWithAccessKey(t, server, "nate", + `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`) + + for _, action := range accessKeyImplicitUserNameActions { + t.Run(action, func(t *testing.T) { + params := accessKeyActionParams(action) + params.Set("UserName", "") + resp := doSignedIAMActionAs(t, server, accessKeyID, secret, "", params) + requireIAMError(t, resp, http.StatusBadRequest, "Sender", "ValidationError", + "The specified value for userName is invalid. It must contain only alphanumeric characters and/or the following: +=,.@_-") + }) + } +} + func TestIAMApiControllerCreateUserValidationErrors(t *testing.T) { tests := []struct { name string @@ -3481,6 +3632,25 @@ func TestIAMApiControllerCreateOIDCProviderAutoFetchSSRFGuard(t *testing.T) { "Could not connect to https://127.0.0.1") } +// accessKeyImplicitUserNameActions are the four access-key actions that +// accept an omitted UserName and infer it from the calling access key. +var accessKeyImplicitUserNameActions = []string{"CreateAccessKey", "UpdateAccessKey", "DeleteAccessKey", "ListAccessKeys"} + +// accessKeyActionParams builds a request for action carrying every +// parameter but UserName, so a test can exercise the omitted-UserName path +// without each case repeating the action's other required parameters. +func accessKeyActionParams(action string) url.Values { + params := url.Values{"Action": {action}} + switch action { + case "UpdateAccessKey": + params.Set("AccessKeyId", "AKIAIOSFODNN7EXAMPLE") + params.Set("Status", "Inactive") + case "DeleteAccessKey": + params.Set("AccessKeyId", "AKIAIOSFODNN7EXAMPLE") + } + return params +} + func newIAMControllerTestServer(t *testing.T) *IAMApiServer { t.Helper() diff --git a/iamapi/iamerr/errors.go b/iamapi/iamerr/errors.go index bf6831ca..20b409eb 100644 --- a/iamapi/iamerr/errors.go +++ b/iamapi/iamerr/errors.go @@ -435,6 +435,10 @@ func InvalidUserName(field string) Error { return ValidationError(fmt.Sprintf("The specified value for %s is invalid. It must contain only alphanumeric characters and/or the following: +=,.@_-", field)) } +func MustSpecifyUserName() Error { + return ValidationError("Must specify userName when calling with non-User credentials") +} + func UserNameTooLong(field string, maxLength int) Error { return ValidationError(fmt.Sprintf("1 validation error detected: Value at '%s' failed to satisfy constraint: Member must have length less than or equal to %d", field, maxLength)) } diff --git a/iamapi/internal/iammiddleware/policy.go b/iamapi/internal/iammiddleware/policy.go index 6747d604..b5c1f36c 100644 --- a/iamapi/internal/iammiddleware/policy.go +++ b/iamapi/internal/iammiddleware/policy.go @@ -170,11 +170,10 @@ func resourceForAction(ctx fiber.Ctx, store iamutil.IdentityStore, action string switch action { case "CreateUser": return newUserResource(ctx), nil - case "GetUser": - return getUserResource(ctx, store) - case "DeleteUser", "UpdateUser", "CreateAccessKey", "UpdateAccessKey", "DeleteAccessKey", - "ListAccessKeys", "PutUserPolicy", "GetUserPolicy", "DeleteUserPolicy", "ListUserPolicies", - "TagUser", "UntagUser", "ListUserTags": + case "GetUser", "CreateAccessKey", "UpdateAccessKey", "DeleteAccessKey", "ListAccessKeys": + return callerOrNamedUserResource(ctx, store) + case "DeleteUser", "UpdateUser", "PutUserPolicy", "GetUserPolicy", "DeleteUserPolicy", + "ListUserPolicies", "TagUser", "UntagUser", "ListUserTags": return existingUserResource(ctx, store) case "GetAccessKeyLastUsed": return accessKeyOwnerResource(ctx, store) @@ -231,13 +230,14 @@ func existingUserResource(ctx fiber.Ctx, store iamutil.IdentityStore) (string, [ return user.Arn, user.Tags } -// getUserResource resolves GetUser's target: the named user's stored Arn and -// Tags, or — when UserName is omitted, matching the controller's (and real -// IAM's) "look up the caller's own identity" behavior — the calling user's -// own Arn and Tags. A session (assumed role) has no self IAM user to -// resolve, so it falls back to ("", nil), the same lookup-failure fallback -// used elsewhere. -func getUserResource(ctx fiber.Ctx, store iamutil.IdentityStore) (string, []types.Tag) { +// callerOrNamedUserResource resolves the target of the actions that accept +// an omitted UserName — GetUser and the four access-key APIs: the named +// user's stored Arn and Tags, or, when UserName is left out, the calling +// user's own Arn and Tags, matching the controllers' (and real IAM's) +// "operate on the caller's own identity" behavior. A caller with no IAM +// user of its own (a session, or root) has nothing to resolve, so it falls +// back to ("", nil), the same lookup-failure fallback used elsewhere. +func callerOrNamedUserResource(ctx fiber.Ctx, store iamutil.IdentityStore) (string, []types.Tag) { userName, ok := iamutil.RequestParam(ctx, "UserName") if !ok || userName == "" { identity, _ := httpctx.ContextKeyCallerIdentity.Get(ctx).(types.Identity) diff --git a/iamapi/internal/iamutil/user.go b/iamapi/internal/iamutil/user.go index c746811b..cd3ca86c 100644 --- a/iamapi/internal/iamutil/user.go +++ b/iamapi/internal/iamutil/user.go @@ -26,6 +26,7 @@ import ( "github.com/versity/versitygw/debuglogger" "github.com/versity/versitygw/iamapi/iamerr" "github.com/versity/versitygw/iamapi/types" + "github.com/versity/versitygw/internal/httpctx" ) const ( @@ -114,6 +115,33 @@ func GetUserName(ctx fiber.Ctx, operation string, maxLen int, missingErr error) return userName, nil } +// GetUserNameOrCaller resolves the UserName request parameter like +// GetUserName, except that an omitted parameter resolves to the calling +// user's own name instead of being an error — matching real IAM, which +// infers the user from the access key signing the request when UserName is +// left out. +// +// Only an entirely absent parameter is inferred. A UserName that is present +// but empty stays a ValidateName rejection, as on real IAM, so a client that +// sends the parameter with no value is told the value is invalid rather than +// silently acting on a different user than it named. +func GetUserNameOrCaller(ctx fiber.Ctx, operation string, maxLen int) (string, error) { + userName, ok := RequestParam(ctx, "UserName") + if !ok { + identity, _ := httpctx.ContextKeyCallerIdentity.Get(ctx).(types.Identity) + if identity.User == nil { + debuglogger.Logf("%s omitted UserName with credentials that have no IAM user", operation) + return "", iamerr.MustSpecifyUserName() + } + return identity.User.UserName, nil + } + if err := ValidateName("userName", userName, maxLen); err != nil { + return "", err + } + + return userName, nil +} + // GetRoleName resolves the RoleName request parameter and validates it // against maxLen, returning missingErr if the parameter is absent or empty. func GetRoleName(ctx fiber.Ctx, operation string, maxLen int, missingErr error) (string, error) { diff --git a/tests/integration/group-tests.go b/tests/integration/group-tests.go index 58ad7627..af8cbebc 100644 --- a/tests/integration/group-tests.go +++ b/tests/integration/group-tests.go @@ -1243,6 +1243,8 @@ func TestIAMListUserTags(ts *TestState) { func TestIAMCreateAccessKey(ts *TestState) { ts.Run(IAMCreateAccessKey_missing_user_name) + ts.Run(IAMCreateAccessKey_empty_user_name) + ts.Run(IAMCreateAccessKey_infers_caller_user_name) ts.Run(IAMCreateAccessKey_invalid_user_name) ts.Run(IAMCreateAccessKey_long_user_name) ts.Run(IAMCreateAccessKey_non_existing_user) @@ -1252,6 +1254,8 @@ func TestIAMCreateAccessKey(ts *TestState) { func TestIAMUpdateAccessKey(ts *TestState) { ts.Run(IAMUpdateAccessKey_missing_user_name) + ts.Run(IAMUpdateAccessKey_infers_caller_user_name) + ts.Run(IAMUpdateAccessKey_inferred_user_name_scoped_to_caller) ts.Run(IAMUpdateAccessKey_invalid_user_name) ts.Run(IAMUpdateAccessKey_long_user_name) ts.Run(IAMUpdateAccessKey_missing_access_key_id) @@ -1267,6 +1271,8 @@ func TestIAMUpdateAccessKey(ts *TestState) { func TestIAMDeleteAccessKey(ts *TestState) { ts.Run(IAMDeleteAccessKey_missing_user_name) + ts.Run(IAMDeleteAccessKey_infers_caller_user_name) + ts.Run(IAMDeleteAccessKey_inferred_user_name_scoped_to_caller) ts.Run(IAMDeleteAccessKey_invalid_user_name) ts.Run(IAMDeleteAccessKey_long_user_name) ts.Run(IAMDeleteAccessKey_missing_access_key_id) @@ -1289,6 +1295,7 @@ func TestIAMGetAccessKeyLastUsed(ts *TestState) { func TestIAMListAccessKeys(ts *TestState) { ts.Run(IAMListAccessKeys_missing_user_name) + ts.Run(IAMListAccessKeys_infers_caller_user_name) ts.Run(IAMListAccessKeys_invalid_user_name) ts.Run(IAMListAccessKeys_long_user_name) ts.Run(IAMListAccessKeys_invalid_max_items) @@ -2327,12 +2334,16 @@ func GetIntTests() IntTests { "IAMListUserTags_success": IAMListUserTags_success, "IAMListUserTags_pagination": IAMListUserTags_pagination, "IAMCreateAccessKey_missing_user_name": IAMCreateAccessKey_missing_user_name, + "IAMCreateAccessKey_empty_user_name": IAMCreateAccessKey_empty_user_name, + "IAMCreateAccessKey_infers_caller_user_name": IAMCreateAccessKey_infers_caller_user_name, "IAMCreateAccessKey_invalid_user_name": IAMCreateAccessKey_invalid_user_name, "IAMCreateAccessKey_long_user_name": IAMCreateAccessKey_long_user_name, "IAMCreateAccessKey_non_existing_user": IAMCreateAccessKey_non_existing_user, "IAMCreateAccessKey_limit_exceeded": IAMCreateAccessKey_limit_exceeded, "IAMCreateAccessKey_success": IAMCreateAccessKey_success, "IAMUpdateAccessKey_missing_user_name": IAMUpdateAccessKey_missing_user_name, + "IAMUpdateAccessKey_infers_caller_user_name": IAMUpdateAccessKey_infers_caller_user_name, + "IAMUpdateAccessKey_inferred_user_name_scoped_to_caller": IAMUpdateAccessKey_inferred_user_name_scoped_to_caller, "IAMUpdateAccessKey_invalid_user_name": IAMUpdateAccessKey_invalid_user_name, "IAMUpdateAccessKey_long_user_name": IAMUpdateAccessKey_long_user_name, "IAMUpdateAccessKey_missing_access_key_id": IAMUpdateAccessKey_missing_access_key_id, @@ -2345,6 +2356,8 @@ func GetIntTests() IntTests { "IAMUpdateAccessKey_non_existing_access_key": IAMUpdateAccessKey_non_existing_access_key, "IAMUpdateAccessKey_success": IAMUpdateAccessKey_success, "IAMDeleteAccessKey_missing_user_name": IAMDeleteAccessKey_missing_user_name, + "IAMDeleteAccessKey_infers_caller_user_name": IAMDeleteAccessKey_infers_caller_user_name, + "IAMDeleteAccessKey_inferred_user_name_scoped_to_caller": IAMDeleteAccessKey_inferred_user_name_scoped_to_caller, "IAMDeleteAccessKey_invalid_user_name": IAMDeleteAccessKey_invalid_user_name, "IAMDeleteAccessKey_long_user_name": IAMDeleteAccessKey_long_user_name, "IAMDeleteAccessKey_missing_access_key_id": IAMDeleteAccessKey_missing_access_key_id, @@ -2361,6 +2374,7 @@ func GetIntTests() IntTests { "IAMGetAccessKeyLastUsed_non_existing_access_key": IAMGetAccessKeyLastUsed_non_existing_access_key, "IAMGetAccessKeyLastUsed_success": IAMGetAccessKeyLastUsed_success, "IAMListAccessKeys_missing_user_name": IAMListAccessKeys_missing_user_name, + "IAMListAccessKeys_infers_caller_user_name": IAMListAccessKeys_infers_caller_user_name, "IAMListAccessKeys_invalid_user_name": IAMListAccessKeys_invalid_user_name, "IAMListAccessKeys_long_user_name": IAMListAccessKeys_long_user_name, "IAMListAccessKeys_invalid_max_items": IAMListAccessKeys_invalid_max_items, diff --git a/tests/integration/iam_create_access_key.go b/tests/integration/iam_create_access_key.go index 78155743..1daf00a2 100644 --- a/tests/integration/iam_create_access_key.go +++ b/tests/integration/iam_create_access_key.go @@ -17,8 +17,11 @@ package integration import ( "context" "fmt" + "net/http" + "net/url" "regexp" "strings" + "time" "github.com/aws/aws-sdk-go-v2/aws" awsmiddleware "github.com/aws/aws-sdk-go-v2/aws/middleware" @@ -29,11 +32,57 @@ import ( var integrationIAMAccessKeyIDPattern = regexp.MustCompile(`^AKIA[A-Z2-7]{17}$`) +// IAMCreateAccessKey_missing_user_name calls as root, which is a configured +// credential rather than a stored IAM user and so has no user name to infer +// — unlike a caller signing with an IAM user's own access key, covered by +// IAMCreateAccessKey_infers_caller_user_name. func IAMCreateAccessKey_missing_user_name(s *S3Conf) error { testName := "IAMCreateAccessKey_missing_user_name" return iamActionHandler(s, testName, func(client *iam.Client) error { _, err := createIAMAccessKey(client, &iam.CreateAccessKeyInput{}) - return checkIAMApiErr(err, iamerr.MissingParameter("UserName")) + return checkIAMApiErr(err, iamerr.MustSpecifyUserName()) + }) +} + +// IAMCreateAccessKey_empty_user_name confirms only an entirely absent +// UserName is inferred: sending the parameter with an empty value stays a +// validation failure rather than silently creating a key for the caller. +func IAMCreateAccessKey_empty_user_name(s *S3Conf) error { + testName := "IAMCreateAccessKey_empty_user_name" + body := []byte(url.Values{ + "Action": {"CreateAccessKey"}, + "Version": {"2010-05-08"}, + "UserName": {""}, + }.Encode()) + return authHandler(s, &authConfig{ + testName: testName, + method: http.MethodPost, + service: "iam", + region: iamAuthRegion, + body: body, + date: time.Now().UTC(), + headers: map[string]string{ + "Content-Type": "application/x-www-form-urlencoded", + }, + }, func(req *http.Request) error { + return checkIAMAuthRequest(s, req, iamerr.InvalidUserName("userName")) + }) +} + +func IAMCreateAccessKey_infers_caller_user_name(s *S3Conf) error { + testName := "IAMCreateAccessKey_infers_caller_user_name" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:CreateAccessKey") + if err != nil { + return err + } + defer cleanup() + + out, err := createIAMAccessKey(caller.client, &iam.CreateAccessKeyInput{}) + if err != nil { + return err + } + return checkCreateAccessKeyOutput(out, caller.userName) }) } @@ -149,3 +198,20 @@ func checkCreateAccessKeyOutput(out *iam.CreateAccessKeyOutput, userName string) return nil } + +// newAccessKeyCaller creates an IAM user with one access key and an inline +// policy granting actions on its own user ARN, plus an *iam.Client signing +// as that user — the fixture every "UserName inferred from the calling +// access key" test in the access-key files needs. Scoping the grant to the +// caller's own ARN (rather than "*") means a passing test also proves the +// inferred name reaches the resource-level authorization check, not just +// the controller. +func newAccessKeyCaller(root *iam.Client, s *S3Conf, actions ...string) (*accessControlCaller, func(), error) { + userName := newIAMUserName() + grant := policyDoc(accessStatement{ + Effect: "Allow", + Action: actions, + Resource: "arn:aws:iam::" + testAccountID + ":user/" + userName, + }) + return newAccessControlCaller(root, s, userName, map[string]string{"self-access-keys": grant}) +} diff --git a/tests/integration/iam_delete_access_key.go b/tests/integration/iam_delete_access_key.go index 36897bfb..3d8bf41d 100644 --- a/tests/integration/iam_delete_access_key.go +++ b/tests/integration/iam_delete_access_key.go @@ -16,6 +16,7 @@ package integration import ( "context" + "fmt" "net/http" "net/url" "strings" @@ -23,14 +24,89 @@ import ( "github.com/aws/aws-sdk-go-v2/aws" "github.com/aws/aws-sdk-go-v2/service/iam" + iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" "github.com/versity/versitygw/iamapi/iamerr" ) +// IAMDeleteAccessKey_missing_user_name calls as root, which is a configured +// credential rather than a stored IAM user and so has no user name to infer +// — unlike a caller signing with an IAM user's own access key, covered by +// IAMDeleteAccessKey_infers_caller_user_name. func IAMDeleteAccessKey_missing_user_name(s *S3Conf) error { testName := "IAMDeleteAccessKey_missing_user_name" return iamActionHandler(s, testName, func(client *iam.Client) error { err := deleteIAMAccessKey(client, "", genRandString(20)) - return checkIAMApiErr(err, iamerr.MissingParameter("UserName")) + return checkIAMApiErr(err, iamerr.MustSpecifyUserName()) + }) +} + +func IAMDeleteAccessKey_infers_caller_user_name(s *S3Conf) error { + testName := "IAMDeleteAccessKey_infers_caller_user_name" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:DeleteAccessKey") + if err != nil { + return err + } + defer cleanup() + + created, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &caller.userName}) + if err != nil { + return err + } + accessKeyID := aws.ToString(created.AccessKey.AccessKeyId) + + if err := deleteIAMAccessKey(caller.client, "", accessKeyID); err != nil { + return err + } + + listOut, err := listIAMAccessKeys(root, &iam.ListAccessKeysInput{UserName: &caller.userName}) + if err != nil { + return err + } + for _, key := range listOut.AccessKeyMetadata { + if aws.ToString(key.AccessKeyId) == accessKeyID { + return fmt.Errorf("expected access key %q to be deleted", accessKeyID) + } + } + return nil + }) +} + +// IAMDeleteAccessKey_inferred_user_name_scoped_to_caller confirms the +// inferred user name is the caller's own and nothing else: another user's +// access key is simply not found, rather than being deleted. +func IAMDeleteAccessKey_inferred_user_name_scoped_to_caller(s *S3Conf) error { + testName := "IAMDeleteAccessKey_inferred_user_name_scoped_to_caller" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:DeleteAccessKey") + if err != nil { + return err + } + defer cleanup() + + otherName := newIAMUserName() + if _, err := createIAMUser(root, &iam.CreateUserInput{UserName: &otherName}); err != nil { + return err + } + defer deleteIAMUserAndAccessKeys(root, otherName) + + otherKey, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &otherName}) + if err != nil { + return err + } + otherKeyID := aws.ToString(otherKey.AccessKey.AccessKeyId) + + err = deleteIAMAccessKey(caller.client, "", otherKeyID) + if err := checkIAMApiErr(err, iamerr.NoSuchEntityAccessKey(otherKeyID)); err != nil { + return err + } + + listOut, err := listIAMAccessKeys(root, &iam.ListAccessKeysInput{UserName: &otherName}) + if err != nil { + return err + } + return checkIAMListAccessKeys(listOut.AccessKeyMetadata, otherName, + map[string]iamtypes.StatusType{otherKeyID: iamtypes.StatusTypeActive}) }) } diff --git a/tests/integration/iam_list_access_keys.go b/tests/integration/iam_list_access_keys.go index 505158a0..dea4c138 100644 --- a/tests/integration/iam_list_access_keys.go +++ b/tests/integration/iam_list_access_keys.go @@ -31,11 +31,64 @@ import ( "github.com/versity/versitygw/iamapi/iamerr" ) +// IAMListAccessKeys_missing_user_name calls as root, which is a configured +// credential rather than a stored IAM user and so has no user name to infer +// — unlike a caller signing with an IAM user's own access key, covered by +// IAMListAccessKeys_infers_caller_user_name. func IAMListAccessKeys_missing_user_name(s *S3Conf) error { testName := "IAMListAccessKeys_missing_user_name" return iamActionHandler(s, testName, func(client *iam.Client) error { _, err := listIAMAccessKeys(client, &iam.ListAccessKeysInput{}) - return checkIAMApiErr(err, iamerr.MissingParameter("UserName")) + return checkIAMApiErr(err, iamerr.MustSpecifyUserName()) + }) +} + +// IAMListAccessKeys_infers_caller_user_name also covers the scoping the +// other inferred-user-name tests assert through a NoSuchEntity: a listing +// with no UserName returns the caller's own keys and only those, so a +// second user's key must not appear in it. +func IAMListAccessKeys_infers_caller_user_name(s *S3Conf) error { + testName := "IAMListAccessKeys_infers_caller_user_name" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:ListAccessKeys") + if err != nil { + return err + } + defer cleanup() + + if _, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &caller.userName}); err != nil { + return err + } + + otherName := newIAMUserName() + if _, err := createIAMUser(root, &iam.CreateUserInput{UserName: &otherName}); err != nil { + return err + } + defer deleteIAMUserAndAccessKeys(root, otherName) + if _, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &otherName}); err != nil { + return err + } + + callerKeys, err := listIAMAccessKeys(root, &iam.ListAccessKeysInput{UserName: &caller.userName}) + if err != nil { + return err + } + if len(callerKeys.AccessKeyMetadata) != 2 { + return fmt.Errorf("expected the caller to own 2 access keys, instead got %d", len(callerKeys.AccessKeyMetadata)) + } + expected := make(map[string]iamtypes.StatusType, len(callerKeys.AccessKeyMetadata)) + for _, key := range callerKeys.AccessKeyMetadata { + expected[aws.ToString(key.AccessKeyId)] = key.Status + } + + out, err := listIAMAccessKeys(caller.client, &iam.ListAccessKeysInput{}) + if err != nil { + return err + } + if out.IsTruncated { + return fmt.Errorf("expected ListAccessKeys not to be truncated") + } + return checkIAMListAccessKeys(out.AccessKeyMetadata, caller.userName, expected) }) } diff --git a/tests/integration/iam_update_access_key.go b/tests/integration/iam_update_access_key.go index 0170054b..e208c71e 100644 --- a/tests/integration/iam_update_access_key.go +++ b/tests/integration/iam_update_access_key.go @@ -29,6 +29,10 @@ import ( "github.com/versity/versitygw/iamapi/iamerr" ) +// IAMUpdateAccessKey_missing_user_name calls as root, which is a configured +// credential rather than a stored IAM user and so has no user name to infer +// — unlike a caller signing with an IAM user's own access key, covered by +// IAMUpdateAccessKey_infers_caller_user_name. func IAMUpdateAccessKey_missing_user_name(s *S3Conf) error { testName := "IAMUpdateAccessKey_missing_user_name" return iamActionHandler(s, testName, func(client *iam.Client) error { @@ -36,7 +40,87 @@ func IAMUpdateAccessKey_missing_user_name(s *S3Conf) error { AccessKeyId: aws.String(genRandString(20)), Status: iamtypes.StatusTypeActive, }) - return checkIAMApiErr(err, iamerr.MissingParameter("UserName")) + return checkIAMApiErr(err, iamerr.MustSpecifyUserName()) + }) +} + +func IAMUpdateAccessKey_infers_caller_user_name(s *S3Conf) error { + testName := "IAMUpdateAccessKey_infers_caller_user_name" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:UpdateAccessKey") + if err != nil { + return err + } + defer cleanup() + + created, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &caller.userName}) + if err != nil { + return err + } + accessKeyID := aws.ToString(created.AccessKey.AccessKeyId) + + if _, err := updateIAMAccessKey(caller.client, &iam.UpdateAccessKeyInput{ + AccessKeyId: &accessKeyID, + Status: iamtypes.StatusTypeInactive, + }); err != nil { + return err + } + + listOut, err := listIAMAccessKeys(root, &iam.ListAccessKeysInput{UserName: &caller.userName}) + if err != nil { + return err + } + for _, key := range listOut.AccessKeyMetadata { + want := iamtypes.StatusTypeActive + if aws.ToString(key.AccessKeyId) == accessKeyID { + want = iamtypes.StatusTypeInactive + } + if key.Status != want { + return fmt.Errorf("expected access key %q status %q, instead got %q", aws.ToString(key.AccessKeyId), want, key.Status) + } + } + return nil + }) +} + +// IAMUpdateAccessKey_inferred_user_name_scoped_to_caller confirms the +// inferred user name is the caller's own and nothing else: another user's +// access key is simply not found, rather than being updated. +func IAMUpdateAccessKey_inferred_user_name_scoped_to_caller(s *S3Conf) error { + testName := "IAMUpdateAccessKey_inferred_user_name_scoped_to_caller" + return iamActionHandler(s, testName, func(root *iam.Client) error { + caller, cleanup, err := newAccessKeyCaller(root, s, "iam:UpdateAccessKey") + if err != nil { + return err + } + defer cleanup() + + otherName := newIAMUserName() + if _, err := createIAMUser(root, &iam.CreateUserInput{UserName: &otherName}); err != nil { + return err + } + defer deleteIAMUserAndAccessKeys(root, otherName) + + otherKey, err := createIAMAccessKey(root, &iam.CreateAccessKeyInput{UserName: &otherName}) + if err != nil { + return err + } + otherKeyID := aws.ToString(otherKey.AccessKey.AccessKeyId) + + _, err = updateIAMAccessKey(caller.client, &iam.UpdateAccessKeyInput{ + AccessKeyId: &otherKeyID, + Status: iamtypes.StatusTypeInactive, + }) + if err := checkIAMApiErr(err, iamerr.NoSuchEntityAccessKey(otherKeyID)); err != nil { + return err + } + + listOut, err := listIAMAccessKeys(root, &iam.ListAccessKeysInput{UserName: &otherName}) + if err != nil { + return err + } + return checkIAMListAccessKeys(listOut.AccessKeyMetadata, otherName, + map[string]iamtypes.StatusType{otherKeyID: iamtypes.StatusTypeActive}) }) }