Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Check session_expires #2377

Merged
merged 1 commit into from
May 10, 2014
Merged

Check session_expires #2377

merged 1 commit into from
May 10, 2014

Conversation

mushketyk
Copy link
Contributor

I am not sure that I made it right, but please write if I am on the right way.
#2344

@Changaco Changaco changed the title Added session_expires check. Resolves #2344 Add session_expires check May 10, 2014
@Changaco Changaco changed the title Add session_expires check Check session_expires May 10, 2014
@Changaco
Copy link
Contributor

@ivanmushketyk I guess my answer on IRC wasn't clear enough. We currently don't modify schema.sql in PRs, if you want to change the DB schema then you need to create a branch.sql that makes the change. The person who merges your PR will run the branch.sql on the production DB, before or after deploying depending on what it does.

In this case it would be before deploying, and the branch.sql would contain something like this:

ALTER TABLE participants ALTER COLUMN session_expires SET DEFAULT CURRENT_TIMESTAMP + INTERVAL '6 hours';

@mushketyk
Copy link
Contributor Author

@Changaco Thank you. Now I got it.
What about the rest? Is session checking implemented correctly?

@Changaco
Copy link
Contributor

@ivanmushketyk

  • pytz.utc.localize(datetime.datetime.now()) is wrong, it should be pytz.utc.localize(datetime.datetime.utcnow())
  • I think it would be better to check session_expires in the Participant.from_session_token() class method instead of in the authentication.inbound() hook.

@mushketyk
Copy link
Contributor Author

@Changaco Fixed. Could you please take a look?

@Changaco
Copy link
Contributor

Looks good.

Changaco added a commit that referenced this pull request May 10, 2014
@Changaco Changaco merged commit fdc1cff into gratipay:master May 10, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants