Fix login redirect for expired sessions #57157
Conversation
There was a problem hiding this comment.
Thanks for fixing this! Your changes address a majority of the issues that I've been able to find with our logout behavior. There's one other super-edge case here that Larry recently fixed with #56786 and #57201. When the user is auto logged out of a non-default space, they're still being redirected to the equivalent of http://localhost/s/foo/login instead of http://localhost/login. At this point, I think it's just cosmetic because the next query-string parameter prevents #56695 from occurring, but there might be some actual issue I'm overlooking or a future issue. How would you feel about using this PR to consume the logout URL injected metadata so we're consistent here also?
Use injected logoutUrl instead of basePath. Also added `provider` param to legacy autoLogout service, and rewrote tests for SessionExpired.
Ah, I also think it's cosmetic but agree it's safer/better to use the single logout URL everywhere. I tested it and it worked fine that way, so I made the change. Good idea! |
|
@elasticmachine merge upstream |
|
ACK: rereviewing |
|
retest |
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
…re/files-and-filetree * 'master' of github.com:elastic/kibana: (139 commits) Move Ace XJSON lexer-rules, worker and utils to es_ui_shared (elastic#57563) [Upgrade Assistant] Fix filter deprecations search filter (elastic#57541) [ML] New Platform server shim: update indices routes (elastic#57685) Bump redux dependencies (elastic#53348) [Index management] Client-side NP ready (elastic#57295) change id of x-pack event_log plugin to eventLog (elastic#57612) [eventLog] get kibana.index name from config instead of hard-coding it (elastic#57607) revert allow using any path to generate fixes ui titles (elastic#57535) Fix login redirect for expired sessions (elastic#57157) Expose Vis on the contract as it requires visTypes (elastic#56968) [SIEM][Detection Engine] Fixes queries to ignore errors when signals index is not present [Remote clusters] Migrate client-side code out of legacy (elastic#57365) Fix failed test reporter for SIEM Cypress use (elastic#57240) skip flaky suite (elastic#45244) update chromedriver to 80.0.1 (elastic#57602) change slack action to only report on whitelisted host name (elastic#57582) [kbn/optimizer] throw errors into stream on invalid completion (elastic#57735) moving visualize/utils to new platform (elastic#56650) ...
This PR changes the behavior of session expiration logouts. It removes logic that strips basePath out of the
nextparameter.Note: there is a known bug, #22440, that prevents the session expiration message and
nextparameter from showing on the login page when the user fails authentication in the legacy platform. That is partially addressed in this PR (for the basic and token auth providers), but it will need to be fully addressed in a separate PR.Resolves #57113.