madhushreeag commented on code in PR #44365:
URL: https://github.com/apache/superset/pull/44365#discussion_r4033179016


##########
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:
   Fair, and worth recording. An attacker who can mint for their own identity 
and induce a victim's browser to navigate to the consume URL within the TTL can 
place that browser in the attacker's session. Single use and a 60s TTL reduces 
the risk but does not eliminate. Also, The attacker here would not get access, 
but victims input. 
   An OAuth-style `state` binding doesn't translate cleanly, if the parent app 
is on a different origin and so can't set a cookie Superset reads. A workable 
variant exists for deployments where parent and Superset share a registrable 
domain, but it's conditional, so it needs to be optional and well documented. 



-- 
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]

Reply via email to