fix: Setting cookie: false is not respected

[Intevel]

Fixing Issue #1436

[bmulholland]

This algorithm was necessary for local storage, as explained with a comment link. But it seems unlikely the identical hacky implementation is needed to check if cookies are enabled. Can you please confirm the best way to check whether cookie support is on, adopt that, and add a comment with details on why you chose that approach?

[Intevel]

There is the Navigator.cookieEnabled property but I dont know if we can use it here

[bmulholland]

Just to walk through the options:

• Server-side (SSR), there's no way to check. Just send the cookies to the browser and it's up to the browser to use them or not. Therefore, there's no additional check needed (beyond the config.cookie check you added -- thank you!). So change request: return true on the server.
• Client-side (browser), there is indeed cookieEnabled, and it is widely supported: https://caniuse.com/?search=cookieEnabled. It seems standard enough that you don't even need a justifying comment :)

[Intevel]

You mean like that?

isCookiesEnabled(): boolean {
    // Disabled by configuration
    if (!this.options.cookie) {
      return false
    }

    if (process.server) {
      return true
    }

    if (process.browser) {
      if (!window.navigator.cookieEnabled) {
        if (!this.options.ignoreExceptions) {
          // eslint-disable-next-line no-console
          console.warn(
            "[AUTH] Cookies is enabled in config, but browser doesn't" +
              ' support it'
          )
          console.warn()
        }
        return false
      } else {
        return true
      }
    }
  }

[bmulholland]

Almost!

• It's process.client, not process.browser
• Don't need to check that anyway, since if it's not server, it's client.
• Best is to explain why things happen -- see extra comment
• Putting the positive case (cookies supported) first helps clarify the primary purpose of the code

Overall result is code that's a lot flatter and easier to understand -- and thus modify in future.

isCookiesEnabled(): boolean {
    // Disabled by configuration
    if (!this.options.cookie) {
      return false
    }

    // Server can only assume cookies are enabled, it's up to the client browser
    // to create them or not
    if (process.server) {
      return true
    }

      if (window.navigator.cookieEnabled) {
        return true
      } else {
          // eslint-disable-next-line no-console
          console.warn(
            "[AUTH] Cookies is enabled in config, but browser doesn't" +
              ' support it'
          )
        }
        return false
     }
  }

(Did this in browser comment text area, please fix formatting)

[Intevel]

Pushed, and added you as Co Author

[bmulholland]

Looks great -- thanks for the fixes!