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

[#2041] Added hard database constraints on User's identifier/login_type fields #990

Closed
wants to merge 1 commit into from

Conversation

Bartvaderkin
Copy link
Contributor

@Bartvaderkin Bartvaderkin commented Jan 25, 2024

I tried to make it impossible to store bad identifier data (like duplicates), or have lingering data (like a bsn when the login type is not digid).

We need to check a few assumptions:

  • user will always have an identifier (email, bsn, oidc_id, rsin or kvk) for their login_type
  • users cannot not switch login types
  • eherkenning users cannot have both rsin and kvk
  • email must be unique for default logins, but not for eherkenning
  • it is intentional OIDC allows non-unique emails

I think I have all cases in the new test, see diff: src/open_inwoner/accounts/tests/test_user_constraints.py

@alextreme
Copy link
Member

Background is that it's possible (once in a blue moon and all within a few seconds) to register multiple Users via DigiD with the same bsn

@Bartvaderkin
Copy link
Contributor Author

The CI is absolute mayhem, at first glance I see some bad factories and sloppy test setups, so I'll look into those and then see if there are actual problems.

@Bartvaderkin
Copy link
Contributor Author

After discussion with Alex I unassigned @stevenbal and @pi-sigma for now because it's slightly too early (although you're welcome to have a look as we'll discuss this monday)

models.Q(("login_type", "eherkenning"), _negated=True),
_connector="OR",
),
name="check_kvk_or_rsin_exclusive_when_login_eherkenning",
Copy link
Contributor

Choose a reason for hiding this comment

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

this will not work with the current implementation of eHerkenning in OIP, because we set both the KVK and RSIN for users

@Bartvaderkin Bartvaderkin added the on hold Pause working for now, continued after decisions label Mar 1, 2024
@alextreme alextreme closed this May 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
on hold Pause working for now, continued after decisions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants