Manual Web User Deactivation #36447

minhaminha · 2025-05-16T19:01:39Z

Product Description

This PR adds the ability to manually deactivate and reactivate web users, exactly the same way as you would mobile workers. Not to be confused with the existing way to deactivate users that use SSO - this deactivation (going to refer to as domain active status) is scoped only to a single domain. Deactivated web users will be:

Unable to log in or access the deactivated domain
Not counted towards the total web user count for billing purposes
Omitted from the recipient list of any scheduled reports/broadcasts/conditional alerts they might've been part of

Additionally, you'll now be able to filter any reports with user type filtering by active vs deactivated web users (this includes web user download). For more things that deactivation affects, please see the design doc below or the commit names.

Technical Summary

Jira epic
Design Doc / Tech Spec

The deactivation state will live in the individual domain_memberships obj a web user has. See this PR to see how that'll be queried in ES.

Feature Flag

This will not be behind a FF (as specified in the design doc)

Safety Assurance

Safety story

Tested locally and on staging.

Automated test coverage

--

QA Plan

QA Ticket

Rollback instructions

This PR can be reverted after deploy with no further considerations

Labels & Review

Risk label is set correctly
The set of people pinged as reviewers is appropriate for the level of risk of the change

minhaminha · 2025-05-22T22:38:23Z

FYI: All the test failures are because DomainMemebership currently lacks the is_active attribute, which is added on the other PR

mkangia · 2025-05-23T08:24:34Z

Can web users not submit data via phone application? I thought they could. What stops them?

minhaminha · 2025-05-23T18:22:14Z

@mkangia could you elaborate? I thought that you had to sign into the mobile app with a mobile account. Can you use a web user to sign into those??

mkangia · 2025-05-25T18:04:18Z

@minhaminha
That is my understanding. You can just login a mobile app using a web user account, it will only fail in one known scenario.
Should be easy to test.

Jtang-1

Got through reviewing 9e84ce2—I'll continue with the rest tomorrow. This looks like a pretty involved change, and you did a great job uncovering all the little places that needed to be touched. Nice work!

Jtang-1 · 2025-05-26T22:08:11Z

corehq/apps/users/urls.py

@@ -106,6 +108,8 @@
    url(r'^web/account/(?P<couch_user_id>[ \w-]+)/$', EditWebUserView.as_view(), name=EditWebUserView.urlname),
    url(r'^web/remove/(?P<couch_user_id>[ \w-]+)/$', remove_web_user, name='remove_web_user'),
    url(r'^web/undo_remove/(?P<record_id>[ \w-]+)/$', undo_remove_web_user, name='undo_remove_web_user'),
+    url(r'^web/deactivate_web/(?P<couch_user_id>[ \w-]+)/$', deactivate_web_user, name='deactivate_web_user'),


super nit: deactivate and activate is clear enough since it's already under web

Jtang-1 · 2025-05-26T22:23:34Z

corehq/apps/users/views/__init__.py

+@always_allow_project_access
+@require_can_edit_web_users
+@require_POST
+@location_safe


location_safe decorator doesn't make the view location safe. It just indicates that the view is location safe and as is, I don't think this or deactivate_web_user are location safe. It need to check user_can_access_other_user

Jtang-1 · 2025-05-26T22:57:52Z

corehq/apps/users/templates/users/web_users.html

+                            {% blocktrans %}
+                              Deactivated web users will not be able to log into the project space.<br />
+                              Read more about
+                              <a href='https://crouton.net' target="_blank">


wrong link?

haha thank you for the reminder. I was going to change that out once the docs were written but I think I'll just link it to the general Web Users for this PR, since that'll probably happen after this is deployed.

corehq/apps/users/templates/users/web_users.html

Jtang-1 · 2025-05-26T23:10:16Z

corehq/apps/domain/views/base.py

@@ -68,9 +68,16 @@ def select(request, do_not_redirect=False, next_view=None):
                or (request.user.is_superuser and not domain_obj.restrict_superusers)
                or domain_obj.is_snapshot
            ):
+                redirect_last_domain = True
+                if (


could this conditional be combined to the above conditional?
Also, do we need the redirect_last_domain flag? Isn't it equivalent to remove the flag and nest the try block within the conditional?

That took me a moment to understand. But I agree. I am reading the code the same way.

yep yall are right I consolidated into the conditional above

Jtang-1 · 2025-05-26T23:23:10Z

corehq/apps/domain/views/base.py

    return sorted([{
        'name': o.name,
        'display_name': o.display_name(),
        'url': reverse(view_name, args=[o.name]),
-    } for o in domain_objects if o], key=lambda link: link['display_name'].lower())
+    } for o in domain_objects if o and user.is_active_in_domain(o.name)],


could Domain.active_for_user be updated to return only the domains that the user is active for? This would avoid having to check is_active_in_domain for each domain

Jtang-1 · 2025-05-26T23:41:29Z

corehq/apps/users/models.py

@@ -748,6 +748,12 @@ class MultiMembershipMixin(_AuthorizableMixin):
    domains = StringListProperty()
    domain_memberships = SchemaListProperty(DomainMembership)

+    @memoized
+    def is_active_in_domain(self, domain):
+        if self.get_domain_membership(domain):


is there overhead to calling get_domain_membership twice? It doesn't require querying the db right since it's just in the user doc? but anyways this would be equivalent

@memoized def is_active_in_domain(self, domain): domain_membership = self.get_domain_membership(domain) return domain_membership.is_active if domain_membership else False

yep it's all in the user doc but true this is better

Jtang-1 · 2025-05-27T01:41:09Z

corehq/apps/users/dbaccessors.py

+        start_date = today - relativedelta(months=1)
+        for u in web_users:
+            if not u.is_active_in_domain(domain):
+                user_history = UserHistory.objects.filter(


I think this business logic ("not counting users only if it has been deactivated for the whole month") is specific to billing so shouldn't be done here

hmm that's why I put it under the exclude_deactivated_web conditional, which is only set to True from accounting contexts. Do you think that's not enough? Maybe a comment to explain it's purpose?

I think it should be done in the get_web_user_count defined in corehq/apps/accounting/models.py.

Since this is a general dbaccessors function it's not intuitive that exclude_deactivated_web=True would only exclude web users that are deactivated AND not active in the past month. I would expect it to exclude all webs users that are deactivated.

So if it was just this, that would be the intuitive behavior (and the "not active in the past month" should be done elsewhere

for u in web_users: if not u.is_active_in_domain(domain): total -= 1

that's fair - moved to the get_web_user_count

Jtang-1 · 2025-05-27T01:46:12Z

corehq/apps/users/dbaccessors.py

+        today = datetime.datetime.today()
+        start_date = today - relativedelta(months=1)
+        for u in web_users:
+            if not u.is_active_in_domain(domain):


I'm not sure - but from a performance consideration, should the view/index be updated so that we're not doing this filtering in memory?

also, can you remind me what the difference between what include_inactive and exclude_deactivated_web is? Is "inactive" a web user that has been soft deleted?

I think I wasn't sure how to do that which is why I'm filtering like this but I can look into that again

inactive in this case refers to the user level is_active status. For web users, this is used to refer to the fact that they use SSO but had their access revoked by their idp. For mobile workers, it refers to their domain level deactivated/active state. Exclude deactivated web refers only to domain level deactivated/active state for web users.

Thanks for the clarifying definitions. Before you look into the view/index again, I would want to get @esoergel thoughts if it's even a concern. Do you think think doing the filtering in memory here is fine?

Jtang-1 · 2025-05-27T01:55:43Z

corehq/apps/api/resources/v0_5.py

@@ -398,6 +398,7 @@ class WebUserResource(v0_1.WebUserResource):
    profile = fields.CharField(null=True)
    user_data = fields.DictField()
    tableau_role = fields.CharField(null=True)
+    is_active_in_domain = fields.BooleanField()


Is there an ask to include this in the response? Otherwise, I think this can be removed. It would be more beneficial as a filter parameter so that the response would include only deactivated users or only activated users. And if that's outside of the scope of this work, then that's something that can be done in the future.

Woody asked about it here but now that I'm going back to read it I guess it doesn't explicitly need to be included in the response - just some way to tell deactivated apart from activated. I can look into using it to filter instead of just returning the state

I think this is fine. Even with a filter, you might not want to have to combine two files just so you can get all users

Part of why i thought of that is because that's also how it's done for mobile workers (https://commcare-hq.readthedocs.io/api/list-mobile-workers.html) where there's an archived parameter. So it would be good for consistency too

I think we're gonna need more input (i.e. Woody's) before committing to either direction.

MartinRiese

Halfway. Just submitting so you get the comments. Will continue shortly

MartinRiese · 2025-05-27T17:23:58Z

corehq/apps/domain/decorators.py

@@ -107,6 +107,9 @@ def call_view():
            return call_view()

        if couch_user.is_member_of(domain_obj, allow_enterprise=True):
+            if (couch_user.is_web_user() and not couch_user.is_superuser
+               and not couch_user.is_active_in_domain(domain_name)):
+                return HttpResponseForbidden(_("You have been deactivated from {domain}.").format(domain=domain))


I think this shows up as just a text in the browser. I wonder, if we can make that a templateResponse like two lines down. So it looks better.

Added a new deactivated_notice.html template to show instead of a blank page with text

MartinRiese · 2025-05-27T17:37:24Z

corehq/apps/domain/views/base.py

@@ -68,9 +68,16 @@ def select(request, do_not_redirect=False, next_view=None):
                or (request.user.is_superuser and not domain_obj.restrict_superusers)
                or domain_obj.is_snapshot
            ):
+                redirect_last_domain = True
+                if (


That took me a moment to understand. But I agree. I am reading the code the same way.

MartinRiese · 2025-05-27T17:39:22Z

corehq/apps/enterprise/api/resources.py

@@ -274,7 +274,8 @@ class WebUserResource(ODataEnterpriseReportResource):
        'last_login': 3,
        'last_access_date': 4,
        'status': 5,
-        'domain': 6,
+        'is_active_in_domain': 6,


Curious why you are reordering the columns?

This was specifically for tests (only tests seem to be using this column map?) which expected a certain order that was impossible to do with is_active_in_domain being 7

MartinRiese · 2025-05-27T17:43:00Z

corehq/apps/hqwebapp/views.py

+        show_domain_login = True
+        if req.couch_user.is_web_user() and not req.couch_user.is_active_in_domain(domain):
+            show_domain_login = False


show_domain_login = not (req.couch_user.is_web_user() and not req.couch_user.is_active_in_domain(domain))?

updated - clearly I was not in the headspace for understanding conditionals this day

Jtang-1 · 2025-05-27T18:25:31Z

corehq/apps/users/dbaccessors.py

@@ -183,6 +191,11 @@ def _get_invitations_by_filters(domain, user_filters, count_only=False):
    support ES search syntax, it's just a case-insensitive substring search.
    Ignores any other filters.
    """
+    only_active = user_filters.get("user_active_status", None)
+    if not only_active and only_active is not None:


Is this conditional right? I'm interpreting user_active_status=True means returning only invitations whose users are active on the domain (which would be 0). But that conditional would evaluate as False

no this is specifically to filter out invitations (or return a count of 0) if you choose the "Only Deactivated" filter in the web user downloads. The options are None (include everything), True (include only active, which all invitations are considered to be) and False (only include deactivated users). Going to add a comment to explain this since I don't really know a better way to convey this through variable naming

ah, i was thinking non accepted invitations would be considered inactive, but active makes more sense (as you have it)

corehq/apps/users/views/__init__.py

MartinRiese · 2025-05-27T18:24:38Z

corehq/apps/user_importer/importer.py

+        if str(current_user.is_active_in_domain(self.domain)) != imported_is_active:
+            if imported_is_active == 'True':
+                current_user.reactivate(self.domain, changed_by=web_user_importer.upload_user)
+            elif imported_is_active == 'False':


Can this not just be an else?

I think I wanted users to be very intentional about deactivating users through the bulk upload. I don't want deactivation to happen if they accidentally leave it blank, misspell 'True', or whatever else.

MartinRiese · 2025-05-27T18:38:24Z

corehq/apps/users/dbaccessors.py

@@ -183,6 +191,11 @@ def _get_invitations_by_filters(domain, user_filters, count_only=False):
    support ES search syntax, it's just a case-insensitive substring search.
    Ignores any other filters.
    """
+    only_active = user_filters.get("user_active_status", None)
+    if not only_active and only_active is not None:


not sure if I understand this. can a user for an invitation ever be active on that same domain?

Jonathan raised the same point haha. copy pasted:

this is specifically to filter out invitations (or return a count of 0) if you choose the "Only Deactivated" filter in the web user downloads. The options are None (include everything), True (include only active, which all invitations are considered to be) and False (only include deactivated users). Going to add a comment to explain this since I don't really know a better way to convey this through variable naming

Move include inactive into domain call

For mass emails we just care that the user is active on at least one domain

Domain now filters out inactive users by default Add right flags to get intended users.

…er-deactivation-manual

…user-deactivation-manual

…ommcare-hq into ml/web-user-deactivation-manual

functions calls were changed in a previous commit

The same check already happens in the requires_privilege_for_commcare_user decorator with addtion of accounting for superusers. Whereas the removed check will throw and exception for superusers

Fix its usage of the UserES filter to match the new functionality. Update the test data

esoergel · 2025-06-06T13:19:58Z

corehq/apps/reports/filters/users.py

@@ -351,6 +351,7 @@ def filter_context(self):
    @classmethod
    def user_es_query(cls, domain, mobile_user_and_group_slugs, request_user):
        # The queryset returned by this method is location-safe
+        # import pdb; pdb.set_trace()


minhaminha added the Open for review: do not merge A work in progress label May 16, 2025

dimagimon added Risk: High Change affects files that have been flagged as high risk. Risk: Medium Change affects files that have been flagged as medium risk. labels May 16, 2025

minhaminha marked this pull request as ready for review May 22, 2025 15:42

minhaminha requested review from esoergel, stephherbers, AddisonDunn, orangejenny, biyeun, snopoke, dannyroberts, nospame and mkangia as code owners May 22, 2025 15:42

minhaminha added awaiting QA QA in progress. Do not merge and removed Open for review: do not merge A work in progress labels May 23, 2025

minhaminha requested a review from MartinRiese May 23, 2025 21:29

Jtang-1 reviewed May 27, 2025

View reviewed changes

MartinRiese reviewed May 27, 2025

View reviewed changes

Jtang-1 reviewed May 27, 2025

View reviewed changes

MartinRiese reviewed May 27, 2025

View reviewed changes

minhaminha requested a review from Robert-Costello as a code owner May 27, 2025 23:02

esoergel and others added 5 commits May 30, 2025 13:37

Remove old once-off migration

321e966

Make is_admin fully available filter

c48a51c

Combine domain and domains filters

9c8887e

Make domain filter handle is_active status

bbb7b7d

Fix usage of UserES

f09bf79

Move include inactive into domain call

MartinRiese and others added 13 commits June 3, 2025 11:09

Fix UserES usage: User active on any domain

3149faf

For mass emails we just care that the user is active on at least one domain

Fix lint errors

7784846

Merge branch 'master' into es/is_active_query

7a505ad

Fix tests

6f3bbf1

Fix UserES usage

262474c

Domain now filters out inactive users by default Add right flags to get intended users.

Fix UserES usage

3435796

Domain now filters out inactive users by default Add right flags to get intended users.

This also includes the demo user by default

979048f

A few easy fixes

298a30e

Make domain available to locations dbaccessors

f9a8fb4

Fix lint error

9517a43

That's the last of 'em

6093dbc

Merge branch 'master' of github.com:dimagi/commcare-hq into ml/web-us…

82a50bb

…er-deactivation-manual

Merge remote-tracking branch 'origin/es/is_active_query' into ml/web-…

33c2dc5

…user-deactivation-manual

esoergel requested a review from AmitPhulera as a code owner June 3, 2025 20:46

dimagimon added the reindex/migration Reindex or migration will be required during or before deploy label Jun 3, 2025

MartinRiese and others added 11 commits June 3, 2025 19:40

Fix lint issue

3ad26a3

Update renamed label in test

52ade7e

Update web users list

a676487

Remove duplicate domains fn in favor of domain

742bd55

Merge branch 'ml/web-user-deactivation-manual' of github.com:dimagi/c…

3e7c186

…ommcare-hq into ml/web-user-deactivation-manual

Fix test

68a5c93

functions calls were changed in a previous commit

Fix test: domain should be passed in the builder

9849d93

Remove additional check

722dd67

The same check already happens in the requires_privilege_for_commcare_user decorator with addtion of accounting for superusers. Whereas the removed check will throw and exception for superusers

Fix expand mobile worker filter

f8414ea

Fix its usage of the UserES filter to match the new functionality. Update the test data

Remove debug statement

81d5e98

Bootstrap 5 Migration - Rebuilt diffs

eb019ae

esoergel reviewed Jun 6, 2025

View reviewed changes

Merge branch 'master' into ml/web-user-deactivation-manual

3ee3979

esoergel mentioned this pull request Jun 18, 2025

Clean up some UserES usages #36594

Merged

3 tasks

Merge branch 'master' into ml/web-user-deactivation-manual

4a794be

Uh oh!

Manual Web User Deactivation #36447

Are you sure you want to change the base?

Manual Web User Deactivation #36447

Uh oh!

Conversation

minhaminha commented May 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Product Description

Technical Summary

Feature Flag

Safety Assurance

Safety story

Automated test coverage

QA Plan

Rollback instructions

Labels & Review

Uh oh!

minhaminha commented May 22, 2025

Uh oh!

mkangia commented May 23, 2025

Uh oh!

minhaminha commented May 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mkangia commented May 25, 2025

Uh oh!

Jtang-1 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinRiese left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

minhaminha commented May 16, 2025 •

edited

Loading

minhaminha commented May 23, 2025 •

edited

Loading

Jtang-1 left a comment •

edited

Loading