Fix OAuth2 login redirect when using context path#1892
Conversation
OAuth2 login did not set an authenticationSuccessHandler, so it fell back to Spring Security's default handler which is not aware of the servlet context path (SERVER_SERVLET_CONTEXT_PATH). When the app is hosted under a base path (e.g. /kafka), the post-login 302 redirect omitted the context path, leaving the user stranded instead of being routed back into the UI. The LOGIN_FORM and LDAP configs already attach emptyRedirectSuccessHandler() (which prepends the context path via EmptyRedirectStrategy); apply the same handler to oauth2Login for parity. Also adds unit coverage for EmptyRedirectStrategy.
📝 WalkthroughWalkthrough
ChangesOAuth Empty Redirect Handler
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hi omerlebo! 👋
Welcome, and thank you for opening your first PR in the repo!
Please wait for triaging by our maintainers.
Please take a look at our contributing guide.
|
AI Summary OAuth2 login redirects ignore the servlet context path, causing users to be redirected to a URL without the base path (e.g., |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/src/test/java/io/kafbat/ui/util/EmptyRedirectStrategyTest.java`:
- Around line 48-57: The test method doesNotRewriteAbsoluteUrlRedirects only
covers the case where an absolute URL has an explicit path segment (like
/login), but the EmptyRedirectStrategy.createLocation() method has a separate
code path for URLs with empty paths. Add a new test method to
EmptyRedirectStrategyTest that tests the scenario with an absolute URL that has
no path (e.g., https://auth.example.com without the /login suffix). In this new
test, create an exchange with a context path, pass an absolute URI with an empty
path to the STRATEGY.sendRedirect() method, and verify the actual behavior when
the path is empty string to ensure the implementation correctly handles this
case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffcf7470-538d-4406-b826-30086b792b84
📒 Files selected for processing (2)
api/src/main/java/io/kafbat/ui/config/auth/OAuthSecurityConfig.javaapi/src/test/java/io/kafbat/ui/util/EmptyRedirectStrategyTest.java
Addresses review feedback: add a case for an absolute URL with an empty path, which hits the createLocation() empty-path branch and is made context-relative. Documents/verifies actual behavior so both branches are covered.
Closes #1893
What's changed
OAuth2 login did not set an
authenticationSuccessHandler, so it fell back to Spring Security's default handler, which is not aware of the servlet context path (SERVER_SERVLET_CONTEXT_PATH/server.servlet.context-path).When kafka-ui is hosted under a base path (e.g.
/kafka), the post-login302redirect omits the context path. The browser is sent to a location that doesn't include/kafka, so the user is stranded after authenticating instead of being routed back into the UI. Manually navigating to/kafkaafter login works, confirming auth itself succeeds — only the redirect target is wrong.The
LOGIN_FORMandLDAPconfigs already attachemptyRedirectSuccessHandler()(which prepends the context path viaEmptyRedirectStrategy):This PR applies the same handler to
oauth2Loginfor parity:emptyRedirectSuccessHandler()is already defined in the sharedAbstractAuthSecurityConfigbase class — it simply wasn't wired into the OAuth2 flow.Tests
Adds
EmptyRedirectStrategyTestcovering the context-path behavior the fix depends on (and which was previously untested):/) redirect →/kafka//kafka/ui/clustersHow to reproduce
auth.type=OAUTH2).SERVER_SERVLET_CONTEXT_PATH=/kafka.302redirect does not point to/kafka, leaving the user on a blank/unrouted page. Manually visiting/kafkaworks.Affects
mainand the latest release (v1.5.0).Summary by CodeRabbit
Bug Fixes
Tests