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

Testers for session lifetime #238

Open
wants to merge 14 commits into
base: development
Choose a base branch
from
Open

Conversation

idoshr
Copy link
Contributor

@idoshr idoshr commented Apr 15, 2024

Add some testers for:

  • session lifetime
  • regenerate session
  • test everything agains SESSION_REFRESH_EACH_REQUEST and SESSION_PERMANENT

seems like there is some issues with the lifetime session in:

  • MongoDB (I think it is minor and there is gap of couple seconds)
  • DynamoDB seems like it doesn't work

EDIT:

prevent session hijacking by generate session already exists

@MauriceBrg
Copy link
Contributor

Is your DynamoDB problem maybe related to this? #240

@idoshr
Copy link
Contributor Author

idoshr commented Apr 18, 2024

Is your DynamoDB problem maybe related to this? #240

I think it is the same issue and we need add filter by expectation time and not relay only on ttl index

@MauriceBrg
Copy link
Contributor

I've added a fix for that issue to this PR - #237

@Lxstr
Copy link
Contributor

Lxstr commented Apr 18, 2024

ok great I will merge #237 first

@Lxstr
Copy link
Contributor

Lxstr commented Apr 18, 2024

@idoshr I've got #237 merged, should we rebase your PR?

@idoshr
Copy link
Contributor Author

idoshr commented Apr 18, 2024

I will pull development to my branch

@idoshr
Copy link
Contributor Author

idoshr commented Apr 18, 2024

@Lxstr
Merged

@MauriceBrg
Copy link
Contributor

I think you should update the target branch to development for this PR

@idoshr idoshr changed the base branch from main to development April 19, 2024 07:36
@idoshr
Copy link
Contributor Author

idoshr commented Apr 19, 2024

I think you should update the target branch to development for this PR

Changed now
@MauriceBrg
@Lxstr

@idoshr idoshr changed the title Testers Testers for session lifetime & Prevent session hijacking Apr 25, 2024
@Lxstr
Copy link
Contributor

Lxstr commented Apr 25, 2024

Hi @idoshr thanks for your continued work. Could you please remove the code changed in commit 4fb799 as we do have need for this should_set_storage method? Also, can you separate the code you added for regeneration into a new PR? This is out of scope, better to just focus on testing for one PR I think

@idoshr idoshr changed the title Testers for session lifetime & Prevent session hijacking Testers for session lifetime Apr 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants