codeant-ai-for-open-source[bot] commented on code in PR #44365:
URL: https://github.com/apache/superset/pull/44365#discussion_r4032946466
##########
superset/security/api.py:
##########
@@ -268,6 +285,128 @@ def guest_token(self) -> Response:
except ValidationError as error:
return self.response_400(message=error.messages)
+ @expose("/login-token/", methods=("POST",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ @transaction()
+ def login_token(self) -> Response:
+ """Mint a one-time login token for iframe embedding.
+ ---
+ post:
+ summary: Mint a one-time login token
+ description: >-
+ Exchanges a caller-supplied proof of identity for an opaque,
single-use
+ token that GET on this same path trades for a session cookie.
Intended
+ to be called server-to-server by a trusted parent application so
the
+ underlying credential never reaches the browser. The
+ LOGIN_TOKEN_IDENTITY_RESOLVER hook decides what counts as proof.
+ responses:
+ 200:
+ description: The minted token and its expiry
+ content:
+ application/json:
+ schema: LoginTokenResponseSchema
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not login_token_utils.is_enabled():
+ # 404 rather than 403: with the feature off there is nothing here
to
+ # be forbidden from, and this keeps the surface closed by default.
+ return self.response_404()
+
+ if (userinfo := login_token_utils.resolve_identity(request)) is None:
+ return self.response_401()
+
+ token, expires_on = login_token_utils.mint(userinfo)
+ logger.info(
+ "One-time login token minted for '%s' from %s",
+ userinfo.get("username") or userinfo.get("email"),
+ request.remote_addr,
+ )
+ return self.response(
+ 200,
+ access_token=token,
+ expires_at=int(expires_on.timestamp()),
+ )
+
+ @expose("/login-token/", methods=("GET",))
+ @event_logger.log_this
+ @statsd_metrics
+ @transaction()
+ def login_with_token(self) -> Response:
Review Comment:
**Suggestion:** The unauthenticated GET accepts any bearer token without
binding it to the intended browser or parent, enabling login CSRF that switches
a victim into an attacker's account.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Security`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=42dc3c0f96e148dd9885eacad77a7b86&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=42dc3c0f96e148dd9885eacad77a7b86&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/api.py
**Line:** 337:341
**Comment:**
*Security: The unauthenticated GET accepts any bearer token without
binding it to the intended browser or parent, enabling login CSRF that switches
a victim into an attacker's account.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=f0fa39e43cf5a4248bdb4de712725510262be232fd5679101492461083e89f71&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=f0fa39e43cf5a4248bdb4de712725510262be232fd5679101492461083e89f71&reaction=dislike'>๐</a>
##########
superset/security/api.py:
##########
@@ -268,6 +285,128 @@ def guest_token(self) -> Response:
except ValidationError as error:
return self.response_400(message=error.messages)
+ @expose("/login-token/", methods=("POST",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ @transaction()
+ def login_token(self) -> Response:
+ """Mint a one-time login token for iframe embedding.
+ ---
+ post:
+ summary: Mint a one-time login token
+ description: >-
+ Exchanges a caller-supplied proof of identity for an opaque,
single-use
+ token that GET on this same path trades for a session cookie.
Intended
+ to be called server-to-server by a trusted parent application so
the
+ underlying credential never reaches the browser. The
+ LOGIN_TOKEN_IDENTITY_RESOLVER hook decides what counts as proof.
+ responses:
+ 200:
+ description: The minted token and its expiry
+ content:
+ application/json:
+ schema: LoginTokenResponseSchema
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not login_token_utils.is_enabled():
+ # 404 rather than 403: with the feature off there is nothing here
to
+ # be forbidden from, and this keeps the surface closed by default.
+ return self.response_404()
+
+ if (userinfo := login_token_utils.resolve_identity(request)) is None:
+ return self.response_401()
+
+ token, expires_on = login_token_utils.mint(userinfo)
+ logger.info(
+ "One-time login token minted for '%s' from %s",
+ userinfo.get("username") or userinfo.get("email"),
+ request.remote_addr,
+ )
+ return self.response(
+ 200,
+ access_token=token,
+ expires_at=int(expires_on.timestamp()),
+ )
+
+ @expose("/login-token/", methods=("GET",))
+ @event_logger.log_this
+ @statsd_metrics
+ @transaction()
+ def login_with_token(self) -> Response:
+ """Consume a one-time login token and establish a session.
+ ---
+ get:
+ summary: Consume a one-time login token
+ description: >-
+ Reached by navigating an iframe to this URL. Exchanges the token
for a
+ standard session cookie and redirects to `next`, so the frame
holds an
+ ordinary Superset session with the user's own roles and row-level
+ security. The token is deleted on use.
+ parameters:
+ - in: query
+ name: token
+ required: true
+ schema:
+ type: string
+ description: The opaque token returned by POST on this path
+ - in: query
+ name: next
+ required: false
+ schema:
+ type: string
+ description: Internal URL to redirect to; rejected if not internal
+ responses:
+ 302:
+ description: Session established; redirect to `next`
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not login_token_utils.is_enabled():
+ return self.response_404()
Review Comment:
**Suggestion:** Consumption also requires the minting resolver to remain
configured, so removing or rotating that resolver strands already-issued tokens
even though consumption needs no resolver.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Stale reference`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=f5981bc7f91f45ca87240dc32a8fd5bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=f5981bc7f91f45ca87240dc32a8fd5bb&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/api.py
**Line:** 374:375
**Comment:**
*Stale Reference: Consumption also requires the minting resolver to
remain configured, so removing or rotating that resolver strands already-issued
tokens even though consumption needs no resolver.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=8d137e6b232e15f663ad8bc2469b00d4bdcf41c57e530d3d983481d6712a68da&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=8d137e6b232e15f663ad8bc2469b00d4bdcf41c57e530d3d983481d6712a68da&reaction=dislike'>๐</a>
##########
superset/security/api.py:
##########
@@ -268,6 +285,128 @@ def guest_token(self) -> Response:
except ValidationError as error:
return self.response_400(message=error.messages)
+ @expose("/login-token/", methods=("POST",))
+ @event_logger.log_this
+ @safe
+ @statsd_metrics
+ @transaction()
+ def login_token(self) -> Response:
+ """Mint a one-time login token for iframe embedding.
+ ---
+ post:
+ summary: Mint a one-time login token
+ description: >-
+ Exchanges a caller-supplied proof of identity for an opaque,
single-use
+ token that GET on this same path trades for a session cookie.
Intended
+ to be called server-to-server by a trusted parent application so
the
+ underlying credential never reaches the browser. The
+ LOGIN_TOKEN_IDENTITY_RESOLVER hook decides what counts as proof.
+ responses:
+ 200:
+ description: The minted token and its expiry
+ content:
+ application/json:
+ schema: LoginTokenResponseSchema
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not login_token_utils.is_enabled():
+ # 404 rather than 403: with the feature off there is nothing here
to
+ # be forbidden from, and this keeps the surface closed by default.
+ return self.response_404()
+
+ if (userinfo := login_token_utils.resolve_identity(request)) is None:
+ return self.response_401()
+
+ token, expires_on = login_token_utils.mint(userinfo)
+ logger.info(
+ "One-time login token minted for '%s' from %s",
+ userinfo.get("username") or userinfo.get("email"),
+ request.remote_addr,
+ )
+ return self.response(
+ 200,
+ access_token=token,
+ expires_at=int(expires_on.timestamp()),
+ )
+
+ @expose("/login-token/", methods=("GET",))
+ @event_logger.log_this
+ @statsd_metrics
+ @transaction()
+ def login_with_token(self) -> Response:
+ """Consume a one-time login token and establish a session.
+ ---
+ get:
+ summary: Consume a one-time login token
+ description: >-
+ Reached by navigating an iframe to this URL. Exchanges the token
for a
+ standard session cookie and redirects to `next`, so the frame
holds an
+ ordinary Superset session with the user's own roles and row-level
+ security. The token is deleted on use.
+ parameters:
+ - in: query
+ name: token
+ required: true
+ schema:
+ type: string
+ description: The opaque token returned by POST on this path
+ - in: query
+ name: next
+ required: false
+ schema:
+ type: string
+ description: Internal URL to redirect to; rejected if not internal
+ responses:
+ 302:
+ description: Session established; redirect to `next`
+ 401:
+ $ref: '#/components/responses/401'
+ 404:
+ $ref: '#/components/responses/404'
+ 500:
+ $ref: '#/components/responses/500'
+ """
+ if not login_token_utils.is_enabled():
+ return self.response_404()
+
+ token = request.args.get("token", "")
+ # A single failure mode for unknown, malformed, expired and
already-spent
+ # tokens, so the response cannot be used to probe which one it was.
+ if not token or (userinfo := login_token_utils.consume(token)) is None:
+ return self.response_401()
+
+ user = self.appbuilder.sm.auth_user_oauth(userinfo)
Review Comment:
**Suggestion:** If `auth_user_oauth` raises after consumption, the
transaction rolls back the deletion, making the supposedly single-use token
reusable on retry.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Logic error`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=11085a6d385441b0921a4d035bbd0356&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=11085a6d385441b0921a4d035bbd0356&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/security/api.py
**Line:** 383:383
**Comment:**
*Logic Error: If `auth_user_oauth` raises after consumption, the
transaction rolls back the deletion, making the supposedly single-use token
reusable on retry.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=1a1bda43f3ccf39e0b39ea98af63ac3610c38e270149cfb3856c5f750d30030f&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44365&comment_hash=1a1bda43f3ccf39e0b39ea98af63ac3610c38e270149cfb3856c5f750d30030f&reaction=dislike'>๐</a>
--
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]
