Add config.expireInactiveSession to add support for non-expiring inactive sessions

[steven-supersolid]

No description provided.

[codecov-io]

Current coverage is 92.31%

Merging #1536 into master will increase coverage by +<.01%

@@             master      #1536   diff @@
==========================================
  Files            87         87          
  Lines          6148       6153     +5   
  Methods        1062       1062          
  Messages          0          0          
  Branches       1260       1263     +3   
==========================================
+ Hits           5675       5680     +5   
  Misses          473        473          
  Partials          0          0          

Powered by Codecov. Last updated by 0b88302...de42ace

[drew-gross]

I don't think this is the behaviour we want. Being able to create a session that is already expired is useful to test your client-side code that handles session expiry.

My recommendation would be to add an expireInactiveSessions setting similar to Parse.com, and throw an error if they try to set a sessionLength and also set expireInactiveSessions to false.

[facebook-github-bot]

@steven-supersolid updated the pull request.

[steven-supersolid]

I've made some of the suggested changes but didn't want to change the current default behavior. Please let me know what you think about this:

Default: expireInactiveSessions = true, sessionLength = 1 year
Case A: Developer can change the value of sessionLength as desired
Case B: Developer can set expireInactiveSessions to false to disable with no other change required (such as changing sessionLength to an invalid value)

[drew-gross]

This looks good to me, cc @nlutsenko @gfosco for comments because this involves a new public API.

[steven-supersolid]

I think I missed the following checks when expireInactiveSessions is false:

• Don't delete the session document where the User has an existing expired session
• Delete the session expiration for any users with existing sessions

Will address tomorrow

[drew-gross]

Adding that would be a much larger and probably backwards incompatible change. Probably to support the use case you want would require a major and backwards incompatible overhaul to sessions, which I would like to see, but probably not for awhile. Essentially I'd like to remove expiresAt from sessions and check for expiry by subtracting the expiration time from the current time and checking that against updatedAt or some other field.

[nlutsenko]

Having expiresAt is still helpful, since otherwise we are relying on magic when the user uses the session token for example, but I feel that's a topic for some other day.

This looks good to me, and is helpful for testing, and when the security model of Parse Server is not required, so I am ok with this change.

[facebook-github-bot]

@steven-supersolid updated the pull request.

[drew-gross]

Parse.com sets/deletes expiresAt when making a request using a session token, rather than when the setting is enabled/disabled. I think thats probably the right approach. I'd actually be fine with merging this as is. I do think 0 seconds should be a valid session length, but thats something we can change later.

[ghost]

@steven-supersolid updated the pull request.

[ghost]

@steven-supersolid updated the pull request.