Doesn't need to happen for a few months at least, so removing from current sprint.

@Niharika should the user have to re-accept the use agreements if they are given access again? Alternatively, is it okay if the use agreement checkboxes are kept checked so the user does not have to re-accept the agreement?

In T375115#10985959, @Dreamy_Jazz wrote:

@Niharika should the user have to re-accept the use agreements if they are given access again? Alternatively, is it okay if the use agreement checkboxes are kept checked so the user does not have to re-accept the agreement?

Good question. I think the proper thing to do would be to have them re-accept the agreements. If someone has been inactive for a long time, they may not remember the terms they agreed to.
I'll update the task description.

Inactivity is defined as no edits or logged actions (any actions, including private ones)

We only track private actions for 90 days (via CheckUser tables) so I think this part of the specs would need to be reworded. I would suggest we change it to:

Inactivity is defined as no edits or publicly logged actions

cc @Niharika

Change #1175891 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/CheckUser@master] Add logging support for automatic inactive group membership removal

https://gerrit.wikimedia.org/r/1175891

Change #1175894 had a related patch set uploaded (by STran; author: STran):

[mediawiki/core@master] Auto-remove IP reveal rights from inactive users via maintenance script

https://gerrit.wikimedia.org/r/1175894

Change #1175894 abandoned by STran:

[mediawiki/core@master] Auto-remove IP reveal rights from inactive users via maintenance script

Reason:

prefer I11ff6ddc1b697ddac2b2b1528ed90fd6fc72b2f2

https://gerrit.wikimedia.org/r/1175894

@Niharika Could you answer a few questions about how we want/expect this to work in these cases?

In T375115#11023981, kostajh wrote:

Inactivity is defined as no edits or logged actions (any actions, including private ones)

We only track private actions for 90 days (via CheckUser tables) so I think this part of the specs would need to be reworded. I would suggest we change it to:

Inactivity is defined as no edits or publicly logged actions

cc @Niharika

Additionally, for IPInfo, the tos agreement preference can be set globally and this overrides local value setting. Should we:

• add a local exception
• not unset the preference at all
• attempt to ascertain if the user is globally inactive (feels like scope creep, do not recommend)

Additionally, for IPInfo,

Just to note that this also applies to the IP reveal preference. It can be set globally in the same way, so we would need to answer that for this one too.

In T375115#11023981, @kostajh wrote:

Inactivity is defined as no edits or logged actions (any actions, including private ones)

We only track private actions for 90 days (via CheckUser tables) so I think this part of the specs would need to be reworded. I would suggest we change it to:

Inactivity is defined as no edits or publicly logged actions

Sounds OK to me.

Additionally, for IPInfo, the tos agreement preference can be set globally and this overrides local value setting. Should we:

• add a local exception
• not unset the preference at all
• attempt to ascertain if the user is globally inactive (feels like scope creep, do not recommend)

Between the first two options, I think adding a local exception seems like the more logical step. I think it is better to unset the preference so the user can re-review the ToS before accepting them. @Dreamy_Jazz I'd welcome your thoughts on this (with your CU hat on!).

This might be out of scope but this script doesn't inform the user that their access has been revoked and should it? I guess if you're gone without notice for a year you're not coming back or you should expect to have privileged revoked but an indicator/notification might be nice regardless in case you do? It might make setting the local exception clearer, as @Dreamy_Jazz did note on the task:

I think it would be confusing because it would be not clear how a user would re-enable the preference locally.

Between the first two options, I think adding a local exception seems like the more logical step. I think it is better to unset the preference so the user can re-review the ToS before accepting them.

Adding a local exception could be confusing IMO because:

• The user may be inactive on only one wiki and so they may still be using access elsewhere
• We don't provide a route for users to see that the local preference is disabled, so they may be unable to see why

Could we alternatively consider just unsetting the preference globally if the user is inactive on all wikis? Tbh I don't see why we would need to differentiate between a user agreeing to the preference on multiple wikis, because agreeing to the preference appears to be agreeing to the policy that applies globally.

Some notes from a quick discussion we had off-band:

• In the interest of unblocking myself, I'm going to move forward with "Do nothing" wrt to the ip info preferences for now. We have time to decide how we want to treat this, as no one's access will be expiring any time soon. If we resolve it differently before the patch goes in, that's fine but we shouldn't mind a follow-up patch if necessary.
• We should decide if "because agreeing to the preference appears to be agreeing to the policy that applies globally." is true. If it is, the local exception could cause a mismatch in ToS agreement if the global agreement still stands but no longer counts locally.
• Expiring a global preference based on on global inactivity is a different ask than expiring a local preference based on local inactivity and imo should be a follow-up/different thing regardless.

We've been discussing this on-patch and @Dreamy_Jazz brought up this point. @Niharika could you take a look and give your opinion on how to move forward? I'm not sure what the call is, given there are legal obligations etc.

Personally I think we should use the rights log to indicate removal and not create a log in the temporary account reveal log.

This is because:

• If we create the log but then don't create one if the user gets access again, we risk users being confused (because the IP reveal log says that they had access removed for inactivity)
• The user rights log for the group change should be enough to indicate removal and we will need to create it anyway.
• No reason in the user rights log will be confusing for users who cannot see the IP reveal log (i.e. most users), as they will see no reason for the removal provided in any associated log.

For context, the specs don't ask for a log in the user rights change log but for consistency/clarity we added it there since it would be confusing if someone looked at Special:UserRights and didn't see why someone's group membership was revoked. As of writing, this means the same action is logged to both the rights log and the temp accounts log.

Change #1179638 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/IPInfo@master] Add logs for when the system autorevokes a user's IPInfo access

https://gerrit.wikimedia.org/r/1179638

In T375115#11084841, @STran wrote:

We've been discussing this on-patch and @Dreamy_Jazz brought up this point. @Niharika could you take a look and give your opinion on how to move forward? I'm not sure what the call is, given there are legal obligations etc.

Personally I think we should use the rights log to indicate removal and not create a log in the temporary account reveal log.

This is because:

• If we create the log but then don't create one if the user gets access again, we risk users being confused (because the IP reveal log says that they had access removed for inactivity)
• The user rights log for the group change should be enough to indicate removal and we will need to create it anyway.
• No reason in the user rights log will be confusing for users who cannot see the IP reveal log (i.e. most users), as they will see no reason for the removal provided in any associated log.

For context, the specs don't ask for a log in the user rights change log but for consistency/clarity we added it there since it would be confusing if someone looked at Special:UserRights and didn't see why someone's group membership was revoked. As of writing, this means the same action is logged to both the rights log and the temp accounts log.

It makes sense to me only to log it in the user rights log.

In T375115#11082366, @STran wrote:

Some notes from a quick discussion we had off-band:

• In the interest of unblocking myself, I'm going to move forward with "Do nothing" wrt to the ip info preferences for now. We have time to decide how we want to treat this, as no one's access will be expiring any time soon. If we resolve it differently before the patch goes in, that's fine but we shouldn't mind a follow-up patch if necessary.
• We should decide if "because agreeing to the preference appears to be agreeing to the policy that applies globally." is true. If it is, the local exception could cause a mismatch in ToS agreement if the global agreement still stands but no longer counts locally.
• Expiring a global preference based on on global inactivity is a different ask than expiring a local preference based on local inactivity and imo should be a follow-up/different thing regardless.

I would support leaving the preferences alone, as this makes the logic and the user experience simpler. I don't think it makes sense to regard them as no longer agreeing to the agreement if they lose their access automatically, as they have taken no action to imply that this is the case. If they ever need to re-enable their access, it would be a confusing experience to need to discover that they need to disable a local override that was automatically made to disable their preference.

Change #1179638 abandoned by STran:

[mediawiki/extensions/IPInfo@master] Add logs for when the system autorevokes a user's IPInfo access

Reason:

product specs updated, no longer needed

https://gerrit.wikimedia.org/r/1179638

(Moving out of Needs Review as the associated patch is marked as WIP)

The maintenance script patch was +2ed but I'm moving this back into In Progress because it also needs an accompanying cron job setup so it actually runs. I've updated acceptance criteria to reflect this and also what we agreed on wrt to local preference modifications (don't).

Change #1175891 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Auto-remove IP reveal rights from inactive users via maintenance script

https://gerrit.wikimedia.org/r/1175891

Change #1181654 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/CheckUser@master] Condense query used in revokeTemporaryAccountViewerGroup maint script

https://gerrit.wikimedia.org/r/1181654

Change #1181662 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/WikimediaMessages@master] Override `checkuser-temporary-account-autorevoke-userright-reason`

https://gerrit.wikimedia.org/r/1181662

Change #1181689 had a related patch set uploaded (by STran; author: STran):

[operations/puppet@production] mediawiki: Run CheckUser/revokeTemporaryAccountViewerGroup.php

https://gerrit.wikimedia.org/r/1181689

Change #1181654 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] Clean up revokeTemporaryAccountViewerGroup maint script

https://gerrit.wikimedia.org/r/1181654

Change #1181662 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] Override `checkuser-temporary-account-autorevoke-userright-reason`

https://gerrit.wikimedia.org/r/1181662

Change #1181689 merged by JHathaway:

[operations/puppet@production] mediawiki: Run CheckUser/revokeTemporaryAccountViewerGroup.php

https://gerrit.wikimedia.org/r/1181689

First run on WMF wikis should have happened and it looks like it didn't remove any users (as expected) per https://pl.wikipedia.org/wiki/Specjalna:U%C5%BCytkownicy?username=&group=temporary-account-viewer&wpsubmit=&wpFormIdentifier=mw-listusers-form&limit=50&uselang=en having users still.

We may want to grant access to an inactive test account on test.wikipedia.org and see if the script removes access from them to be sure that it is working.

Change #1184049 had a related patch set uploaded (by STran; author: STran):

[mediawiki/extensions/WikimediaMessages@master] Respect user language in temp account viewer autorevoke policy link

https://gerrit.wikimedia.org/r/1184049

Change #1184049 merged by jenkins-bot:

[mediawiki/extensions/WikimediaMessages@master] Respect user language in temp account viewer autorevoke policy link

https://gerrit.wikimedia.org/r/1184049