Raid v2 improvments by maximeroucher · Pull Request #1054 · aeecleclair/Hyperion

maximeroucher · 2026-08-24T11:38:40Z

Description

Summary

Updating Raid to match the first points for data security

Changes Made

Adding consent field to the security file
Adding read_medical_data permission gating the zip download (now only source of security file read)
Logging when a zip download is attempted
Adding course responsible as emergency contact into the security file, as the Raid requested

Additional Notes

First step toward a better security data handling.

Classification

Type of Change

🐛 Bug fix (non-breaking change which fixes an issue)
✨ New feature (non-breaking change which adds functionality)
🔨 Refactor (non-breaking change that neither fixes a bug nor adds a feature)
🔧 Infra CI/CD (changes to configs of workflows)
💥 BREAKING CHANGE (fix or feature that require a new minimal version of the front-end)
😶‍🌫️ No impact for the end-users

Impact & Scope

Testing

1. Tested this locally
2. Added/modified tests that pass the CI (or tested in a downstream fork)
3. Tested in a deployed pre-prod
0. Untestable (exceptionally), will be tested in prod directly

Documentation

Updated the docs accordingly :
" Docstrings
# Inline comments
No documentation needed

Marc-Andrieu

Looks promising! That's a clean start about the medical data issue

codecov · 2026-08-24T12:49:26Z

Codecov Report

❌ Patch coverage is 79.34783% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.95%. Comparing base (e9b5f53) to head (a6dcabb).

Files with missing lines	Patch %	Lines
app/modules/raid/endpoints_raid.py	70.73%	12 Missing ⚠️
app/modules/raid/cruds_raid.py	60.00%	6 Missing ⚠️
app/modules/raid/dependencies_raid.py	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1054      +/-   ##
==========================================
- Coverage   84.99%   84.95%   -0.05%     
==========================================
  Files         220      220              
  Lines       16273    16308      +35     
==========================================
+ Hits        13831    13854      +23     
- Misses       2442     2454      +12

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Daihecyy

Possible to split PR ?
consent_given and course_responsible have nothing to do with each other, comments will get muddled

Daihecyy · 2026-08-24T14:43:10Z

+    if not security_file.consent_given:
+        raise HTTPException(
+            status_code=400,
+            detail="Consent must be given to register medical data",
+        )
+


❓ Could replace this with consent_given: Literal[True] in the schema ?

Not sure to get it either

You are doing schema validation in the endpoint
you could replace this snippet by tightening the schema validator

At this point, why even bother to put it in the schema, just put it at True no matter what

I disagree with this point. Putting Literal[True] would return a 422 validation error.
In this endpoint, a value False should be accepted (in term of schema validation). It should however return a "400 Consent must be given to register medical data", as it violates a business rules: we can not proceed given a falsy consent. When receving this error, the frontend will explain to the user why it can not proceed

Daihecyy · 2026-08-24T14:44:31Z

+    hyperion_security_logger.info(
+        "Medical data access",
+        extra={
+            "accessed_by_user_id": user.id,
+            "edition_id": str(edition.id),
+            "access_type": "download",
+        },
+    )


💡 Archi wise, get_all_security_files_zip should own the logging for me, not the endpoint, it is the one doing the critical step
This way, we futureproof it and won't forget to log elsewhere
I would also have put the permission check in it

I don't see any loggers in cruds, except those for a db error, so I stuck to that architecture, maybe need the opinion of someone with more knowledge on the backend, at least on my side

@armanddidierjean Any advice on this one will be welcomed

Daihecyy · 2026-08-24T14:46:41Z

    emergency_person_name: Mapped[str | None]
    emergency_person_phone: Mapped[str | None]
    file_id: Mapped[str | None]
+    consent_given: Mapped[bool] = mapped_column(default=False)


🛑 consent_given as a boolean doesn't represent a consent for me
First, it is mandatory to fill in the mandatory security sheet so not free at all (in set_security_file)
Second, we could have wanted a timestamp and/or a version number of the consent text. Here, we don't know what the user consented to nor when
Retractation doesn't seem to be handled that well, especially considering we have no historization

Something along the lines of MyPayment
https://github.com/aeecleclair/Hyperion/blob/acba6e2/app/core/mypayment/models_mypayment.py#L244

It doesn't handle retractation either but it would be a step in the right direction

What do we do if a user rectract after the data are exported ?

And does versioning suppose to had their respective text in db as well ? If we change the text during a registering, what does happen, do we just remove registered info because the participant did not consent with the latest version, or do we consider that consenting to one imply consenting to all the following ones ?

Well, it seems like medical consent is a freaking pain in the ass, who would have thought ?

I agree, storing the datetime the consent was given at would be a good addition. I think automatic retractation is out of scope of this PR. I propose we start with manual suppression of data (including the deletion of files that were downloaded by the RAID association) if someone ask ECLAIR/the RAID for its data to be removed

I added the date, how would you design the retractation feature ? a simple endpoint marking consent as false and clearing the data ?

maximeroucher · 2026-08-24T15:24:36Z

Possible to split PR ? consent_given and course_responsible have nothing to do with each other, comments will get muddled

Given the time constraint and the time taken to review and merge a PR, I decided to put all the changes in one. I acknowledge that it will be a bit harder to read, that's why I ensure to produce really thin commit to allow a good review

to prevent greenlet

Marc-Andrieu approved these changes Aug 24, 2026

View reviewed changes

Comment thread app/modules/raid/coredata_raid.py

Comment thread app/modules/raid/endpoints_raid.py

maximeroucher force-pushed the raidV2-improvments branch 3 times, most recently from 88021b7 to d3331a4 Compare August 24, 2026 12:41

Daihecyy reviewed Aug 24, 2026

View reviewed changes

maximeroucher force-pushed the raidV2-improvments branch from d3331a4 to 64ecdb5 Compare August 24, 2026 15:32

maximeroucher added 13 commits August 24, 2026 21:17

feat: adding consent fields to model

51fea38

feat(migration): adding column do db

cd5d8b1

feat: adding consent to schema

7ef77a9

feat: excluding security file when pulling all participants

9c49af6

feat: adding consent when creating security file

5a195d0

feat: gating security file update based on consent only

94b32eb

feat: adding medical read permission and logging

c00b1aa

feat: removing security file on admin access of a participant

2edd5d5

feat: adding course responsible as emergency contact

4ed79d8

feat: generating security file with course responsible data

12866f0

feat: updating testing setup with mocked course responsible

0ad7654

feat: adding consent default data for test

e7069a2

feat: renaming is admin to is raid admin

7e9da86

maximeroucher force-pushed the raidV2-improvments branch from 38100f3 to 8d80f96 Compare August 24, 2026 19:18

feat: gating security data read via selecting load restriction

08471eb

maximeroucher force-pushed the raidV2-improvments branch from 8d80f96 to 08471eb Compare August 24, 2026 19:58

armanddidierjean added 4 commits August 24, 2026 22:13

fix: nullable consent_given_at in migration

baa1ecd

fix: remove print

7df50a6

fix: don't copy list before selecting relationship in cruds

8d93b4b

fix: select relationship

2930855

to prevent greenlet

armanddidierjean force-pushed the raidV2-improvments branch 2 times, most recently from 72f76e7 to ef26b01 Compare August 25, 2026 08:24

feat: use restricted participant without securityfile when possible

c53ee18

armanddidierjean force-pushed the raidV2-improvments branch from ef26b01 to c53ee18 Compare August 25, 2026 08:46

Return progress for GET admin participant

a6dcabb

Uh oh!

Conversation

maximeroucher commented Aug 24, 2026

Description

Summary

Changes Made

Additional Notes

Classification

Type of Change

Impact & Scope

Testing

Documentation

Uh oh!

Marc-Andrieu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Aug 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Daihecyy 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Daihecyy Aug 24, 2026 • 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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Daihecyy Aug 24, 2026 • 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!

Daihecyy Aug 24, 2026 • 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!

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!

maximeroucher commented Aug 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

codecov Bot commented Aug 24, 2026 •

edited

Loading

Daihecyy left a comment •

edited

Loading

Daihecyy Aug 24, 2026 •

edited

Loading

Daihecyy Aug 24, 2026 •

edited

Loading

Daihecyy Aug 24, 2026 •

edited

Loading