Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support cross domain credentials #422

Merged
merged 4 commits into from
Jun 28, 2021
Merged

Support cross domain credentials #422

merged 4 commits into from
Jun 28, 2021

Conversation

jamescryer
Copy link
Contributor

Description

I have two or more Next.js applications which are deployed independently. They provide different services to users, and live under different subdomains e.g. login.mywebapp.com and people.mywebapp.com. For the user this is a single web application with a single session.

I'd like for the "login" web application to implement the nextjs-auth0 APIs, and have the other applications make use nextjs-auth0 frontend helpers without needing to implement the APIs as well. The other applications should use the APIs under the "login" sub domain.

I can almost make this work by

  • Setting the environment variables NEXT_PUBLIC_AUTH0_LOGIN and NEXT_PUBLIC_AUTH0_PROFILE in all web apps
  • Setting the header Access-Control-Allow-Origin and Access-Control-Allow-Credentials in the API handler in the login web app.
  • Setting the session.cookie.domain as the .mywebapp.com so that it is shared across sub domains.

But the cookie is not sent with the request to https://login.mywebapp.com/api/auth/me, returning an "unauthorised" response.

This PR makes it possible to send the auth cookie across sub domains by setting the { credentials: 'include' } option in the checkSession() fetch call.

Testing

  • Set the environment variable NEXT_PUBLIC_AUTH0_PROFILE to a different URL, and add any cookie to that domain. Upon loading the page a request will be sent to https://login.mywebapp.com/api/auth/me with the cookie sent on the request.

  • This change adds test coverage for new/changed/fixed functionality

Checklist

  • I have added documentation for new/changed functionality in this PR or in auth0.com/docs
  • All active GitHub checks for tests, formatting, and security are passing
  • The correct base branch is being used, if not main

@vercel
Copy link

vercel bot commented Jun 22, 2021

@jamescryer is attempting to deploy a commit to the Auth0 Team on Vercel.

A member of the Team first needs to authorize it.

@jamescryer
Copy link
Contributor Author

Hey. I'm leaving this as a draft PR for now as for some yet unknown reason I'm unable to install the examples to try the end-to-end tests. It errors with Could not resolve dependency: ... peer next@">=9.4.4" from @serverless-jwt/[email protected]. I don't have time to work through this right now.

But if automation can run the those test and you're all happy with the idea then perhaps we can mark as "ready for review"? Also, I've not added any documentation, let me know if you'd like something added.

@adamjmcgrath
Copy link
Contributor

Hi @jamescryer - thanks for raising this. I'd be happy to accept a PR that allowed you to optionally pass a custom fetcher (similar to useSWR) which should resolve your issue, eg

<Auth0Provider
  fetcher={async (url) => {
      const response = await fetch(url, { credentials: 'include' });
      return response.ok ? response.json() : undefined;
  }}

@jamescryer jamescryer marked this pull request as ready for review June 23, 2021 14:58
@jamescryer jamescryer requested a review from a team as a code owner June 23, 2021 14:58
@jamescryer
Copy link
Contributor Author

@adamjmcgrath Thanks for your feedback. Do my changes work for you?

Copy link
Contributor

@adamjmcgrath adamjmcgrath left a comment

Choose a reason for hiding this comment

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

lgtm - thanks @jamescryer

just a small request on the test case

const fetchSpy = jest.fn();
(global as any).fetch = fetchSpy;

const customFetcher = jest.fn().mockReturnValue(Promise.resolve());
Copy link
Contributor

Choose a reason for hiding this comment

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

const customFetcher = jest.fn().mockReturnValue(Promise.resolve('foo'));
// then    
expect(result.current.user).toBe('foo');

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good shout. I've made that change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks - I just need to wait a day or two for something before I can merge. Will keep you posted

@adamjmcgrath adamjmcgrath merged commit 594d752 into auth0:main Jun 28, 2021
@adamjmcgrath
Copy link
Contributor

Thanks @jamescryer - I'll try and get round to releasing it this week

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants