codeant-ai-for-open-source[bot] commented on code in PR #38033:
URL: https://github.com/apache/superset/pull/38033#discussion_r3556209224
##########
superset-frontend/src/pages/Login/index.tsx:
##########
@@ -95,15 +96,19 @@ export default function Login() {
}, []);
const loginEndpoint = useMemo(
- () => (nextUrl ? `/login/?next=${encodeURIComponent(nextUrl)}` :
'/login/'),
+ () =>
+ ensureAppRoot(
+ nextUrl ? `/login/?next=${encodeURIComponent(nextUrl)}` : '/login/',
+ ),
Review Comment:
**Suggestion:** `SupersetClient.postForm` already prefixes endpoints with
the configured app root internally, so wrapping the login endpoint with
`ensureAppRoot` here causes double-prefix URLs (for example
`/superset/superset/login/`) in subpath deployments and breaks DB/LDAP login
with 404s. Keep this endpoint as a raw relative path when calling `postForm`.
[api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ DB/LDAP login POST hits double-prefixed, non-existent endpoint.
- ❌ Subpath deployments cannot authenticate using database credentials.
- ⚠️ OAuth/OIDC logins work but endpoint handling inconsistent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Load the login page implemented in
`superset-frontend/src/pages/Login/index.tsx:83-160`. The `loginEndpoint`
memo is defined
at lines 98-104 and now wraps the relative login path with `ensureAppRoot`:
- Line 98: `const loginEndpoint = useMemo(`
- Lines 99-102: `() => ensureAppRoot(nextUrl ? '/login/?next=...' :
'/login/')`
2. Inspect `ensureAppRoot` in
`superset-frontend/src/utils/pathUtils.ts:44-60`. For a
subpath deployment where `applicationRoot()` returns `'/superset'` (derived
from
`getBootstrapData().common.application_root` in
`src/utils/getBootstrapData.ts:68-82`),
calling `ensureAppRoot('/login/')` returns `'/superset/login/'` (line 59:
`return
\`\${root}\${normalizedPath}\`;`).
3. For database or LDAP authentication, the login form submit handler
`onFinish` at
`superset-frontend/src/pages/Login/index.tsx:133-141` invokes the shared
client:
- Line 139: comment about standard form submission for Flask-AppBuilder
compatibility
- Line 140: `SupersetClient.postForm(loginEndpoint, values, '');`
In
`packages/superset-ui-core/src/connection/SupersetClientClass.ts:119-151`,
`postForm` builds the URL using `this.getUrl({ endpoint })`, and `getUrl`
at lines
26-42 constructs:
- `return
\`\${this.protocol}//\${cleanHost}\${this.appRoot}/\${endpoint[0] === '/' ?
endpoint.slice(1) : endpoint}\`;`
This means the configured `appRoot` is always prefixed to the provided
`endpoint`.
4. In a subpath deployment where SupersetClient is configured with `appRoot
= '/superset'`
(matching the behaviour validated by
`packages/superset-ui-core/test/connection/SupersetClient.test.ts:109-112`,
which expects
`getUrl({ endpoint: '/some/path' })` to contain `'/app/some/path'` when
`appRoot:
'/app'`), the combination of steps 2 and 3 yields:
- `loginEndpoint` from React: `'/superset/login/?next=...'`
- `getUrl` with `appRoot = '/superset'`:
`https://host/superset/superset/login/?next=...`
This double-prefixed path (`/superset/superset/login/`) does not match
the backend
login route (`/superset/login/`), so the POST from
`SupersetClient.postForm` hits a
non-existent endpoint and the DB/LDAP login request fails with a 404 in
subpath
deployments.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2b1c2332e5e7414f97226c017cc98c00&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=2b1c2332e5e7414f97226c017cc98c00&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/pages/Login/index.tsx
**Line:** 99:102
**Comment:**
*Api Mismatch: `SupersetClient.postForm` already prefixes endpoints
with the configured app root internally, so wrapping the login endpoint with
`ensureAppRoot` here causes double-prefix URLs (for example
`/superset/superset/login/`) in subpath deployments and breaks DB/LDAP login
with 404s. Keep this endpoint as a raw relative path when calling `postForm`.
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%2F38033&comment_hash=aa2598df29589a1ce038f30475cf9396b51777318e3d240363b9580b0430ded5&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38033&comment_hash=aa2598df29589a1ce038f30475cf9396b51777318e3d240363b9580b0430ded5&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]
