Added session length option for session tokens to server configuration

[Kenishi]

I'm not sure if this is the best way to go about doing this, but it seems like the least painless route.
This allows you to specify a session length in milliseconds for how long session tokens should be good for. If one isn't specified then it falls back to 1 year.

Added some test coverage for the implementation of this on RestWrite. I think there should also ideally be one test in the index.js spec as well, but I'm not sure what the best way of doing that is.

[codecov-io]

Current coverage is 93.07%

Merging #997 into master will increase coverage by +0.03% as of 3be04c9

@@            master    #997   diff @@
======================================
  Files           84      84       
  Stmts         5304    5316    +12
  Branches       970     975     +5
  Methods          0       0       
======================================
+ Hit           4935    4948    +13
  Partial         11      11       
+ Missed         358     357     -1

Review entire Coverage Diff as of 3be04c9

Powered by Codecov. Updated on successful CI builds.

[facebook-github-bot]

@Kenishi updated the pull request.

[flovilmart]

I would put the default value at the top, when we spread the options

[flovilmart]

you should add the validation that the sessionLength is valid in Config.validate, and throw an error if the session length is not valid

[facebook-github-bot]

@Kenishi updated the pull request.

[flovilmart]

remove space

[Kenishi]

By "each place" do you mean in each location where a session object is created?
For Example

[flovilmart]

That is exactly what I mean, maybe refactor that code in order to have something centralized and not all over the place, but not refactor by overriding the value before saving

[facebook-github-bot]

@Kenishi updated the pull request.

[Kenishi]

Just to make sure I understand, changing to something like this would be preferred?

var sessionLength = this.config.sessionLength,
    now = new Date(),
    expiresAt = new Date(now.getTime() + (sessionLength*1000));
var sessionData = {
  sessionToken: token,
  user: {
    __type: 'Pointer',
    className: '_User',
    objectId: this.auth.user.id
  },
  createdWith: {
    'action': 'create'
  },
  restricted: true,
  expiresAt: Parse._encode(expiresAt)
};

This would be done at 2 locations in RestWrite and 1 location in UsersRouter.

Unless what you are suggesting is to have a function somewhere that can be called to get a JS Date object for the expiresAt time? Something like:

var expiresAt = SomeClass.generateExpiresAt()
// or
var expiresAt = SomeClass.generateExpiresAt(this.config.sessionLength);

If thats the case, any suggestions in what class this should go? Config seems the most straight forward but I'm not sure it makes sense.

[flovilmart]

I makes sense to put it in config for many reasons,

• it holds the option
• it's widely exposed through the stack

[facebook-github-bot]

@Kenishi updated the pull request.

[Kenishi]

Let me know if you need me to do anything about the fails. I ran npm test on my end and the Monbgo3.0.8 was passing.

[facebook-github-bot]

@Kenishi updated the pull request.

[facebook-github-bot]

@Kenishi updated the pull request.

[Kenishi]

Did I rebase that correctly?

[gfosco]

The only real problem here is that session expiration isn't actually implemented. Are you interested in working on that as well?

As for rebasing, there are still 7 commits listed. Ideally there should only be one. What I've been doing to update a single commit during PR development is:

git add .
git commit --amend
# ^ by default this opens an editor with a commit message, just save/quit.
git push -f
# ^ force pushes the now amended commit

[Kenishi]

The only real problem here is that session expiration isn't actually implemented.

I'm not sure I completely follow. Do you mean like having the session deleted from the database after it has expired? I'd be interested in working on this if it isn't very complicated.

[gfosco]

At the moment it's just a date field, no logic changes based on its value. After the expire time, it still works. During the beginning of the request cycle, probably in middlewares.js, the expire time should be validated.

[Kenishi]

I looked at this the other day. I think I can probably add something to middleware for handling expired sessions. My main question is how should an expired token be handled? Should it just reject the request with an error like expired session token? Would that break current functionality on the various SDKs?

[drew-gross]

In general we do our best to match the behaviour of Parse.com. If you have a Parse.com account, try it on an app on Parse.com and then duplicate that behaviour. If you don't, send me a message with your email address and I can create one for you.

[Kenishi]

@drew-gross I might have some time to get the middleware stuff in place. I tried following your suggestion and checking how the Parse.com handles an expired session token, but I can't test it because the server won't allow the expiresAt field to be modified. It kicks back an error about expiresAt being read-only (when directly editing in dashboard) or a reserved word (REST API).

[drew-gross]

@Kenishi OK, I checked in the source code. The response for an expired session should be the exact same response as for a revoked or invalid session token.

[facebook-github-bot]

@Kenishi updated the pull request.

[Kenishi]

Okay. I added a check in Auth.getAuthForSessionToken to see if the session has expired. If it has then it throws a Parse.Error.INVALID_SESSION_TOKEN with a message about the token being expired. I added a check in the middleware catch() to see if the error is an instance of Parse.Error and if so then it passes the error on in the next(). The handleParseErrors middleware will then properly handle it.

[drew-gross]

That error handling looks a little weird but seems to work, and the code is otherwise solid. Merging.

[flovilmart]

merging!