feat: expo auth

[juliusmarminge]

Closes #486

[juliusmarminge]

• feat: expo auth #720 👈
• nativewind v4 #715
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @juliusmarminge and the rest of your teammates on Graphite

[ochicf]

This doesn't work for me, as the cookie starts differently and does not find any, throwing the error just below. Here's an example of my value:

authis.pkce.code verifier=; Max-Age=0; Path=/: HttpOnly: SameSite=Lax, authis.session-token=270ca062-6d2e-4f78-82c9-7690aaff8a6f: Path=/: Expires=Fri, 21 Jun 2024 07:20:50 GMT; HttpOnly; SameSite=Lax

Since the cookie pattern is defined as a regex in this file, could we just use it here to match the cookie?

const setCookie = authResponse.headers
  .getSetCookie()
  .find((cookie) => AUTH_COOKIE_PATTERN.test(cookie));

[Wundero]

The pattern itself is hardcoded as well - if you change the authjs cookie name, this will have issues no matter what. Perhaps it might make more sense to customize the cookie to be acme.session-token instead, and that way users of the app can just find+replace acme and will cover the cookie as well?

An alternative would be to just ignore the prefix with something like this:

const AUTH_COOKIE_PATTERN = /(?:\w+)\.session-token=([^;]+)/;

And then apply your suggestion to test the cookie.

[ochicf]

@Wundero I just realised that I misscopied the contents of the cookie 🤦

The part of the authjs.session-token does not change (at least on my case), I just wanted to highlight that the cookie contains another starting part (the authjs.pkce.code_verifier=... which caused that the .startsWith(...) condition to not comply.

authjs.pkce.code_verifier=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax, authjs.session-token=e9bf1a44-44f1-453f-b492-fc26adec22ef; Path=/; Expires=Sat, 22 Jun 2024 07:33:02 GMT; HttpOnly; SameSite=Lax

With that being clarified, your point is still valid as NextAuth allows to configure cookies (including changing its names), so even the .session-token part could be changed. Is this something that needs to be supported? IMHO it's too of an edge case and not worth it, but still, adding a possible way below.

Details

I guess the regex could be created from exported authConfig's cookie configuration (if any), defaulting to the current one:

const AUTH_COOKIE_NAME =
  authConfig.cookies?.sessionToken?.name || "authjs.session-token";
const AUTH_COOKIE_PATTERN = new RegExp(
  `${AUTH_COOKIE_NAME.replace(/\./g, "\\.")}=([^;]+)`,
);

Notes:

• because of the satisfies NextAuthConfig: the actual exported authConfig does not have the cookies and the nested properties, so TS coplains of the authConfig.cookies?.sessionToken?.name access, so this would need to be handled
• the authConfig object needs to be exported from @acme/auth
• there's some string manipulation things going on (replacing . with \\. so the regexp is properly created... there could be more edge cases so things could break

[Wundero]

The startsWith should still work, since the getSetCookie will return a list of all named cookies - the authjs.pkce.code_verifier (or any other authjs cookie) is a separate entry in the list, and should fail that check. In your example, it would split the cookies at the , and return these:

['authjs.pkce.code_verifier=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax', 'authjs.session-token=e9bf1a44-44f1-453f-b492-fc26adec22ef; Path=/; Expires=Sat, 22 Jun 2024 07:33:02 GMT; HttpOnly; SameSite=Lax']

[ochicf]

I'm not sure why (maybe different system, code or package versions?), but In my case it does not return the same as you.

What you provided:

// outer array
[
  // item 1
  'authjs.pkce.code_verifier=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax',
  // item 2
  'authjs.session-token=e9bf1a44-44f1-453f-b492-fc26adec22ef; Path=/; Expires=Sat, 22 Jun 2024 07:33:02 GMT; HttpOnly; SameSite=Lax'
]

What I'm getting:

// outer array
[
  // item 1 - the comma is inside the string -------------------------- v
  'authjs.pkce.code_verifier=; Max-Age=0; Path=/; HttpOnly; SameSite=Lax, authjs.session-token=e9bf1a44-44f1-453f-b492-fc26adec22ef; Path=/; Expires=Sat, 22 Jun 2024 07:33:02 GMT; HttpOnly; SameSite=Lax'
]

So, in my case there's a single item that does not start with authjs.session-token.

To add a bit more into the mix, I'm doing some tests with HTTPS and the name of the cookie changes, which would make the startsWith to not work even if your case (note the __Secure-authjs.session-token names). TBH I'm trying with nextjs --experimental-https, haven't tried this in a production site or anything, so not sure if that would affect:

[
  '__Secure-authjs.pkce.code_verifier=; Max-Age=0; Path=/; HttpOnly; Secure; SameSite=Lax, __Secure-authjs.session-token=d99b92b9-acf2-4f01-b1bc-91cff0bff535; Path=/; Expires=Thu, 27 Jun 2024 07:33:38 GMT; HttpOnly; Secure; SameSite=Lax'
]

In any case, IMHO it feels more consistent and resilient doing the AUTH_COOKIE_PATTERN.test(...), as just below it's doing a setCookie?.match(AUTH_COOKIE_PATTERN) which also allows the cookie to be in any place of the string.

[juliusmarminge]

Hey guys,

Sorry for not keeping up here, been busy.

You seem to have done a lot of exploration into getting the IP to work, I wonder if it wouldn't be easier to work around the csrf stuff in dev?

Have you tried what Nico suggested with the trust host thing?

[Wundero]

Hey guys,

Sorry for not keeping up here, been busy.

You seem to have done a lot of exploration into getting the IP to work, I wonder if it wouldn't be easier to work around the csrf stuff in dev?

Have you tried what Nico suggested with the trust host thing?

The branch I shared does this - afaik my branch works, and the checks can probably be turned back on for production, so I think it's a viable option. I can make a PR into this branch (like what ochicf has done) for you to review it a bit more, if that helps, but there's more than just CSRF that's a problem.

The big one is that NextJS sets request.url to localhost:3000 even if you make requests from other IPs, and that becomes a problem for callbacks. You need to patch it to be the Host header they requested the API from, as per my comment before.

I think both of these routes can work, and each one has pros and cons, so I think we can decide how we want to go forward once you have taken a look into both optins.

[ochicf]

@juliusmarminge I've been fiddling a bit more on the environment variables approach and pushed some new code to #1045. I've updated the description there writing the highlights, pros and cons, so you can have more info and evaluate whether this is a valid approach. I've also added some screenshots and a screen capture (which I'm adding here to make this comment sexier)

Screen.Recording.2024-05-28.at.16.47.51.mov

Screen.Recording.2024-05-28.at.16.29.45.mov

@Wundero would love your inputs aswell. Also I'll try to look deeper on your code and do some attempts aswell (probably to EOW/during weekend). If you push your working version in a MR would be great ❤️

[Wundero]

@juliusmarminge I've been fiddling a bit more on the environment variables approach and pushed some new code to #1045. I've updated the description there writing the highlights, pros and cons, so you can have more info and evaluate whether this is a valid approach. I've also added some screenshots and a screen capture (which I'm adding here to make this comment sexier)

Screen.Recording.2024-05-28.at.16.47.51.mov
Screen.Recording.2024-05-28.at.16.29.45.mov
@Wundero would love your inputs aswell. Also I'll try to look deeper on your code and do some attempts aswell (probably to EOW/during weekend). If you push your working version in a MR would be great ❤️

I've gone ahead and filed the MR here: #1054. I am gonna continue trying to get the stuff to work for me at some point, but for now I believe this is sufficient to get the app working.

I haven't verified that the login step completes at the end because I ran into linking issues, and I have recently reinstalled my boot drive and thus don't have the software setup (namely android studio) and haven't had the time to get back around to that yet.

As far as input goes, I think your method works quite well (as demonstrated), but there is definitely a lot of complexity as well. I think your implementation does the best with the concept of changing the env vars, but to me that feels fundamentally finicky (not that my approach of rewriting requests is much better haha).

Let me know what you think of my PR and whether you could get it working, since I got 99% of the way there and the roadblock I hit was that expo wasn't processing the acme://... links (i changed the slug on my end to ct3t since i called the project that on expo.dev, but the core issue of ct3t://login?session_token=... not being a valid link was the crux of the issue). Any advice on how to resolve that issue would be appreciated, since that's really my final blocker for validation.

[ochicf]

@Wundero thanks for this. I've tried your MR and worked fine for me with Discord on first try with my existing project, and also from a clean project. Definitely much more simpler and less code, so I think it's a better take!

I still had to add the metro IP in the allowed list from Discord app to be able to complete login from Expo, which maybe would not be super clear if I didn't faced with this issue already, so it could be worth adding this in the FAQ instead of the current AUTH_URL explanation?

BTW I added a question on your MR.

[Wundero]

@Wundero thanks for this. I've tried your MR and worked fine for me with Discord on first try with my existing project, and also from a clean project. Definitely much more simpler and less code, so I think it's a better take!

I still had to add the metro IP in the allowed list from Discord app to be able to complete login from Expo, which maybe would not be super clear if I didn't faced with this issue already, so it could be worth adding this in the FAQ instead of the current AUTH_URL explanation?

BTW I added a question on your MR.

I actually only tested this when using the deployed auth proxy nitro app, so that was what I was using, but adding the IP to discord or the auth proxy to discord are both valid (but at least one needs to be done). Glad to hear it works though. I will update the FAQ/etc. sometime in the coming day or two, when I have a bit more time.

[Wundero]

I've fixed all the issues I was having with my PR and it should be good to go now. Tested the auth on a real android device and it works seamlessly, and all security concerns should be alleviated since I only make the security relaxations in dev mode. Works well with the auth proxy, and supports localhost:3000 for the web without issue as well.

@juliusmarminge I think #1054 is now a feature-complete solution that doesn't really have compromises, so I would appreciate if you could take some time to review the PR and we could maybe merge it or come up with next steps.

[juliusmarminge]

Okay this works great! Think we're good to merge it now.

Some heads up for those who's been lurking this PR (for far too long), you need to either deploy the proxy (RECOMMENDED) or set your host IP in the accepted redirect URIs on the OAuth provider. The latter might not be valid if you're using e.g. Github OAuth which only allows a single URL per project.

[kiikoh]

Many thanks to all involved contributors