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

fix(storage): fix system test and change scope for iam access token #47

Merged
merged 6 commits into from Feb 12, 2020

Conversation

HemangChothani
Copy link
Contributor

Fixes #46

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Feb 6, 2020
@HemangChothani
Copy link
Contributor Author

HemangChothani commented Feb 6, 2020

System test throws 'Identity and Access Management (IAM) API has not been used in project before or it is disabled. ' error, so need (IAM) permission for this project.

@HemangChothani HemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
tests/system.py Outdated
@@ -1066,7 +1067,7 @@ def test_create_signed_read_url_v4_w_access_token(self):
client = iam_credentials_v1.IAMCredentialsClient()
service_account_email = Config.CLIENT._credentials.service_account_email
name = client.service_account_path("-", service_account_email)
scope = ["https://www.googleapis.com/auth/devstorage.read_write"]
scope = ["https://www.googleapis.com/auth/cloud-platform"]
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like a lot of access. @jkwlui @frankyn is this scope needed for this?

Copy link
Contributor

Choose a reason for hiding this comment

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

(we don't have to block merging on this as it is a test, but if this is needed it seems like a large scope for narrow use?)

Copy link
Contributor

@tseaver tseaver Feb 11, 2020

Choose a reason for hiding this comment

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

FWIW, I can confirm that both tests fail on master with a 403 without this patch.

Update: it fails even with the cloud-platform scope for me on master.

Copy link
Member

Choose a reason for hiding this comment

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

Could you narrow the scope down to: https://www.googleapis.com/auth/iam

Documented at the bottom of the following document: https://cloud.google.com/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/signBlob

Copy link
Member

Choose a reason for hiding this comment

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

To clarify, you can do join both:

scope = ['https://www.googleapis.com/auth/devstorage.read_write', 'https://www.googleapis.com/auth/iam']

tests/system.py Outdated Show resolved Hide resolved
@HemangChothani HemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@crwilcox crwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@crwilcox crwilcox merged commit bc5375f into googleapis:master Feb 12, 2020
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…oogleapis#47)

* fix(storage): change scope for iam access token

* fix: narrow scope

* fix: trailing commas

* chore: blacken

Co-authored-by: Christopher Wilcox <crwilcox@google.com>
cojenco pushed a commit to cojenco/python-storage that referenced this pull request Oct 13, 2021
…oogleapis#47)

* fix(storage): change scope for iam access token

* fix: narrow scope

* fix: trailing commas

* chore: blacken

Co-authored-by: Christopher Wilcox <crwilcox@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Storage: access token field is unused in system test 'test_create_signed_read_url_*_w_access_token'
6 participants