Skip to content

Conversation

@brkalow
Copy link
Member

@brkalow brkalow commented Dec 3, 2024

Description

Introduce a new Clerk option, allowedRedirectProtocols that allows users to specify custom protocols to be allowed as part of user-provided redirect URLs. This is necessary for cases where deep-linking is used, in an Electron app for example.

As part of this, I've removed the protocol check from windowNavigate. This is an internal method that is used in only a few places, none of which should be accepting arbitrary, user-provided URLs. I've also added a JSDoc comment indicating windowNavigate should never be used when accepting user-provided URLs.

fixes SDKI-782

Closes: #4667

Checklist

  • pnpm test runs as expected.
  • pnpm build runs as expected.
  • (If applicable) JSDoc comments have been added or updated for any package exports
  • (If applicable) Documentation has been updated

Type of change

  • 🐛 Bug fix
  • 🌟 New feature
  • 🔨 Breaking change
  • 📖 Refactoring / dependency upgrade / documentation
  • other:

@brkalow brkalow requested a review from a team December 3, 2024 18:16
@changeset-bot
Copy link

changeset-bot bot commented Dec 3, 2024

🦋 Changeset detected

Latest commit: dce268f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 22 packages
Name Type
@clerk/clerk-js Minor
@clerk/types Minor
@clerk/chrome-extension Patch
@clerk/clerk-expo Patch
@clerk/astro Patch
@clerk/backend Patch
@clerk/elements Patch
@clerk/expo-passkeys Patch
@clerk/express Patch
@clerk/fastify Patch
@clerk/localizations Patch
@clerk/nextjs Patch
@clerk/nuxt Patch
@clerk/clerk-react Patch
@clerk/remix Patch
@clerk/clerk-sdk-node Patch
@clerk/shared Patch
@clerk/tanstack-start Patch
@clerk/testing Patch
@clerk/themes Patch
@clerk/ui Patch
@clerk/vue Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@vercel
Copy link

vercel bot commented Dec 3, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
clerk-js-sandbox ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 3, 2024 6:16pm

Comment on lines +17 to +18
* Note that this utility should **never** be called with a user-provided URL. We make no specific checks against the contents of the URL here and assume it is safe. Use `Clerk.navigate()` instead for user-provided URLs.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we replace the usage for windowNavigate from SignIn.ts and SignUp.ts with Clerk.navigate() ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need to, as the url is provided from our API and it will always be external.

@jacekradko jacekradko mentioned this pull request Dec 3, 2024
4 tasks
@brkalow brkalow merged commit 4e5e7f4 into main Dec 3, 2024
36 checks passed
@brkalow brkalow deleted the brk.feat/clerk-js-allowed-protocols branch December 3, 2024 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"tauri:" is not a valid protocol

5 participants