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

[V2] [Sessions] Simplify logout #659

Merged
merged 5 commits into from
Mar 17, 2020
Merged

[V2] [Sessions] Simplify logout #659

merged 5 commits into from
Mar 17, 2020

Conversation

LoicPoullain
Copy link
Member

@LoicPoullain LoicPoullain commented Mar 6, 2020

Issue

Version 2

Logging users out in Foal is not simple and intuitive. This PR reduces the amount of code to be produced and makes it clearer.

Specification

Without cookies

Before

import {
  dependency, Context, HttpResponseNoContent, Post,
  Session, TokenRequired
} from '@foal/core';
import { TypeORMStore } from '@foal/typeorm';

export class AuthController {

  @dependency
  store: TypeORMStore;

  @Post('/logout')
  @TokenRequired({
    extendLifeTimeOrUpdate: false,
    store: TypeORMStore,
  })
  async logout(ctx: Context<any, Session>) {
    await this.store.destroy(ctx.session.sessionID);

    return new HttpResponseNoContent();
  }

}

After

import {
  Context, HttpResponseNoContent, Post, TokenOptional
} from '@foal/core';
import { TypeORMStore } from '@foal/typeorm';

export class AuthController {

  @Post('/logout')
  @TokenOptional({
    store: TypeORMStore,
  })
  async logout(ctx: Context) {
    if (ctx.session) {
      await ctx.session.destroy();
    }

    return new HttpResponseNoContent();
  }

}

With cookies

Before

import {
  dependency, Context, HttpResponseNoContent, Post,
  removeSessionCookie, Session, TokenRequired
} from '@foal/core';
import { TypeORMStore } from '@foal/typeorm';

export class AuthController {

  @dependency
  store: TypeORMStore;

  @Post('/logout')
  @TokenRequired({
    cookie: true,
    extendLifeTimeOrUpdate: false,
    store: TypeORMStore,
  })
  async logout(ctx: Context<any, Session>) {
    await this.store.destroy(ctx.session.sessionID);

    const response = new HttpResponseNoContent();
    removeSessionCookie(response);
    return response;
  }

}

After

import {
  Context, HttpResponseNoContent, Post, TokenOptional
} from '@foal/core';
import { TypeORMStore } from '@foal/typeorm';

export class AuthController {

  @Post('/logout')
  @TokenOptional({
    cookie: true,
    store: TypeORMStore,
  })
  async logout(ctx: Context) {
    if (ctx.session) {
      await ctx.session.destroy();
    }

    return new HttpResponseNoContent();
  }

}

Solution and steps

Breaking change: The Session know expects the store to be passed to the constructor upon instantiation.

  • Add Session.store.
  • Set Session.store with this in all the methods read and createAndSaveSession of all stores.
  • Add Session.destroy which calls Store.destroy.
  • Add the public property Session.isDestroyed and set it to true when Session.destroy is called.
  • In @Token, do not update the session or extend its lifetime if session.isDestroyed is true.
  • In @Token, remove the session cookie if session.isDestroyed is true.

Checklist

  • Add/update/check docs (code comments and docs/ folder).
  • Add/update/check tests.
  • Update/check the cli generators.

@LoicPoullain LoicPoullain added this to Work In Progress in Issue tracking via automation Mar 6, 2020
@LoicPoullain LoicPoullain merged commit 87417cf into v2-0-0 Mar 17, 2020
Issue tracking automation moved this from Work In Progress to Done / Closed This Release Mar 17, 2020
@LoicPoullain LoicPoullain deleted the simplify-logout branch March 17, 2020 15:39
@LoicPoullain LoicPoullain mentioned this pull request Mar 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Issue tracking
  
Done / Closed This Release
Development

Successfully merging this pull request may close these issues.

None yet

1 participant