-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: add check for client authorization before retrieving token string #2327
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
Conversation
Summary of ChangesHello @Yuan325, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request rectifies a security and functional regression by re-establishing crucial client authorization checks across various Cloud Healthcare API tools. The core change involves conditionally parsing bearer tokens only when client authorization is enabled, preventing unnecessary token operations and ensuring that API calls are properly authenticated and authorized as intended. This fix enhances the robustness and security posture of the affected tools. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request addresses a regression by re-introducing a necessary authorization check before parsing bearer tokens. The fix is applied consistently across 15 different tool implementations. The logic of the fix is correct. However, this change highlights a significant amount of code duplication for token handling across these files. I've added a review comment with a suggestion to refactor this duplicated logic into a single helper function to improve the overall maintainability of the codebase.
internal/tools/cloudhealthcare/cloudhealthcarefhirfetchpage/cloudhealthcarefhirfetchpage.go
Show resolved
Hide resolved
googleapis#2327) Previous refactoring (googleapis#2273) accidentally removed the authorization checks prior to token retrieval. This issue went unnoticed because the integration tests were disabled. I am re-adding the necessary checks.
🤖 I have created a release *beep* *boop* --- ## [0.26.0](v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([#2305](#2305)) ([5054212](5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([#2288](#2288)) ([6b02591](6b02591)) ### Features * Add new `user-agent-metadata` flag ([#2302](#2302)) ([adc9589](adc9589)) * Add remaining flag to Toolbox server in MCP registry ([#2272](#2272)) ([5e0999e](5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([#2310](#2310)) ([e4f60e5](e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([#2262](#2262)) ([4abf0c3](4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([#2141](#2141)) ([8e0fb03](8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([#2171](#2171)) ([00c3e6d](00c3e6d)) * Support combining multiple prebuilt configurations ([#2295](#2295)) ([e535b37](e535b37)) * Support MCP specs version 2025-11-25 ([#2303](#2303)) ([4d23a3b](4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([#2333](#2333)) ([15101b1](15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([#2327](#2327)) ([c25a233](c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com>
🤖 I have created a release *beep* *boop* --- ## [0.26.0](v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([#2305](#2305)) ([5054212](5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([#2288](#2288)) ([6b02591](6b02591)) ### Features * Add new `user-agent-metadata` flag ([#2302](#2302)) ([adc9589](adc9589)) * Add remaining flag to Toolbox server in MCP registry ([#2272](#2272)) ([5e0999e](5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([#2310](#2310)) ([e4f60e5](e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([#2262](#2262)) ([4abf0c3](4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([#2141](#2141)) ([8e0fb03](8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([#2171](#2171)) ([00c3e6d](00c3e6d)) * Support combining multiple prebuilt configurations ([#2295](#2295)) ([e535b37](e535b37)) * Support MCP specs version 2025-11-25 ([#2303](#2303)) ([4d23a3b](4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([#2333](#2333)) ([15101b1](15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([#2327](#2327)) ([c25a233](c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 86bf7bf
🤖 I have created a release *beep* *boop* --- ## [0.26.0](googleapis/genai-toolbox@v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([googleapis#2305](googleapis#2305)) ([5054212](googleapis@5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([googleapis#2288](googleapis#2288)) ([6b02591](googleapis@6b02591)) ### Features * Add new `user-agent-metadata` flag ([googleapis#2302](googleapis#2302)) ([adc9589](googleapis@adc9589)) * Add remaining flag to Toolbox server in MCP registry ([googleapis#2272](googleapis#2272)) ([5e0999e](googleapis@5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([googleapis#2310](googleapis#2310)) ([e4f60e5](googleapis@e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([googleapis#2262](googleapis#2262)) ([4abf0c3](googleapis@4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([googleapis#2141](googleapis#2141)) ([8e0fb03](googleapis@8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([googleapis#2171](googleapis#2171)) ([00c3e6d](googleapis@00c3e6d)) * Support combining multiple prebuilt configurations ([googleapis#2295](googleapis#2295)) ([e535b37](googleapis@e535b37)) * Support MCP specs version 2025-11-25 ([googleapis#2303](googleapis#2303)) ([4d23a3b](googleapis@4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([googleapis#2333](googleapis#2333)) ([15101b1](googleapis@15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([googleapis#2327](googleapis#2327)) ([c25a233](googleapis@c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 86bf7bf
🤖 I have created a release *beep* *boop* --- ## [0.26.0](googleapis/genai-toolbox@v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([googleapis#2305](googleapis#2305)) ([5054212](googleapis@5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([googleapis#2288](googleapis#2288)) ([6b02591](googleapis@6b02591)) ### Features * Add new `user-agent-metadata` flag ([googleapis#2302](googleapis#2302)) ([adc9589](googleapis@adc9589)) * Add remaining flag to Toolbox server in MCP registry ([googleapis#2272](googleapis#2272)) ([5e0999e](googleapis@5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([googleapis#2310](googleapis#2310)) ([e4f60e5](googleapis@e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([googleapis#2262](googleapis#2262)) ([4abf0c3](googleapis@4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([googleapis#2141](googleapis#2141)) ([8e0fb03](googleapis@8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([googleapis#2171](googleapis#2171)) ([00c3e6d](googleapis@00c3e6d)) * Support combining multiple prebuilt configurations ([googleapis#2295](googleapis#2295)) ([e535b37](googleapis@e535b37)) * Support MCP specs version 2025-11-25 ([googleapis#2303](googleapis#2303)) ([4d23a3b](googleapis@4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([googleapis#2333](googleapis#2333)) ([15101b1](googleapis@15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([googleapis#2327](googleapis#2327)) ([c25a233](googleapis@c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 86bf7bf
🤖 I have created a release *beep* *boop* --- ## [0.26.0](googleapis/genai-toolbox@v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([googleapis#2305](googleapis#2305)) ([5054212](googleapis@5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([googleapis#2288](googleapis#2288)) ([6b02591](googleapis@6b02591)) ### Features * Add new `user-agent-metadata` flag ([googleapis#2302](googleapis#2302)) ([adc9589](googleapis@adc9589)) * Add remaining flag to Toolbox server in MCP registry ([googleapis#2272](googleapis#2272)) ([5e0999e](googleapis@5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([googleapis#2310](googleapis#2310)) ([e4f60e5](googleapis@e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([googleapis#2262](googleapis#2262)) ([4abf0c3](googleapis@4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([googleapis#2141](googleapis#2141)) ([8e0fb03](googleapis@8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([googleapis#2171](googleapis#2171)) ([00c3e6d](googleapis@00c3e6d)) * Support combining multiple prebuilt configurations ([googleapis#2295](googleapis#2295)) ([e535b37](googleapis@e535b37)) * Support MCP specs version 2025-11-25 ([googleapis#2303](googleapis#2303)) ([4d23a3b](googleapis@4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([googleapis#2333](googleapis#2333)) ([15101b1](googleapis@15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([googleapis#2327](googleapis#2327)) ([c25a233](googleapis@c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 86bf7bf
🤖 I have created a release *beep* *boop* --- ## [0.26.0](googleapis/genai-toolbox@v0.25.0...v0.26.0) (2026-01-22) ### ⚠ BREAKING CHANGES * Validate tool naming ([googleapis#2305](googleapis#2305)) ([5054212](googleapis@5054212)) * **tools/cloudgda:** Update description and parameter name for cloudgda tool ([googleapis#2288](googleapis#2288)) ([6b02591](googleapis@6b02591)) ### Features * Add new `user-agent-metadata` flag ([googleapis#2302](googleapis#2302)) ([adc9589](googleapis@adc9589)) * Add remaining flag to Toolbox server in MCP registry ([googleapis#2272](googleapis#2272)) ([5e0999e](googleapis@5e0999e)) * **embeddingModel:** Add embedding model to MCP handler ([googleapis#2310](googleapis#2310)) ([e4f60e5](googleapis@e4f60e5)) * **sources/bigquery:** Make maximum rows returned from queries configurable ([googleapis#2262](googleapis#2262)) ([4abf0c3](googleapis@4abf0c3)) * **prebuilt/cloud-sql:** Add create backup tool for Cloud SQL ([googleapis#2141](googleapis#2141)) ([8e0fb03](googleapis@8e0fb03)) * **prebuilt/cloud-sql:** Add restore backup tool for Cloud SQL ([googleapis#2171](googleapis#2171)) ([00c3e6d](googleapis@00c3e6d)) * Support combining multiple prebuilt configurations ([googleapis#2295](googleapis#2295)) ([e535b37](googleapis@e535b37)) * Support MCP specs version 2025-11-25 ([googleapis#2303](googleapis#2303)) ([4d23a3b](googleapis@4d23a3b)) * **tools:** Add `valueFromParam` support to Tool config ([googleapis#2333](googleapis#2333)) ([15101b1](googleapis@15101b1)) ### Bug Fixes * **tools/cloudhealthcare:** Add check for client authorization before retrieving token string ([googleapis#2327](googleapis#2327)) ([c25a233](googleapis@c25a233)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please). --------- Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Co-authored-by: Yuan Teoh <45984206+Yuan325@users.noreply.github.com> 86bf7bf
Previous refactoring (#2273) accidentally removed the authorization checks prior to token retrieval. This issue went unnoticed because the integration tests were disabled. I am re-adding the necessary checks.