rusackas commented on code in PR #41965:
URL: https://github.com/apache/superset/pull/41965#discussion_r3566226551
##########
tests/unit_tests/security/api_test.py:
##########
@@ -66,6 +66,53 @@ def test_csrf_exempt_blueprints_with_api_key(app: Any,
app_context: None) -> Non
assert "ApiKeyApi" in {blueprint.name for blueprint in
csrf._exempt_blueprints}
+def test_security_api_trailing_slash_matches_route_ownership(client: Any) ->
None:
+ """Regression for #29934: sibling ``/api/v1/security/*`` endpoints respond
to
+ a misspelled (wrong trailing-slash) URL differently, and that difference is
+ the *intended* behavior — a Werkzeug routing artifact of who owns each
route,
+ not a bug.
+
+ Three routes live under the same ``/api/v1/security/`` prefix but are
+ declared with different slash conventions because they come from different
+ owners:
+
+ * ``login`` -> ``@expose("/login")`` (no trailing slash)
+ Flask-AppBuilder's own route. Superset does not own or register it, so
+ it inherits FAB's no-trailing-slash convention. Werkzeug hard-404s a
+ request that adds a stray trailing slash to a no-slash route (there is
+ no canonical slashed URL to redirect to).
+ * ``csrf_token`` -> ``@expose("/csrf_token/")`` (trailing slash)
+ * ``guest_token`` -> ``@expose("/guest_token/")`` (trailing slash)
+ Superset's own routes, whose trailing-slash URLs are the documented
+ canonical URLs (the Embedded SDK depends on them). Werkzeug
308-redirects
+ a request that omits the trailing slash to the canonical slashed URL.
+
+ Unifying the two would either break the documented ``csrf_token`` /
+ ``guest_token`` URLs the Embedded SDK relies on, or require patching FAB /
+ an app-wide routing change. So the divergence is working-as-designed. This
+ test pins that intended per-route contract so the behavior stays documented
+ and any accidental future change is caught.
+ """
+ # FAB-owned no-trailing-slash route: adding a stray slash hard-404s because
+ # there is no canonical slashed URL to redirect to.
+ response = client.open(
+ "/api/v1/security/login/", method="POST", follow_redirects=False
+ )
+ assert response.status_code == 404
Review Comment:
Good catch, added a control assertion that the canonical
`/api/v1/security/login` route (no stray slash) does not 404, so the test
actually pins the route existing.
##########
tests/unit_tests/security/api_test.py:
##########
@@ -66,6 +66,53 @@ def test_csrf_exempt_blueprints_with_api_key(app: Any,
app_context: None) -> Non
assert "ApiKeyApi" in {blueprint.name for blueprint in
csrf._exempt_blueprints}
+def test_security_api_trailing_slash_matches_route_ownership(client: Any) ->
None:
+ """Regression for #29934: sibling ``/api/v1/security/*`` endpoints respond
to
+ a misspelled (wrong trailing-slash) URL differently, and that difference is
+ the *intended* behavior — a Werkzeug routing artifact of who owns each
route,
+ not a bug.
+
+ Three routes live under the same ``/api/v1/security/`` prefix but are
+ declared with different slash conventions because they come from different
+ owners:
+
+ * ``login`` -> ``@expose("/login")`` (no trailing slash)
+ Flask-AppBuilder's own route. Superset does not own or register it, so
+ it inherits FAB's no-trailing-slash convention. Werkzeug hard-404s a
+ request that adds a stray trailing slash to a no-slash route (there is
+ no canonical slashed URL to redirect to).
+ * ``csrf_token`` -> ``@expose("/csrf_token/")`` (trailing slash)
+ * ``guest_token`` -> ``@expose("/guest_token/")`` (trailing slash)
+ Superset's own routes, whose trailing-slash URLs are the documented
+ canonical URLs (the Embedded SDK depends on them). Werkzeug
308-redirects
+ a request that omits the trailing slash to the canonical slashed URL.
+
+ Unifying the two would either break the documented ``csrf_token`` /
+ ``guest_token`` URLs the Embedded SDK relies on, or require patching FAB /
+ an app-wide routing change. So the divergence is working-as-designed. This
+ test pins that intended per-route contract so the behavior stays documented
+ and any accidental future change is caught.
+ """
+ # FAB-owned no-trailing-slash route: adding a stray slash hard-404s because
+ # there is no canonical slashed URL to redirect to.
+ response = client.open(
+ "/api/v1/security/login/", method="POST", follow_redirects=False
+ )
+ assert response.status_code == 404
+
+ # Superset-owned canonical trailing-slash routes: omitting the trailing
+ # slash 308-redirects to the documented canonical URL.
+ response = client.open(
+ "/api/v1/security/csrf_token", method="GET", follow_redirects=False
+ )
+ assert response.status_code == 308
+
+ response = client.open(
+ "/api/v1/security/guest_token", method="POST", follow_redirects=False
+ )
+ assert response.status_code == 308
Review Comment:
Good catch, added `Location` header assertions for both redirects so a
misrouted redirect would actually fail this test.
--
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]
