Skip to content

Testers for session lifetime#238

Open
idoshr wants to merge 14 commits into
pallets-eco:developmentfrom
idoshr:testers
Open

Testers for session lifetime#238
idoshr wants to merge 14 commits into
pallets-eco:developmentfrom
idoshr:testers

Conversation

@idoshr

@idoshr idoshr commented Apr 15, 2024

Copy link
Copy Markdown
Contributor

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
Copy Markdown
Contributor

Is your DynamoDB problem maybe related to this? #240

@idoshr

idoshr commented Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
Contributor

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

@Lxstr

Lxstr commented Apr 18, 2024

Copy link
Copy Markdown
Contributor

ok great I will merge #237 first

@Lxstr

Lxstr commented Apr 18, 2024

Copy link
Copy Markdown
Contributor

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

@idoshr

idoshr commented Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

I will pull development to my branch

@idoshr

idoshr commented Apr 18, 2024

Copy link
Copy Markdown
Contributor Author

@Lxstr
Merged

@MauriceBrg

Copy link
Copy Markdown
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

idoshr commented Apr 19, 2024

Copy link
Copy Markdown
Contributor Author

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

Lxstr commented Apr 25, 2024

Copy link
Copy Markdown
Contributor

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