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

Show pseudonymized names #1211

Merged
merged 6 commits into from
Apr 4, 2024

Conversation

Mikael-Lenander
Copy link
Contributor

@Mikael-Lenander Mikael-Lenander commented Jun 28, 2023

This turned out to be very difficult. The only way I came up with is to modify the user instance's fields without storing the changes to the database. I don't know if this is the cleanest way to do it but the upside is that is doesn't require changing the API of user of userprofile. I couldn't figure out a way to pseudonymize the 'All results' page since it doesn't fetch the user data in the view but through a JavaScript script so I can't access the request.session object which contains a boolean value 'pseudonymize' that determines whether personal data should be pseudonymized. Also, the forms are difficult to pseudonymize without changing the form field values which sounds dangerous.

#533

userprofile/pseudonymize.py Fixed Show fixed Hide fixed
def get(self, request: HttpRequest) -> HttpResponse:
pseudonymize = request.session.get("pseudonymize", False)
request.session["pseudonymize"] = not pseudonymize
return HttpResponseRedirect(request.META.get("HTTP_REFERER", "/"))

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.
userprofile/views.py Fixed Show fixed Hide fixed
@mikaelGusse mikaelGusse force-pushed the feature/pseudonymized-data branch 8 times, most recently from efe9dc9 to 000f732 Compare March 12, 2024 14:21
- Added studentID to be pseudonymizized
- Fixed hashing issues
@mikaelGusse mikaelGusse marked this pull request as ready for review March 13, 2024 08:30
@ihalaij1 ihalaij1 assigned ihalaij1 and unassigned mikaelGusse Mar 19, 2024
@ihalaij1 ihalaij1 requested review from murhum1 and removed request for ihalaij1 March 21, 2024 11:21
@ihalaij1 ihalaij1 assigned murhum1 and unassigned ihalaij1 Mar 21, 2024
@@ -66,6 +75,9 @@ def get_common_objects(self) -> None:
Prefetch('submitters', UserProfile.objects.prefetch_tags(self.instance)),
)
)
for submission in qs:
format_submission(submission, self.pseudonymize)
print(submission.submitters)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should remove debug print

Copy link
Contributor

Choose a reason for hiding this comment

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

Removed this print

@mikaelGusse mikaelGusse requested a review from murhum1 April 3, 2024 10:16
Copy link
Contributor

@murhum1 murhum1 left a comment

Choose a reason for hiding this comment

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

Just wrote up comments earlier and didn't actually submit them, sorry! New to this review business. Anyway, comments up now.

exercise/templates/exercise/_submission_info.html Outdated Show resolved Hide resolved
exercise/templates/exercise/staff/_assessment_panel.html Outdated Show resolved Hide resolved
external_services/templatetags/external_services.py Outdated Show resolved Hide resolved
userprofile/pseudonymize.py Show resolved Hide resolved
@mikaelGusse
Copy link
Contributor

Just wrote up comments earlier and didn't actually submit them, sorry! New to this review business. Anyway, comments up now.

Thanks for the comments! Just went through them, do you think the seed system there is ok for keeping the pseudonymizations consistent?

Copy link
Contributor

@murhum1 murhum1 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks fine now.

@murhum1 murhum1 merged commit 5290e8b into apluslms:master Apr 4, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants