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

Add basic structure for GDrive OAuth tool #269

Merged
merged 5 commits into from
Jun 24, 2024
Merged

Conversation

BeatrixCohere
Copy link
Collaborator

@BeatrixCohere BeatrixCohere commented Jun 24, 2024

Add basic structure for GDrive OAuth tool

Doesn't include:

  • More error handling
  • Testing
  • storing (user_id,tool_id) in state
  • correct UI flow

AI Description

Summary

This PR adds a new tool to the backend, Google Drive, and its associated authentication. It also updates the tool authentication system to include tool-based authentication.

Changes

  • Added a new tool, Google Drive, which searches Google Drive for relevant files.
  • Added tool-based authentication, which is currently experimental and in development.
  • Added a new table, tool_auth, to store access tokens for tools that require authentication.
  • Added a new column, is_auth_required, to the ManagedTool model to indicate whether a tool requires authentication.
  • Added a new column, auth_url, to the ManagedTool model to store the authentication URL for a tool.
  • Updated the list_tools function to include the is_auth_required and auth_url fields for each tool.
  • Added a new abstract base class, BaseAuth, for tool authentication.
  • Added new functions, get_auth_url, is_auth_required, and process_auth_token, to the BaseAuth class for tool authentication.
  • Added a new class, GoogleDriveAuth, which inherits from BaseAuth and implements authentication for the Google Drive tool.

Copy link
Collaborator

@lusmoura lusmoura left a comment

Choose a reason for hiding this comment

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

LGTM!

"""

@classmethod
def is_available(cls) -> bool:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any chance we can check if the user is authenticated here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could but I think it is better to have the error in is_auth_required?

@giannis2two
Copy link
Contributor

Should we add a tool_id property in BaseTool for all tools to share?

@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 53.21637% with 80 lines in your changes missing coverage. Please review.

Please upload report for BASE (main@0bf8b73). Learn more about missing BASE report.

Files Patch % Lines
src/backend/tools/google_drive.py 35.80% 52 Missing ⚠️
src/backend/crud/tool_auth.py 31.57% 13 Missing ⚠️
src/backend/routers/auth.py 35.29% 11 Missing ⚠️
src/backend/alembic/versions/a6efd9f047b4_.py 84.61% 2 Missing ⚠️
src/backend/routers/tool.py 81.81% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #269   +/-   ##
=======================================
  Coverage        ?   85.65%           
=======================================
  Files           ?      153           
  Lines           ?     5716           
  Branches        ?        0           
=======================================
  Hits            ?     4896           
  Misses          ?      820           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@BeatrixCohere
Copy link
Collaborator Author

tool_id

Agreed, won't do here but will follow up with it

@BeatrixCohere BeatrixCohere merged commit 9496189 into main Jun 24, 2024
3 checks passed
@BeatrixCohere BeatrixCohere deleted the beatrix/GDriveOauth branch June 24, 2024 14:23
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.

4 participants