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

🗃️ [#4246] Expand AuthInfo model to capture mandate context #4339

Merged

Conversation

sergei-maertens
Copy link
Member

@sergei-maertens sergei-maertens commented May 30, 2024

Partly closes #4246

Changes

  • Added fields for the (optional) legal subject (authorizee)
  • Added fields for the (optional) acting subject (authorizee)
  • Added check constraints to enforce data integrity
  • Organized the admin in logical groups

The authentication context data model identifies two parties: the representee and the authorizee. There is always an authorizee - it can be the same actor as the representee if no mandate is in involved.

An authorizee has two aspects: legal subject and acting subject. There is always a legal subject. If no acting subject is provided, it is inferred from the legal subject.

So, a simple DigiD login in this model is represented by a legal subject authorizee of type BSN. We still keep storing this data in the attribute + value fields, but in the event of a mandate, we will store the additional information for the legal/acting subject.

Checklist

Check off the items that are completed or not relevant.

  • Impact on features

    • Checked copying a form
    • Checked import/export of a form
    • Config checks in the configuration overview admin page
    • Problem detection in the admin email digest is handled
  • Release management

    • I have labelled the PR as "needs-backport" accordingly
  • I have updated the translations assets (you do NOT need to provide translations)

    • Ran ./bin/makemessages_js.sh.sh
    • Ran ./bin/compilemessages_js.sh
  • Commit hygiene

    • Commit messages refer to the relevant Github issue
    • Commit messages explain the "why" of change, not the how

Copy link

codecov bot commented May 30, 2024

Codecov Report

Attention: Patch coverage is 97.22222% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.26%. Comparing base (3e8f3cb) to head (ff451e3).
Report is 717 commits behind head on master.

Files with missing lines Patch % Lines
src/openforms/authentication/models.py 96.29% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #4339   +/-   ##
=======================================
  Coverage   96.26%   96.26%           
=======================================
  Files         731      731           
  Lines       23756    23786   +30     
  Branches     2801     2803    +2     
=======================================
+ Hits        22868    22898   +30     
  Misses        617      617           
  Partials      271      271           

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

@sergei-maertens sergei-maertens force-pushed the feature/4246-store-authentication-context-data branch from e66224c to 3366046 Compare May 31, 2024 07:14
@sergei-maertens
Copy link
Member Author

For the reviewer, this relates to https://github.com/maykinmedia/authentication-context-schemas

@sergei-maertens sergei-maertens requested a review from Viicos May 31, 2024 08:10
src/openforms/authentication/models.py Outdated Show resolved Hide resolved
Comment on lines +176 to +179
# deprecated!
machtigen = models.JSONField(
verbose_name=_("machtigen"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess there's no way to extract things from this deprecated fields to set them to the mandate/authorizee fields?

Copy link
Member Author

Choose a reason for hiding this comment

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

even worse - the data stored in there is just plain wrong now :)

* Added fields for the (optional) legal subject (authorizee)
* Added fields for the (optional) acting subject (authorizee)
* Added check constraints to enforce data integrity
* Organized the admin in logical groups

The authentication context data model identifies two parties:
the representee and the authorizee. There is always an
authorizee - it *can* be the same actor as the representee
if no mandate is in involved.

An authorizee has two aspects: legal subject and acting
subject. There is always a legal subject. If no acting
subject is provided, it is inferred from the legal subject.

So, a simple DigiD login in this model is represented by
a legal subject authorizee of type BSN. We still keep
storing this data in the attribute + value fields, but
in the event of a mandate, we will store the additional
information for the legal/acting subject.
@sergei-maertens sergei-maertens force-pushed the feature/4246-store-authentication-context-data branch from 3366046 to ff451e3 Compare May 31, 2024 10:22
@sergei-maertens sergei-maertens merged commit 4576c74 into master May 31, 2024
27 checks passed
@sergei-maertens sergei-maertens deleted the feature/4246-store-authentication-context-data branch May 31, 2024 11:11
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.

Capture authentication context data
2 participants