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

Create 11-Testing_for_Simultaneous_Sessions #1111

Merged
merged 19 commits into from
Feb 6, 2024
Merged

Conversation

0xmaximus
Copy link
Contributor

@0xmaximus 0xmaximus commented Nov 24, 2023

This PR fixes #1110.

  • This PR handles the issue and requires no additional PRs.
  • You have validated the need for this change.

What did this PR accomplish?

Adding "Test for Simultaneous sessions" in Session Management Testing.

@kingthorin
Copy link
Collaborator

I'd suggest adding this to 01 or 08.

@0xmaximus
Copy link
Contributor Author

I can work on it, but I think we should care about how big 01 section must be. Also, section 08 might not be the best place to include this information.
The main goal of this section is to ensure users are aware of malicious behaviors by receiving log-out notifications or alerts. Also, adding this to other sections will miss other ideas like implementing a dedicated page to display and enable the termination of active sessions in remediation part.

@kingthorin
Copy link
Collaborator

Okay, I'll wait for a few others to chime in with their thoughts.

@0xmaximus
Copy link
Contributor Author

Should I tag someone?

@kingthorin
Copy link
Collaborator

Nope, just give it a few days, they should get GitHub notifications. If they don't add their thoughts I'll hit them up out of band.

@rbsec
Copy link
Collaborator

rbsec commented Nov 25, 2023

I think that this would make more sense in its own section - it could be squeezed into 01 or 08, but doesn't really seem to fit too well there.

Copy link
Collaborator

@rbsec rbsec left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally looks good, and it's a useful check to add. I'm always a bit twitchy about this issue though, because it's something that can often be indiscriminately reported regardless of whether it's actually a security issue in the context of the application - so I think some guidance around that might be useful.

Fixed response sample and added some notes for improved testing.
@ThunderSon
Copy link
Collaborator

ThunderSon commented Nov 28, 2023

@0xmaximus can you please have the file extension? .md
It's currently without an extension

As for the file location, I'm okay with this being its own file.
I'll give this another look by EOW, looks good from a first look, probably won't be big/major comments, rbsec seems to have done the heavy lifting 😉

Thank you for taking care of this PR!

@ThunderSon ThunderSon self-requested a review November 28, 2023 23:18
@kingthorin
Copy link
Collaborator

kingthorin commented Nov 28, 2023

It will need to be linked appropriately in the section index and overall.

This comment has been minimized.

improve the summary part and add some essential factors

This comment has been minimized.

Add more test cases for "How to test" and "Remediation" part.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@0xmaximus 0xmaximus requested a review from rbsec December 4, 2023 20:18
@0xmaximus
Copy link
Contributor Author

I would appreciate it if any other potential issues are mentioned.

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still needs to be linked in the various indexes.

This comment was marked as outdated.

@0xmaximus
Copy link
Contributor Author

Still needs to be linked in the various indexes.

If I understand you correctly, I should modify these files and add new link:
checklists/checklist.md
document/4-Web_Application_Security_Testing/06-Session_Management_Testing/README.md
document/README.md
checklists/checklist.json
Also, Should I modify them in separate PRs?

…ent_Testing/11-Testing_for_Concurrent_Sessions.md

Co-authored-by: Rick M <kingthorin@users.noreply.github.com>

This comment was marked as outdated.

@kingthorin
Copy link
Collaborator

kingthorin commented Feb 6, 2024

Don't worry about the checklists they're generated. There should be 3 READMEs. This PR.

Copy link
Collaborator

@kingthorin kingthorin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Getting close

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

@kingthorin kingthorin merged commit a9d92d0 into OWASP:master Feb 6, 2024
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding "Test for Simultaneous sessions" in Session Management Testing
4 participants