Safe redirect action#7695
Conversation
…t instead - note: I added a TODO for refactoring this but that should wait until we merge to develop since it will save us from having to bump the package version in 26.3 for this
|
|
||
| window.location.href = this.state.returnUrl || defaultUrl; | ||
| const redirectUrl = this.state.returnUrl || defaultUrl; | ||
| // TODO refactor this and other usages in platform/core/src/client to a helper safeRedirect() function from @labkey/components |
There was a problem hiding this comment.
as noted in the commit message, I'll wait until this gets merged to develop to address this TODO so that I don't need to bump @labkey/components for this branch (and have to handle the merge forward conflicts)
| } | ||
|
|
||
| // Called by various client components to ensure safe redirects, GitHub Issue #1023. This action redirects to | ||
| // local URLs only, never to an external site, even if the host is on the "Allowed External Redirect Hosts" list. |
There was a problem hiding this comment.
What is it that assures this uses local URLs only? Doesn't seem to be codified here.
There was a problem hiding this comment.
First, ActionURL is guaranteed to be a local URL. You can give it an external URL and it'll ignore the schema, host, and port, always using the local AppProps values (e.g., when calling getURIString()). Second, SimpleRedirectAction throws RedirectException, which now guarantees a local redirect. This is the result of a recent refactor I did to tighten up our server-side redirect handling. The only way to redirect externally now is via ExternalRedirectException (which ensures an allowed host) or UnsafeExternalRedirectException (where caller is responsible for ensuring the URL is safe).
There was a problem hiding this comment.
FYI: I expanded the action's comment to lay out the safety guarantees similar to the above
… should not redirect to external host
3 participants
Rationale
Simple server-side action to help with https://github.com/LabKey/internal-issues/issues/1023
Tasks 📍
Testing and verification of the issue fix will come after the client changes are implemented.
Related PR