Skip to content

Add expiration update on validate. Add rememberMeFor property.#28

Closed
phess101 wants to merge 2 commits intohapijs:masterfrom
codeandfury:master
Closed

Add expiration update on validate. Add rememberMeFor property.#28
phess101 wants to merge 2 commits intohapijs:masterfrom
codeandfury:master

Conversation

@phess101
Copy link
Copy Markdown

@phess101 phess101 commented Sep 2, 2014

I needed the ability to update expiration every time the page is accessed (pushing out the expiration ttl time).
Also added Remember Me functionality.

@mattapperson
Copy link
Copy Markdown

+1

@JustinSchneider
Copy link
Copy Markdown

+1. This would be the bee's knees.

@davemackintosh
Copy link
Copy Markdown

Had a look, can't find any issues with it +1

@evankeller
Copy link
Copy Markdown

+1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to specify the input type (boolean?)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the expiration time in milliseconds (just like ttl property).

@hueniverse
Copy link
Copy Markdown
Contributor

Missing tests and full coverage.

@hueniverse hueniverse added the feature New functionality or improvement label Sep 16, 2014
@hueniverse hueniverse self-assigned this Sep 16, 2014
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like this. It's a bit of a hack. Why not just change the set() signature to set(session, ttl) so you can override it per set?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, but to be clear, are you suggesting that the rememberMe logic should be on the validate function? (line 84). Then I'd just be providing the ttl to the set function.

@hueniverse
Copy link
Copy Markdown
Contributor

I don't like this approach. Replacing it with something similar.

@hueniverse hueniverse closed this Nov 12, 2014
@lock
Copy link
Copy Markdown

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants