atinux / nuxt-auth-utils

Add Authentication to Nuxt applications with secured & sealed cookies sessions.
MIT License
872 stars 83 forks source link

nuxt-auth-utils throws `Unauthorized (/api/_auth/session)` on any 404 page #115

Closed cosbgn closed 2 months ago

cosbgn commented 3 months ago

I added sentry to my project and within days I had hundreds and hundreds of Unauthorized errors coming up. After investigating I arrived to the conclusion that:

nuxt-auth-utils throws `Unauthorized (/api/_auth/session)` on any 404 page

So since bots scan for /wp-admin etc I get a ton of these. This is a small reproduction (create a nitro plugin):

// server/plugins/server_error_handler.js

export default defineNitroPlugin((nitroApp) => {
    nitroApp.hooks.hook('error', (err, context) => {
        console.error(`${err?.message || "Server Error"} (${context?.event?.path})
    }
})

Open any 404 page like /api/does-not-exists or even client pages like /does-not-exists

The console will show:

 ERROR  Unauthorized (/api/_auth/session)

Not sure why this is happening but it doesn't seem like a normal behavior.

Yizack commented 3 months ago

Is your site prerendered or SSR disabled?

cosbgn commented 3 months ago

No it's the default Nuxt start app deployed on cloudflare pages.

atinux commented 2 months ago

Hi @cosbgn

This is tricky as the error page can have a header and display the user state.

Any unauthenticated call to any pages (200 or 404) will lead to a 401 for the /api/_auth/session, this is how we know if the user is authenticated or not.

You may want to only keep the 500 errors for Sentry don't you think?

cosbgn commented 2 months ago

If I'm using useUserSession() in the page it would make sense, but if my page is just: <template>hello</template> I would expect that nuxtAuthUtils won't call my backend. Also for my blog posts, 404 etc, I don't want the API call on every page.

In my opinion a 401 should be thrown only if I'm trying to access a protected page, not when I'm looking at the homepage.

I do use this in my sentry plugin, so it's not a big issue:

        if (context?.event?.path === "/api/_auth/session"){
            return; // Skip because we get a ton of unauthorized without reason
        }

Anyways, If this is by design, and it's the only way to get it working I'm happy to close this.

atinux commented 2 months ago

I tend to agree with you, the /api/_auth/session should not return a 401 as it is called on each page (once) (https://github.com/Atinux/nuxt-auth-utils/blob/main/src/runtime/app/plugins/session.server.ts)

Will make a PR to avoid returning a 401 of this API call.

cosbgn commented 2 months ago

Yes, ideally /api/_auth/session returns session || null and always 200.

atinux commented 2 months ago

Can you try with the v0.3.1 ?

cosbgn commented 2 months ago

Seems to work, I'll comment here if I spot some issue, but for now looks perfect. Thanks a lot!