Log out a user when user is not active #35143
Conversation
jingcheng16
requested review from
millerdev,
biyeun,
calellowitz,
gherceg and
dannyroberts
jingcheng16
added
the
product/prod-india-all-users
There was a problem hiding this comment.
I'm not too concerned about the user experience since this doesn't change any existing behavior apart from ending a user's session early if they are no longer active. The bad user experience for deactivated users already exists which is that if they try to login, it fails with a message saying their credentials were incorrect is displayed, so it is fair to say that is outside of the scope of this change.
| @@ -15,6 +16,9 @@ def process_view(self, request, view_func, view_args, view_kwargs): | |||
| if 'org' in view_kwargs: | |||
| request.org = view_kwargs['org'] | |||
| if request.user and request.user.is_authenticated: | |||
| if not request.user.is_active: | |||
| logout(request) | |||
| return self.get_response(request) | |||
There was a problem hiding this comment.
I'm not sure this is actually what we want in the long run based on docs, as it seems by calling self.get_response(request) we will continue to pass the request down the middleware chain. Since the user has been logged out, I imagine it hits a point where an authenticated session is required and returns a response then. This doesn't seem too risky, and the overall change is still an improvement so I don't consider this blocking, but would like to see this potentially addressed if needed in the future.
gherceg
dismissed
their stale review
Dismissing my own review as I think we need to understand sef.get_response(request) better.
Product Description
When a user signed in, even if the user get deactivated, as long as the user is still in his session, he can still use commcarehq, which sounds unsafe.
This PR will make sure, after the user is deactivated, the next request he sent, will log him out.
I know it is not user friendly enough, so I created a follow up ticket: https://dimagi.atlassian.net/browse/SAAS-16009. We want to roll this out sooner to solve the issue for CRS.
Technical Summary
Ticket: https://dimagi.atlassian.net/browse/SAAS-15996
I modified the UsersMiddleWare that checks if a user is active during each request
logout function is used in Django documentation: https://docs.djangoproject.com/en/4.2/topics/auth/default/#how-to-log-a-user-out
Safety Assurance
Safety story
This ticket improved safety as we only allow activated user access hq.
Tested locally and on staging.
Automated test coverage
QA Plan
No QA.
Rollback instructions
Labels & Review