Re: [PR] feat: upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4031378778 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4030695324 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
netlify[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4030599917 ### β Deploy Preview for *superset-docs-preview* ready! | Name | Link | |:-:|| |π¨ Latest commit | 672fbd6af8936393818d4be8563d9a6c8674b1f8 | |π Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69affb682980190008776840 | |π Deploy Preview | [https://deploy-preview-38469--superset-docs-preview.netlify.app](https://deploy-preview-38469--superset-docs-preview.netlify.app) | |π± Preview on mobile | Toggle QR Code... _Use your smartphone camera to open QR code link._ | --- _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
github-advanced-security[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2910842135
##
superset/utils/oauth2.py:
##
@@ -276,3 +280,103 @@
if database.is_oauth2_enabled() and
database.db_engine_spec.needs_oauth2(ex):
database.db_engine_spec.start_oauth2_dance(database)
raise
+
+
+def save_user_provider_token(
+user_id: int,
+provider: str,
+token_response: dict[str, Any],
+) -> None:
+"""
+Upsert an UpstreamOAuthToken row for the given user + provider.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+token = UpstreamOAuthToken(user_id=user_id, provider=provider)
+
+token.access_token = token_response.get("access_token")
+expires_in = token_response.get("expires_in")
+token.access_token_expiration = (
+datetime.now() + timedelta(seconds=expires_in) if expires_in else None
+)
+token.refresh_token = token_response.get("refresh_token")
+db.session.add(token)
+db.session.commit()
+
+
+def get_upstream_provider_token(provider: str, user_id: int) -> str | None:
+"""
+Retrieve a valid access token for the given provider and user.
+
+If the token is expired and a refresh token exists, attempt to refresh it.
+Returns None if no valid token is available.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+return None
+
+now = datetime.now()
+if token.access_token_expiration is None or token.access_token_expiration
> now:
+return token.access_token
+
+# Token is expired
+if token.refresh_token:
+return _refresh_upstream_provider_token(token, provider)
+
+db.session.delete(token)
+db.session.commit()
+return None
+
+
+def _refresh_upstream_provider_token(
+token: UpstreamOAuthToken,
+provider: str,
+) -> str | None:
+"""
+Use the refresh token to obtain a new access token from the provider.
+Updates and persists the token if successful; deletes it on failure.
+"""
+from flask import current_app as flask_app
+
+try:
+remote_app =
flask_app.extensions["authlib.integrations.flask_client"][provider]
+token_response = remote_app.fetch_access_token(
+grant_type="refresh_token",
+refresh_token=token.refresh_token,
+)
+except Exception: # pylint: disable=broad-except
+logger.warning(
+"Failed to refresh upstream OAuth token for provider %s",
+provider,
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (password)](1) as clear text.
[Show more
details](https://github.com/apache/superset/security/code-scanning/2263)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
netlify[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4030332015 ### β Deploy Preview for *superset-docs-preview* ready! | Name | Link | |:-:|| |π¨ Latest commit | 38ca71cb1bf9161f25711f7c9ffbc4a507a311bd | |π Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69aff1a0574cfe00086e95bb | |π Deploy Preview | [https://deploy-preview-38469--superset-docs-preview.netlify.app](https://deploy-preview-38469--superset-docs-preview.netlify.app) | |π± Preview on mobile | Toggle QR Code... _Use your smartphone camera to open QR code link._ | --- _To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4029998308 Code Review Agent Run #15e3c5 Actionable Suggestions - 0 Review Details Files reviewed - 1 Β· Commit Range: 64b8ced..87a4108 superset/utils/oauth2.py Files skipped - 0 Tools Whispers (Secret Scanner) - βοΈ SuccessfulDetect-secrets (Secret Scanner) - βοΈ SuccessfulMyPy (Static Code Analysis) - βοΈ SuccessfulAstral Ruff (Static Code Analysis) - βοΈ Successful Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ AI Code Review powered by https://bito.ai/"; target="_blank">https://bito.ai/wp-content/uploads/2023/10/Logo-Bito-Black-cropped.svg"; alt="Bito Logo" width="50" height="20" /> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
github-advanced-security[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2910110960
##
superset/utils/oauth2.py:
##
@@ -276,3 +280,104 @@
if database.is_oauth2_enabled() and
database.db_engine_spec.needs_oauth2(ex):
database.db_engine_spec.start_oauth2_dance(database)
raise
+
+
+def save_user_provider_token(
+user_id: int,
+provider: str,
+token_response: dict[str, Any],
+) -> None:
+"""
+Upsert an UpstreamOAuthToken row for the given user + provider.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+token = UpstreamOAuthToken(user_id=user_id, provider=provider)
+
+token.access_token = token_response.get("access_token")
+expires_in = token_response.get("expires_in")
+token.access_token_expiration = (
+datetime.now() + timedelta(seconds=expires_in) if expires_in else None
+)
+token.refresh_token = token_response.get("refresh_token")
+db.session.add(token)
+db.session.commit()
+
+
+def get_upstream_provider_token(provider: str, user_id: int) -> str | None:
+"""
+Retrieve a valid access token for the given provider and user.
+
+If the token is expired and a refresh token exists, attempt to refresh it.
+Returns None if no valid token is available.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+return None
+
+now = datetime.now()
+if token.access_token_expiration is None or token.access_token_expiration
> now:
+return token.access_token
+
+# Token is expired
+if token.refresh_token:
+return _refresh_upstream_provider_token(token, provider)
+
+db.session.delete(token)
+db.session.commit()
+return None
+
+
+def _refresh_upstream_provider_token(
+token: UpstreamOAuthToken,
+provider: str,
+) -> str | None:
+"""
+Use the refresh token to obtain a new access token from the provider.
+Updates and persists the token if successful; deletes it on failure.
+"""
+from flask import current_app as flask_app
+
+try:
+remote_app =
flask_app.extensions["authlib.integrations.flask_client"][provider]
+token_response = remote_app.fetch_access_token(
+grant_type="refresh_token",
+refresh_token=token.refresh_token,
+)
+except Exception: # pylint: disable=broad-except
+logger.warning(
+"Failed to refresh upstream OAuth token for provider %s",
+provider,
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (password)](1) as clear text.
[Show more
details](https://github.com/apache/superset/security/code-scanning/2261)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4029328727 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
codeant-ai-for-open-source[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2909654135
##
superset/utils/oauth2.py:
##
@@ -276,3 +276,102 @@ def check_for_oauth2(database: Database) ->
Iterator[None]:
if database.is_oauth2_enabled() and
database.db_engine_spec.needs_oauth2(ex):
database.db_engine_spec.start_oauth2_dance(database)
raise
+
+
+def save_user_provider_token(
+user_id: int,
+provider: str,
+token_response: dict[str, Any],
+) -> None:
+"""
+Upsert an UpstreamOAuthToken row for the given user + provider.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+token = UpstreamOAuthToken(user_id=user_id, provider=provider)
Review Comment:
**Suggestion:** Using `.one_or_none()` on `UpstreamOAuthToken` without a
uniqueness constraint on `(user_id, provider)` can raise `MultipleResultsFound`
at runtime under concurrent logins (two rows created for the same
user/provider), so the "upsert" helper should tolerate duplicates by selecting
the first row instead of assuming at most one. [race condition]
Severity Level: Major β οΈ
```mdx
- β OAuth login fails when duplicate upstream token rows exist.
- β οΈ Any token maintenance task crashes on MultipleResultsFound.
```
```suggestion
token: UpstreamOAuthToken | None = (
db.session.query(UpstreamOAuthToken)
.filter_by(user_id=user_id, provider=provider)
.first()
)
```
Steps of Reproduction β
```mdx
1. Inspect the UpstreamOAuthToken model at
`superset/models/core.py:1372-1392`; note there
is only an index `idx_upstream_oauth_tokens_user_provider` on `(user_id,
provider)` and no
UNIQUE constraint, so multiple rows per user/provider are allowed at the
database level.
2. Insert at least two rows into `upstream_oauth_tokens` with the same
`user_id` and
`provider` (e.g., via SQL console or a migration script) so that `SELECT *
FROM
upstream_oauth_tokens WHERE user_id= AND provider='keycloak'` returns
more than one
row.
3. Start Superset with this PR, configured with OAuth login as described in
the PR
(`OAUTH_PROVIDERS = [{"name": "keycloak", "save_token": True, ...}]`), and
log in via that
OAuth provider so that `auth_user_oauth()` (mentioned in the PR description)
executes and
calls `save_user_provider_token()` in `superset/utils/oauth2.py` for the
logged-in user.
4. During login, `save_user_provider_token()` at
`superset/utils/oauth2.py:281-305`
executes the snippet at `293-297`, calling `.one_or_none()` on the query;
because multiple
rows match, SQLAlchemy raises `MultipleResultsFound`, causing the login
request to fail
with a 500 error instead of updating one of the existing token rows.
```
Prompt for AI Agent π€
```mdx
This is a comment left during a code review.
**Path:** superset/utils/oauth2.py
**Line:** 293:297
**Comment:**
*Race Condition: Using `.one_or_none()` on `UpstreamOAuthToken` without
a uniqueness constraint on `(user_id, provider)` can raise
`MultipleResultsFound` at runtime under concurrent logins (two rows created for
the same user/provider), so the "upsert" helper should tolerate duplicates by
selecting the first row instead of assuming at most one.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
π
| π
##
superset/utils/oauth2.py:
##
@@ -276,3 +276,102 @@ def check_for_oauth2(database: Database) ->
Iterator[None]:
if database.is_oauth2_enabled() and
database.db_engine_spec.needs_oauth2(ex):
database.db_engine_spec.start_oauth2_dance(database)
raise
+
+
+def save_user_provider_token(
+user_id: int,
+provider: str,
+token_response: dict[str, Any],
+) -> None:
+"""
+Upsert an UpstreamOAuthToken row for the given user + provider.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+token = UpstreamOAuthToken(user_id=user_id, provider=provider)
+
+token.access_token = token_response.get("access_token")
+expires_in = token_response.get("expires_in")
+token.access_token_expiration = (
+datetime.now() + timedelta(seconds=expires_in) if expires_in else None
+)
+token.refresh_token = token_response.get("refresh_token")
+db.session.add(tok
Re: [PR] feat: upstream oauth token forwarding master [superset]
aurokk commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4016543212 Hello! Please review and provide me some guidance on what to get this accepted! Thanks! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] feat: upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4011545074 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). Bito Usage Guide **Commands** Type the following command in the pull request comment and save the comment. - `/review` - Manually triggers a full AI review. - `/pause` - Pauses automatic reviews on this pull request. - `/resume` - Resumes automatic reviews. - `/resolve` - Marks all Bito-posted review comments as resolved. - `/abort` - Cancels all in-progress reviews. Refer to the https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands"; target="_blank" rel="noopener noreferrer">documentation for additional commands. **Configuration** This repository uses `Superset` You can customize the agent settings https://alpha.bito.ai/home/ai-agents/code-review-agent"; target="_blank" rel="noopener noreferrer">here or contact your Bito workspace admin at [email protected]. **Documentation & Help** - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance"; target="_blank" rel="noopener noreferrer">Customize agent settings - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules"; target="_blank" rel="noopener noreferrer">Review rules - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent"; target="_blank" rel="noopener noreferrer">General documentation - https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs"; target="_blank" rel="noopener noreferrer">FAQ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] (feat): upstream oauth token forwarding master [superset]
github-advanced-security[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2895387129
##
superset/security/manager.py:
##
@@ -480,6 +480,63 @@
return self.get_guest_user_from_request(request)
return None
+def set_oauth_session(self, provider: str, oauth_response: dict[str, Any])
-> None:
+"""
+Override to persist the full OAuth token response before FAB reduces
it to
+``session["oauth"] = (access_token, secret)`` tuple.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import session
+
+super().set_oauth_session(provider, oauth_response)
+# FAB stores only (access_token, secret) in session["oauth"].
+# We need the full response (expires_in, refresh_token, β¦) for
upstream forwarding.
+session["oauth_full_token"] = dict(oauth_response) # noqa: S105
+
+def auth_user_oauth(self, userinfo: dict[str, Any]) -> Any:
+def auth_user_oauth(self, userinfo: dict[str, Any]) -> Any:
+"""
+Override to save the upstream OAuth token when a user logs in via
OAuth.
+
+If ``save_token: True`` is set in the matching OAUTH_PROVIDERS entry
and
+``DATABASE_OAUTH2_UPSTREAM_PROVIDERS`` maps a database to this
provider,
+the token will be forwarded to that database instead of triggering a
+separate OAuth2 dance.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import current_app as flask_app, session
+
+user = super().auth_user_oauth(userinfo)
+if user:
+provider = session.get("oauth_provider")
+# Use the full token dict saved by set_oauth_session, not the
+# (access_token, secret) tuple that FAB stores in session["oauth"].
+token = session.get("oauth_full_token")
+if token and provider:
+provider_config = next(
+(
+p
+for p in flask_app.config.get("OAUTH_PROVIDERS", [])
+if p.get("name") == provider
+),
+None,
+)
+if provider_config and provider_config.get("save_token"):
+from superset.utils.oauth2 import save_user_provider_token
+
+try:
+save_user_provider_token(user.id, provider, token)
+except Exception: # pylint: disable=broad-except
+logger.warning(
+"Failed to save upstream OAuth token for provider
%s",
+provider,
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (password)](1) as clear text.
[Show more
details](https://github.com/apache/superset/security/code-scanning/2257)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] (feat): upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2895356898
##
superset/security/manager.py:
##
@@ -480,6 +480,60 @@ def request_loader(self, request: Request) ->
Optional[User]:
return self.get_guest_user_from_request(request)
return None
+def set_oauth_session(self, provider: str, oauth_response: dict[str, Any])
-> None:
+"""
+Override to persist the full OAuth token response before FAB reduces
it to
+``session["oauth"] = (access_token, secret)`` tuple.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import session
+
+super().set_oauth_session(provider, oauth_response)
+# FAB stores only (access_token, secret) in session["oauth"].
+# We need the full response (expires_in, refresh_token, β¦) for
upstream forwarding.
+session["oauth_full_token"] = dict(oauth_response) # noqa: S105
+
+def auth_user_oauth(self, userinfo: dict[str, Any]) -> Any:
+"""
+Override to save the upstream OAuth token when a user logs in via
OAuth.
+
+If ``save_token: True`` is set in the matching OAUTH_PROVIDERS entry
and
+``DATABASE_OAUTH2_UPSTREAM_PROVIDERS`` maps a database to this
provider,
+the token will be forwarded to that database instead of triggering a
+separate OAuth2 dance.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import current_app as flask_app, session
+
+user = super().auth_user_oauth(userinfo)
+if user:
+provider = session.get("oauth_provider")
+# Use the full token dict saved by set_oauth_session, not the
+# (access_token, secret) tuple that FAB stores in session["oauth"].
+token = session.get("oauth_full_token")
+if token and provider:
+provider_config = next(
+(
+p
+for p in flask_app.config.get("OAUTH_PROVIDERS", [])
+if p.get("name") == provider
+),
+None,
+)
+if provider_config and provider_config.get("save_token"):
+from superset.utils.oauth2 import save_user_provider_token
+
+try:
+save_user_provider_token(user.id, provider, token)
+except Exception: # pylint: disable=broad-except
Review Comment:
Blind exception catch without specificity
Replace the broad `Exception` catch with specific exception types. Consider
catching `Exception` only if necessary, or specify the expected exceptions
(e.g., `OSError`, `ValueError`).
Code suggestion
Check the AI-generated fix before applying
suggestion
from superset.utils.oauth2 import
save_user_provider_token
try:
save_user_provider_token(user.id, provider, token)
except (OSError, ValueError) as e: # pylint:
disable=broad-except
Code Review Run #429f28
---
Should Bito avoid suggestions like this for future reviews? (https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules)
- [ ] Yes, avoid them
##
superset/migrations/versions/2026-03-06_12-00_a1b2c3d4e5f6_add_upstream_oauth_tokens.py:
##
@@ -0,0 +1,73 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""add_upstream_oauth_tokens
+
+Revision ID: a1b2c3d4e5f6
+Revises: f5b5f88d8526
+Create Date: 2026-03-06 12:00:00.00
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = "a1b2c3d4e5f6"
+down_revision = "f5b5f88d8526"
+
+
+def upgrade() -> None:
+op.create_table(
+"upstream_oauth_tokens",
+sa.Column("id", sa.Integer(), nullable=False),
+sa.Column("user_id", sa.Integer(), nullable=False),
+sa.Column("provider", sa.String(256), nullable=False),
+sa.Column("access_tok
Re: [PR] (feat): upstream oauth token forwarding master [superset]
github-advanced-security[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2895347996
##
superset/security/manager.py:
##
@@ -480,6 +480,60 @@
return self.get_guest_user_from_request(request)
return None
+def set_oauth_session(self, provider: str, oauth_response: dict[str, Any])
-> None:
+"""
+Override to persist the full OAuth token response before FAB reduces
it to
+``session["oauth"] = (access_token, secret)`` tuple.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import session
+
+super().set_oauth_session(provider, oauth_response)
+# FAB stores only (access_token, secret) in session["oauth"].
+# We need the full response (expires_in, refresh_token, β¦) for
upstream forwarding.
+session["oauth_full_token"] = dict(oauth_response) # noqa: S105
+
+def auth_user_oauth(self, userinfo: dict[str, Any]) -> Any:
+"""
+Override to save the upstream OAuth token when a user logs in via
OAuth.
+
+If ``save_token: True`` is set in the matching OAUTH_PROVIDERS entry
and
+``DATABASE_OAUTH2_UPSTREAM_PROVIDERS`` maps a database to this
provider,
+the token will be forwarded to that database instead of triggering a
+separate OAuth2 dance.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import current_app as flask_app, session
+
+user = super().auth_user_oauth(userinfo)
+if user:
+provider = session.get("oauth_provider")
+# Use the full token dict saved by set_oauth_session, not the
+# (access_token, secret) tuple that FAB stores in session["oauth"].
+token = session.get("oauth_full_token")
+if token and provider:
+provider_config = next(
+(
+p
+for p in flask_app.config.get("OAUTH_PROVIDERS", [])
+if p.get("name") == provider
+),
+None,
+)
+if provider_config and provider_config.get("save_token"):
+from superset.utils.oauth2 import save_user_provider_token
+
+try:
+save_user_provider_token(user.id, provider, token)
+except Exception: # pylint: disable=broad-except
+logger.warning(
+"Failed to save upstream OAuth token for provider
%s",
+provider,
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (password)](1) as clear text.
[Show more
details](https://github.com/apache/superset/security/code-scanning/2256)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
-
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
Re: [PR] Feat/upstream oauth token forwarding master [superset]
codecov[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4011231586 ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38469?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report :white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 65.37%. Comparing base ([`0d5ade6`](https://app.codecov.io/gh/apache/superset/commit/0d5ade6dd31bfdb2dc253642b1d9917f668d5cc7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`b99811d`](https://app.codecov.io/gh/apache/superset/commit/b99811d6cbcd9b8308d5e55820bb99bee27c3725?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)). :warning: Report is 5 commits behind head on master. :x: Your project check has failed because the head coverage (99.24%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage. Additional details and impacted files ```diff @@Coverage Diff @@ ## master #38469 +/- ## == + Coverage 64.84% 65.37% +0.52% == Files1815 1824 +9 Lines 7204973142+1093 Branches2294623094 +148 == + Hits4672047813+1093 Misses 2532925329 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/38469/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | | |---|---|---| | [python](https://app.codecov.io/gh/apache/superset/pull/38469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (?)` | | | [unit](https://app.codecov.io/gh/apache/superset/pull/38469/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ΓΈ> (?)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38469?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache). :rocket: New features to boost your workflow: - :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems. - :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
Re: [PR] Feat/upstream oauth token forwarding master [superset]
codeant-ai-for-open-source[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2895294791
##
superset/migrations/versions/2026-03-06_13-00_b2c3d4e5f6a7_fix_upstream_oauth_tokens_column_types.py:
##
@@ -0,0 +1,81 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""fix_upstream_oauth_tokens_column_types
+
+Recreate upstream_oauth_tokens with EncryptedType columns instead of plain
Text.
+The previous migration incorrectly used sa.Text() for access_token and
+refresh_token, causing a TypeError when SQLAlchemy tried to read them back.
+
+Revision ID: b2c3d4e5f6a7
+Revises: a1b2c3d4e5f6
+Create Date: 2026-03-06 13:00:00.00
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+from sqlalchemy_utils import EncryptedType
+
+# revision identifiers, used by Alembic.
+revision = "b2c3d4e5f6a7"
+down_revision = "a1b2c3d4e5f6"
+
+
+def upgrade() -> None:
+# Drop and recreate the table with correct EncryptedType columns.
+# The table is empty at this point (token saves were failing due to the
+# previous bug), so no data migration is needed.
+op.drop_index(
+"idx_upstream_oauth_tokens_user_provider",
+table_name="upstream_oauth_tokens",
+)
+op.drop_table("upstream_oauth_tokens")
+
+op.create_table(
+"upstream_oauth_tokens",
+sa.Column("id", sa.Integer(), nullable=False),
+sa.Column("user_id", sa.Integer(), nullable=False),
+sa.Column("provider", sa.String(256), nullable=False),
+sa.Column("access_token", EncryptedType(), nullable=True),
+sa.Column("access_token_expiration", sa.DateTime(), nullable=True),
+sa.Column("refresh_token", EncryptedType(), nullable=True),
Review Comment:
**Suggestion:** This follow-up migration again declares `access_token` and
`refresh_token` as `EncryptedType()` with no constructor arguments, which is
invalid for `sqlalchemy_utils.EncryptedType` and will cause the Alembic
migration to fail at runtime; switch these to a concrete SQLAlchemy column type
such as `sa.Text()` so the upgrade can run successfully. [type error]
Severity Level: Major β οΈ
```mdx
- β `superset db upgrade` fails at this migration.
- β Upstream OAuth tokens feature cannot be initialized.
- β οΈ Suggestion reintroduces Text columns, losing intended encryption.
- β οΈ Suggestion may revive prior TypeError described in migration docstring.
```
```suggestion
sa.Column("access_token", sa.Text(), nullable=True),
sa.Column("refresh_token", sa.Text(), nullable=True),
```
Steps of Reproduction β
```mdx
1. Start a Superset instance that includes this migration file at
`superset/migrations/versions/2026-03-06_13-00_b2c3d4e5f6a7_fix_upstream_oauth_tokens_column_types.py`.
2. Run the database upgrade command (which uses Alembic), e.g. `superset db
upgrade` or
`alembic upgrade head`, which loads this revision (revision ID
`b2c3d4e5f6a7`,
down_revision `a1b2c3d4e5f6`).
3. Alembic calls `upgrade()` in this file (lines 38β74). Execution reaches
the
`op.create_table(...)` call where `access_token` and `refresh_token` columns
are defined
at lines 53 and 55 using `EncryptedType()` with no arguments.
4. At runtime, Python attempts to instantiate `EncryptedType()` (from
`sqlalchemy_utils`)
but its `__init__` requires at least the underlying type and an encryption
key; the
missing required positional arguments cause a `TypeError` and the migration
fails,
aborting the upgrade.
```
Prompt for AI Agent π€
```mdx
This is a comment left during a code review.
**Path:**
superset/migrations/versions/2026-03-06_13-00_b2c3d4e5f6a7_fix_upstream_oauth_tokens_column_types.py
**Line:** 53:55
**Comment:**
*Type Error: This follow-up migration again declares `access_token` and
`refresh_token` as `EncryptedType()` with no constructor arguments, which is
invalid for `sqlalchemy_utils.EncryptedType` and will cause the Alembic
migration to fail at runtime; switch these to a concrete SQLAlchemy column type
such as `sa.Text()` so the upgrade can run successfully.
Re: [PR] Feat/upstream oauth token forwarding master [superset]
github-advanced-security[bot] commented on code in PR #38469:
URL: https://github.com/apache/superset/pull/38469#discussion_r2895286430
##
superset/utils/oauth2.py:
##
@@ -276,3 +276,102 @@
if database.is_oauth2_enabled() and
database.db_engine_spec.needs_oauth2(ex):
database.db_engine_spec.start_oauth2_dance(database)
raise
+
+
+def save_user_provider_token(
+user_id: int,
+provider: str,
+token_response: dict[str, Any],
+) -> None:
+"""
+Upsert an UpstreamOAuthToken row for the given user + provider.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+token = UpstreamOAuthToken(user_id=user_id, provider=provider)
+
+token.access_token = token_response.get("access_token")
+expires_in = token_response.get("expires_in")
+token.access_token_expiration = (
+datetime.now() + timedelta(seconds=expires_in) if expires_in else None
+)
+token.refresh_token = token_response.get("refresh_token")
+db.session.add(token)
+db.session.commit()
+
+
+def get_upstream_provider_token(provider: str, user_id: int) -> str | None:
+"""
+Retrieve a valid access token for the given provider and user.
+
+If the token is expired and a refresh token exists, attempt to refresh it.
+Returns None if no valid token is available.
+"""
+from superset.models.core import UpstreamOAuthToken
+
+token: UpstreamOAuthToken | None = (
+db.session.query(UpstreamOAuthToken)
+.filter_by(user_id=user_id, provider=provider)
+.one_or_none()
+)
+if token is None:
+return None
+
+now = datetime.now()
+if token.access_token_expiration and token.access_token_expiration > now:
+return token.access_token
+
+# Token is expired
+if token.refresh_token:
+return _refresh_upstream_provider_token(token, provider)
+
+db.session.delete(token)
+db.session.commit()
+return None
+
+
+def _refresh_upstream_provider_token(
+token: UpstreamOAuthToken,
+provider: str,
+) -> str | None:
+"""
+Use the refresh token to obtain a new access token from the provider.
+Updates and persists the token if successful; deletes it on failure.
+"""
+from flask import current_app as flask_app
+
+try:
+remote_app =
flask_app.extensions["authlib.integrations.flask_client"][provider]
+token_response = remote_app.fetch_access_token(
+grant_type="refresh_token",
+refresh_token=token.refresh_token,
+)
+except Exception: # pylint: disable=broad-except
+logger.warning(
+"Failed to refresh upstream OAuth token for provider %s",
provider, exc_info=True
Review Comment:
## Clear-text logging of sensitive information
This expression logs [sensitive data (password)](1) as clear text.
[Show more
details](https://github.com/apache/superset/security/code-scanning/2255)
##
superset/security/manager.py:
##
@@ -480,6 +480,59 @@
return self.get_guest_user_from_request(request)
return None
+def set_oauth_session(self, provider: str, oauth_response: dict[str, Any])
-> None:
+"""
+Override to persist the full OAuth token response before FAB reduces
it to
+``session["oauth"] = (access_token, secret)`` tuple.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import session
+
+super().set_oauth_session(provider, oauth_response)
+# FAB stores only (access_token, secret) in session["oauth"].
+# We need the full response (expires_in, refresh_token, β¦) for
upstream forwarding.
+session["oauth_full_token"] = dict(oauth_response)
+
+def auth_user_oauth(self, userinfo: dict[str, Any]) -> Any:
+"""
+Override to save the upstream OAuth token when a user logs in via
OAuth.
+
+If ``save_token: True`` is set in the matching OAUTH_PROVIDERS entry
and
+``DATABASE_OAUTH2_UPSTREAM_PROVIDERS`` maps a database to this
provider,
+the token will be forwarded to that database instead of triggering a
+separate OAuth2 dance.
+"""
+# pylint: disable=import-outside-toplevel
+from flask import current_app as flask_app, session
+
+user = super().auth_user_oauth(userinfo)
+if user:
+provider = session.get("oauth_provider")
+# Use the full token dict saved by set_oauth_session, not the
+# (access_token, secret) tuple that FAB stores in session["oauth"].
+token = session.get("oauth_full_token")
+if token and provider:
+provider_config = next(
+(
+
Re: [PR] Feat/upstream oauth token forwarding master [superset]
bito-code-review[bot] commented on PR #38469: URL: https://github.com/apache/superset/pull/38469#issuecomment-4011185087 AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] - To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
