feat: add end user oauth apis#29683
Conversation
Summary of ChangesHello @CourTeous33, 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 Dify's capability by introducing end-user specific authentication for tools. It allows applications to leverage external services that require individual user authorization, such as OAuth-based integrations, directly through the end-user's context. This change provides greater flexibility and personalization for how tools are used within Dify applications, moving beyond workspace-wide credentials to support a more granular, user-centric authentication model. Highlights
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.
Code Review
This pull request introduces a comprehensive set of APIs and services for end-user OAuth authentication for tools. The changes are well-structured, with new services for managing authentication requirements, credentials, and the OAuth flow itself. The core logic in ToolManager is updated to handle end-user credentials, including on-the-fly token refreshing. The addition of unit tests is also a great step towards ensuring the stability of this new feature.
My review includes suggestions for refactoring to reduce code duplication, improving code style by moving local imports, and enhancing test coverage for the new token refresh mechanism. Overall, this is a solid contribution that significantly enhances the authentication capabilities of the platform.
| if enduser_provider.expires_at != -1 and (enduser_provider.expires_at - 60) < int(time.time()): | ||
| from core.plugin.impl.oauth import OAuthHandler | ||
| from services.tools.builtin_tools_manage_service import BuiltinToolManageService | ||
|
|
||
| # refresh the credentials | ||
| tool_provider = ToolProviderID(provider_id) | ||
| provider_name = tool_provider.provider_name | ||
| # Use the same redirect URI as workspace OAuth to reuse the same OAuth client | ||
| redirect_uri = f"{dify_config.CONSOLE_API_URL}/console/api/oauth/plugin/{provider_id}/tool/callback" | ||
| system_credentials = BuiltinToolManageService.get_oauth_client(tenant_id, provider_id) | ||
|
|
||
| oauth_handler = OAuthHandler() | ||
| # refresh the credentials | ||
| refreshed_credentials = oauth_handler.refresh_credentials( | ||
| tenant_id=tenant_id, | ||
| user_id=end_user_id, | ||
| plugin_id=tool_provider.plugin_id, | ||
| provider=provider_name, | ||
| redirect_uri=redirect_uri, | ||
| system_credentials=system_credentials or {}, | ||
| credentials=decrypted_credentials, | ||
| ) | ||
| # update the credentials | ||
| enduser_provider.encrypted_credentials = json.dumps( | ||
| encrypter.encrypt(refreshed_credentials.credentials) | ||
| ) | ||
| enduser_provider.expires_at = refreshed_credentials.expires_at | ||
| db.session.commit() | ||
| decrypted_credentials = refreshed_credentials.credentials | ||
| cache.delete() |
There was a problem hiding this comment.
This block of code for refreshing OAuth tokens is very similar to the logic for workspace authentication token refresh on lines 361-390. This duplication can make future maintenance more difficult.
Additionally, the local import of services.tools.builtin_tools_manage_service introduces a circular dependency (tool_manager -> builtin_tools_manage_service -> tool_manager), which is a code smell.
To improve this, consider extracting the common token refresh logic into a private helper method within the ToolManager class. This helper could take parameters like tenant_id, provider_id, user_id, and decrypted_credentials, and return the refreshed credentials. The calling code would then be responsible for updating the specific provider object (enduser_provider or builtin_provider) and committing the changes.
This refactoring would reduce code duplication and centralize the token refresh logic, making the circular dependency more contained and explicit within a single helper method. A long-term solution would involve refactoring the service layers to break the cycle entirely.
| def get(self, app_model, provider_id: str): | ||
| """Handle OAuth callback and store credentials.""" | ||
| # Get OAuth context ID from cookie | ||
| context_id = request.cookies.get("oauth_context_id") |
| required_type = None | ||
| if supported_types: | ||
| # Prefer OAuth2, then API key | ||
| from core.plugin.entities.plugin_daemon import CredentialType |
| # Calculate expiration timestamp | ||
| expires_at = -1 | ||
| if credentials_response.expires_in and credentials_response.expires_in > 0: | ||
| import time |
| """ | ||
| Unit tests for end-user tool authentication. | ||
|
|
||
| Tests the integration of end-user authentication with tool runtime resolution. | ||
| """ |
There was a problem hiding this comment.
The added unit tests cover the basic flows for end-user authentication, which is great. However, the automatic OAuth token refresh logic implemented in tool_manager.py is a critical and complex part of this feature that is not currently tested.
Please consider adding a test case to verify that expired or soon-to-expire OAuth tokens are correctly refreshed on-the-fly when ToolManager.get_tool_runtime is called with auth_type=ToolAuthType.END_USER. This would involve mocking the expires_at field of the EndUserAuthenticationProvider and verifying that the refresh logic is triggered and the credentials are updated.
|
@CourTeous33 Please ensure that the database migration files are applied in a strictly linear, ordered manner, and avoid using the merge function, as it makes downgrades difficult. |
Important
Fixes #<issue number>.Summary
added end user oauth apis
For details, please check design doc
Screenshots
Checklist
dev/reformat(backend) andcd web && npx lint-staged(frontend) to appease the lint gods