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

Add siret attribute and mapper to Keycloak #577

Merged
merged 2 commits into from
Dec 23, 2024
Merged

Conversation

Morendil
Copy link
Collaborator

@Morendil Morendil commented Dec 3, 2024

Purpose

Improve developer experience by making local dev resemble production conditions more closely.

Proposal

We can now find organization data as provided by ProConnect in user_info, specifically SIRET.

Also in this PR:

Add OIDC_ORGANIZATION_REGISTRATION_ID_FIELD to local dev settings

Otherwise local development would be blind to registration ID fields (e.g. SIRET)… and these will become more important as we start dealing with a variety of organizations both private and public (such as French communes).

Expose organization registration ID field (e.g. SIRET) in /users/me endpoint

This is mostly to have a convenient way of testing the delivered feature, via an API-oriented E2E test. This isn't entirely satisfactory: as this is a very important feature, we should be able to unit test it. The E2E test depends on technical details such as the fact that we use Keycloak to simulate the ProConnect flow.

Add jq to Python dev dependencies, so we can perform "surgery" on JSON responses

This is necessary for several backend tests that query the /users endpoint. These tests retrieve a JSON object and test for strict equality. But strict equality is much more stringent than what these tests intend to assert ! We only want to assert that queries for a given set of users will return all the users, and only those users, that the query designates. We are not interested in, for instance, the exact registration IDs of each of the users' organizations. (It is a matter for a more focused unit test to test, for instance, that the User serializer returns certain Organization fields.)

Also, if we tested against the actual, exact registration IDs of organizations, then our tests would become dependent on the order in which they are run, which is unacceptable.

Copy link
Collaborator

@qbey qbey left a comment

Choose a reason for hiding this comment

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

nice :)

@Morendil Morendil force-pushed the add-siret-to-keycloak branch 4 times, most recently from 9acb4ef to a811e03 Compare December 23, 2024 16:56
We can now find organization data as provided by ProConnect in user_info
@Morendil Morendil force-pushed the add-siret-to-keycloak branch 3 times, most recently from fe048f5 to d9d724c Compare December 23, 2024 18:19
We also add registration ID info to the /me endpoint, via serializers
@Morendil Morendil force-pushed the add-siret-to-keycloak branch from d9d724c to d9c6f63 Compare December 23, 2024 18:24
@Morendil Morendil merged commit 8fd55a6 into main Dec 23, 2024
20 checks passed
@Morendil Morendil deleted the add-siret-to-keycloak branch December 23, 2024 19:18
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.

2 participants