Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

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

Merged
crwilcox merged 6 commits into
googleapis:masterfrom
HemangChothani:storage_fix_system_test
Feb 12, 2020
Merged

fix(storage): fix system test and change scope for iam access token#47
crwilcox merged 6 commits into
googleapis:masterfrom
HemangChothani:storage_fix_system_test

Conversation

@HemangChothani

Copy link
Copy Markdown
Contributor

Fixes#46

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

HemangChothani commented Feb 6, 2020

Copy link
Copy Markdown
ContributorAuthor

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.

@HemangChothaniHemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@crwilcoxcrwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 6, 2020
Comment threadtests/system.py Outdated
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
Copy Markdown
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
Copy Markdown
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?)

@tseavertseaverFeb 11, 2020

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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
Copy Markdown
Contributor

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']

Comment threadtests/system.py Outdated
@HemangChothaniHemangChothani added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoroyoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@crwilcoxcrwilcox added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Feb 12, 2020
@yoshi-kokoroyoshi-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:masterFeb 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>
@release-pleaserelease-pleaseBot mentioned this pull request Jan 12, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla: yesThis 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

@HemangChothani@tseaver@crwilcox@frankyn@googlebot@yoshi-kokoro