Feature/email login - #271
Merged
Merged
Conversation
The email is what links an ngen account to the identity of the sso provider, and from now on what the login takes besides the username, so it has to name one account and only one: it is required and unique, and it is stored without its case so that two of them cannot differ only in that. The emails that could not exist under that rule are rewritten by the migration, which is the only thing it can do without choosing for the administrator: of the accounts that share an address the oldest keeps it and the others get a numbered variation of it on the same domain, and an account with no address at all gets one on a reserved domain. Both are meant to be found and replaced from the users page. The unique validator that comes with the field is written as an exact match, so the serializer asks for it without the case as well: a duplicate is a 400 that names the field instead of an integrity error.
Everything that asks for a user and a password goes through django's authenticate(), so the api, the admin and the browsable api all read the identifier the same way once the backend does: the account whose username or email is the given one, in any case. Usernames are only unique as written, so an identifier can still reach more than one account. The one that owns it as its username wins, and anything still ambiguous is refused rather than guessed, which is also why a wrong identifier is made to cost the same as a wrong password.
The account of an identity is the one that has its email, and the lookup asked for it as written: an account saved as Name@corp.com was not the one behind name@corp.com, so the provider got a second account instead of the one that already existed, with its groups, its permissions and its cases. The lookup that could return more than one user only handled finding none, which is a 500 in the callback rather than a login. It cannot happen now that the email is unique, but which account an identity owns is not something to settle by accident.
The sso login leaves the state of the flow in the cache and reads it back on the callback, and django keeps the cache inside each process unless it is told otherwise: with more than one worker the callback lands on a process that never saw the login start and the flow fails for no visible reason. Redis is already there for celery and for constance, so the cache goes to a database of its own. The suite keeps a cache of its own too, in memory.
…hing Nothing limited how many times a password could be tried: 40 attempts took four seconds. The count is kept in the authentication backend, so every way in is covered by the same rule, the api, the django admin and the browsable api alike, and it is kept per account and address so that nobody can lock an account out of its owner by failing on purpose. The account is what is counted, not how it was named, or the same one could be tried twice over by its username and by its email. On top of that the endpoints that answer without credentials carry a rate: the counter is per account and would not notice a list of accounts being tried one attempt each. Signing up was open, and it was also a way around the password policy the users page asks for: the endpoint checked nothing and the frontend only hid the link behind a flag nothing ever wrote. It is off unless the deployment asks for it, it asks for the same password as everywhere else, and the frontend reads whether to offer it from the api.
…at asked The refresh token is a session in a cookie and it was handed over without asking for https and without saying which sites may send it, and it outlived by two weeks the token it carries. It now says both, and lasts what the token lasts. The sso login was not tied to anything either: the state lived in a cache that any browser could complete, so an attacker could start a login and hand the finished callback to somebody else, who would land inside the attacker's account. The browser that starts the login now carries the state in a cookie and the callback asks for it back. Two more things the provider was not asked for: a nonce, so the id token has to answer this login and not one replayed from another, and a pkce challenge, so an authorization code is only worth something to whoever started the flow. The code the frontend trades for a session travels in the fragment now, which the browser keeps to itself, instead of the query string, which ends up in its history and in the referer of whatever the page loads.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR strengthens authentication and user identity handling by allowing login via username or email, enforcing unique case-insensitive emails, and tightening security controls around login/signup/SSO flows across backend and frontend.
Changes:
- Backend: added an email-or-username auth backend, login attempt limiting, DRF throttling scopes/rates, and SSO hardening (state cookie binding, PKCE, nonce validation, fragment-based exchange code).
- Data model: made
User.emailrequired+unique, lowercased on clean, and introduced a migration to repair missing/duplicate emails. - Frontend: updated login/signup toggles and SSO callback handling, improved user form email validation, and updated i18n strings.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| project/urls.py | Switches the “simple token” endpoint to a class-based view (ObtainApiKeyView). |
| project/settings.py | Adds scoped throttling rates, shared Redis cache config, signup flag, and tightens credentialed CORS behavior. |
| ngen/views/tools.py | Adds throttling scope for export and exposes ALLOW_SIGNUP via settings endpoint. |
| ngen/views/sso.py | Hardens SSO flow (PKCE, nonce, state-cookie binding) and moves exchange code to URL fragment. |
| ngen/views/auth.py | Centralizes refresh-cookie handling, adds throttling scopes to auth endpoints, and gates signup behind ALLOW_SIGNUP. |
| ngen/tests/models/test_users.py | Adds model/migration-focused tests for email normalization and uniqueness. |
| ngen/tests/models/test_tasks.py | Updates test user creation to include required email. |
| ngen/tests/models/test_state.py | Updates test user creation to include required email. |
| ngen/tests/models/test_events.py | Updates test user creation to include required email. |
| ngen/tests/models/test_email_auto_send.py | Updates test user creation to include required email. |
| ngen/tests/models/test_constituency.py | Updates test user creation to include required email. |
| ngen/tests/models/test_announcement.py | Updates test user creation to include required email. |
| ngen/tests/filters/test_events.py | Updates test user creation to include required email. |
| ngen/tests/api/test_users.py | Adds API tests for email-based login + case-insensitivity + email uniqueness behaviors. |
| ngen/tests/api/test_sso.py | Adds tests for OIDC backend email-linking and case-insensitive matching. |
| ngen/tests/api/test_playbook.py | Updates test user creation to include required email. |
| ngen/tests/api/test_login_security.py | Adds tests for login attempt limiting, throttling behavior, signup gating, and refresh-cookie properties. |
| ngen/services/login_attempts.py | Introduces cache-backed login failure limiter keyed per account+IP. |
| ngen/services/init.py | Re-exports the new login attempt limiter service. |
| ngen/serializers/auth.py | Tightens signup password validation and enforces case-insensitive email uniqueness in serializers. |
| ngen/models/auth.py | Makes email unique and normalizes email to lowercase in clean(). |
| ngen/migrations/0038_user_email_unique.py | Migrates existing users to have unique/placeholder emails and alters the email field to unique. |
| ngen/backends.py | Adds email-or-username backend with ambiguity handling and integrates login attempt limiting + OIDC email iexact handling. |
| frontend/src/views/user/components/FormUser.jsx | Makes email effectively required in the UI and blocks submit if email invalid. |
| frontend/src/views/auth/sso/SsoCallback.jsx | Parses exchange code from fragment rather than query string. |
| frontend/src/views/auth/signin/SignIn1.jsx | Reads and persists ALLOW_SIGNUP setting to control signup availability in UI. |
| frontend/src/views/auth/signin/RestLogin.jsx | Updates login UX copy to “username or email” and validation messages. |
| frontend/src/api/services/users.jsx | Surfaces backend validation reasons for rejected user create/edit requests. |
| frontend/public/locales/es/translation.json | Adds Spanish i18n keys for “username or email” validation/copy. |
| frontend/public/locales/en/translation.json | Adds English i18n keys for “username or email” validation/copy. |
| docker/.env/ngen.dev.env.example | Documents new env vars for CORS origin naming, Redis cache URL, and signup flag. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The variation was drawn at random until it landed on a free one, which is a loop with nothing stopping it: with every number taken it would spin forever, in the middle of a migration, and the test that covered it leaned on the draw eventually coming up with the only number left. It draws a few times, which is what keeps the rewritten addresses from spelling out how many accounts were sharing one, and counts from there, which always ends because only so many addresses are taken. The login field says what it takes to a screen reader too: it carried a label attribute that an input does not have, so it had no name of its own, only its placeholder. And an username is not how that is written.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces several important changes to user authentication and management, focusing on allowing users to log in with either their username or email, enforcing unique emails for all accounts, and improving error handling and validation in both the backend and frontend. There are also updates to localization and environment configuration.
Authentication and User Identity
EmailOrUsernameModelBackend) that allows users to log in using either their username or email, ensuring case-insensitive matching and proper handling of ambiguous identifiers. Also integrates login attempt limiting. (ngen/backends.py)Usermodel to require unique emails. (ngen/migrations/0038_user_email_unique.py,ngen/models/auth.py) [1] [2]ngen/backends.py)Frontend Login and Validation
frontend/src/views/auth/signin/RestLogin.jsx,frontend/public/locales/en/translation.json,frontend/public/locales/es/translation.json) [1] [2] [3] [4] [5] [6]frontend/src/api/services/users.jsx) [1] [2] [3]frontend/src/views/user/components/FormUser.jsx) [1] [2]SSO and Environment Configuration
frontend/src/views/auth/sso/SsoCallback.jsx) [1] [2] [3]docker/.env/ngen.dev.env.example,frontend/src/views/auth/signin/SignIn1.jsx) [1] [2] [3]These changes collectively improve security, user experience, and administrative clarity in user management and authentication.