Fix login redirects when metabot is enabled#73717
Open
npfitz wants to merge 1 commit intomastermetabase/metabase:masterfrom
uxw-3939-redirect-after-login-is-brokenmetabase/metabase:uxw-3939-redirect-after-login-is-brokenCopy head branch name to clipboard
Open
Fix login redirects when metabot is enabled#73717npfitz wants to merge 1 commit intomastermetabase/metabase:masterfrom uxw-3939-redirect-after-login-is-brokenmetabase/metabase:uxw-3939-redirect-after-login-is-brokenCopy head branch name to clipboard
npfitz wants to merge 1 commit intomastermetabase/metabase:masterfrom
uxw-3939-redirect-after-login-is-brokenmetabase/metabase:uxw-3939-redirect-after-login-is-brokenCopy head branch name to clipboard
Conversation
Contributor
e2e tests failed on
|
| File | Test Name |
|---|---|
model-actions.cy.spec.js |
Write actions on model detail page (postgres) > should respect impersonated permission |
source-replacement.cy.spec.ts |
scenarios > data-studio > source replacement > Native queries > replaces a table referenced in a native SQL question |
usage-auditing.cy.spec.ts |
scenarios > metabot > usage auditing > renders usage stats charts for selected date shortcuts on conversations |
usage-auditing.cy.spec.ts |
scenarios > metabot > usage auditing > renders usage stats charts for selected date shortcuts on tokens |
dependency-unreferenced-list.cy.spec.ts |
scenarios > dependencies > unreferenced list > analysis > should not show referenced entities |
usage-auditing.cy.spec.ts |
scenarios > metabot > usage auditing > renders usage stats charts for selected date shortcuts on messages |
table-editing.cy.spec.ts |
scenarios > table-editing > table edit mode > should allow to view row details |
native-database-source.cy.spec.js |
scenarios > question > native > database source > selecting a database in native editor for model actions should not persist the database |
usage-auditing.cy.spec.ts |
scenarios > metabot > usage auditing > drills through from conversation charts to the conversations list and updates list filters |
usage-auditing.cy.spec.ts |
scenarios > metabot > usage auditing > drills through from the conversations by day chart to the conversations list |
kevinkelleher12
approved these changes
May 6, 2026
Contributor
kevinkelleher12
left a comment
There was a problem hiding this comment.
Looks good to me. Did you think about adding redirects to the 401 handler in app-main.js? It seems like from the PR description you think this might be tricky, but it would be a more robust solution.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This issue came from the unauthenticated -> redirect logic at the API layer, rather than the route guard layer. When metabot is enabled and a user is unauthenticated, the App (and App Bar) are still mounted, causing
use-user-metabot-permissions.tsto reach out to/user-permissionsand get a 401. This caused a hard redirect to/auth/loginwithout the redirect search param. The fix here is to only make this call if a user is logged in, though this is a class of bug that requires a broader solution to prevent in the future.How to verify
Checklist