feat: Notion & Linear OAuth connectors, enhanced security of OAuth connectors and few fixes #663
Conversation
- Adjusted the height of the dialog content in the connector popup for better layout. - Enhanced the last indexed date display with a new function for contextual formatting, providing clearer time references. - Updated various text sizes for consistency across the connector card and dialog header components. - Minor layout adjustments in the connector dialog header and active connectors tab for improved spacing.
…ents - Commented out the write_todos tracking and messaging logic in the stream_new_chat.py file. - Disabled the import and usage of WriteTodosToolUI in the new-chat page component. - Updated related logic in the active connectors tab to remove indexing state handling for write_todos. - These changes are part of a temporary disablement of the write_todos feature for further evaluation.
|
@AnishSarkar22 is attempting to deploy a commit to the Rohan Verma's projects Team on Vercel. A member of the Team first needs to authorize it. |
![recurseml[bot]](https://avatars.githubusercontent.com/in/1054195?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 70890bd..2b01120
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (7)
• surfsense_backend/app/tasks/chat/stream_new_chat.py
• surfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsx
• surfsense_web/components/assistant-ui/connector-popup.tsx
• surfsense_web/components/assistant-ui/connector-popup/components/connector-dialog-header.tsx
• surfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsx
• surfsense_web/components/assistant-ui/document-upload-popup.tsx
• surfsense_web/components/sources/DocumentUploadTab.tsx
- Introduced Notion OAuth support with new environment variables for client ID, client secret, and redirect URI. - Implemented Notion connector routes for OAuth flow, including authorization and callback handling. - Updated existing components to accommodate Notion integration, including validation changes and connector configuration. - Enhanced the Notion indexer to utilize OAuth access tokens instead of integration tokens. - Adjusted UI components to reflect the new Notion connector without requiring special configuration.
AnishSarkar22
changed the title
- Introduced Linear OAuth support with new environment variables for client ID, client secret, and redirect URI. - Implemented Linear connector routes for OAuth flow, including authorization and callback handling. - Updated existing components to accommodate Linear integration, including validation changes and connector configuration. - Enhanced the Linear indexer to utilize OAuth access tokens instead of API keys. - Adjusted UI components to reflect the new Linear connector without requiring special configuration.
AnishSarkar22
changed the title
…n connectors if user deny access - Updated callback routes to handle optional error parameters for OAuth flows, improving user experience during authorization failures. - Implemented error logging and redirection logic to provide feedback when access is denied or errors occur. - Added validation for required parameters to ensure proper handling of authorization codes and state parameters. - Enhanced documentation in the callback functions to clarify the purpose of each parameter.
…ctors - Added encryption for sensitive tokens (access token, refresh token, client secret) in Google Calendar, Google Drive, Gmail, Linear, and Notion connectors to enhance security. - Introduced OAuthStateManager for secure state parameter generation and validation, improving the integrity of OAuth flows. - Updated callback routes to handle state validation and error management, ensuring robust handling of authorization processes. - Enhanced indexers to support decryption of tokens for backward compatibility, maintaining functionality with existing encrypted credentials. - Improved validation for date parameters in connector routes to ensure proper input handling.
AnishSarkar22
changed the title
AnishSarkar22
changed the title
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 2b01120..645e849
| Severity | Location | Issue | Delete |
|---|---|---|---|
 |
surfsense_backend/app/tasks/connector_indexers/linear_indexer.py:95 | Breaking change without migration |  |
|
surfsense_backend/app/tasks/connector_indexers/notion_indexer.py:98 | Breaking change without migration | |
|
surfsense_backend/app/utils/oauth_security.py:190 | Incorrect encryption detection heuristic | |
 |
surfsense_backend/app/tasks/connector_indexers/linear_indexer.py:112 | Unhandled ValueError in encryption check | |
✅ Files analyzed, no issues (37)
• surfsense_backend/.env.example
• surfsense_backend/app/config/__init__.py
• surfsense_backend/app/connectors/google_calendar_connector.py
• surfsense_backend/app/connectors/google_drive/credentials.py
• surfsense_backend/app/connectors/linear_connector.py
• surfsense_backend/app/connectors/notion_history.py
• surfsense_backend/app/routes/__init__.py
• surfsense_backend/app/routes/airtable_add_connector_route.py
• surfsense_backend/app/routes/google_calendar_add_connector_route.py
• surfsense_backend/app/routes/google_drive_add_connector_route.py
• surfsense_backend/app/routes/google_gmail_add_connector_route.py
• surfsense_backend/app/routes/linear_add_connector_route.py
• surfsense_backend/app/routes/notion_add_connector_route.py
• surfsense_backend/app/tasks/connector_indexers/airtable_indexer.py
• surfsense_backend/app/tasks/connector_indexers/google_calendar_indexer.py
• surfsense_backend/app/tasks/connector_indexers/google_drive_indexer.py
• surfsense_backend/app/tasks/connector_indexers/google_gmail_indexer.py
• surfsense_backend/app/utils/validators.py
• surfsense_web/components/assistant-ui/connector-popup/components/connector-card.tsx
• surfsense_web/components/assistant-ui/connector-popup/components/connector-dialog-header.tsx
• surfsense_web/components/assistant-ui/connector-popup/connect-forms/components/circleback-connect-form.tsx
• surfsense_web/components/assistant-ui/connector-popup/connect-forms/components/linear-connect-form.tsx
• surfsense_web/components/assistant-ui/connector-popup/connect-forms/components/notion-connect-form.tsx
• surfsense_web/components/assistant-ui/connector-popup/connect-forms/index.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/components/linear-config.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/components/notion-config.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/index.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-connect-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/connector-edit-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/connector-configs/views/indexing-configuration-view.tsx
• surfsense_web/components/assistant-ui/connector-popup/constants/connector-constants.ts
• surfsense_web/components/assistant-ui/connector-popup/hooks/use-connector-dialog.ts
• surfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsx
• surfsense_web/components/assistant-ui/document-upload-popup.tsx
• surfsense_web/components/editConnector/types.ts
• surfsense_web/components/sources/DocumentUploadTab.tsx
• surfsense_web/hooks/use-connector-edit-page.ts
| linear_token = connector.config.get("LINEAR_API_KEY") | ||
| if not linear_token: | ||
| # Get the Linear access token from the connector config | ||
| linear_access_token = connector.config.get("access_token") |
There was a problem hiding this comment.
Critical breaking change: The Linear indexer now requires 'access_token' in connector config, but existing Linear connectors created before this PR use 'LINEAR_API_KEY' instead. When the indexer runs on an existing connector, it will fail at line 96-103 with error 'Linear access token not found in connector config' because connector.config.get('access_token') returns None. This will cause all existing Linear connectors to crash during indexing until users manually reconnect them via OAuth. There is no migration path or backward compatibility handling for existing connectors.
Similar issue exists in notion_indexer.py line 98 where it expects 'access_token' but old connectors have 'NOTION_INTEGRATION_TOKEN'.
Evidence:
- File 5 (linear_connector.py) changed from API key authentication to OAuth Bearer token (lines 17-34)
- File 21 (validators.py) removed 'LINEAR_API_KEY' requirement (line 529 removed)
- File 25 (linear-connect-form.tsx) - entire old API key form was REMOVED
- File 18 (linear_indexer.py) line 95 now expects 'access_token' which won't exist in old connectors
- No migration logic exists to convert old connector configs to new format
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| # Get the Notion token from the connector config | ||
| notion_token = connector.config.get("NOTION_INTEGRATION_TOKEN") | ||
| # Get the Notion access token from the connector config (OAuth-based) | ||
| notion_token = connector.config.get("access_token") |
There was a problem hiding this comment.
Critical breaking change: The Notion indexer now requires 'access_token' in connector config, but existing Notion connectors created before this PR use 'NOTION_INTEGRATION_TOKEN' instead. When the indexer runs on an existing connector, it will fail at line 99-106 with error 'Notion access token not found in connector config' because connector.config.get('access_token') returns None. This will cause all existing Notion connectors to crash during indexing until users manually reconnect them via OAuth. There is no migration path or backward compatibility handling for existing connectors.
Evidence:
- File 6 (notion_history.py) line 10 comment changed from 'integration token' to 'OAuth access token'
- File 21 (validators.py) removed 'NOTION_INTEGRATION_TOKEN' requirement (lines 517-520 removed)
- File 26 (notion-connect-form.tsx) - entire old integration token form was REMOVED
- File 19 (notion_indexer.py) line 98 now expects 'access_token' which won't exist in old connectors
- Frontend code still references NOTION_INTEGRATION_TOKEN (file 38, surfsense_web/hooks/use-connector-edit-page.ts lines 80, 117, 271-278) but backend no longer accepts it
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
| token_encrypted = connector.config.get("_token_encrypted", False) | ||
| if token_encrypted or ( | ||
| config.SECRET_KEY | ||
| and TokenEncryption(config.SECRET_KEY).is_encrypted(linear_access_token) |
There was a problem hiding this comment.
Potential runtime error: The code creates a new TokenEncryption instance on every check to determine if a token is encrypted. If config.SECRET_KEY is None or empty, this will raise a ValueError from TokenEncryption.init (oauth_security.py line 144: 'raise ValueError("secret_key is required for token encryption")'). This check happens on line 110-113 even if token_encrypted flag is False. The issue is that the condition checks 'config.SECRET_KEY' for truthiness but then immediately uses it to create TokenEncryption, which could fail if SECRET_KEY is an empty string or other falsy value that passes the truthiness check but is invalid for encryption.
The same issue exists in notion_indexer.py line 110-113.
Evidence:
- Line 112 creates TokenEncryption(config.SECRET_KEY) without try-catch
- oauth_security.py lines 137-145 show TokenEncryption.init raises ValueError if secret_key is falsy
- The guard 'config.SECRET_KEY' on line 111 checks truthiness but doesn't validate if it's a valid encryption key
React with 👍 to tell me that this comment was useful, or 👎 if not (and I'll stop posting more comments like this in the future)
|
@AnishSarkar22 Can you verify the recurse ml pr review once. |
Yeah checking it. |
- Enhanced token decryption logic in Airtable, Google Drive, Linear, and Notion indexers to only attempt decryption when tokens are explicitly marked as encrypted. - Added error handling for missing SECRET_KEY when tokens are marked as encrypted, improving robustness and clarity in error reporting. - Updated comments to clarify the handling of plaintext tokens when encryption is not indicated.
…ctors - Updated documentation for the LinearConnector and NotionHistoryConnector classes to specify that the access token can also be an API key or integration token, enhancing clarity for users regarding token usage.
…ors similar to google oauth based ones - Enhanced LinearConnector and NotionHistoryConnector classes to support automatic token refresh, improving reliability in accessing APIs. - Updated initialization to require session and connector ID, allowing for dynamic credential management. - Introduced new credential schemas for Linear and Notion, encapsulating access and refresh tokens with expiration handling. - Refactored indexers to utilize the new connector structure, ensuring seamless integration with the updated authentication flow. - Improved error handling and logging during token refresh processes for better debugging and user feedback.
|
@MODSetter Please check it, the issues should be fixed now. |
|
Clone of #664 |
There was a problem hiding this comment.
Review by RecurseML
🔍 Review performed on 645e849..76de0b5
✨ No bugs found, your code is sparkling clean
✅ Files analyzed, no issues (9)
• surfsense_backend/app/routes/linear_add_connector_route.py
• surfsense_backend/app/routes/notion_add_connector_route.py
• surfsense_backend/app/schemas/linear_auth_credentials.py
• surfsense_backend/app/schemas/notion_auth_credentials.py
• surfsense_backend/app/tasks/connector_indexers/airtable_indexer.py
• surfsense_backend/app/tasks/connector_indexers/google_drive_indexer.py
• surfsense_backend/app/tasks/connector_indexers/linear_indexer.py
• surfsense_backend/app/tasks/connector_indexers/notion_indexer.py
• surfsense_web/components/new-chat/document-mention-picker.tsx
2 participants
oauth_securitysecurity module for centralized management for oauth connectors. Previously it only had base64 encoding without state or timestamp validation.write_todostool.Description
Motivation and Context
FIX #
Screenshots
API Changes
Change Type
Testing Performed
Checklist
High-level PR Summary
This PR introduces three distinct improvements: disabling the
write_todostool functionality across the backend and frontend by commenting out related code, enhancing the last indexed date display for connectors with more human-readable relative time formatting (e.g., "Just now", "Today at 2:30 PM", "3 days ago"), and fixing mobile responsiveness for the connector and document upload popups by adjusting heights, padding, and layout to improve usability on smaller screens.⏱️ Estimated Review Time: 5-15 minutes
💡 Review Order Suggestion
surfsense_web/components/assistant-ui/connector-popup/components/connector-card.tsxsurfsense_web/components/assistant-ui/connector-popup/tabs/active-connectors-tab.tsxsurfsense_web/components/assistant-ui/connector-popup/components/connector-dialog-header.tsxsurfsense_web/components/assistant-ui/connector-popup.tsxsurfsense_web/components/sources/DocumentUploadTab.tsxsurfsense_web/components/assistant-ui/document-upload-popup.tsxsurfsense_web/app/dashboard/[search_space_id]/new-chat/[[...chat_id]]/page.tsxsurfsense_backend/app/tasks/chat/stream_new_chat.py