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

GDPR compliance #1134

Merged
merged 3 commits into from
Oct 31, 2023
Merged

GDPR compliance #1134

merged 3 commits into from
Oct 31, 2023

Conversation

JP5457
Copy link
Contributor

@JP5457 JP5457 commented Oct 22, 2023

three main features:
-A mandatory privacy policy that users must accept on their first attempt to login
-the ability to opt into and out of account deletion
-php scripts for emailing users eligible for deletion and for deleting users

src/Controllers/gdpremail.php Outdated Show resolved Hide resolved
src/Controllers/gdpremail.php Outdated Show resolved Hide resolved
src/Templates/MyRadio/privacy.twig Show resolved Hide resolved
src/Templates/MyRadio/privacy.twig Show resolved Hide resolved
src/Templates/MyRadio/privacy.twig Show resolved Hide resolved
src/Templates/MyRadio/privacy.twig Outdated Show resolved Hide resolved
src/Controllers/gdprdelete.php Outdated Show resolved Hide resolved
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Since I was in the area.

My primary concerns are conceptual though - are you sure that this is actually required? What's prompted adding an automatic deletion to MyRadio? Deletion requests should of course be followed, but what's collected currently in my view falls under "acceptable use" (except certain bits of personal data)

The URY database has such a wealth of knowledge in it regarding everything people did in their time at URY, going back over 20 years now.

Once it's gone it's gone, and it'd be a shame to delete anything unnecessarily - I wouldn't be surprised if this would delete 90% of the "members".

src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/MyRadio/MyRadioForm.php Outdated Show resolved Hide resolved
src/Controllers/gdprdelete.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Controllers/gdprdelete.php Outdated Show resolved Hide resolved
src/Templates/MyRadio/privacy.twig Outdated Show resolved Hide resolved
@JP5457 JP5457 force-pushed the data-protection-is-probably-a-good-idea-yes branch 2 times, most recently from 364463b to 7d70345 Compare October 24, 2023 01:47
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

I do not like forcing this, but I'm seriously concerned by the direction this is going in, and I'd like some answers

Why are you doing this? What issue is being solved by deleting all 20 years worth of user data about shows and everything they did at URY?

@LordAro
Copy link
Member

LordAro commented Oct 24, 2023

I reiterate - once you implement this, you cannot undo it. You should be very very sure the data that is being deleted is only what's desired. Have you checked backups (and restoring from those backups) recently?

@LordAro
Copy link
Member

LordAro commented Oct 24, 2023

Since I can feel the inevitable "because GDPR" response, I'm going to point you at https://ico.org.uk/for-organisations/uk-gdpr-guidance-and-resources/data-protection-principles/a-guide-to-the-data-protection-principles/the-principles/storage-limitation/

You can keep personal data for longer if you are only keeping it for public interest archiving, scientific or historical research, or statistical purposes.

Delete the bits that are not needed if you feel you must (I'm trying not to think about why is_wheelchair stored at all), but my view is that the database is primarily covered by the above and should not be touched

I feel the need to say that I'm not opposed to deletion in general - I'm the one that deleted the 'gender' field from the database after the thorny question of how that should be represented came up. The answer was of course that we didn't need it and so shouldn't be keeping it. And that was before GDPR...

@adfw
Copy link
Member

adfw commented Oct 24, 2023

To put my tuppenth' worth in, since the initial GDPR regulation came about when I was Head of Computing in 2016 (e.g. it got passed and appeared upon the horizon), think a discussion was briefly held... (it certainly later led to the "why do we record gender" discussion as @LordAro mentions above).

Just to stick my nose in here too, I think there are several 'classes' of members you might want to consider before doing this:

  1. Members who signed up but never presented a show or did anything substantive (e.g. signed up at freshers fair, maybe got studio trained but progressed no further)
  2. Members who never presented a show but perhaps were on a team, had a committee position (perhaps rare, but certainly historically there were for e.g. a few people in the technical teams who never had a show)
  3. Most importantly, members who were studio trained and presented shows

For 1., I fully agree that under GDPR those members details are fully deprecated, no longer need to be held, and there is no good reason to hold such data - agreed that given a grace period, their details should be removed.
For 2., I think there is a reasonable argument that if a member held a position of note in the society (e.g. was on committee), that the society should keep a record of such.
For 3., If a member in the database appeared in a show (there may be debate as to how 'important' they are - e.g. presenter, engineer, guest?), and given that URY has always been a broadcast radio station, those members should be fully aware that their participation and being a presenter (or similar) on a licensed broadcaster in the UK, then there is a reasonable 'public interest' in keeping a record, at a minimum, of their appearance on the schedule.

That does not mean that a Subject Access Request, or a Deletion Request, can't occur and those details be made non-public, but my view would be that by presenting on a broadcast radio station, you lose a certain amount of right to privacy at a very high level - e.g. show credit. Mixcloud recordings, photos etc. should of course be deleted on request.

I would also note that any email is unlikely to reach many people who didn't update their contact email address after leaving UoY, and their york.ac.uk account will now go nowhere.

I would certainly see if any advice is available from Ofcom on this in terms of retaining data about presenters beyond the statutory 48-day logs retention period - I would suggest contacting them for advice.

Happy Compliancing,
Anthony
URY, 2011-2016

@LordAro
Copy link
Member

LordAro commented Oct 27, 2023

Also come on dude, commit messages are there for a reason

@JP5457
Copy link
Contributor Author

JP5457 commented Oct 27, 2023

Also come on dude, commit messages are there for a reason

I'm sorry if you're getting notifications when I commit. I'm commiting a lot because its by far the easiest way to get code onto myradio dev.

@LordAro
Copy link
Member

LordAro commented Oct 27, 2023

Sounds suspiciously like an actual staging environment :o (though I might suggest actual dev work should be done on your local computer first)

Back in my day we just edited it directly on the server. Often production itself. waves walking stick

@LordAro
Copy link
Member

LordAro commented Oct 27, 2023

Still, it's not the notifications I'm bothered about - future you will thank past you if you actually write out what you're doing. Helps reduce the "why on earth is that there" factor that inevitably happens months (or weeks or days or hours) later

@michael-grace
Copy link
Member

This is why we do rebasing when it's ready. Myradio is difficult to work on locally, so we have testing versions on server. We do care about what commit messages end up in history.

@JP5457 JP5457 requested a review from LordAro October 29, 2023 13:37
@JP5457
Copy link
Contributor Author

JP5457 commented Oct 29, 2023

I've made some significant changed to how this works:

  • users now have the option of hiding their details in MyRadio
  • automatic deletion now only deletes a few unnecessary columns
  • script for deleting almost all data from users that explicitly request deletion

In the meeting we decided that any data visible on ury.org.uk we could defend keeping because it is required for the website to function as advertised.

src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
scripts/gdpremail.php Show resolved Hide resolved
scripts/gdpremail.php Outdated Show resolved Hide resolved
scripts/gdprdeleteall.php Show resolved Hide resolved
Copy link
Member

@LordAro LordAro left a comment

Choose a reason for hiding this comment

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

Yeah, this feels like a much more reasonable approach to me, thanks for taking the time to address our concerns :)

Do update the PR description to keep it up to date, and don't forget to update previous review comments (mark them as resolved when.. they are) so you don't get overwhelmed by them

Also, codestyle :)

scripts/gdprdeleteall.php Show resolved Hide resolved
scripts/gdprdeleteuser.php Outdated Show resolved Hide resolved
scripts/gdprdeleteall.php Outdated Show resolved Hide resolved
scripts/gdprdeleteuser.php Outdated Show resolved Hide resolved
scripts/gdprdeleteuser.php Outdated Show resolved Hide resolved
scripts/gdpremail.php Outdated Show resolved Hide resolved
scripts/gdpremail.php Outdated Show resolved Hide resolved
scripts/gdpremail.php Show resolved Hide resolved
src/Classes/ServiceAPI/MyRadio_User.php Outdated Show resolved Hide resolved
scripts/gdprdeleteall.php Show resolved Hide resolved
@ColinRoitt
Copy link
Contributor

Future non-essential changes moved to #1137

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

test

moved file

moved file

moved file

moved file

moved file

moved file

moved file

moved file

test

cleaning

test

test

test

Update src/Classes/ServiceAPI/MyRadio_User.php

Co-authored-by: Charles Pigott <[email protected]>

test

test

test

test

test

testing

test

gide officership

hide shows

hide shows

hide shows

hide shows

hide profile button

delete scripts

fix

fix

test

test

test

test

enumification

clarified some comments

cleaned up toDataSource

added joined to email querys and fixed typos

email wording change

changed for PR comments

code clean up and confirmation messages on scripts

typo

typo
@JP5457 JP5457 force-pushed the data-protection-is-probably-a-good-idea-yes branch from 9289264 to 9d88f4a Compare October 31, 2023 22:57
@JP5457 JP5457 merged commit c988fa9 into master Oct 31, 2023
@michael-grace michael-grace deleted the data-protection-is-probably-a-good-idea-yes branch May 31, 2024 21:09
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.

6 participants