Core: [Auth] logged user persist on all requests even from other clients

[helnokaly]

I noticed that after a user logs in, that user becomes logged in on other browsers, even on Postman !!.

My config:

authenticator: session
session: cookie

So to make reproduction of the issue easier I packaged a minimal Adonis app having the issue

yardstick.zip

Steps to reproduce:

1. npm install
2. ./ace migration:run
3. npm run dev
4. Browse to http://localhost:3333
5. Create a user
6. Login
7. Browse/request to http://localhost:3333 from other browsers, postman or curl and notice the currentUser username on the top of the page

curl -X GET -H "Cache-Control: no-cache" "http://localhost:3333"

[RomainLanz]

Hey @helnokaly !

Thanks for your report! ❤️

@thetutlage and myself will look into it soon!

[kuntapong]

It happened even from different domains as well. But change session driver to file may solve this. Not sure why.

[RomainLanz]

Thanks for your contribution @kuntapong !

I have tried to debug it this morning but I can't figure out how this is possible (at the moment). The weird thing is I have an application build with Adonis running in production without this problem (https://www.qualitas-helvetica.ch/).

Going to debug deeper this afternoon. 💪

[thetutlage]

@helnokaly Can u please confirm by installing adonis-framework from github? Later I will publish it to npm.

[howdoicomputer]

I think I may have filed a duplicate issue here: https://github.com/adonisjs/adonis-auth/issues/22

[howdoicomputer]

Also, that commit seems to break the auth middleware entirely. I used npm to grab the latest commit from GitHub and now it... looks like the CSRF token is stripped before the auth middleware can validate it?

Error: csrf secret missing
    at Shield._validateCsrf (/home/tyler/workspace/work/noc_dashboard/node_modules/adonis-middleware/src/Shield/index.js:162:21)
    at Shield.handle (/home/tyler/workspace/work/noc_dashboard/node_modules/adonis-middleware/src/Shield/index.js:248:12)
    at handle.next (<anonymous>)
    at onFulfilled (/home/tyler/workspace/work/noc_dashboard/node_modules/co/index.js:65:19)
    at process._tickCallback (internal/process/next_tick.js:103:7)

[howdoicomputer]

   * @public
   */
  _validateCsrf (request, csrfSecret) {
    /**
     * @description throw error when unable to find csrf secret
     */
    if (!csrfSecret) {
      const error = new Error('csrf secret missing')
      error.status = 403
      error.code = 'EBADCSRFTOKEN'

Another stacktrace. I'm playing around on my fork.

[howdoicomputer]

@thetutlage Also, it doesn't seem like the auth middleware supports different session drivers? I've tried the Redis and File driver to back sessions and I get Error: CSRF secret missing when I attempt a login.

Works fine with cookies as the driver but then... everyone can login when a single user is logged in.

[howdoicomputer]

I slept and woke up and poked at it some more and my yowling was mostly my fault.

I got session based authentication working using the Redis driver. The problem I was having before was that the secure option was set to true (my bad) in config/session.js and that messed with the CSRF token being received by the backend when serving the website over HTTP. Since I was just running on localhost it _looked_ like everything was broken since everything ran fine with the session being established via the cookie driver - when I switched to the latest code/used a different driver everything broke.

However, I revisited testing out your change @thetutlage and can verify that it works when the secure option for sessions is set to false. I sign in, access an auth middleware controlled route, open an incognito window, and I'm denied access like expected. Thanks for the commit!

[thetutlage]

Cool, will release it soon with some other changes

[thetutlage]

Published as 3.0.4 in adonis-framework

[helnokaly]

@thetutlage sorry for the late reply, I tested it an it's fixed but is there a possibility that concurrent requests may share the same session before it is cleared?

[thetutlage]

@helnokaly Seems correct. Since drivers are singleton this will cause issue with Cookie driver.

Let me add a concept of singleton and non-singleton drivers since drivers like Redis should not open a new connection on each request and should be singleton, whereas drivers like Cookie should have a new instance tied to each request.

Thanks for bringing this up

[thetutlage]

@helnokaly What about something like this
https://github.com/adonisjs/adonis-framework/commit/c784fbb5a70d189957e27885ec49fc6d4e7f8159

In short drivers can decide whether they need a fresh instance per request or not. If they want they should implement fresh method, which will return the instance to be treated as a valid driver.

[helnokaly]

@thetutlage looks good and reasonable to me, thanks.
I'm curious if there is a big performance hit with one instance per request approach.

[thetutlage]

Nope as soon as request finishes GC will clear it

[helnokaly]

Great :+1:

[lock[bot]]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.