Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSFS | NC | IAM Service - Access Keys CRUD API Implementation #8119

Merged
merged 1 commit into from
Jun 23, 2024

Conversation

shirady
Copy link
Contributor

@shirady shirady commented Jun 9, 2024

Explain the changes

  1. Implement the functions related to access keys in AccountSpaceFS (CRUD).
  2. Add more properties to nsfs_account_schema inside access_key object (all of them are not required):
    creation_date = the date the access-key object was created.
    deactivated = boolean (we need to translate it to status which is 'Active' or 'Inactive').
  3. Clean the account cache after updating the account config using the IAM API.
  4. Remove unused errors that were copied from STS' errors in IamErrors.
  5. Change the param name from user_name to username in the ops.
  6. Remove the mock variables that we used in the IAM boilerplate implementation.
  7. Update unit test for access_keys function in accountspace_fs.
  8. Add unit test in account_schema_validation related to the schema changes.
  9. Update Get Started section that would be the demo for IAM access keys management (docs/dev_guide/nc_nsfs_iam_developer_doc.md).
  10. Changes in nc_master_key_manager to allow an access key object with more properties and add tests for the edited functions (encrypt_access_keys and decrypt_access_keys of account).
  11. Add the support for denying a request whose access key status is Inactive (in all services: S3, STS, IAM).
  12. When creating a new user instead of copying the master_key_id from the root account, we now take the active master_key_id.
  13. Add a new RPC code for a deactivated access key.
  14. Edit the function authorize_request_account_by_token adding the access key deactivated check, but also throwing errors on every missing component instead of using optional chaining.

Issues:

List of GAPs:

  1. Parsing and validating the params (for example: username).
  2. No pagination in the list_access_keys implementation.
  3. No NoobaaEvent at this point.
  4. In get_access_key_last_used we send dummy values (region, last_used_date, service_name).
  5. Change the IamError class to have a template message.

Testing Instructions:

Unit Tests

Please run:

  1. sudo npx jest test_accountspace_fs.test.js
  2. npx jest test_iam_utils.test.js
  3. npx jest test_nc_nsfs_account_schema_validation.test.js
  4. sudo npx jest test_nc_master_keys.test.js (the teardown removes what you have in /etc/noobaa.conf.d).

Manual Tests

IAM changes in NC NSFS
Currently, we do not validate the input, so the test should use only valid input.

  1. Create the root user account with the CLI: sudo node src/cmd/manage_nsfs account add --name shira-1002 --new_buckets_path /tmp/nsfs_root1 --access_key <access-key> --secret_key <secret-key> --uid <uid> --gid <gid>
    Note: before creating the account need to give permission to the new_buckets_path: chmod 777 /tmp/nsfs_root1.
  2. Start the NSFS server with: sudo node src/cmd/nsfs --debug 5 --https_port_iam 7005
    Note: before starting the server please add this line: process.env.NOOBAA_LOG_LEVEL = 'nsfs'; in the endpoint.js (before the condition if (process.env.NOOBAA_LOG_LEVEL) {)
  3. Create the alias for IAM service: alias s3-nc-user-1-iam='AWS_ACCESS_KEY_ID=<acess-key> AWS_SECRET_ACCESS_KEY=<secret-key> aws --no-verify-ssl --endpoint-url https://localhost:7005'.
  4. Use AWS CLI to send requests to the IAM service, for example:
    s3-nc-user-1-iam iam create-user --user-name Bob --path '/division_abc/subdivision_xyz/'
    s3-nc-user-1-iam iam create-access-key --user-name Bob
    s3-nc-user-1-iam iam get-access-key-last-used --access-key-id <access-key>
    s3-nc-user-1-iam iam update-access-key --access-key-id <access-key> --user-name Bob --status Inactive
    s3-nc-user-1-iam iam delete-access-key --access-key-id <access-key> --user-name Bob
    s3-nc-user-1-iam iam list-access-keys --user-name Bob

Other
Since I changed the function authorize_request_account_by_token I also tested it in a containerized environment with the operator.

  • Doc added/updated
  • Tests added

@shirady shirady self-assigned this Jun 9, 2024
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch from 5874f8a to c70583f Compare June 10, 2024 12:43
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please squash commits

src/sdk/accountspace_fs.js Outdated Show resolved Hide resolved
src/endpoint/iam/iam_utils.js Outdated Show resolved Hide resolved
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch from c70583f to 7aa9ded Compare June 13, 2024 06:34
@shirady
Copy link
Contributor Author

shirady commented Jun 13, 2024

please squash commits

@guymguym I will squash before merging (you can leave the request for changes until then).
Meanwhile, I think it lets everyone better see the changes.

@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 8 times, most recently from 1aa05e3 to 8520bd5 Compare June 16, 2024 13:02
@guymguym
Copy link
Member

@shirady please squash to be ready to merge

src/sdk/account_sdk.js Outdated Show resolved Hide resolved
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comments posted above

@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 4 times, most recently from 36e39fa to fda5b8d Compare June 17, 2024 08:37
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 5 times, most recently from ecffc64 to 238e76e Compare June 17, 2024 13:11
@shirady shirady marked this pull request as ready for review June 17, 2024 13:11
@shirady shirady requested review from guymguym and romayalon June 17, 2024 13:40
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 3 times, most recently from b846b7a to 74456d7 Compare June 18, 2024 10:15
Copy link
Member

@guymguym guymguym left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

posted some comments above. is the signature_utils change stable?

@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 2 times, most recently from 521a32e to b738275 Compare June 19, 2024 15:07
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch from b738275 to 4e8c0c0 Compare June 20, 2024 07:09
@shirady
Copy link
Contributor Author

shirady commented Jun 20, 2024

Testing Instructions - Manual Testing in Containerized Environment:

Background

Since I changed the function authorize_request_account_by_token I also tested it in a containerized environment with the operator.
Tested on the default backing store (in the local cluster it is PVC) using the first.bucket.

Debugging addition:

We added printings in signature_utils inside authorize_request_account_by_token, and also warp this line inside the RPC auth server check signature (here)

Requirements:

  1. Build the image and deploy noobaa using Rancher desktop (see guide).
    Notes:
  • nb is an alias that runs the local operator from build/_output/bin (alias created by devenv script).
  • The namespace noobaa is installed on is -n test1 in this example.
  1. (optional) User port-forward for the services (because of an issue I have on my MacOS M1):
  • kubectl port-forward -n test1 service/s3 12443:443 (12443 port in localhost for S3 service)
  • kubectl port-forward -n test1 service/sts 12444:443 (12444 port in localhost for STS service)
  1. Copy the noobaa admin access key and secret key from the output of nb status --show-secrets -n test1 (we will use it later)

S3 Test

  1. Create the alias for sending the S3 requests using the admin credentials (taken from previous step): alias s3-user-1='AWS_ACCESS_KEY_ID=<admin-access-key> AWS_SECRET_ACCESS_KEY=<admin-secret-key> aws --no-verify-ssl --endpoint-url https://localhost:12443'
  2. Send a request, for example, to list the buckets: s3-user-1 s3 ls (expected result is to see the first.bucket in the output and see the debug added printings).

STS test

  1. Create a new noobaa user without the ability to create buckets (username is user1 in this example):
    nb account create user1 -n test1 --allow_bucket_create=false –show-secrets.
    In the output you will see the access key and secret key of the user:
INFO[0000] ✅ Exists: NooBaaAccount "user1"
INFO[0000] ✅ NooBaaAccount "user1" Phase is Ready

# NooBaaAccount spec:
allow_bucket_creation: false
default_resource: noobaa-default-backing-store
force_md5_etag: false

INFO[0000] ✅ Exists: Secret "noobaa-account-user1"
Connection info:
  AWS_ACCESS_KEY_ID      : <user1 access key id>
  AWS_SECRET_ACCESS_KEY  : <user1 secret key id>
  1. Assign a new role policy to the user (username is user1 from the previous step and role name is my-role in this example):
    nb sts assign-role --email user1 --role_config '{"role_name": "my-role", "assume_role_policy": {"version": "2012-10-17", "statement": [ {"action": ["sts:AssumeRole"], "effect": "allow", "principal": ["*"]} ]}}' -n test1.
  2. Use the noobaa admin credentials to assume the role of the user (username is user1 , role name is my-role and seesion name is my-session in this example):
  • alias sts-user-1='AWS_ACCESS_KEY_ID=<admin-credentials> AWS_SECRET_ACCESS_KEY=<admin-credentials> aws --no-verify-ssl --endpoint-url https://localhost:12444' # 12444 for sts service
  • sts-user-1 sts assume-role --role-arn arn:aws:sts::<copy the access key of the assumed role>:role/my-role --role-session-name my-seesion, you will see an output like this (I removed the information):
{
   “Credentials”: {
       “AccessKeyId”: “<>”,
       “SecretAccessKey”: “<>”,
       “SessionToken”: “<>”,
       “Expiration”: “<>”
   },
   “AssumedRoleUser”: {
       “AssumedRoleId”: “<>:my-session”,
       “Arn”: “arn:aws:sts::<>:assumed-role/my-role/my-session”
   },
   “PackedPolicySize”: 0
}
  1. With the credentials that the CLI returned in the previous step:
  • alias s3-user-assumed='AWS_ACCESS_KEY_ID=<previous step> AWS_SECRET_ACCESS_KEY=<previous step> AWS_SESSION_TOKEN="<previous step>" aws --no-verify-ssl --endpoint-url https://localhost:12443/' # 12443 for s3 service
  • try to list the buckets (should succeed, see the first.bucket in the output): s3-user-assumed s3 ls.
  • try to create a new S3 bucket (should be blocked): S3-user-assumed s3 mb:test-bucket
    output:
make_bucket failed: s3://test-bucket An error occurred (AccessDenied) when calling the CreateBucket operation: Access Denied

Endpoint logs:

  • [RpcError: Not allowed to create new buckets] { rpc_code: 'UNAUTHORIZED' }

Note related to STS:
Currently, there is an issue in STS (it uses RPC token instead of creating its token), this is the workaround for it (until we merge the fix - put this line in the comment: if (config.NOOBAA_AUTH_TOKEN) return config.NOOBAA_AUTH_TOKEN; in the file jwt_utils):

function make_auth_token(object = {}, jwt_options = {}) {
    // Remote services/endpoints should not sign tokens
    // if (config.NOOBAA_AUTH_TOKEN) return config.NOOBAA_AUTH_TOKEN; //SDSD try
    // create and return the signed token
    return jwt.sign(object, get_jwt_secret(), jwt_options);
}

@shirady shirady force-pushed the nsfs-iam-account-access-keys branch 2 times, most recently from 0958460 to 2efdab5 Compare June 20, 2024 11:05
1. Implement the functions related to access keys in AccountSpaceFS (CRUD).
2. Add more properties to nsfs_account_schema inside access_key object (all of them are not required):
  - creation_date = the date the access-key object was created.
  - deactivated = boolean (we need to translate it to status which is 'Active' or 'Inactive').
3. Clean the account cache after updating the account config using the IAM API.
4. Remove unused errors that were copied from STS' errors in IamErrors.
5. Change the param name from user_name to username in the ops.
6. Remove the mock variables that we used in the IAM boilerplate implementation.
7. Update unit test for access_keys function in accountspace_fs.
8. Add unit test in account_schema_validation related to the schema changes.
9. Update Get Started section that would be the demo for IAM access keys management (docs/dev_guide/nc_nsfs_iam_developer_doc.md).
10. Changes in nc_master_key_manager to allow an access key object with more properties and add tests for the edited functions (encrypt_access_keys and decrypt_access_keys of account).
11. Add the support for denying a request whose access key status is Inactive (in all services: S3, STS, IAM).
12. Instead of copying the master_key_id from the root account, we now take the most updated master_key_id.
13. Add a new RPC code for a deactivated access key.
14. Edit the function authorize_request_account_by_token adding the access key deactivated check, but also throwing errors on every missing component instead of using optional chaining.

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
@shirady shirady force-pushed the nsfs-iam-account-access-keys branch from 2efdab5 to d416089 Compare June 23, 2024 05:09
@shirady shirady merged commit d5921f4 into noobaa:master Jun 23, 2024
10 checks passed
@Neon-White Neon-White mentioned this pull request Jun 26, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants