Allow custom callback_url

[kengreeff]

Explain your changes

Hey team, I have added the ability to store the options object in the verifier cookie so that once verified we can check if the callback_url has been set and redirect there instead of the defaults.

Checklist

• I have read the “Pull requests” section in the contributing guidelines.
• I agree to the terms within the code of conduct.

🛟 If you need help, consider asking for advice over in the Kinde community.

[DaveOrDead]

Hey @kengreeff thanks so much for the PR.

Love your work but I do have one concern from a security perspective about passing the redirect url as part of the query string.

Essentially I could share a link https://app.kengreeff.com/login?callback_url=https://daves-dodgy-app.com and share this around which means when people log into your app they would get redirected to my nefarious one.

I have a couple of ideas on how we could achieve this securely and would love your thoughts:

1. We use something like a nextPath argument instead of a whole url, which means at least the user can't be sent outside the confines of the call site.
2. We take a different approach and use middleware to intercept a route request and set a cookie - similar to @peterphanouvong's proposal here except we would call the cookie something like kinde_next_url so that in the case of restricted actions you could simply set a cookie with the same name.

[kengreeff]

@DaveOrDead Yea, good thinking - realistically it is always a path, not a full url - I just copied the naming convention from next auth.

Let's roll with nextPath - I dont know if the middleware approach works because it isn't always page based. In our use case we have it on a "like" button, so only the button requires auth, not the whole page. We just redirect back to the page

Do you want to use nextPath or next_path - I wasn't sure as I couldn't identify a common convention

[kengreeff]

Would callback_path be more clear?

[DaveOrDead]

@kengreeff ah I wasn't clear enough in what I meant by saying:

in the case of restricted actions you could simply set a cookie with the same name.

that was to address this use case:

I dont know if the middleware approach works because it isn't always page based. In our use case we have it on a "like" button, so only the button requires auth, not the whole page. We just redirect back to the page

If we set a convention for the cookie name, for example kinde_next_url then when a user clicks your "like" button you could set a cookie called kinde_next_url with the current page as the value and redirect to /login.

After the user authenticates on Kinde they are redirected to your app and the middleware reads that cookie and puts them back on the page they were on.

This means that for the Pages use case, nothing needs to be set by the developer as it is just handled by default by the middleware, but in the case of actions such as "like" you could opt in to setting a cookie.

Otherwise the developer would have to manipulate the login/register url in the case of both page restriction and actions restriction.

[kengreeff]

@DaveOrDead that seems like it might be quite a bit of work when compared to a callback path in the query string.

This is an example of what we do now:

<Button
  as={isAuthenticated ? 'button' : 'a'}
  colorScheme={userPostLikesCount ? 'red' : 'gray'}
  leftIcon={userPostLikesCount ? <HiHeart fontSize={24} /> : <HiOutlineHeart fontSize={24} />}
  href={isAuthenticated
    ? undefined
    : `/users/login?redirect=${redirect || `/${post?.id}`}`}
  onClick={userPostLikesCount
    ? () => deletePostLikeFn({
      id: post?.id,
    })
    : () => createPostLikeFn({
      id: post?.id,
    })}
  size="md"
>
  {postLikesCount} {postLikesCount === 1 ? 'Like' : 'Likes'}
</Button>

I feel like the cookie route would add a lot more code for the developer.

Could you possibly show me what your implementation would look like in this code block?

[kengreeff]

looks like nextauth handles redirects like this:

redirect({ url, baseUrl }) {
  if (url.startsWith("/")) return `${baseUrl}${url}`
  else if (new URL(url).origin === baseUrl) return url
  return baseUrl
},

https://github.com/nextauthjs/next-auth/blob/main/packages/next-auth/src/core/lib/default-callbacks.ts#L7

Seems like a simple way to prevent dodgy redirects - thoughts?

[DaveOrDead]

@kengreeff looks good to me

[kengreeff]

this was a real gotcha in dev!

[kengreeff]

@DaveOrDead ready for you mate - I have also removed some unused code from files where I found it to clean up.

I have left as callback_url for now as it can be a full url (if it matches the origin) - let me know if you want to change.

I tested the old code and could be redirected to google, the new code prevents this.

[kengreeff]

Added this to make dev a bit easier yarn build:watch

[DaveOrDead]

@kengreeff all looks awesome. The only thing I'd say it I'm not sure on calling the option callback_url just because it has a specific meaning in oauth flows that it is the url being called back to after authentication to complete the flow. Whereas this is specifically the url to pass onto post-authenticating and after the callback has completed.

Maybe next_url?

What are your thoughts @peterphanouvong ?

[kengreeff]

How about redirect_url?

[DaveOrDead]

Thank for the PR 🚀