sidebase / nuxt-auth

Authentication built for Nuxt 3! Easily add authentication via OAuth providers, credentials or Email Magic URLs!
https://auth.sidebase.io
MIT License
1.31k stars 164 forks source link

fix: properly disable `getSession` endpoint for local and refresh #768

Closed Armillus closed 4 months ago

Armillus commented 5 months ago

πŸ”— Linked issue

https://github.com/sidebase/nuxt-auth/issues/761

❓ Type of change

πŸ“š Description

The linked issue just covered half of the problem ; in the current form, getSession cannot be disabled, even with a configuration extracted from the documentation such as:

{
  auth: {
    baseURL: '/api/auth',
    provider: {
      type: 'local',
      endpoints: {
        getSession: false,
      }
    }
  }
}

It's not an issue while you're not calling it, except that it is called by signIn anyways, at least with the local and refresh providers. Eventually, it means that an unexpected HTTP request is sent, which on one hand is inefficient, and on the other hand throws an exception from signIn, thus making it fail.

To fix those problems, this PR is applying the same strategy for getSession as for signOut, in both the function implementation and the exported type.

πŸ“ Checklist

zoey-kaiser commented 4 months ago

Hi @Armillus πŸ‘‹

Firstly, I love the PR title πŸ˜†. Thank you for pushing this change, and I apologize for the short delay. I had just run the CI checks on your PR and sadly there are some prepack issues. Could you please look into resolving these πŸ€—

Armillus commented 4 months ago

Hi @zoey-kaiser,

I'm glad you liked the title πŸ˜„ No problem regarding the delay! The prepack issue should be fixed now.

zoey-kaiser commented 4 months ago

The prepack issue should be fixed now.

They are indeed! Thank you so much! There is one small linting issue and otherwise I think we are good to go. Once we get this PR through, I will release a first RC of 0.8.0!

Armillus commented 4 months ago

Wonderful! I fixed linting issues as well, sorry for the slight inconvenience.

zoey-kaiser commented 4 months ago

Hi @Armillus πŸ‘‹

After testing this PR in 0.8.0, we decided to revert it. You can find a full explanation here: https://github.com/sidebase/nuxt-auth/issues/781#issuecomment-2209074510