Add registration variables for auth context (objects API registration v2) #4449
Conversation
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
2 times, most recently
from
141e98c to
144be1c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #4449 +/- ##
==========================================
+ Coverage 96.51% 96.57% +0.06%
==========================================
Files 720 720
Lines 23899 24000 +101
Branches 2817 2843 +26
==========================================
+ Hits 23065 23179 +114
+ Misses 566 559 -7
+ Partials 268 262 -6 ☔ View full report in Codecov by Sentry. |
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
from
422518c to
62f2041
Compare
sergei-maertens
changed the title
src/openforms/registrations/contrib/objects_api/tests/test_backend_v2.py
Outdated
Show resolved
Hide resolved
src/openforms/registrations/contrib/objects_api/registration_variables.py
Show resolved
Hide resolved
src/openforms/registrations/contrib/objects_api/tests/test_backend_v2.py
Outdated
Show resolved
Hide resolved
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
from
a9da7c3 to
a5ad075
Compare
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
3 times, most recently
from
4c19b80 to
a8b802f
Compare
|
@stevenbal can you review the documentation changes, as they're in Dutch? 😬 |
docs/manual/forms/variables.rst
Outdated
| Er zijn geen plannen om deze laatste te verwijderen. | ||
|
|
||
| De variabele bevat een bak aan informatie, gestructureerd volgens het | ||
| authenticatiecontextdatamodel (TODO: add link). De structuur is als volgt: |
There was a problem hiding this comment.
will this be added later?
There was a problem hiding this comment.
yes, though I need to check if I can share those gitbook links and people outside of our team probably need to review/tweak the content, so I don't think we should block this PR on that TODO.
| Merk op dat niet alle attributen aanwezig zijn, dit hangt af van het inlogmiddel ( | ||
| DigiD, eHerkenning) en of er wel/niet sprake is van een machtiging én de soort |
There was a problem hiding this comment.
| Merk op dat niet alle attributen aanwezig zijn, dit hangt af van het inlogmiddel ( | |
| DigiD, eHerkenning) en of er wel/niet sprake is van een machtiging én de soort | |
| Merk op dat niet alle attributen aanwezig zijn, dit hangt af van het inlogmiddel (DigiD, eHerkenning) en of er wel/niet sprake is van een machtiging én de soort |
There was a problem hiding this comment.
I don't see the difference or mistake 😅
docs/manual/forms/variables.rst
Outdated
| De onderdelen van deze structuur worden ook als individuele variabelen aangeboden: | ||
|
|
||
| ``auth_context_source`` | ||
| Middel van inloggen: de waarde is ``"digid"`` of ``eherkenning``, of een lege string |
There was a problem hiding this comment.
| Middel van inloggen: de waarde is ``"digid"`` of ``eherkenning``, of een lege string | |
| Middel van inloggen: de waarde is ``"digid"`` of ``"eherkenning"``, of een lege string |
There was a problem hiding this comment.
I removed the quotes on digid instead to make it consistent with other example values.
| ``auth_context_acting_subject_identifier_type`` | ||
| In de praktijk zal de waarde altijd ``opaque`` of leeg zijn. Geeft aan hoe de | ||
| identificatie van de handelende persoon ("de persoon aan de knoppen") | ||
| geïnterpreteerd moet worden. |
There was a problem hiding this comment.
I'm not sure what this means, does this mean that the value literally is "opaque"? Is this like a convenience variable to not have to check if machtigen is used when determining the acting subjects auth info?
There was a problem hiding this comment.
the identifier type value is indeed "opaque", as opposed to "bsn" or "kvkNummer" (or the future "RSIN").
Currently it's the only possible type, but in the future more types could be possible (this could for example be used for a registrator at the service desk too, at some point). It basically means that you have zero guarantees about the meaning of the identifier value except that it's a value tied to a particular staff member. It could look like a BSN, but if you use it to consult the BRP, it may sometimes work, it will most likely not. It is not expected to be decrypted.
| Zie :ref:`manual_forms_variables_auth_context` voor een voorbeeld van de structuur, en | ||
| een overzicht van alle "onderdelen" waaruit de ``auth_context`` variabele bestaat. Je | ||
| kan deze allemaal individueel gebruiken in de sjablonen. |
There was a problem hiding this comment.
Is there a preference when it comes to using these variables and does it make a difference (aside from number of characters), e.g. auth_context_loa vs auth_context.levelOfAssurance?
There was a problem hiding this comment.
there is a slight difference in that the static variables will always yield a string, while missing other attributes could theoretically return None and template authors would have to check for that.
In practice, I think the DTL fallback to empty string will result in the same outcome (most of the time).
| @@ -26,6 +26,17 @@ class Registry(BaseRegistry[BaseStaticVariable]): | |||
| """The Objects API registration variables registry.""" | |||
|
|
|||
|
|
|||
| @register("public_reference") | |||
| class PublicReference(BaseStaticVariable): | |||
There was a problem hiding this comment.
I don't see this variable mentioned in the Vaste variabelen section, should this be mentioned there? Or is this mentioned in a specific page for Objects API registration?
There was a problem hiding this comment.
it's not a static variable, but a registration variable and visible in the UI when you select an objects API registration backend :) and there the title + type are displayed (it's "self-documenting" via the registry).
| "auth_context": ( | ||
| submission.auth_info.to_auth_context_data() | ||
| if submission.is_authenticated | ||
| else None | ||
| ), |
There was a problem hiding this comment.
Just for my understanding, this is no longer needed because the auth_context variable is always available for the registration backend now?
There was a problem hiding this comment.
yeah, precisely. I assumed wrongly that the auth_context should be provided only as objects registration variables, but it acts at the same level as the existing auth static variable, so we can expose it directly to all registration backends automatically :)
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
from
66ce62a to
e92ed0d
Compare
This was reported verbally at some point by DH when they want to use the 'objects API contract' feature.
Tests for the happy flow where all information is available.
We notice now logic is written based on auth.plugin rather than auth.attribute, which makes using a different auth backend providing the same attribute hard. Instead, provide the auth_type variable to have a better name, and update the docs to favour this one.
Instead of only exposing them as objects API registration variables. These variables are available at all times, after authenticating, so they belong in the static variables.
The static variables can also be used as-is in the templates, no special treatment is required. The documentation is updated accordingly, with a pointer to the static variables section that describes the authentication context data.
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
3 times, most recently
from
ba61561 to
dc23c68
Compare
* Use nicer way to check if a registrator is present or not * Documentation fixes
sergei-maertens
force-pushed
the
feature/4246-register-auth-context-in-objects-api-v2
branch
from
dc23c68 to
e1e6a11
Compare
sergei-maertens
deleted the
feature/4246-register-auth-context-in-objects-api-v2
branch
Partly closes #4246
Changes
Checklist
Check off the items that are completed or not relevant.
Impact on features
Release management
I have updated the translations assets (you do NOT need to provide translations)
./bin/makemessages_js.sh./bin/compilemessages_js.shCommit hygiene