Make shared login the only way into the admin - #4
Open
alexeygrigorev wants to merge 1 commit into
Open
Conversation
`/admin/login/?local=1` returned Django's password form even where Cognito was configured. The escape hatch arrived with the OIDC adapter and nothing ever used it: operator accounts are created by the callback without a usable password, no deploy script or document mentions it, and no test covered it. On a public hostname it is a password endpoint on the open internet, which is why the infrastructure change fronting Relay carries a WAF rule refusing the parameter. Authentication policy belongs in the application, and that rule stops helping the moment a request reaches the app another way. The parameter is gone. Django's password form survives only where there is no identity provider to talk to: DEBUG with no AUTH_* configured, which is a developer's machine. A deployed host that has lost its auth configuration now answers 503 instead of quietly degrading to passwords. The AUTH_* settings also defaulted to the sandbox host and pool whenever DEBUG was off, so any other hostname would have sent operators to relay.dtcdev.click's callback and failed there with nothing in the app noticing. A deployed host now states its own pool and callback, and Django refuses to start with DEBUG=False and them unset. The sandbox deploy script supplies the values it already knew.
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.
The bypass
GET /admin/login/?local=1returned Django's username/password form even on a hostwith Cognito configured.
relay/urls.pyroutesadmin/login/throughoidc.admin_loginahead ofadmin.site.urls, so this is the single door to both/admin/and the operator UI (@staff_member_requiredredirects here).History: the branch arrived in
aaec99c"Add shared Cognito login" together with theadapter, undocumented and untested. At that moment
AUTH_*defaulted to emptyeverywhere, so local development was already covered by the
configured()checkalone —
?local=1only ever mattered on a deployment that had an identityprovider. Seven minutes later
5c2fecb"Default production to shared Cognito login"made non-DEBUG default to the sandbox pool, which is the point at which
?local=1became the only way to reach a password form on a deployed host. Nothing depends on
it: the callback creates operator accounts with no usable password, no deploy script
or runbook mentions it, no test covered it.
With
relay.datatalks.clubabout to be exposed, that is a password endpoint on theopen internet. DataTalksClub/aws-infra#36 compensates with a WAF rule refusing
local=1; authentication policy belongs in the application, and that rule stopshelping the moment a request reaches the app another way.
What changed
relay/oidc.py—admin_loginno longer reads any query parameter. Configured →always redirect to
/auth/login. Not configured and notDEBUG→503, the sameanswer
begin()already gives, so a deployed host that loses its authconfiguration fails closed instead of degrading to passwords. Django's password
form survives only for
DEBUGwith noAUTH_*, which is a developer's machinewith no provider to talk to. Recovery on a deployed host is a shell, not a login page.
relay/settings.py—AUTH_*no longer default torelay.dtcdev.clickand thesandbox pool when
DEBUGis off. A deployment that did not set them would havesent operators to the sandbox callback and failed there with nothing in the app
noticing.
DEBUG=FalsewithAUTH_BASE_URL,AUTH_CLIENT_ID,AUTH_CALLBACK_URLor
AUTH_ISSUERunset now raisesImproperlyConfiguredat import, the same shapeas the existing
SECRET_KEYrule: it fails at rollout, when someone is watching,rather than the first time the owner tries to sign in.
scripts/deploy_relay_sandbox.sh— writes the sandbox's ownAUTH_*into/etc/relay/runtime.env, next to therelay.dtcdev.clickvalues it already sets.Without this the sandbox would stop starting.
.env.example,docs/relay-deployment.md— document the variables and the"no local escape hatch on a deployed host" rule.
Tests
tests/test_oidc_auth.py, 8 new cases:test_admin_login_has_no_local_password_escape_hatch— parametrised over?local=1,?local=1&next=/admin/,?next=/admin/&local=1,?LOCAL=1,?local=truein a production-like configuration (DEBUG=False,AUTH_*set):302 to
/auth/login, no password input.test_admin_login_fails_closed_when_shared_auth_is_unconfigured_in_production— 503.test_admin_login_keeps_the_password_form_for_local_development— the dev path still works.test_shared_auth_settings_have_no_host_defaults_and_fail_loudly— imports thesettings module in a subprocess with
DEBUG=Falseand emptyAUTH_*and assertsit refuses to load, naming the four variables.
Five of these fail against the parent commit. Suite: 520 passed before, 528 passed
after;
ruff,makemigrations --check,manage.py checkclean.Terraform lane
main/relaymust supply, forrelay.datatalks.club:and the pool client must list that callback as an allowed redirect URI, or sign-in
stops at the provider. Without them the container will not start — deliberately.
The WAF rule refusing
local=1can stay as defence in depth; it is no longer thecontrol.
Other escape hatches found
SES_WEBHOOKS_SIGNATURE_MODE(relay/settings.py) defaults tomockunderDEBUGorTESTINGand is env-overridable, so setting it on a deployed hostdisables SNS signature verification. Not changed here — it is an ingress
authenticity control, not a login, and belongs in its own change.
mailing/management/commands/seed_demo_data.pysets a password but refuses to rununless
DEBUG/TESTING./internal/ops/status(mailing/ops_views.py) fails closed: no token, no endpoint.?u=on the click redirect is validated against the campaign's links.admin.site.urls; no header-based auth branches.