versity / versitygw

versity s3 gateway
https://www.versity.com/products/versitygw/
Apache License 2.0
167 stars 21 forks source link

IAM user management should return better http status codes #613

Closed benmcclelland closed 3 months ago

benmcclelland commented 3 months ago

Describe the bug Currently user management errors set 500 status. It would probably be better for these to be bad request or something more appropriate.

To Reproduce create a user that already exists, gets this error server side:

09:26:10 | 500 |     722.458µs | 127.0.0.1 | PATCH | /create-user | failed to create user: update iam data: user already exists

Expected behavior A 400 level status code instead of a 500.

thenitai commented 3 months ago

On this, and maybe related, I just saw that our whole user.json file got deleted. Trying to add another user results in an error:

failed to create user: update iam data: get iam data: failed to parse the config file: unexpected end of JSON input
thenitai commented 3 months ago

Ok, I've found the issue, though I'm not sure why this is happening.

We use nodejs to with shelljs to execute commands against the versitygw cli. It looks like that issuing a delete-user within that deletes the whole file. However, issuing the very same command in the cli works.

On that, is there a better way to manage buckets and users?

jonaustin09 commented 3 months ago

Another way to interact with admin actions would be directly calling admin apis, without using admin cli actions.

For example the following http request to the gateway creates a new user:

[PATCH] /create-user Body:

{
  "access": "user_access_key_id",
  "secret": "user_secret_access_key",
  "role": "user | userplus | admin",
  "userID": 3,
  "groupID": 5,
}

Note that the request should be signed with AWS SigV4 using an existing admin/root user credentials (access, secret ...).

thenitai commented 3 months ago

Another way to interact with admin actions would be directly calling admin apis, without using admin cli actions.

For example the following http request to the gateway creates a new user:

[PATCH] /create-user Body:

{
  "access": "user_access_key_id",
  "secret": "user_secret_access_key",
  "role": "user | userplus | admin",
  "userID": 3,
  "groupID": 5,
}

Note that the request should be signed with AWS SigV4 using an existing admin/root user credentials (access, secret ...).

Did I miss something or is the API not documented? According to the doc at https://github.com/versity/versitygw/wiki/Multi-Tenant I was under the impression that the IAM S3 is something else. Please correct me if I'm wrong as I would rather use a standard API :) Thank you.

benmcclelland commented 3 months ago

Another way to interact with admin actions would be directly calling admin apis, without using admin cli actions. For example the following http request to the gateway creates a new user: [PATCH] /create-user Body:

{
  "access": "user_access_key_id",
  "secret": "user_secret_access_key",
  "role": "user | userplus | admin",
  "userID": 3,
  "groupID": 5,
}

Note that the request should be signed with AWS SigV4 using an existing admin/root user credentials (access, secret ...).

Did I miss something or is the API not documented? According to the doc at https://github.com/versity/versitygw/wiki/Multi-Tenant I was under the impression that the IAM S3 is something else. Please correct me if I'm wrong as I would rather use a standard API :) Thank you.

We are missing documentation for the admin API. I'll add that to the todo list. Correct that IAM S3 is not aws IAM API. It is the same as the internal IAM, except the json file is stored in an s3 object instead of a file. At this time, we are not planning on an aws compatible IAM API for the gateway, but it is something we have considered.

The Sigv4 comment was that the admin API uses signed requests that use the same signature the S3 API requests use. This was convenient for the gateway since we already had all of the v4 authentication code already. The versitygw admin commands use the API internally and sign the requests as well, for example: https://github.com/versity/versitygw/blob/d98ca9b0346d8f097ab42d8832895a248756259a/cmd/versitygw/admin.go#L172-L232

benmcclelland commented 3 months ago

I am able to confirm some problems if we have multiple simultaneous create/delete user commands. I'll open up a new ticket for this.

A more robust IAM service would probably be the Vault integrations: https://github.com/versity/versitygw/wiki/IAM-Vault

thenitai commented 3 months ago

I agree. However, we really just need a simple setup. It's not public and only customers who have a userid and password can connect. Also, as there is no option to update a user password we opted to remove the user, add again, and then assign the bucket. If there would be a way (unless I missed something again) to update user records it would work with just one call.

benmcclelland commented 3 months ago

630 added for the unexpected issues with racing account updates, pr #628 should fix this

631 added for doc request for admin api

632 added for feature request to allow updating account info