-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat(nuxt): navigateTo
supports external redirects
#5022
Conversation
β Deploy Preview for nuxt3-docs canceled.
|
Should it only allow http/https based urls? Or would mailto and tel also need to work? A check that does include mailto and tel. try {
url = new URL(string);
//url route
} catch (_) {
//normal route
} Maybe it would be even better to have no check and have a additional entry in the to object so that the following would work: navigateTo({url: "https://github.com"}); |
Great idea! Will incorporate that in the code
That would be one option but would leave the "check" up to the user. IMO the DX is better when Nuxt is doing the check for the user instead of leaving it up to them. |
Thanks for this PR @manniL π In general this is a really nice improvement that we support redirection to external URLs but I'm also little bit concerned about potential security risks implied by combination of this new feature and developer's mistake with a programmatic call. I suggest some options:
|
Thanks for the feedback! π What would be your favorite solution here? |
I would probably do both:
|
That means not executing any links that start with
By throwing an error with the approriate message, right?
I'd probably go with |
Considering the possibility of native app linking with custom protocols, yes the best we can exclude unsafe
Would be an overkill to use good-old window.confirm as the default fallback? For server yes error is the best!
|
I will add a little feedback as developer who faced with this problem. In general, reading the documentation, one gets the impression that the |
601fd83
to
0299ab2
Compare
Updated the PR with the recommended changes π |
Until this is merged βΒ I've worked up a small composable function to allow explicit external redirects from route middleware: https://gist.github.com/justin-schroeder/f583797c702155f32c9078d1add2a594 |
Co-authored-by: Koen van Staveren <koenvanstaveren@hotmail.com>
Co-authored-by: Daniel Roe <daniel@roe.dev>
b0fb76f
to
633e6e3
Compare
Hello! Are there any updates on this? It is quite critical for important workflows like OAuth 2.0 (being able to follow a server redirection to the login form, etc.), unless there is some other way to do it I missed. |
Found an interesting workaround for this one, prepending your URL with Works because of parser differentials between the browser and vue-router/ufo. Couldn't find a way to do this for other protocols. |
@pi0 Please let me know what is left to do here besides the docs PR |
Thanks again for PR @manniL and sorry it took long. Finally, I've made few refactors to inline and simplify logic, avoid extra parse calls, improve messages and make the option simpler |
} else { | ||
location.href = toPath | ||
} | ||
return Promise.resolve() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One last thing to ensure: In nuxt 2, we had this problem with external navigations that during navigation, rest of logic goes on. And solution was a custom made error because it was sync. I think we could do something like a never resolving promise that avoids this. (will make a reproduction and implement in later PR btw need more testing) => https://github.com/nuxt/framework/issues/6906
(We also need to document new |
if (process.client && isProcessingMiddleware()) { | ||
|
||
const toPath = typeof to === 'string' ? to : ((to as RouteLocationPathRaw).path || '/') | ||
const isExternal = hasProtocol(toPath, true) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why you check protocol for external? may be I want to redirect within current domain, i.e. respond with headers
HTTP/1.1 302 Found
Location: /my-legacy-page
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would you please open an issue with a sandbox to track? ππΌ
π Linked issue
https://github.com/nuxt/framework/discussions/4997, resolves nuxt/nuxt#14067
β Type of change
π Description
Right now,
navigateTo
supports only internal redirects (either viavue-router
or by using thebaseURL
).This PR adds rudimentary support for external redirects and protocol checking:
ufo
)script:
, Nuxt will immediately throw an error on client- and server-sideallowExternal
is not set to true, then a confirmation dialog will pop up on client-side and an error will be thrown on server-sideh3
on the server-side orlocation.replace
/location.href = X
on the client-sideAlso feel free to tell me if that's the right approach or if we want another function for this behavior π€
Doc update will follow when the PR is more mature.
π Checklist