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

[Feature Request] Optional "Remember me" functionality #2877

Closed
dbrxnds opened this issue Oct 25, 2021 · 3 comments
Closed

[Feature Request] Optional "Remember me" functionality #2877

dbrxnds opened this issue Oct 25, 2021 · 3 comments
Labels

Comments

@dbrxnds
Copy link
Contributor

dbrxnds commented Oct 25, 2021

What do you want and why?

The ability to set whether a user should be remembered only for the duration of their browser session or for a longer period of time (As is now)

Possible implementation(s)

We were thinking you could add a property to session.$create which allows you to set rememberMe to true or false.
If this value is false, the relevant cookies get set with the expiry set to session. This should cause the user to have to log in again after reopening their browser.

Additional context

It might be useful to allow developers to set a property in the config for how long a user should be remembered, if they choose to do so. I believe the current expiry value is set to a month.

It may be useful for some use cases to be able to set the expiry length dynamically in the session.$create method. But this is a detail that we don't need, so it might be unnecessary to add it for now.

@beerose
Copy link
Contributor

beerose commented Oct 26, 2021

It might be useful to allow developers to set a property in the config for how long a user should be remembered, if they choose to do so. I believe the current expiry value is set to a month.

Currently, you can set sessionExpiryMinutes in the session middleware config. Docs: https://blitzjs.com/docs/session-management#session-configuration.

As for the setting expiry dynamically use case, I think, in order to preserve the current functionality, what about making it more opt-out? By default, if you don't pass any additional arguments, it would behave as it does so far. However, you could pass some value, e.g. skipExpiry (or any better name 😬) so that users can opt-out from sessionExpiryMinutes and exipryAt will be set to null (will setting it to null achieve automatic expiry after browser session end?).

@flybayer any thoughts?

@flybayer
Copy link
Member

You can make a cookie expire at the end of a session by setting the expire time to 0 or omitting it altogether.

It seems adding ability to set an expire time from session.$create() could be a good solution. Because then you can do session.$create({userId, role}, {}, {expiresAt: 0}) to "not remember".

However, that's not super intuitive. So maybe in addition to that, we can add a "shortcut" for expiresAt: 0 like session.$create({userId, role}, {}, {remember: false}) ?

Morever, instead of adding a third param to $create(), I think we should make a breaking change and change second argument from privateData to an object that includes options + privateData.

// new
session.$create({userId, role}, {privateData: {boo: 1}, remember: false})

Workaround

From: https://discord.com/channels/802917734999523368/901997467871113276/902845303848325143

This was a simple solution I made by modifying the login mutation. The resolver takes in a 3rd param -- a boolean. If true, update the session expiresAt to expire one year from session creation.

export default resolver.pipe(resolver.zod(Login), async ({ email, password, remember }, ctx) => {
  // 1. Run authenticateUser() function above passing in email and password. Returns user from above function
  const user = await authenticateUser(email, password)

  // Create a new db Session and assign to client Session (Public Session Data)
  // A. userId
  // B. User Role array for multitenancy - [ GlobalRole, OrganizationRole ] (declared in top level types.ts)
  // C. Organization ID. The default ID that the member is a part of.
  await ctx.session.$create({
    userId: user.id,
    roles: [user.role, user.organization.role!],
    orgId: user.memberships[0]?.organizationId
  })


  if(remember) {
    const neverExpiresDate = new Date()
    neverExpiresDate.setFullYear(neverExpiresDate.getFullYear() + 1)

    await db.session.update({
      where: { handle: ctx.session.$handle!},
      data: {
        expiresAt: neverExpiresDate
      }
    })
  }

  // 2. Return the user.
  return user
})

@rene-demonsters
Copy link
Contributor

You can make a cookie expire at the end of a session by setting the expire time to 0 or omitting it altogether.

It seems adding ability to set an expire time from session.$create() could be a good solution. Because then you can do session.$create({userId, role}, {}, {expiresAt: 0}) to "not remember".

However, that's not super intuitive. So maybe in addition to that, we can add a "shortcut" for expiresAt: 0 like session.$create({userId, role}, {}, {remember: false}) ?

Morever, instead of adding a third param to $create(), I think we should make a breaking change and change second argument from privateData to an object that includes options + privateData.

// new
session.$create({userId, role}, {privateData: {boo: 1}, remember: false})

Workaround

From: https://discord.com/channels/802917734999523368/901997467871113276/902845303848325143

This was a simple solution I made by modifying the login mutation. The resolver takes in a 3rd param -- a boolean. If true, update the session expiresAt to expire one year from session creation.

export default resolver.pipe(resolver.zod(Login), async ({ email, password, remember }, ctx) => {
  // 1. Run authenticateUser() function above passing in email and password. Returns user from above function
  const user = await authenticateUser(email, password)

  // Create a new db Session and assign to client Session (Public Session Data)
  // A. userId
  // B. User Role array for multitenancy - [ GlobalRole, OrganizationRole ] (declared in top level types.ts)
  // C. Organization ID. The default ID that the member is a part of.
  await ctx.session.$create({
    userId: user.id,
    roles: [user.role, user.organization.role!],
    orgId: user.memberships[0]?.organizationId
  })


  if(remember) {
    const neverExpiresDate = new Date()
    neverExpiresDate.setFullYear(neverExpiresDate.getFullYear() + 1)

    await db.session.update({
      where: { handle: ctx.session.$handle!},
      data: {
        expiresAt: neverExpiresDate
      }
    })
  }

  // 2. Return the user.
  return user
})

This introduces a subtle bug with leap years. If somebody signs in on the 29th of february and you add a year, it won't be able to safe. Better to use the date-fns addYears function which should take that in account.

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

No branches or pull requests

5 participants