nextauthjs / next-auth

Authentication for the Web.
https://authjs.dev
ISC License
24.96k stars 3.52k forks source link

Per Path authorization example doesn't work since adding signIn and signOut to SvelteKitAuth() #10187

Closed jdh148 closed 8 months ago

jdh148 commented 8 months ago

What is the improvement or update you wish to see?

The example on this page shows using the sequence() function to combine SvelteKithAuth() with a custom authorization function, but that is no longer possible because SvelteKitAuth() now returns { handle: Handle; signIn: Action; signOut: Action; } instead of a Handle.

Is there any context that might help us understand?

This was added in #9714 and the comments mention it affecting hooks.server.ts but "it doesn't change much", but no more details given.

Does the docs page already exist? Please link to it.

https://authjs.dev/reference/sveltekit#per-path

ndom91 commented 8 months ago

Thanks for pointing this out! We'll get it updated.

The "small change" is that SvelteKitAuth() returns an object now, one of the members of which is the handle function. Instead of only returning the handle function. That means that we'd recommend putting all the SvelteKitAuth stuff in a separate file now (i.e. src/auth.ts) and that returned handle function can then be imported in your hooks.server.ts

That means that it's supposed to be used like this now:

image

See: https://authjs.dev/reference/sveltekit#usage

ndom91 commented 8 months ago

Fixed in https://github.com/nextauthjs/next-auth/pull/10191, let us know if per path doesn't work for you now :+1:

jdh148 commented 8 months ago

Thanks for the quick response and update!

I was getting some errors with that new sample code, and I seem to have fixed the one error, but unsure how to fix the other one.

First, the import { handle: authenticationHandle } from './auth'; line in hooks.server.ts was throwing a few different errors:

Individual declarations in merged declaration 'handle' must be all exported or all local.ts(2395)
Import declaration conflicts with local declaration of 'handle'.ts(2440)
Module '"./auth"' has no exported member 'authenticationHandle'.ts(2305)

But I changed it to import { handle as authenticationHandle } from './auth'; and that resolved them. I'll need to adapt the sample to my project to verify it's actually taking effect, but this change let it compile successfully.

However, there's a second error that it seems to be ignoring. The satisfies Handle at the end of auth.ts gives this error:

Type '{ handle: Handle; signIn: Action; signOut: Action; }' does not satisfy the expected type 'Handle'.
  Type '{ handle: Handle; signIn: Action; signOut: Action; }' provides no match for the signature '(input: { event: RequestEvent<Partial<Record<string, string>>, string | null>; resolve(event: RequestEvent<Partial<Record<string, string>>, string | null>, opts?: ResolveOptions | undefined): MaybePromise<...>; }): MaybePromise<...>'.ts(1360)

Is this something that needs to be addressed? Until I finish adapting the sample to my project I can't be certain if it's causing a problem or not.

UPDATE: The code is working perfectly well despite the error around satisfies Handle - so I'm thinking it's safe to simply remove it?

Thanks again for the quick update to the documentation.

ndom91 commented 8 months ago

@jdh148 ah yeah good catch with the :/as with the import example, that was just a typo on my side. I'll fix that.

Regarding the satisfies handle typescriot issue, as you mentioned, it is in fact just a TS issue. It shouldn't impact runtime. Well take a look though 👍