adonisjs / ally

AdonisJS Social Authentication Provider
MIT License
159 stars 53 forks source link

Two set-cookie, one overwriting the other #133

Closed eyglys closed 2 years ago

eyglys commented 2 years ago

Package version

4.1.1

Node.js and npm version

Node: v16.13.2, npm: 8.3.2, yarn: 1.22.15

Sample Code (to reproduce the issue)

### Controller code

public async Redirect({ ally } : HttpContextContract) {
  return ally.use('github').redirect()
}

public async callback({ ally, auth, response } : HttpContextContract) {
  const gith = ally.use('github')

  if (gith.accessDenied()) {
    return 'Access was denied'
  }

  /**
   * Unable to verify the CSRF state
   */
  if (gith.stateMisMatch()) {
    return 'Request expired. Retry again'
  }

 if (gith.hasError()) {
   return suap.getError()
 }
 //continues...
}

When redirecting, Ally sends two set-cookies, one of which is encrypted (as expected) and the second one without any encryption. Debugging the code, I realized that the encrypted set-cookie stores the correct state and the decrypted one stores a second state completely different from the first.

Below are the HTTP headers, the state passed by the URL is OKAb_ICu0VGAzaHA6H39JGla_K-XYF55. I checked this state, it's the same one that arrives at the Oauth2Driver.persistState method.

set-cookie: gh_oauth_state=eyJtZXNzYWdlIjoiIn0; Max-Age=-1; Path=/; Expires=Thu, 01 Jan 1970 00:00:00 GMT; HttpOnly
set-cookie: gh_oauth_state=e%3ADN_HCjMSzCNnSlB_NSzEMRTX15GL8Rn8mAqD3ktLHkmFkDJYnh6xHhzgI3oqREznyDYuhQJZm3HyHjYZSK0eWHGp5WmQIbDKIplBYmAJyas.akJWMTVYYmxtU2FjT0tmRA.d8dUoYeyNmJir7CgpvusTrQQeEE0t_IPjQEr7l9UR28; Max-Age=7200; Path=/; HttpOnly

On return to the callback, the encrypted cookie no longer exists, it has been overwritten by the decrypted cookie, causing an error 'Request expired. Retry again'.

//config/app.ts
export const http: ServerConfig = {
//...
  cookie: {
    domain: '',
    path: '/',
    maxAge: '2h',
    httpOnly: true,
    secure: false,
    sameSite: false,
  },
//...
}
//config/session.ts
const sessionConfig: SessionConfig = {
//...
  cookie: {
    path: '/',
    httpOnly: true,
    sameSite: false,
  },
//...
}

The application is configured to use the AdonisJS web version, using sessions to keep the user authenticated.

stale[bot] commented 2 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

NullVoxPopuli commented 4 months ago

what was the solution?