ADR-004: Authentication API #251
Conversation
reinkrul
requested review from
Dirklectisch,
JorisHeadease,
Copilot,
gasperr and
stevenvegt
There was a problem hiding this comment.
Pull Request Overview
This PR introduces an Architecture Decision Record (ADR) documenting the authentication API strategy for the Knooppunt system. The ADR evaluates different approaches for EHR-to-Knooppunt authentication in both machine-to-machine and end-user scenarios.
- Evaluates three main authentication options: Nuts v2 auth API, OAuth2/OpenID Connect, and various OAuth2 grant types
- Documents trade-offs between simplicity, security, and integration complexity
- Proposes using optional OIDC Provider for end-user auth and OAuth2 API with Client Credentials and Token Exchange grants
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
docs/adr/004-authentication-api.md
Outdated
| - The EHR will still need to integrate DEZI (an OpenID Connect API) for caregiver authentication. | ||
| - The DEZI id_token will have to be wrapped in a Verifiable Credential for usage in Nuts, adding complexity. | ||
|
|
||
| ### OAuth2 / OpenID Connect |
There was a problem hiding this comment.
OAuth for machine-2-machine authentication makes semantically no sense. You are pretending to be an OAuth token endpoint, but it is only an endpoint to initiate the request to another token endpoint. This can lead to confusion and misuse.
|
@stevenvegt I've updated the ADR;
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 9 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - **Security**: Should promote secure deployments. | ||
| - **Simplicity**: Should be as simple as possible to implement and deploy. | ||
|
|
||
| ## Decision outcome |
There was a problem hiding this comment.
The section is titled "Decision outcome" but appears before the "Considered Options" section. Standard ADR format typically presents options first, then the decision. Consider either:
- Moving this section after "Considered Options", or
- Renaming it to "Proposal" or "Recommended Solution" to clarify this is the proposed approach being evaluated
| ``` | ||
| grant_type=urn:ietf:params:oauth:grant-type:token-exchange | ||
| &audience=<remote EHR system (remote OAuth2 issuer URL)> | ||
| &subject_token=<Dezi id_token> | ||
| &subject_token_type=urn:ietf:params:oauth:token-type:id_token | ||
| &actor_token=<Nuts subject ID> | ||
| &actor_token_type=nuts-subject-id | ||
| &requested_token_type=urn:ietf:params:oauth:token-type:access_token | ||
| &scope=<requested scopes> | ||
| &client_id=<EHR client ID> | ||
| &client_secret=<EHR client secret> | ||
| ``` |
There was a problem hiding this comment.
Inconsistent formatting: The code block at lines 104-111 uses a multi-line format with ampersands on new lines and leading spaces, while the code block at lines 163-174 uses the same multi-line format. However, both should clarify that this is illustrative formatting for readability, as actual OAuth2 requests would be URL-encoded on a single line or properly formatted as HTTP requests.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
docs/adr/004-authentication-api.md
Outdated
| &client_id=<EHR client ID> | ||
| &client_secret=<EHR client secret> | ||
| &dezi_id_token=<Dezi id_token> (optional) | ||
| &nuts_subject_id=<Nuts subject ID> |
There was a problem hiding this comment.
The parameter dezi_id_token is marked as optional but nuts_subject_id is not. The description doesn't clarify whether nuts_subject_id is always required or only in specific scenarios. Consider documenting when each parameter is required/optional.
| &nuts_subject_id=<Nuts subject ID> | |
| &nuts_subject_id=<Nuts subject ID> (required; always needed to identify the organization or end-user context) |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
See "Proposal" at the bottom of the document.