diff --git a/README.md b/README.md index ae3453a..60ca555 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,32 @@ but membership no longer checks directory recreation during login. Preserve the connector mapping for other services; migrating Dex to immutable user IDs is a separate coordinated change, not an automatic part of this release. +### Database ProgrammingError during callback + +`Dex callback rejected kind=ProgrammingError` in older images is a database +error, not a password rejection. Updated images log a redacted `sqlstate`: +`42703` means an undefined column, `42P01` an undefined table, and `42501` +insufficient privileges. Raw SQL/exception messages are deliberately omitted +because they may contain identity data. + +For `42703` after the Dex-only release, check that `members.0004_dex_login_identity` +is applied to the **same database/schema used by web and worker**. Using the +release image and its production environment, inspect `showmigrations members` +and `migrate --plan`, then run `migrate --noinput` through the deployment's +migration service/Job. These are Django management commands. Do not use +`makemigrations` or `--fake` as a production repair, or delete database volumes. +Keep the original `DEX_ISSUER` while applying this migration. + +On Kubernetes, publish a new immutable membership image tag/digest and rerun the +role so it completes a new migration Job before deploying web/worker. Reusing an +image tag can leave both an old cached migration image (`IfNotPresent`) and an +already-completed Job: a rollout restart alone does not rerun migrations. Use +`www_rollout_revision` to request a fresh Job when appropriate, but prefer a new +immutable image reference. If the migration is marked applied yet columns are +missing, investigate the database/schema and migration history before any manual +DDL. For other SQLSTATEs, investigate the indicated permissions/schema problem +rather than assuming an unapplied migration. + ### LDAP service credentials still required for directory writes LDAP is still needed for registration and worker group synchronization. Configure diff --git a/membership/members/tests/test_membership.py b/membership/members/tests/test_membership.py index 0495a03..c1d7108 100755 --- a/membership/members/tests/test_membership.py +++ b/membership/members/tests/test_membership.py @@ -274,6 +274,35 @@ class IdentityTests(TestCase): self.assertContains(response, "could not be verified") self.assertNotIn("account_id", self.client.session) + @patch("members.views.dex") + @patch("members.views.link_identity") + def test_database_failure_logs_only_safe_sqlstate_and_recovery_hint(self, link, dex): + from django.db import ProgrammingError + from psycopg.errors import InsufficientPrivilege, UndefinedColumn, UndefinedTable + + dex.return_value.authorize_access_token.return_value = {"id_token": "private-token", "userinfo": self.claims()} + for cause, expected, hint in ( + (UndefinedColumn("private SQL and email alice@example.org"), "42703", "members.0004_dex_login_identity"), + (UndefinedTable("private SQL"), "42P01", "undefined_table"), + (InsufficientPrivilege("private SQL"), "42501", "insufficient_privilege"), + (None, "unknown", "matching application/migration images"), + ): + with self.subTest(sqlstate=expected): + error = ProgrammingError("private database details") + error.__cause__ = cause + link.side_effect = error + with self.assertLogs("members.views", level="ERROR") as logs: + response = self.client.get("/members/oidc/callback/?code=private-code") + self.assertContains(response, "could not be verified") + self.assertNotIn("account_id", self.client.session) + output = " ".join(logs.output) + self.assertIn(f"sqlstate={expected}", output) + self.assertIn(hint, output) + for private in ("private", "alice@example.org", "Traceback"): + self.assertNotIn(private, output) + self.assertNotIn("sqlstate", response.content.decode()) + self.assertFalse(Account.objects.exists()) + @patch("members.views.dex") @patch("members.directory.Connection") @override_settings(LDAP_URL="", LDAP_BIND_DN="", LDAP_BIND_PASSWORD="", LDAP_PASSWORD_SCHEME="{CRYPT}") diff --git a/membership/members/views.py b/membership/members/views.py index 71def4a..2bf1751 100755 --- a/membership/members/views.py +++ b/membership/members/views.py @@ -12,7 +12,7 @@ from authlib.integrations.django_client import OAuth from django.conf import settings from django.core.validators import validate_email from django.core.mail import send_mail -from django.db import transaction +from django.db import ProgrammingError, transaction from django.http import HttpResponse from django.shortcuts import get_object_or_404, redirect, render from django.db.models import Q @@ -213,6 +213,18 @@ def callback(request): expires_at = proof.get("expires_at") if isinstance(proof, dict) else None registration_account_id = proof.get("account_id") if isinstance(expires_at, (int, float)) and expires_at > timezone.now().timestamp() else None account = link_identity(token.get("userinfo") or {}, registration_account_id=registration_account_id) + except ProgrammingError as exc: + # PostgreSQL diagnostics and SQL text can contain identity data. Log + # only a validated SQLSTATE, never the exception message or traceback. + sqlstate = getattr(exc.__cause__, "sqlstate", None) + sqlstate = sqlstate if isinstance(sqlstate, str) and re.fullmatch(r"[0-9A-Z]{5}", sqlstate) else "unknown" + hint = { + "42703": "undefined_column: check pending migrations, especially members.0004_dex_login_identity, against the web database", + "42P01": "undefined_table: check migrations and the configured database/schema", + "42501": "insufficient_privilege: check database role permissions", + }.get(sqlstate, "check database schema, permissions and matching application/migration images") + logger.error("Membership database failure during Dex callback kind=ProgrammingError sqlstate=%s; %s", sqlstate, hint) + return page(request, title="Sign-in could not be verified", message="Try signing in again, or contact the administrator.") except Exception as exc: logger.error("Dex callback rejected kind=%s", type(exc).__name__) return page(request, title="Sign-in could not be verified", message="Try signing in again, or contact the administrator.")