Feature/auth flow - #457
Conversation
- Add AuthConfig, AuthCredential, AuthScheme types - Add AuthHandler for OAuth2 flow orchestration - Add CredentialManager and CredentialService for token management - Add OAuth2 token exchange helpers - Add unit tests for auth package
Add support for tools to request authentication credentials during execution: - Add RequestedAuthConfigs field to EventActions for storing auth requests - Implement GenerateAuthEvent to convert auth requests to adk_request_credential function calls - Add RequestAuthConfig method to ToolContext for tools to request authentication - Add AuthConfig field to Tool interface for tools to declare auth requirements - Integrate auth event generation into the LLM base flow Also fix test comparisons to ignore RequestedAuthConfigs field initialization and add UTC timezone setup in database tests for consistent timestamp formatting.
Summary of ChangesHello @cpunion, 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 significantly enhances the agent's capabilities by introducing a robust and flexible authentication framework. It allows agents to dynamically request and manage user credentials for interacting with authenticated APIs, particularly for OAuth2 and OpenID Connect. The new system includes dedicated components for defining authentication schemes, handling credential exchange and refresh, and integrates seamlessly into the agent's execution flow to enable a 'Surgical Resumption' pattern, where tools can pause for user authentication and then automatically resume once credentials are provided. This foundational work enables more secure and interactive agent behaviors. 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
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive authentication package to handle various auth flows, primarily OAuth2. It includes new data structures for auth schemes and credentials, a credential manager for handling the token lifecycle (exchange, refresh), and integration into the core agent flow for requesting and processing user credentials. The changes are extensive, adding a new auth package and modifying several core components to support the new functionality. The implementation appears to be a faithful port of the Python ADK's auth logic. My review focuses on correctness and maintainability of the new Go code, and I've identified a few areas for improvement.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive authentication package (auth) to handle various authentication schemes, with a focus on OAuth2/OIDC flows. The changes are extensive, adding new data structures for auth configurations, credentials, and schemes, along with handlers and managers to orchestrate the entire authentication lifecycle, including credential exchange and token refresh.
The implementation correctly integrates with the existing agent and tool execution flow by introducing:
- An
authPreprocessorto handle user-provided credentials and resume tool execution. - An
adk_request_credentialsystem tool to request user authorization. - New methods on the
tool.Contextfor tools to interact with the auth system.
The code is well-structured and includes thorough testing for the new components. My review includes a couple of suggestions to improve the robustness of credential key generation and clarify the behavior of the credential refresh logic on failure. Overall, this is a solid and well-engineered feature addition.
| if schemePart == "" && credPart == "" { | ||
| return "adk_" + uuid.NewString(), nil | ||
| } | ||
| return fmt.Sprintf("adk_%s_%s", schemePart, credPart), nil |
There was a problem hiding this comment.
The current string formatting for the credential key can result in keys with double underscores (e.g., adk__credpart) or trailing underscores if either schemePart or credPart is empty. A more robust approach would be to conditionally build the key parts and then join them. This ensures a clean, well-formed key in all cases.
Note: You'll also need to add "strings" to your imports for this change.
| if schemePart == "" && credPart == "" { | |
| return "adk_" + uuid.NewString(), nil | |
| } | |
| return fmt.Sprintf("adk_%s_%s", schemePart, credPart), nil | |
| parts := []string{"adk"} | |
| if schemePart != "" { | |
| parts = append(parts, schemePart) | |
| } | |
| if credPart != "" { | |
| parts = append(parts, credPart) | |
| } | |
| if len(parts) == 1 { | |
| return "adk_" + uuid.NewString(), nil | |
| } | |
| return strings.Join(parts, "_"), nil |
|
|
||
| refreshed, err := ref.Refresh(ctx, cred, m.authConfig.AuthScheme) | ||
| if err != nil { | ||
| return cred, false, fmt.Errorf("failed to refresh credential: %w", err) |
There was a problem hiding this comment.
When a credential refresh fails, this function returns the original (and likely expired) credential along with an error. While the caller currently handles this correctly, it's safer for this function to return a nil credential on failure. This makes the function's contract clearer: a non-nil credential is only returned on success.
| return cred, false, fmt.Errorf("failed to refresh credential: %w", err) | |
| return nil, false, fmt.Errorf("failed to refresh credential: %w", err) |
1 participant
Base on:
#455 feat(auth): add OAuth2/OIDC authentication package