Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

one of the 2 cookies are not cleared #172

Open
artofspeed opened this issue May 9, 2019 · 2 comments
Open

one of the 2 cookies are not cleared #172

artofspeed opened this issue May 9, 2019 · 2 comments

Comments

@artofspeed
Copy link

artofspeed commented May 9, 2019

Say cookie name is foo. When you do ctx.session = null;, on client side, the cookie foo.sig is cleared, the cookie foo is still there with its original value.

Why is it done this way? How to do a full clean?

@laurazenc
Copy link

I'm having the same problem, is there any information of how to fully destroy a session and do not return something like koa:sess= ; koa:sess.sig=123213123 ?

@jmitchell38488
Copy link
Contributor

jmitchell38488 commented Aug 16, 2019

tl;dr currently the destroy method in lib/context.js only sets the cookie data to be blank, instead of expiring it. This is because koa uses the cookies module, but doesn't take advantage of the additional options, or the available API on the cookie itself.

Long story:
The code only destroys the data contained in the cookie, not the cookie itself, that is referenced with opts.key:

session/lib/context.js

Lines 275 to 284 in 10bb122

async remove() {
const opts = this.opts;
const ctx = this.ctx;
const key = opts.key;
const externalKey = this.externalKey;
if (externalKey) await this.store.destroy(externalKey);
ctx.cookies.set(key, '', opts);
}

Once the session is destroyed by the key name, the session named by opts.key is destroyed. This session is called koa:sess by default, or can be the passed cookie name.

session/index.js

Lines 60 to 75 in 10bb122

function formatOpts(opts) {
opts = opts || {};
// key
opts.key = opts.key || 'koa:sess';
// back-compat maxage
if (!('maxAge' in opts)) opts.maxAge = opts.maxage;
// defaults
if (opts.overwrite == null) opts.overwrite = true;
if (opts.httpOnly == null) opts.httpOnly = true;
if (opts.signed == null) opts.signed = true;
if (opts.autoCommit == null) opts.autoCommit = true;
debug('session options %j', opts);

The .sig cookie exists because by default, the options declare the cookie should be signed, which is part of the cookies library that koa uses.

module: cookies/index.js

Cookies.prototype.get = function(name, opts) {
  var sigName = name + ".sig"
    , header, match, value, remote, data, index
    , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys

  header = this.request.headers["cookie"]
  if (!header) return

  match = header.match(getPattern(name))
  if (!match) return

  value = match[1]
  if (!opts || !signed) return value

  remote = this.get(sigName)
  if (!remote) return

  data = name + "=" + value
  if (!this.keys) throw new Error('.keys required for signed cookies');
  index = this.keys.index(data, remote)

  if (index < 0) {
    this.set(sigName, null, {path: "/", signed: false })
  } else {
    index && this.set(sigName, this.keys.sign(data), { signed: false })
    return value
  }
};

Cookies.prototype.set = function(name, value, opts) {
  var res = this.response
    , req = this.request
    , headers = res.getHeader("Set-Cookie") || []
    , secure = this.secure !== undefined ? !!this.secure : req.protocol === 'https' || req.connection.encrypted
    , cookie = new Cookie(name, value, opts)
    , signed = opts && opts.signed !== undefined ? opts.signed : !!this.keys

  if (typeof headers == "string") headers = [headers]

  if (!secure && opts && opts.secure) {
    throw new Error('Cannot send secure cookie over unencrypted connection')
  }

  cookie.secure = secure
  if (opts && "secure" in opts) cookie.secure = opts.secure

  if (opts && "secureProxy" in opts) {
    deprecate('"secureProxy" option; use "secure" option, provide "secure" to constructor if needed')
    cookie.secure = opts.secureProxy
  }

  pushCookie(headers, cookie)

  if (opts && signed) {
    if (!this.keys) throw new Error('.keys required for signed cookies');
    cookie.value = this.keys.sign(cookie.toString())
    cookie.name += ".sig"
    pushCookie(headers, cookie)
  }

  var setHeader = res.set ? http.OutgoingMessage.prototype.setHeader : res.setHeader
  setHeader.call(res, 'Set-Cookie', headers)
  return this
};

I'll create a pull request to fix this behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants