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

Support for Assume Role #871

Closed
hardbyte opened this issue Apr 7, 2020 · 7 comments
Closed

Support for Assume Role #871

hardbyte opened this issue Apr 7, 2020 · 7 comments

Comments

@hardbyte
Copy link
Contributor

hardbyte commented Apr 7, 2020

This is a feature request for the Python API to support Assume Role.

@harshavardhana
Copy link
Member

harshavardhana commented Apr 7, 2020

Feel free to send a PR @hardbyte - we already support credentials mechanism https://github.com/minio/minio-py/tree/master/minio/credentials

@hardbyte
Copy link
Contributor Author

hardbyte commented Apr 7, 2020

Thanks @harshavardhana I had a brief look at this and I think it would involve modifying the sign_v4 API in signer.py as the request has to be signed for the service sts instead of s3 in generate_signing_key(). If you're happy to have service="s3" as an optional parameter I could probably put together a PR.

@hardbyte
Copy link
Contributor Author

hardbyte commented Apr 7, 2020

How about something like this? https://gist.github.com/hardbyte/8d6261154b5c840a0f29e6d34cb181c9

Before making a PR I'm not sure if assume_role() should hang off the Minio object, or perhaps as a standalone function?

@harshavardhana
Copy link
Member

How about something like this? https://gist.github.com/hardbyte/8d6261154b5c840a0f29e6d34cb181c9

Before making a PR I'm not sure if assume_role() should hang off the Minio object, or perhaps as a standalone function?

It should be credentials object similar in style to other implementations

@hardbyte
Copy link
Contributor Author

hardbyte commented Apr 7, 2020

Sure that is why I'm proposing returning a Credentials instance (see here)

Are you happy for assume_role to be a method of Minio, or should it be a top level function?

Or do you mean there should be a new class e.g. AssumeRoleCredentials as the main interface? I suspect that wouldn't be that clean as you need to have (non-root) credentials to assume a role.

My current assume_role api takes these parameters:

:param mc: Minio client needed to get endpoint, and credentials for the assume role request.
:param RoleArn: Ignored by minio, but required by STS.
param RoleSessionName: Ignored by minio, but required by STS.
:param Policy: Optional policy dict.
:param DurationSeconds: Number of seconds the assume role credentials will remain valid.

@harshavardhana
Copy link
Member

Are you happy for assume_role to be a method of Minio, or should it be a top level function?

it shouldn't be a method of Minio constructor it should be just a credentials mechanism @hardbyte

@hardbyte
Copy link
Contributor Author

hardbyte commented Apr 7, 2020

Sweet that is not a big change from my gist prototype - I'll make a new AssumeRoleCredentials class - again I'll note that it will require an existing minio client as a constructor argument.

One other observation is that a primary reason for using AssumeRole is to then serialize the credentials to share with a user (e.g. for uploading to the object store).

hardbyte added a commit to hardbyte/minio-py that referenced this issue Apr 8, 2020
hardbyte added a commit to hardbyte/minio-py that referenced this issue Apr 13, 2020
hardbyte added a commit to hardbyte/minio-py that referenced this issue Apr 14, 2020
hardbyte added a commit to hardbyte/minio-py that referenced this issue Apr 15, 2020
hardbyte added a commit to hardbyte/minio-py that referenced this issue Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants