Fix: check_connection and enhance the authorization URL#12298
Conversation
![entelligence-ai-pr-reviews[bot]](https://avatars.githubusercontent.com/in/932812?s=60&v=4){kind=small}
| { | ||
| "datasource_name": self.datasource_name, | ||
| "integrations_name": "hubspot", | ||
| "client_id": self.client_id, | ||
| "client_secret": self.client_secret, | ||
| "redirect_uri": redirect_uri, | ||
| "scope": " ".join(self.scopes) if self.scopes else "oauth", | ||
| "optional_scope": " ".join(self.optional_scopes) if self.optional_scopes else "", | ||
| } | ||
| ) | ||
| auth_url += f"&state={urllib.parse.quote(state_data)}" | ||
|
|
There was a problem hiding this comment.
Correctness: The client_secret is embedded in the state query parameter of the auth URL, which will appear in browser history, server access logs, and HTTP Referer headers — this is a critical credential leak.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/hubspot_handler/hubspot_oauth.py, around line 90, the `client_secret` field is included in the `state_data` dict that gets URL-encoded and appended to the auth URL. Remove `"client_secret": self.client_secret` from the state_data dictionary. The client_secret must never be exposed in URLs as it will appear in browser history, server logs, and HTTP Referer headers, constituting a critical security vulnerability.
| """Exchange an authorization code for access and refresh tokens.""" | ||
| logger.error( | ||
| f"DEBUG EXCHANGING CODE: code={code}, redirect_uri={self._get_redirect_uri()}, client_id={self.client_id}" |
There was a problem hiding this comment.
Correctness: This logger.error call logs the OAuth authorization code and client_id at ERROR level — the code is a sensitive single-use secret and should never appear in logs. This is a debug statement that was accidentally left in and must be removed before merging.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/hubspot_handler/hubspot_oauth.py, lines 113-115, remove the debug logger.error statement that logs sensitive OAuth credentials (authorization code and client_id). This is a leftover debug statement that exposes secrets in logs and must be deleted entirely before merging.
There was a problem hiding this comment.
yes we can remove this debug message
|
|
||
| if not self.handler_storage.encrypted_json_get(_STORAGE_KEY): |
There was a problem hiding this comment.
Correctness: self.handler_storage is assigned from kwargs.get('handler_storage') and can be None. Calling self.handler_storage.encrypted_json_get(_STORAGE_KEY) when it is None will raise AttributeError and crash check_connection instead of proceeding normally.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/hubspot_handler/hubspot_handler.py, around line 221-222, `self.handler_storage` can be None (set via `kwargs.get('handler_storage')`). Before calling `self.handler_storage.encrypted_json_get(_STORAGE_KEY)`, add a None guard: `if not self.handler_storage or not self.handler_storage.encrypted_json_get(_STORAGE_KEY):`
🔍 Duplicate Code DetectedFound 1 duplicate function(s) in this PR. Consider consolidating to reduce code duplication. File: This function 📍 Original Location: Function: 💡 Recommendation: Consider importing and reusing the existing function instead of duplicating the logic. |
| # even when localStorage context is missing (e.g. script-initiated flows). | ||
| if self.datasource_name: | ||
| state_data = urllib.parse.urlencode( | ||
| { | ||
| "datasource_name": self.datasource_name, | ||
| "integrations_name": "hubspot", | ||
| "client_id": self.client_id, | ||
| "client_secret": self.client_secret, | ||
| "redirect_uri": redirect_uri, | ||
| "scope": " ".join(self.scopes) if self.scopes else "oauth", |
There was a problem hiding this comment.
Correctness: The client_secret is included in the OAuth state parameter, which is passed as a URL query parameter and visible in browser history, server logs, and the HubSpot redirect. This fully exposes the client secret to anyone who can observe the redirect URL.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/hubspot_handler/hubspot_oauth.py, around line 87, the `client_secret` is being included in the OAuth `state` parameter which gets appended to the auth URL as a query parameter. This exposes the client secret in browser history, server logs, and to HubSpot's servers. Remove `client_secret` from the state_data dictionary. The client_secret should never be transmitted via URL parameters.
| """Exchange an authorization code for access and refresh tokens.""" | ||
| logger.error( | ||
| f"DEBUG EXCHANGING CODE: code={code}, redirect_uri={self._get_redirect_uri()}, client_id={self.client_id}" |
There was a problem hiding this comment.
Correctness: This logger.error call logs the OAuth authorization code and client_id at ERROR level — the code is a sensitive single-use credential and should never appear in logs. This is a debug statement that was accidentally committed and must be removed before production.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In file `mindsdb/integrations/handlers/hubspot_handler/hubspot_oauth.py`, lines 113-115, remove the `logger.error(...)` debug statement inside `_exchange_code()` that logs the OAuth authorization code, redirect URI, and client ID. This leaks sensitive credentials to logs. Delete those three lines entirely.
| # later requests then fail with BAD_AUTH_CODE. Exchange only when a request | ||
| if self.connection_data.get("code") and not self.is_connected: | ||
| from mindsdb.integrations.handlers.hubspot_handler.hubspot_oauth import _STORAGE_KEY | ||
|
|
There was a problem hiding this comment.
Correctness: self.handler_storage can be None (it's set from kwargs.get('handler_storage') with no default), so calling self.handler_storage.encrypted_json_get(_STORAGE_KEY) will raise AttributeError when handler_storage is not provided.
🤖 AI Agent Prompt for Cursor/Windsurf
📋 Copy this prompt to your AI coding assistant (Cursor, Windsurf, etc.) to get help fixing this issue
In mindsdb/integrations/handlers/hubspot_handler/hubspot_handler.py, at the line `if not self.handler_storage.encrypted_json_get(_STORAGE_KEY):` (inside the `check_connection` method, within the new OAuth deferral block), `self.handler_storage` can be None because it is assigned via `kwargs.get('handler_storage')` without a default value. Add a None guard before calling the method: change the condition to `if not self.handler_storage or not self.handler_storage.encrypted_json_get(_STORAGE_KEY):`.
tino097
left a comment
tino097
left a comment
There was a problem hiding this comment.
LGTM just remove the debug message
2 participants
Description
Please include a summary of the change and the issue it solves.
Fixes #issue_number
Type of change
(Please delete options that are not relevant)
Verification Process
To ensure the changes are working as expected:
Additional Media:
Checklist:
Confidence Score: 1/5 - Blocking Issues
client_secretis embedded in the OAuthstateURL parameter, exposing it in browser history, server logs, and HTTP Referer headers — this is a severe credential leak that must be fixed before merging.self.handler_storagecan beNone(no default inkwargs.get), causingAttributeErrorwhen calling.encrypted_json_get()— this will crash in production environments where handler_storage is not provided.Files requiring special attention
mindsdb/integrations/handlers/hubspot_handler/hubspot_oauth.pymindsdb/integrations/handlers/hubspot_handler/hubspot_handler.py