Page MenuHomePhabricator

Remove production data access for former WMDE staff member goransm
Closed, ResolvedPublic

Description

User 'goransm' is no longer employed by WMDE (and hasn't been for a while) but was never removed from analytics_privatedata_users and analytics_wmde_users_members groups.

And his entry still has ensure: present

Event Timeline

Thank you @AndrewTavis_WMDE for alerting us of this. Pinging @Manuel for visibility.

Gehel triaged this task as High priority.Jan 31 2024, 4:32 PM

Change 994799 had a related patch set uploaded (by Btullis; author: Btullis):

[operations/puppet@production] Revoke production shell access from goransm

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

It isn't 100% clear from the description whether the user should still have production shell access but without analytics_privatedata_users and analytics_wmde_users_members membership.

I have decided to err on the side of safety and the patch I have submitted revokes all production shell access.

@Manuel do you know if the user should retain any shell access, or am I right to revoke it completely? Thanks.

Adding @MoritzMuehlenhoff for visibility - Do we need to do anything else regarding an off-boarding checklist for this formed WMDE user?
I will revoke the kerberos principal.

Change 994799 merged by Btullis:

[operations/puppet@production] Revoke production shell access from goransm

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

I looked into this: Goran, upon quitting his contract with WMDE, requested continued private data access as a volunteer. This was granted and he signed a volunteer NDA (see T310055). It is important to note that any remaining data access for Goran should not anymore be tied to his former contract with WMDE.

Ah, thanks @Manuel. It seems that I have acted with too much haste. I can revert the change then and restore access. I have not yet removed the kerberos principal.

It is important to note that any remaining data access for Goran should not anymore be tied to his former contract with WMDE.

I'm not quite sure that I understand what this means, from a technical perpective.
We should add expiry_date and expiry_contact fields to reflect the NDA, but is there anything else that we need to do to make this clear?

Change 994715 had a related patch set uploaded (by Btullis; author: Btullis):

[operations/puppet@production] Revert "Revoke production shell access from goransm"

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

We should add expiry_date and expiry_contact fields to reflect the NDA

@KFrancis are you still the contact for Goran's continued access to sensitive information as a volunteer? Is there an expiration date for his NDA?

Change 994715 merged by Btullis:

[operations/puppet@production] Revert "Revoke production shell access from goransm"

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

I have just deployed the revert, so the changes should be undone and the user should still have access.
Any scripts that are still running (as mentioned at T310055#8002568) should once again be able to use the analytics-privatedata user and access kerberised services.

We can follow up with another patch to add the expiry details.
Apologies for jumping the gun and revoking access before having thoroughly checked. I hope that this does not cause any inconvenience to you in terms of data processing etc.

I'm not quite sure that I understand what this means,

I just wanted to emphasize that WMDE has no oversight anymore, and my assumption was that there is a WMF process for managing data access for volunteers that now needs to be followed.

from a technical perpective.

I don't know. Looking at the YAML, I wonder if analytics_wmde_users_members still make sense or not.

We should add expiry_date and expiry_contact fields to reflect the NDA, but is there anything else that we need to do to make this clear?

The community NDA don't have an end date per se. They usually get revoked either when someone states the access is no longer needed (https://phabricator.wikimedia.org/T356203) of via activity pings (typically by a staff member working with the volunteer)

Apologies for jumping the gun and revoking access before having thoroughly checked.

All good from my side, when in doubt, protecting our private data has priority!

from a technical perpective.

I don't know. Looking at the YAML, I wonder if analytics_wmde_users_members still make sense or not.

He's a member of the analytics-wmde-users shell group, it grants running commands under the analytics-wmde user on the stat* hosts. Not sure what this used for, but if it's specific to WMDE staff, it should rather be dropped?

Shouldn't we add the affected user to this ticket and ask them about all this?

it grants running commands under the analytics-wmde user on the stat* hosts

Reading T310055#8002568 together with your explanation about the group sounds to me as if some of the data pipelines that Goran built for WMDE as a contractor may still depend on his user (and it being in this group).

it should rather be dropped?

I am a Product Manager, so I don't know for sure, but I would prefer to not drop that group for now. I will involve our developers tomorrow morning, and come back to you.

I'm also sorry for making the request without knowing about the prior request.

Shouldn't we add the affected user to this ticket and ask them about all this?

Fair point, yeah.

@GoranSMilovanovic Hello hello! Long time no see :) Question for you – do you still need the various production data & systems accesses & membership to privileged groups? Is the access purely until your data pipelines are migrated from your user account to WMDE's Airflow instance?

via activity pings (typically by a staff member working with the volunteer)

That's the thing – I'm not sure there is one? At least not on file?

I apologize for my part of bringing this up with the assumption that such access isn't required anymore. My assumption was that at the very least wmde wouldn't have been necessary (now shown to be incorrect), and that lead me to believe that access rights had not been checked in the first place (a valid concern given other circumstances).

Again my apologies for the initial haste.

We should add expiry_date and expiry_contact fields to reflect the NDA

@KFrancis are you still the contact for Goran's continued access to sensitive information as a volunteer? Is there an expiration date for his NDA?

There is no official expiration date for the NDA. Access is usually ended when the person is no longer working in the project or volunteering.

Change 995007 had a related patch set uploaded (by Arnaudb; author: Arnaudb):

[operations/puppet@production] admin: remove goransm production access

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

Change 995007 abandoned by Arnaudb:

[operations/puppet@production] admin: remove goransm production access

Reason:

unnecessary

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

ABran-WMF lowered the priority of this task from High to Medium.Feb 1 2024, 10:51 AM

Before closing this task / withdrawing the request I'd like to get a confirmation from Goran whether the level of access is still needed and if it's just for the WMDE pipelines, in which case it would make sense to prioritize migrating those to WMDE's Airflow instance so that we can eventually revoke the unrestricted access to highly sensitive information.

Thank you for the continued attention here, @mpopov. Final investigations of this infrastructure and the options that we have are of high priority at this time. We'll communicate with you all as progress is made.

BTullis subscribed.

It's worth noting that there is only one remaining active entry in the crontab for goransm on stat1007.
There are several others, but they have been commented out.

I'm fairly sure that the remaining WMDE analytics scheduled tasks wmde/graphite and wmde/wdcm are all now using the analytics-wmde system user, as opposed to any individual's user account.

I agree with @mpopov that migrating all of the WMDE pipelines to airflow is the best course of action, especially any that are still tied to individual user accounts.

If it's OK with you, I'll unassign myself from the task for now. Feel free to reassign me or ping if I can help at all in any other way.

Should we close this ticket as "invalid"? It seems the best course of action might be a new ticket like "migrate all WMDE pipelines to airflow" and close this one?

Dzahn changed the task status from Open to In Progress.Feb 7 2024, 6:35 PM
Dzahn moved this task from Untriaged to In Discussion on the SRE-Access-Requests board.

Not yet. I believe @AndrewTavis_WMDE will be sharing some findings from WMDE side soon.

@Manuel and I would suggest that this task remain open. Decisions on the data processes that require this account's private data access have now shifted to WMDE's Product Team, with a final decision expected next week. From my end I've been doing some documentation of the process, which can be seen here: T356618: [EPIC] Check of legacy wmde analytics infrastructure.

Dzahn changed the task status from In Progress to Stalled.Feb 8 2024, 4:09 PM

@AndrewTavis_WMDE Ok, thanks for the update. Confirmed. Keeping open and just setting to stalled for the moment (because we have rotating clinic duty working on access requests).

Has there been a final decision on access and the scope of this ticket?

Thank you @hnowlan for the check in here. Final word on this is coming from @Manuel who will be back in the office on Wednesday. I appreciate everyone's patience.

Based on Andrew's analysis of the current state (T356618) and product strategy, I have decided that we should deprecate all remaining data processes that still rely on the user account in question. Therefore, from this point on, we are confident that it is safe from our side to remove all group memberships from the user account. We understand that this will result in some data processes breaking and sending error messages, and clean-up tasks have already been planned for when @ItamarWMDE returns in a few weeks. For any questions or concerns regarding this decision, please contact either @AndrewTavis_WMDE or myself directly.

We understand that the jobs will be failing and we will do the needed steps to shut down the following machines in the coming weeks once @ItamarWMDE is available:

Please let me know if there are error messages, job failed emails or other issues that we should take care of sooner rather than later. I am happy to prioritize shutting down the machines directly if we’re able to get support with this as I don't have a full understanding of the complete process that needs to be done.

Beyond this I'd also like to draw attention to T357697: Archive WMDE analytics Gerrit repositories that I've now tagged appropriately. With these processes being deprecated, it would be good for us to archive the remaining associated Gerrit repos.

Please let us know if there is further work to be done that we're not seeing!

Thank you all again for your attention to this situation.

@AndrewTavis_WMDE Thanks! This is a long task and to make things explicit: Is the summary below a correct summary of next steps:

  • Membership in analytics-privatedata-users is covered by the ongoing community NDA and persists
  • Membership of analytics-wmde-users should be removed

And one followup: If that is confirmed, when should the analytics-wmde-users membership be removed? Should this happen now or in the coming weeks (when ItammarWMDE is available)?

Thanks for checking in @MoritzMuehlenhoff! A correction to one of your points:

  • Membership in analytics-privatedata-users should be removed as the user in question is no longer directly working with or volunteering for WMDE
    • The original access was granted in part because the user was to continue to work on the aforementioned processes
    • Yes there was also mention of private projects, but from our end the NDA was signed off on for further volunteer work with WMDE
    • If the user still needs the access, then for us a new NDA should be signed that states a new purpose
  • As you read: membership of analytics-wmde-users should also be removed

To your follow up, we are comfortable with access being removed now for both analytics-privatedata-users and analytics-wmde-users. Yes the jobs running on the account that need these permissions will fail, but we feel that at this point the private data access is not warranted, and access should be removed directly.

Thanks for checking in @MoritzMuehlenhoff! A correction to one of your points:

  • Membership in analytics-privatedata-users should be removed as the user in question is no longer directly working with or volunteering for WMDE
    • The original access was granted in part because the user was to continue to work on the aforementioned processes
    • Yes there was also mention of private projects, but from our end the NDA was signed off on for further volunteer work with WMDE
    • If the user still needs the access, then for us a new NDA should be signed that states a new purpose
  • As you read: membership of analytics-wmde-users should also be removed

To your follow up, we are comfortable with access being removed now for both analytics-privatedata-users and analytics-wmde-users. Yes the jobs running on the account that need these permissions will fail, but we feel that at this point the private data access is not warranted, and access should be removed directly.

Ok! So I'll go ahead and make a patch to drop analytics-wmde-users right now.

To also remove analytics-privatedata-users I'll first clarify with the person within Wikimedia Legal handling NDAs (since that means that the NDA in general is no longer needed), I'll add you to that mail as well.

Change 1005753 had a related patch set uploaded (by Muehlenhoff; author: Muehlenhoff):

[operations/puppet@production] Remove goransm from analytics-wmde-users

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

Thank you for the help with this, @MoritzMuehlenhoff! Please also add in @Manuel to the email, if you would.

Change 1005753 merged by Muehlenhoff:

[operations/puppet@production] Remove goransm from analytics-wmde-users

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

MoritzMuehlenhoff changed the task status from Stalled to Open.Feb 22 2024, 1:15 PM
MoritzMuehlenhoff claimed this task.

Change 1005942 had a related patch set uploaded (by Muehlenhoff; author: Muehlenhoff):

[operations/puppet@production] Remove access for goransm

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

Change 1005942 merged by Muehlenhoff:

[operations/puppet@production] Remove access for goransm

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

@AndrewTavis_WMDE @Manuel I've removed the access also for analytics-privatedata-users now. The data cleanup task is https://phabricator.wikimedia.org/T358311 for which you are both subscribed.

Thank you, @MoritzMuehlenhoff! Really grateful to have this finalized. I'll write any information I have into the new task.