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

storage: fix incorrect API scopes for IAM SignBlob API #629

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Apr 9, 2024

Previously when a service account attempted to use the IAM SignBlob API, the request would fail with a 403
ACCESS_TOKEN_SCOPE_INSUFFICIENT because the wrong scope was requested.

As documented in
https://cloud.google.com/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/signBlob, either https://www.googleapis.com/auth/iam or
https://www.googleapis.com/auth/cloud-platform is needed.

This commit fixes an issue where the default authorization header with the https://www.googleapis.com/auth/devstorage.full_control scope was being used by the IAM service. This occurred because the previous code did not actually set the scope properly, and for the IAM service to work properly, we need to request a new access token with the correct scope.

Note that the service account in question needs to have the Service Account Token Creator IAM role to work.

Closes #599

Comment on lines 146 to 147
apply_client_options(@iam_service, @options)
iam_options = @options.merge(google_api_scope_url: GOOGLE_STORAGE_JSON_IAM_API_SCOPE_URLS.join(" ")))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the future, it might be nicer to distinguish between options that are only used by this gem and options that are used by the Google SDK. It's not obvious that google_api_scope_url isn't passed all the way to the Google SDK.

@stanhu
Copy link
Contributor Author

stanhu commented Apr 9, 2024

@Temikus Would you mind reviewing this? This seems to be a pretty critical fix for Google Kubernetes Engine users.

Copy link
Member

@Temikus Temikus left a comment

Choose a reason for hiding this comment

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

LGTM - feel free to drop the client too

@Temikus
Copy link
Member

Temikus commented Apr 9, 2024

And thanks for your contributions @stanhu as usual ❤️

@Temikus
Copy link
Member

Temikus commented Apr 10, 2024

Kicked off the CI, once it passes will merge and release as soon as I can.

@Temikus
Copy link
Member

Temikus commented Apr 10, 2024

@stanhu looks like there’s a small typo causing some errors - you can see the unit test run failing ‘rake test:unit’ for local if you need it.

Previously when a service account attempted to use the IAM SignBlob
API, the request would fail with a 403
`ACCESS_TOKEN_SCOPE_INSUFFICIENT` because the wrong scope was
requested.

As documented in
https://cloud.google.com/iam/docs/reference/credentials/rest/v1/projects.serviceAccounts/signBlob,
either `https://www.googleapis.com/auth/iam` or
`https://www.googleapis.com/auth/cloud-platform` is needed.

This commit fixes an issue where the default authorization header with
the `https://www.googleapis.com/auth/devstorage.full_control` scope
was being used by the IAM service. This occurred because the previous
code did not actually set the scope properly, and for the IAM service
to work properly, we need to request a new access token with the
correct scope.

Note that the service account in question needs to have the `Service
Account Token Creator` IAM role to work.

Closes fog#599
@stanhu stanhu force-pushed the sh-fix-storage-iam-scope branch from 1866bc0 to f8ca6ce Compare April 10, 2024 02:27
@stanhu
Copy link
Contributor Author

stanhu commented Apr 10, 2024

@stanhu looks like there’s a small typo causing some errors - you can see the unit test run failing ‘rake test:unit’ for local if you need it.

Thanks. That was a last-minute refactor, should be fixed now.

@Temikus
Copy link
Member

Temikus commented Apr 11, 2024

LGTM, merging 👍 Will aim to release today if I can - if not - will push Friday.

@Temikus Temikus merged commit 9089d06 into fog:master Apr 11, 2024
24 of 29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fog::Storage::GoogleJSON::Real#iam_signer is using the wrong API scopes with Workload Identity
3 participants