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

fix: check the user's state when handling a request #113

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

NinhDoan
Copy link

@NinhDoan NinhDoan commented Apr 11, 2024

Hello @dkmstr ,

I recently performed a penetration test (pentest) on the system and discovered a bug in session management. Specifically, when a user is inactive, they can still make requests to the system if their session has not been destroyed.

To fix this bug, I added a user state check to the request middleware and handler. Please review and confirm that it works as expected.

Copy link
Collaborator

@dkmstr dkmstr left a comment

Choose a reason for hiding this comment

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

This is a nice solution.
Previously, we always think that the setting of that kind of flag will be available "on next login", but in this case, this is a much better solution :)

Thank you very much, i proceed to merge ;-)

Regards, good work.

@dkmstr dkmstr merged commit 871538f into VirtualCable:master Apr 11, 2024
1 of 2 checks passed
@dkmstr
Copy link
Collaborator

dkmstr commented Apr 11, 2024

I'll also add this to 3.6 release

@dkmstr
Copy link
Collaborator

dkmstr commented Apr 11, 2024

Mmmm, it was a 3.6 patch :). Didnt work against.
Also, i have noticed this:
# self._user = User() # Empty user for non authenticated handlers
raise AccessDenied()

And this cannot be, because some REST apis are PUBLIC, without users or authentication.

It should be like this:
self._user = User() # Empty user for non authenticated handlers
self._user.state = types.states.State.ACTIVE # Ensure it's active

Dont worry, i'll fix these issues, again, thanks for your work! ;-)

@NinhDoan NinhDoan deleted the fix-check-user-state branch April 12, 2024 02:10
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.

2 participants