phuoc/ctp_blogr

Fork 0

Files

T

phuoc 187fe49c3c modified google-oauth-migration.md

2026-10-06 18:44:22 +02:00

56 KiB

Raw Blame History

google-oauth-migration - Work Plan

TL;DR (For humans)

What you'll get: Password login is gone. Visitors sign in with a Google account, and only the two email addresses you already have in the database are allowed in. Signing in as phuoc@caotek.fr keeps admin powers; signing in as ctphuoc@gmail.com gets you in without them.

Why this approach: The app keeps its own list of permitted emails and checks it once, at sign-in, so Google's "test user" loophole cannot be used to walk in. Authorization no longer decides who is an admin by comparing a name against the word "admin"; it reads a configured list instead. Sign-in state is protected with Google's recommended anti-hijacking checks.

What it will NOT do: It will not create accounts for strangers, ask Google for any extra information about you, add a server-side session system, or introduce a database migration tool that the project does not have.

Effort: Large Risk: High - it replaces the entire sign-in mechanism and rebuilds a database table inside a database file that is tracked in version control. Decisions to sanity-check: Your admin rights move from a hardcoded name to a list you edit in a config file; the sign-in screen timeout is lengthened so slow Google sign-ins are not thrown away; a broken edit-user page and a crash on the user list are fixed along the way because they sit in the code being changed.

Your next move: approve, or run a high-accuracy review. Full execution detail follows below.

TL;DR (machine): Large/high-risk Pyramid 2.1 auth swap to Google OIDC via Authlib, DB allowlist gate, destructive SQLite users-table rebuild, 14 todos in 5 waves + F1-F4.

Scope

Must have

Google OAuth 2.0 / OIDC sign-in replacing password login entirely. Login becomes a redirect to Google; there is no local credential path left in the codebase.
Authlib 1.8.0 as the only new library, driven by Google's OpenID discovery document at https://accounts.google.com/.well-known/openid-configuration so authorize_url, access_token_url and jwks_uri are never hand-written. It is a requests-based client (authlib.integrations.requests_client.OAuth2Session); Authlib ships integrations for Flask, Django and Starlette but none for Pyramid, which is why the app owns its own state / PKCE / nonce handling. The app fetches the discovery document itself and passes the resolved authorization_endpoint, token_endpoint and jwks_uri into the client: passing server_metadata_url= to OAuth2Session only stores inert metadata, because load_server_metadata exists solely on the Starlette-style sync_app / async_app clients. The document is cached alongside the JWKS rather than refetched per sign-in.
A named, registry-registered collaborator for the Google exchange - not ad-hoc monkeypatching in tests. It exposes create_authorization_url(...) and fetch_claims(...), and it fetches and caches the JWKS itself, because the bare client does not: CodeIDToken + jwt.decode require the app to pull keys from jwks_uri, and an unbounded refetch on every sign-in is a latency and rate-limit problem. Cache TTL is a decision of this plan, not left implicit.
App-owned state, PKCE and nonce handling. state is generated, persisted in the session and consumed (deleted) before the token exchange is attempted; code_verifier is generated by the app with generate_token(48) because create_authorization_url returns only (uri, state) and never hands the verifier back; nonce is generated, persisted, compared against the id_token claim, then deleted.
nonce is REQUIRED by Google's OIDC request parameters.
PKCE with code_challenge_method=S256, the code_verifier persisted between the two hops.
A single, non-replayable code exchange. pyramid_tm tags exceptions as retryable, so a SQLite "database is locked" transient error can make pyramid_retry re-run the callback and re-exchange an already-used authorization code, which fails with invalid_grant and shows the user a broken login. The callback must be non-retryable and fetch_claims must be called at most once per request even with retries enabled.
Auth policy swap to Pyramid's built-in SessionAuthenticationPolicy + SessionAuthenticationHelper, replacing AuthTktAuthenticationPolicy. Pyramid 2.1 ships both with the same three-method interface as AuthTktCookieHelper, so remember()/forget() call sites keep working and security.py is untouched. The hardcoded auth-ticket secret is removed.
The session factory is explicitly reconfigured - it is a first-class part of this change, not an afterthought. SignedCookieSessionFactory('hZug2zPt7hT2MZ') defaults to timeout=1200 (20 minutes) while the outgoing AuthTktAuthenticationPolicy used 36000 (10 hours). A Google consent screen, a Google sign-in and a second factor routinely exceed 20 minutes, so the OAuth values would expire mid-flow. The factory must be constructed with an explicit, longer timeout, a secret of at least 64 characters for hashalg='sha512' (it is currently 16), and httponly=True in both environments plus secure=True under production settings. Today the cookie carries secure=False, httponly=False even though production.ini declares url_scheme = https; once the cookie is the identity, third-party CDN scripts in layout.jinja2:14-15,106-107 can read it via XSS.
No session-id rotation is attempted, because the API does not exist. request.session.renew() is not part of Pyramid 2.1, and neither are the SessionRenew / SessionInvalidate exceptions. The only primitive is invalidate(), which is literally self.clear() carrying the upstream comment # XXX probably needs to unset cookie. There is no server-side session id, so classic session fixation is structurally impossible; the real requirement is to delete the state / code_verifier / nonce keys after the callback so they do not survive client-side.
remember() stores the user's email as authenticated_userid, which keeps all 15 existing request.authenticated_userid call sites working - 7 in Python (services/blog_record.py x3, views/blog.py x2, views/default.py x2) and 8 in templates (layout.jinja2 x4, blog.jinja2 x2, home.jinja2 x1, user_edit.jinja2 x1) - including layout.jinja2:46 (routes /user_edit/{name} by that value) and views/blog.py:64,75 (already write it into entries.creator / entries.editor).
A database allowlist as the mandatory access control, checked exactly once, in the callback. An email that does not match a pre-provisioned users row is rejected and no row is created. This is required, not defensive: Google's own production-readiness guidance states the consent-screen test-user allowlist is bypassed when an app requests only basic identity scopes.
The allowlist check must NOT become the group lookup. pyramid_tm defaults tm.annotate_user to True and then calls request.authenticated_userid inside manager.begin(). A database-backed groupfinder therefore runs a SELECT on every single request, inside the transaction, and any database error turns every route - including /login and the OAuth callback itself - into a 500. This was reproduced against the real app.
Admin rights become configured data, not a literal and not a database lookup. All 4 sites comparing to the literal 'admin' are replaced by 'group:administrators' in request.effective_principals: services/user.py:26, views/default.py:93, templates/layout.jinja2:43, templates/user_edit.jinja2:45. The mapping from email to admin comes from an auth.admin_emails list in settings, so it survives a session loss, takes effect immediately on edit, and costs no query. Templates cannot call a groupfinder, which is why the effective_principals check is the correct template-side expression.
One-shot binding of the two existing accounts, with no first-login claim window: admin (id 1) -> phuoc@caotek.fr (retains manage); phuoc (id 2) -> ctphuoc@gmail.com (authenticated, no manage). Two distinct people, so each row maps to exactly one identity. The Google sub is stored on first successful sign-in and matched first on later sign-ins, because sub is Google's stable identifier and email is mutable.
Removal of the password surface: the users.password column, the passlib dependency, the top-level import passlib in models/user.py:11 (removing the dependency without removing that import breaks the entire app at import time), User.verify_password / set_password, the password / confirm WTForms fields and their length check at views/default.py:116-117, the non-admin self-service password path, and the admin "Nouvel utilisateur" create-user flow at views/default.py:98-102,119-131.
Every template that renders a removed field is fixed in the same change. Jinja2's default Undefined raises on attribute access, so user_edit.jinja2:27-28,36-37 will raise UndefinedError the moment the password fields disappear.
last_logged gets an explicit writer and a NULL-safe renderer. User.verify_password (models/user.py:29) is currently the only code in the app that writes it, so deleting that method deletes the only writer - and templates/users.jinja2:27 calls entry.last_logged.strftime(...) with no NULL guard, which was verified to raise UndefinedError. The callback must set last_logged, and /users must survive a NULL.
The pre-existing delete(...) NameError at views/default.py:142 is fixed in this change. It sits inside user_edit, which this migration edits regardless (lines 93, 98-102, 116-117, 119-131, 127, 137). UserService.delete already exists at services/user.py:18. The trap: if the 'admin' literal in user_edit.jinja2:45 is swapped for the effective_principals check, the delete button still renders for admins and the NameError survives the migration.
Hand-written schema change against the committed ctp_blogr.sqlite, with an explicit warning that this repo has no alembic.ini and no migrations/ directory, so the schema cannot be regenerated. A SQLite table rebuild (not a plain ALTER TABLE) is required because users.password is nullable=False. entries.creator / entries.editor are Unicode(50) and are written straight from request.authenticated_userid with no length guard, so they are widened in the same rebuild.
A branch decision and a documented restore path for ctp_blogr.sqlite are prerequisites, not afterthoughts. There is a single commit on master with main and origin/* at the same commit, and this migration performs a destructive rebuild on a tracked file.
scope=openid email only. profile is not requested (no display name or avatar is used). hd is deliberately omitted: it means "a Workspace account on this domain", and ctphuoc@gmail.com is a personal Gmail account, so setting hd=caotek.fr would lock the second user out. The database allowlist is the control that actually works.
A brand-new WebTest harness. The existing BaseTest builds a testing.DummyRequest, not a WSGI app, so WebTest cannot drive it - and BaseTest.init_database() is defined but never called by either existing test, so no table is ever created. "WebTest on the existing fixture" is impossible; the plan therefore builds its own WSGI app on sqlite:///:memory: with the Google collaborator stubbed and pyramid_retry disabled, covering the happy path and every failure path.
All user-visible strings, comments and flash messages stay in French, matching the app.

Must NOT have (guardrails, anti-slop, scope boundaries)

No implicit sign-up. A Google identity that is not already in users is refused; it is never auto-created. This closes the self-registration hole that a naive OAuth migration opens.
No profile scope, no avatar, no display name, no refresh token, no access_type=offline, no hd parameter, and no call to any Google API. Sign-in only.
No Google-side logout or token revocation. Logout is local session invalidation only.
No database-backed groupfinder and no tm.annotate_user change. The login allowlist is a DB read performed once in the callback; the group mapping is configuration. Silencing annotate_user to dodge the tween problem would break commit/abort semantics and is forbidden.
No pyramid_csrf package and no retrofitting CSRF onto the existing write views. Pyramid 2.1's cookie session already provides new_csrf_token() / get_csrf_token() under _csrft_, so no dependency is needed - but the token is a single long-lived per-session value with no automatic comparison helper, and wiring it into every pre-existing POST view is an unrelated refactor. state is the control for the callback itself (a single-use GET bound to the pre-authentication session). The pre-existing absence of CSRF, and logout being a state-mutating GET, are recorded as accepted risks, not silently fixed and not silently ignored.
No server-side session backend. No pyramid_beaker. Measured, not assumed: identity plus state + code_verifier + nonce serializes to 324 bytes; with ten French flash messages, 662. Pyramid raises a hard ValueError at 4064 bytes, leaving ~3.7 KB of headroom. The only growth vector is unbounded flash accumulation, which is bounded by capping flash length, not by adding a backend.
No migration tooling. Do not introduce Alembic, do not create a migrations/ directory, and do not pretend Base.metadata.create_all() covers the production schema.
No use of coverage as an acceptance gate. .coveragerc still says source = pyramid_blogr (the old package name) so coverage collects nothing; the number would be fiction. Fix the name only if asked.
No unrelated refactors beyond the lines this migration must already touch. Explicitly out of scope: the hardcoded post id 95 in home and home.jinja2:18, the legacy Query.get() call in services/blog_record.py, the non-raw string at routes.py:5, the legacy declarative_base in models/meta.py, the missing initialize_db.py behind the declared initialize_ctp_blogr_db console script, and the dead HistoForm / AllocationForm / ActifForm / Actif2Form forms matching orphaned tables. The pre-existing draft-leak in by_id (no status filter) and the LIKE search leak are recorded, not fixed: they are unchanged by this migration, and widening the user population is what makes them worth a separate decision.
No invented linting, formatting, type-checking, CI or task runner. None exists and this migration does not create any.
No reduction to an MVP or phase 1. This is the complete migration, one plan.

Verification strategy

Zero human intervention for everything the agent can execute. The single exception is F3's real Google sign-in, which needs credentials only the user can issue - see the prerequisite below. Todos 1-14 all run fully offline against a stubbed Google collaborator.

User prerequisite, required before F3 only (not before Wave 1): provision a Google Cloud OAuth 2.0 client, add the Web application redirect URI http://localhost:9280/login/google/callback, and set ENV_GOOGLE_CLIENT_ID / ENV_GOOGLE_CLIENT_SECRET. Todos 1-14 do not need these; F3 cannot pass without them.

Test decision: tests-after, WebTest + pytest, on sqlite:///:memory: via a newly built WSGI test app (the existing BaseTest cannot be reused - see Scope). The Google collaborator and the JWKS fetch are injected behind the named seam from todo 7 so no test performs network I/O. pyramid_retry is disabled in tests, because single-shot side effects such as consuming state are otherwise non-deterministic.
Never assert a bare 200 in this app. Unauthorized requests return 200 with the login page, not 403, because a forbidden_view_config swallows HTTPForbidden. Every protected-route assertion checks status and body content.
Per-todo happy AND failure QA, each writing evidence to <attemptDir>/task-<N>-google-oauth-migration.<ext>.
Structural self-check before handoff: every implementation row is column-zero - [ ] N. <title> and every final-verifier row is column-zero - [ ] F<number>. <title>, both inside their intended section, with no prose bullet standing in for a task.

Acceptance criteria (transcribed from the gap analysis, grouped)

Environment / boot

python -c "from pyramid.paster import get_app; get_app('development.ini')" exits 0, and python -c "import authlib, joserfc; print(authlib.__version__)" succeeds.
python -m pytest -q reports the exact new count, 0 failed, and no no such table error (which proves the new harness actually creates the schema).

Session policy 3. The post-login cookie, read on the next request, contains auth.userid and no key matching state|code_verifier|nonce. 4. A session older than the configured timeout is rejected, asserted with a monkeypatched clock rather than a sleep. 5. The factory is constructed with secure=True under production settings and httponly=True in both; the configured timeout is asserted to exceed the previous 20-minute default and its value is written down.

Callback 6. Unknown email -> assert the actual status (403), that the body contains the French rejection message, and that it contains no <input type="password">. 7. state mismatch, missing state, and replayed state each fail closed, call the token endpoint zero times, and leave no session identity. 8. nonce mismatch is rejected; assert the JWT nonce claim is compared against the stored value (the stub returns a mismatching claim). 9. ?error=access_denied -> French flash, no DB write, no session identity. 10. Retry safety: with retry.attempts=3, a transient error during the callback does not produce a second exchange (assert collaborator call count == 1).

Access control 11. Authenticated as ctphuoc@gmail.com: GET /user_edit/phuoc@caotek.fr is refused, GET /user_edit/0 is refused, GET /users does not contain users_list. 12. Same user: POST /blog_edit/<id> with form.submitted does not modify the post, and with form.deleted the row is still present. 13. users.name is restricted to the two allowlisted emails, and groupfinder('ghost@nowhere.test', request) returns None.

Templates / removals 14. GET /users returns 200 with a row whose last_logged is NULL in the database (no strftime UndefinedError). 15. GET /user_edit/<email> returns 200 and the HTML contains no name="password". 16. grep -c "route_url('user_edit', name='0')" ctp_blogr/templates/users.jinja2 == 0 and GET /user_edit/0 is unreachable. 17. grep -rn passlib ctp_blogr/ returns nothing and the app still boots (the import-order trap). 18. grep -rn "'admin'" ctp_blogr/ returns no authorization comparison.

Schema 19. After the rebuild, PRAGMA table_info(users) shows no password column and name still NOT NULL + UNIQUE; SELECT COUNT(*) FROM users == 2 with the correct emails; PRAGMA foreign_key_check is clean; the alembic_version row is still present and untouched. 20. PRAGMA table_info(entries) shows creator/editor widened.

Regression 21. python -m pytest ctp_blogr/tests.py still passes, or the two legacy duplicate tests are deliberately deleted as part of the harness replacement. 22. An explicit smoke pass over all 15 routes asserting content, never a bare 200.

Execution strategy

Parallel execution waves

Waves are deliberately smaller than the 5-8 target. The gap analysis proved that the session factory, the auth policy and the group mapping are one coupled change (a 20-minute default timeout plus a database-backed callback inside the transaction tween), and that the destructive table rebuild must be isolated. Splitting those further would produce intermediate states that cannot boot.

Wave 1 - prerequisites and foundation (no interdependencies): dependency manifest and authlib install; branch decision plus database backup and restore path; session factory hardening; the settings surface for the client id/secret and auth.admin_emails.
Wave 2 - auth core (coupled, land together): the SessionAuthenticationPolicy swap with the settings-based groupfinder; the destructive schema rebuild on the agreed branch; the named Google collaborator with JWKS caching.
Wave 3 - the flow (depends on Wave 2): the authorize redirect view; the callback view with state consumption, nonce comparison and a single non-retryable exchange; the allowlist gate with the explicit 403, remember(), the last_logged write and OAuth-key cleanup.
Wave 4 - authorization gaps and surface removal (must land in the same change set as Wave 3): the /user_edit permission and ownership fix together with the delete(...) NameError; the four literal 'admin' sites; removal of the password surface and the template fixes it forces.
Wave 5 - tests (depends on all): the new WSGI harness and the full suite.
Wave 6 - final verification wave (F1-F4), all four in parallel, all must APPROVE.

Dependency matrix

Todo	Depends on	Blocks	Can parallelize with
1. Dependency manifest + `authlib` install	-	7, 13	2, 3, 4
2. Branch decision + DB backup/restore path	-	6	1, 3, 4
3. Session factory hardening (timeout, secret, httponly, secure)	-	5, 8, 9	1, 2, 4
4. `settings` surface: client id/secret + `auth.admin_emails`	-	5, 7, 8	1, 2, 3
5. `SessionAuthenticationPolicy` + settings-based groupfinder	3, 4	10, 11, 12	6, 7
6. Schema rebuild: drop `password`, add `google_sub`, widen creator/editor	2	9, 10, 14	5, 7
7. Google collaborator seam + JWKS cache	1, 4	8	5, 6
8. Authorize redirect + callback views	3, 7	9, 13	-
9. Allowlist gate + explicit 403 + `last_logged` + key cleanup	5, 6, 8	13	-
10. `/user_edit` authorization + `delete(...)` `NameError`	5, 6, 9	13	-
11. Replace the 4 literal `'admin'` sites	5, 9	13	10
12. Remove the password surface + template fixes	6, 10, 11	13	-
13. New WSGI test harness + full suite	1-12	F1-F4	-
14. `AGENTS.md` accuracy update	13	F1-F4	-

Todos

Implementation + Test = ONE todo. Never separate.

1. Declare and install authlib without corrupting the shared venv What to do / Must NOT do: Add authlib to install_requires in setup.py (which pulls in joserfc). Install it into the parent venv at /Users/phuoc/pyramid2. MUST NOT casually pip install into that venv - AGENTS.md records that it is shared and still carries a stale editable install of a wiki_test project whose source is gone. MUST NOT add any other new dependency, and MUST NOT introduce pyramid_csrf or pyramid_beaker. Parallelization: Wave 1 | Blocked by: - | Blocks: 7, 13 References (executor has NO interview context - be exhaustive): setup.py (install_requires, currently declares the dead initialize_ctp_blogr_db console script whose target ctp_blogr/scripts/initialize_db.py does not exist); AGENTS.md "Environment" section; venv root is the parent directory, python is already on PATH. Acceptance criteria (agent-executable): python -c "import authlib, joserfc; print(authlib.__version__)" exits 0 and prints 1.8.0; python -c "from pyramid.paster import get_app; get_app('development.ini')" exits 0; git diff setup.py shows exactly one added dependency line. QA scenarios (name the exact tool + invocation): happy - python -c "import authlib, joserfc" succeeds after install. failure - pip install authlib==0.0.0 is NOT run; instead assert grep -c "authlib" setup.py == 1 and that pyramid_csrf/pyramid_beaker are absent. Evidence: <attemptDir>/task-1-google-oauth-migration.<ext> Commit: N - propose feat(deps): add authlib for Google OAuth, commit only on explicit request.
2. Decide the branch and establish a restore path for the tracked database What to do / Must NOT do: Surface the branch decision to the user and get an explicit answer before any schema work. ctp_blogr.sqlite is tracked in git (the *.sqlite line in .gitignore does nothing for an already-tracked file), there is a single commit on master, and main and origin/* point at it. Produce a dated backup of the database file outside the repo and record the exact restore command. MUST NOT create a branch, commit, or push without the user's answer. MUST NOT modify the schema in this todo. Parallelization: Wave 1 | Blocked by: - | Blocks: 6 References (executor has NO interview context - be exhaustive): AGENTS.md "Database" and "Conventions" sections; development.ini and production.ini both point at the file via %(here)s; the file contains alembic_version rev b8f8216d72c7, ~99 posts and 8 tags. Acceptance criteria (agent-executable): a backup file exists outside the working tree and sqlite3 <backup> "PRAGMA integrity_check" returns ok; the restore command is written into the plan's commit strategy; git status --porcelain ctp_blogr.sqlite is still clean at the end of this todo. QA scenarios: happy - backup created and verified with PRAGMA integrity_check. failure - confirm git checkout -- ctp_blogr.sqlite would restore the pre-migration state, and record that the working tree is untouched so far. Evidence: <attemptDir>/task-2-google-oauth-migration.<ext> Commit: N - documentation only, no commit.
3. Harden and reconfigure the session factory What to do / Must NOT do: Replace the bare SignedCookieSessionFactory('hZug2zPt7hT2MZ') with an explicit construction: a secret of at least 64 characters for hashalg='sha512' (currently 16), an explicit timeout long enough to survive a Google consent screen plus sign-in plus 2FA (the default is 1200s / 20 min against the outgoing policy's 36000s / 10 h), httponly=True in both environments, and secure=True driven by settings so production gets it. MUST NOT add pyramid_beaker. MUST NOT call request.session.renew() - that method does not exist in Pyramid 2.1, and neither do SessionRenew / SessionInvalidate. MUST NOT leave the factory defaults in place, because the cookie becomes the identity. Parallelization: Wave 1 | Blocked by: - | Blocks: 5, 8, 9 References (executor has NO interview context - be exhaustive): ctp_blogr/__init__.py:13 (the factory) and :15 (the hardcoded auth-ticket secret being removed); the installed factory source confirms _timeout: 1200, _cookie_secure: False, _cookie_httponly: False, _cookie_samesite: 'Lax', _cookie_on_exception: True; production.ini:29 declares url_scheme = https while the cookie still ships no Secure flag; templates/layout.jinja2:14-15,106-107 load third-party CDN scripts, so httponly=False is a real XSS-to-identity-theft path once the cookie carries the identity. SameSite=Lax is already correct for a top-level GET callback - do not change it to None. Acceptance criteria (agent-executable): grep -n "SignedCookieSessionFactory" ctp_blogr/__init__.py shows named kwargs for timeout, httponly, secure and the secret; the secret is >= 64 chars (assert with a Python one-liner); python -c "from pyramid.paster import get_app; get_app('development.ini')" exits 0; a Set-Cookie from a test request carries HttpOnly, and carries Secure when production settings are loaded. QA scenarios: happy - boot check passes and the header assertions hold. failure - assert a monkeypatched clock past the configured timeout causes the identity to be rejected (criterion 4), with no sleep in the test. Evidence: <attemptDir>/task-3-google-oauth-migration.<ext> Commit: N - propose fix(auth): harden and configure the signed session cookie.
4. Add the settings surface for the Google client and the admin email list What to do / Must NOT do: Read google.client_id and google.client_secret from settings via PasteDeploy environment interpolation (%(ENV_GOOGLE_CLIENT_ID)s, %(ENV_GOOGLE_CLIENT_SECRET)s) in both development.ini and production.ini, and add auth.admin_emails as a parsed list (phuoc@caotek.fr). Fail fast with a clear French message if the credentials are missing. MUST NOT commit any real credential, and MUST NOT invent a settings key that nothing reads. Parallelization: Wave 1 | Blocked by: - | Blocks: 5, 7, 8 References (executor has NO interview context - be exhaustive): ctp_blogr/__init__.py main() (reads settings); development.ini and production.ini; ctp_blogr/security.py (the 3-line ACL whose group:administrators principal this list feeds). Acceptance criteria (agent-executable): grep -n "auth.admin_emails" ctp_blogr/*.ini shows both files; the groupfinder receives ['phuoc@caotek.fr'] from settings; starting the app with the env vars unset raises a clear configuration error naming the missing variable, and git grep -n "client_secret" shows only the interpolation, never a literal value. QA scenarios: happy - app boots with env vars set. failure - app refuses to boot with a readable message when they are absent. Evidence: <attemptDir>/task-4-google-oauth-migration.<ext> Commit: N - propose feat(auth): add Google OAuth and admin email settings.
5. Swap AuthTkt for SessionAuthenticationPolicy with a settings-based groupfinder What to do / Must NOT do: Replace AuthTktAuthenticationPolicy (and its hardcoded secret) with Pyramid's SessionAuthenticationPolicy + SessionAuthenticationHelper, keeping root_factory='ctp_blogr.security.RootFactory' and ACLAuthorizationPolicy so security.py is untouched. Rewrite the groupfinder callback so it maps email -> group purely from auth.admin_emails in settings. MUST NOT query the database from the callback: pyramid_tm defaults tm.annotate_user to True and calls request.authenticated_userid inside manager.begin(), so a DB-backed groupfinder runs a SELECT on every request inside the transaction, and any DB error 500s every route including /login and the callback itself. MUST NOT "fix" this by setting tm.annotate_user = false. Unknown email must return None, not raise. Parallelization: Wave 2 | Blocked by: 3, 4 | Blocks: 10, 11, 12 References (executor has NO interview context - be exhaustive): ctp_blogr/__init__.py:15 (the policy block and secret); ctp_blogr/services/user.py:26 (the current userid == 'admin' literal to remove); ctp_blogr/security.py; installed pyramid_tm/__init__.py annotate_user = asbool(settings.get('tm.annotate_user', True)) and the request.authenticated_userid access inside manager.begin(); models/__init__.py sets settings['tm.manager_hook'] = 'pyramid_tm.explicit_manager'. Acceptance criteria (agent-executable): groupfinder('phuoc@caotek.fr', request) returns ['group:administrators']; groupfinder('ctphuoc@gmail.com', request) returns []; groupfinder('ghost@nowhere.test', request) returns None; no SQL is emitted during a groupfinder call (assert with an engine event listener); grep -rn "'admin'" ctp_blogr/ returns nothing; the ACL file is byte-identical to its pre-change content. QA scenarios: happy - both known identities resolve to the expected principals and /users is reachable only for the admin. failure - groupfinder on an unknown email returns None and creates no row; a request with the database deliberately broken still serves /login (this is the B3 regression test). Evidence: <attemptDir>/task-5-google-oauth-migration.<ext> Commit: N - propose refactor(auth): swap AuthTkt for SessionAuthenticationPolicy.
6. Rebuild the users table and widen entries.creator / entries.editor What to do / Must NOT do: Against the backed-up ctp_blogr.sqlite, rebuild users to drop the password column (it is nullable=False, so a plain ALTER TABLE ... DROP COLUMN is not the safe route), add a nullable unique google_sub, keep id / name (name stores the allowlisted email and stays NOT NULL + UNIQUE) and last_logged. Set row 1 name to phuoc@caotek.fr and row 2 to ctphuoc@gmail.com. Widen entries.creator and entries.editor from Unicode(50) to Unicode(255) in the same rebuild. Back up first (todo 2). MUST NOT introduce Alembic or a migrations/ directory - none exists in this repo. MUST NOT touch the alembic_version row (rev b8f8216d72c7), even though no Alembic is wired up. MUST NOT drop or alter the entries rows themselves. Report clearly that migrations are missing. Parallelization: Wave 2 | Blocked by: 2 | Blocks: 9, 10, 14 References (executor has NO interview context - be exhaustive): ctp_blogr/models/user.py (columns, password nullable=False); ctp_blogr/models/blog_record.py (creator/editor Unicode(50)); ctp_blogr/models/__init__.py:8-9 (Tags is defined in blog_record but not imported here - import it explicitly if mapper config is touched); ctp_blogr/views/blog.py:64,75 (the unguarded writes); AGENTS.md "Database" section (the DB file is the schema source of truth; create_all() is test-only). Acceptance criteria (agent-executable): PRAGMA table_info(users) shows no password column, name still NOT NULL + UNIQUE, and a google_sub column; SELECT COUNT(*) FROM users == 2 with exactly phuoc@caotek.fr and ctphuoc@gmail.com; PRAGMA table_info(entries) shows creator/editor widened; PRAGMA foreign_key_check is clean; SELECT * FROM alembic_version still returns b8f8216d72c7; SELECT COUNT(*) FROM entries is unchanged from the pre-migration count; python -m pytest -q shows no no such table. QA scenarios: happy - all the assertions above pass against the real file. failure - run every assertion against the backup first to confirm the script is not silently a no-op, and confirm the restore path from todo 2 actually restores a working app. Evidence: <attemptDir>/task-6-google-oauth-migration.<ext> Commit: N - propose feat(db): migrate users to Google identity and widen entry authors. The database diff is a real tracked change and must be inspected before any commit.
7. Build the named Google OAuth collaborator with JWKS caching What to do / Must NOT do: Add a registry-registered service (e.g. ctp_blogr/services/google_oauth.py) wrapping OAuth2Session from authlib.integrations.requests_client, configured from the discovery document so authorize_url / access_token_url / jwks_uri are never hand-written. Expose exactly two operations: create_authorization_url(...) and fetch_claims(code, code_verifier, ...). The service MUST fetch and cache the discovery document and the JWKS itself with a bounded TTL, because the bare client does neither: CodeIDToken plus key decoding needs the keys pulled from jwks_uri manually, and refetching on every sign-in is a latency and rate-limit problem. Use joserfc (Authlib's own forward dependency) to decode and verify the id_token, not authlib.jose, which is soft-deprecated in 1.8.0 and warns. Set hd nowhere. MUST NOT use authorize_redirect / save_authorize_data / set_state_data / clear_state_data - those are framework integrations that do not exist for Pyramid. Parallelization: Wave 2 | Blocked by: 1, 4 | Blocks: 8 References (executor has NO interview context - be exhaustive): discovery URL https://accounts.google.com/.well-known/openid-configuration; Authlib entry point authlib.integrations.requests_client.OAuth2Session; create_authorization_url returns the tuple (uri, state) and does not return code_verifier, which the app generates with generate_token(48) and passes in itself; Google's OIDC reference (https://developers.google.com/identity/openid-connect/reference) requires nonce; development.ini / production.ini settings from todo 4. Acceptance criteria (agent-executable): the service is reachable from request.registry.google_oauth; create_authorization_url returns a state and an absolute accounts.google.com URI containing code_challenge_method=S256, a code_challenge, an openid scope and no hd parameter; two consecutive fetch_claims calls hit the JWKS endpoint once (assert with a counting stub); the cache honours its TTL under a monkeypatched clock. QA scenarios: happy - authorization URL is well-formed and JWKS is fetched once. failure - a JWKS fetch failure surfaces a French error and never reaches the token endpoint; a cached-key path still validates a token after the TTL expires. Evidence: <attemptDir>/task-7-google-oauth-migration.<ext> Commit: N - propose feat(auth): add Google OAuth client with state, PKCE and nonce.
8. Add the authorize redirect and the callback views What to do / Must NOT do: Add a /login/google route that generates state, nonce and code_verifier, stores all three in the session, and redirects. Add the /login/google/callback GET which: rejects ?error=access_denied with a French flash and no DB write; consumes (deletes) state before attempting the exchange so a replay cannot reach the token endpoint; exchanges the code exactly once with the stored code_verifier and redirect_uri; compares the id_token nonce claim against the stored value; deletes state / code_verifier / nonce; and then defers to todo 9 for the allowlist gate and remember(). MUST NOT call request.session.renew() (it does not exist). MUST NOT let pyramid_retry replay the exchange: pyramid_tm marks exceptions retryable and a SQLite lock would re-run the callback into invalid_grant. Verify with a call-count assertion under retry.attempts=3. Parallelization: Wave 3 | Blocked by: 3, 7 | Blocks: 9, 13 References (executor has NO interview context - be exhaustive): ctp_blogr/routes.py (add both routes; note :5 is a non-raw string emitting SyntaxWarning on 3.12+ - prefix with r if you edit that file, the new routes are purely additive); ctp_blogr/views/default.py:37-74 (the existing login / logout / forget pattern and its permission / renderer conventions); development.ini (retry.attempts = 3); installed pyramid_tm retryable-error tagging. Acceptance criteria (agent-executable): state mismatch, missing state, and replayed state each fail closed, call the collaborator zero times, and leave no session identity; nonce mismatch is rejected with a stubbed mismatching claim; ?error=access_denied produces a French flash, no DB write and no session identity; with retry.attempts=3 and an injected transient error the collaborator is called exactly once; after a successful callback no session key matches state|code_verifier|nonce. QA scenarios: happy - full redirect-URI round trip with a stubbed token endpoint yields verified claims. failure - the six rejection cases above, each asserting zero token-endpoint calls. Evidence: <attemptDir>/task-8-google-oauth-migration.<ext> Commit: N - propose feat(auth): add authorize and callback views.
9. Gate sign-in on the database allowlist, then remember the identity What to do / Must NOT do: In the callback, after claims are verified, match the identity against the users table: google_sub first, then the allowlisted email. A match updates last_logged (the only writer, since verify_password is being deleted) and stores google_sub on first sign-in, then calls remember(email). No match is refused and no row is ever created. The refusal MUST set request.response.status_int = 403 explicitly and render a French rejection template without raising HTTPForbidden, because forbidden_view_config on login (views/default.py:38) catches that exception and renders login.jinja2 with status 200 - a rejected user would otherwise be shown the dead password form. Replace that forbidden_view_config with a real 403 view rather than inheriting it. MUST NOT auto-provision. MUST NOT query the DB for the group mapping (todo 5). Parallelization: Wave 3 | Blocked by: 5, 6, 8 | Blocks: 13 References (executor has NO interview context - be exhaustive): ctp_blogr/views/default.py:38 (the forbidden_view_config that swallows HTTPForbidden), :53-58 (existing remember() / forget() call style), :68-74 (logout); ctp_blogr/models/user.py:29 (the last_logged write being removed with verify_password); templates/users.jinja2:27 (last_logged.strftime with no NULL guard); Google's production-readiness note that the test-user allowlist is bypassed for basic scopes, which is why this DB gate is mandatory rather than defence-in-depth. Acceptance criteria (agent-executable): unknown email -> status 403, body contains the French rejection message, body contains no <input type="password">, and SELECT COUNT(*) FROM users is unchanged; phuoc@caotek.fr yields auth.userid equal to that email plus the manage principal and a non-NULL last_logged; ctphuoc@gmail.com yields a session without manage; a second sign-in matches on google_sub. QA scenarios: happy - both known identities sign in with the right principals. failure - unknown email refused at 403 with no row created; a tampered sub/email pair refused. Evidence: <attemptDir>/task-9-google-oauth-migration.<ext> Commit: N - propose feat(auth): gate sign-in on a database allowlist.
10. Close the /user_edit authorization hole and fix the delete(...) NameError What to do / Must NOT do: views/default.py:88 declares permission='view', not manage, and the only identity check in the function is == 'admin' for the return URL. Any authenticated user can therefore open /user_edit/<any-email> and rewrite that row, or /user_edit/0 to create one. Change the permission to manage and remove the create-user branch (:98-102,119-131) so name='0' is unreachable; if an admin-only allowlist editor is kept, it must require manage and must not accept a password. In the same change, fix :142 delete(request, user.id) -> UserService.delete(request, user.id); that method already exists at services/user.py:18. Record that it is a filter().delete(synchronize_session=False) with no FK cascade from entries.creator / entries.editor, so deleting a user strands PII in those columns. MUST NOT leave the NameError live: user_edit.jinja2:45 gates the delete button on request.authenticated_userid == 'admin', and swapping that literal for the effective_principals check keeps the button rendering for admins. Parallelization: Wave 4 | Blocked by: 5, 6, 9 | Blocks: 13 References (executor has NO interview context - be exhaustive): ctp_blogr/views/default.py:88 (permission), :93 ('admin' literal), :98-102 and :119-131 (create-user branch), :116-117 (len(form.password.data) < 6), :127 and :137 (set_password), :142 (the NameError); ctp_blogr/services/user.py:18 (delete); ctp_blogr/templates/user_edit.jinja2:45 (delete-button gate); ctp_blogr/templates/users.jinja2 (the route_url('user_edit', name='0') create link). Acceptance criteria (agent-executable): as ctphuoc@gmail.com, GET /user_edit/phuoc@caotek.fr and GET /user_edit/0 are both refused and the response body does not contain the edit form; grep -c "route_url('user_edit', name='0')" ctp_blogr/templates/users.jinja2 == 0; as admin, the delete button renders and submitting it deletes the row without raising; the other user row is untouched. QA scenarios: happy - admin edits and deletes correctly. failure - non-admin is refused on both URLs (content assertion, never a bare 200, because forbidden responses render the login page at 200 in this app). Evidence: <attemptDir>/task-10-google-oauth-migration.<ext> Commit: N - propose fix(auth): require manage on user_edit and fix user deletion.
11. Replace the four literal 'admin' comparisons with effective_principals What to do / Must NOT do: Replace every authorization comparison against the literal 'admin' with 'group:administrators' in request.effective_principals: services/user.py:26, views/default.py:93, templates/layout.jinja2:43, templates/user_edit.jinja2:45. Templates cannot call a groupfinder, which is why effective_principals is the correct template-side expression. Audit all 15 request.authenticated_userid sites (7 Python, 8 templates) so none is left comparing an identity to a name. MUST NOT leave any == 'admin' behind. Parallelization: Wave 4 | Blocked by: 5, 9 | Blocks: 13 References (executor has NO interview context - be exhaustive): ctp_blogr/services/user.py:26; ctp_blogr/views/default.py:93; ctp_blogr/templates/layout.jinja2:43 and :46; ctp_blogr/templates/user_edit.jinja2:45; ctp_blogr/services/blog_record.py x3, ctp_blogr/views/blog.py:64,75, x2, ctp_blogr/views/default.py x2, templates/layout.jinja2 x4, templates/blog.jinja2 x2, templates/home.jinja2 x1, templates/user_edit.jinja2 x1. Acceptance criteria (agent-executable): grep -rn "'admin'" ctp_blogr/ returns no authorization comparison; as phuoc@caotek.fr the admin navigation item renders and /users is reachable; as ctphuoc@gmail.com it does not render and /users does not contain users_list. QA scenarios: happy - both identities see exactly the navigation they should. failure - the non-admin sees no admin-only control in any of the 15 audited sites. Evidence: <attemptDir>/task-11-google-oauth-migration.<ext> Commit: N - propose refactor(auth): replace literal admin checks with effective_principals.
12. Remove the password surface and fix every template it breaks What to do / Must NOT do: Remove the password / confirm WTForms fields and the len(form.password.data) < 6 check, User.verify_password / set_password, the top-level import passlib at models/user.py:11, and the passlib dependency. Fix every template that renders a removed field, because Jinja2's default Undefined raises on attribute access: user_edit.jinja2:27-28,36-37. Make users.jinja2:27 NULL-safe for last_logged. Remove the non-admin self-service password path and the admin "Nouvel utilisateur" button. MUST NOT remove import passlib last - leaving it breaks the entire app at import time, since the app will not even boot. MUST NOT touch the dead HistoForm / AllocationForm / ActifForm / Actif2Form forms or their orphaned histo / allocation / actifs tables. Parallelization: Wave 4 | Blocked by: 6, 10, 11 | Blocks: 13 References (executor has NO interview context - be exhaustive): ctp_blogr/forms.py (password / confirm fields; plus the dead forms to ignore); ctp_blogr/models/user.py:11 (import) and :29 (last_logged writer); ctp_blogr/views/default.py:116-117,127,137,98-102,119-131; ctp_blogr/templates/user_edit.jinja2:27-28,36-37; ctp_blogr/templates/users.jinja2:27; ctp_blogr/templates/login.jinja2 (replaced by a single "Se connecter avec Google" control); setup.py (the passlib requirement). Acceptance criteria (agent-executable): grep -rn passlib ctp_blogr/ returns nothing and python -c "from pyramid.paster import get_app; get_app('development.ini')" still exits 0 (the import-order trap); GET /users returns 200 for a row whose last_logged is NULL; GET /user_edit/<email> returns 200 with no name="password" in the HTML; grep -rn "form.password" ctp_blogr/ returns nothing. QA scenarios: happy - /users and /user_edit/<email> render clean with no password markup. failure - the NULL-last_logged row renders 200; the app boots with passlib uninstalled from the source tree. Evidence: <attemptDir>/task-12-google-oauth-migration.<ext> Commit: N - propose refactor(auth): remove password forms and password storage.
13. Build a new WSGI test harness and the full offline suite What to do / Must NOT do: Build a new WSGI test app - the existing BaseTest constructs a testing.DummyRequest, so WebTest cannot drive it, and BaseTest.init_database() is defined but never called, so no table is ever created. The harness must create a real router on sqlite:///:memory: with Base.metadata.create_all(), register the stubbed Google collaborator from todo 7, and disable pyramid_retry. Cover the happy path and every failure path in the Acceptance criteria above: unknown email, state mismatch / missing / replay, nonce mismatch, error=access_denied, retry-safety call count, the /user_edit and /users authorization refusals, POST /blog_edit not mutating for a non-admin, the NULL-last_logged render, and the two legacy duplicate tests. MUST NOT perform network I/O. MUST NOT assert a bare 200 as proof of access. Decide explicitly whether the two legacy duplicate tests are kept or deliberately deleted. Parallelization: Wave 5 | Blocked by: 1-12 | Blocks: F1-F4 References (executor has NO interview context - be exhaustive): ctp_blogr/tests.py (BaseTest, DummyRequest, the uncalled init_database, the two duplicate tests); pyramid.testing.DummyRequest for the lighter alternative; ctp_blogr/models/__init__.py; the "Testing" section of AGENTS.md (pytest.ini sets testpaths = ctp_blogr and python_files = test*.py, so a new file must be named test*.py inside the package). Acceptance criteria (agent-executable): python -m pytest -q exits 0 with 0 failed and an exact reported count, no no such table errors, and no outbound socket (assert by blocking socket.socket in a fixture); criteria 1-22 from Verification all pass. QA scenarios: happy - the complete sign-in flow passes end to end offline. failure - each of the ten failure criteria fails closed, most importantly that a non-admin cannot reach /user_edit/<other-email> and that a replayed state never reaches the token endpoint. Evidence: <attemptDir>/task-13-google-oauth-migration.<ext> Commit: N - propose test(auth): cover the Google OAuth flow without network access.
14. Update AGENTS.md so it still describes reality What to do / Must NOT do: Rewrite the "Architecture / wiring" section for the new policy and the settings-based admin list, and correct "Known-broken" for the items this migration resolves: the delete(...) NameError, the 200-not-403 forbidden behaviour and its forbidden_view_config cause, and the auth-ticket secret. Remove the now-false statement that admin is a hardcoded userid == 'admin'. MUST NOT invent linting, CI or a task runner, and MUST NOT claim any command was run that was not. Note that coverage is still broken and still must not be trusted. Parallelization: Wave 5 | Blocked by: 13 | Blocks: F1-F4 References (executor has NO interview context - be exhaustive): AGENTS.md "Architecture / wiring", "Known-broken", "Conventions" and "Testing" sections. Acceptance criteria (agent-executable): grep -n "AuthTkt" AGENTS.md returns nothing; grep -n "userid == 'admin'" AGENTS.md returns nothing; every command listed in AGENTS.md "Commands" still runs as written. QA scenarios: happy - the documented wiring matches the code. failure - a reviewer greps each AGENTS.md claim and confirms it against the source. Evidence: <attemptDir>/task-14-google-oauth-migration.<ext> Commit: N - propose docs: update AGENTS.md for the Google OAuth migration.

Final verification wave

Runs in parallel after ALL todos. ALL must APPROVE. Surface results and wait for the user's explicit okay before declaring complete.

F1. Plan compliance audit - confirm every todo's acceptance criteria and all 22 criteria in Verification were actually executed, with evidence files present.
F2. Code quality review - re-read the diff for the failure modes the gap analysis surfaced: retry replay, stale session values, the passlib import order, UndefinedError in templates, and any remaining literal identity comparison.
F3. Real manual QA - boot pserve development.ini, complete a real Google sign-in end to end in a browser, and smoke all 15 routes asserting content rather than bare 200. Requires the Google Cloud OAuth client and registered redirect URI from the Verification prerequisite; if the user has not provisioned them, report F3 as BLOCKED-ON-USER rather than APPROVE or REJECT.
F4. Scope fidelity - confirm nothing outside Scope changed: no pyramid_csrf, no pyramid_beaker, no Alembic, no hd, no offline access, no auto sign-up, no new tooling, and no change to the out-of-scope items (post id 95, Query.get(), routes.py:5, declarative_base, the dead forms, the draft-leak filters).

Commit strategy

The pre-existing history is two commits on master - 411ca8d "Inital commit since swicthing to python 3.14" followed by ea830da "opencode /init done"; main and origin/* point at the same tip and the branch choice is unsettled. Per todo 2, an explicit branch decision is a prerequisite and no branch is created, no push is made, and nothing is committed unless the user asks - AGENTS.md states "Commit only when asked".

Suggested messages, matching the repo's terse existing style:

feat(deps): add authlib for Google OAuth
fix(auth): harden and configure the signed session cookie
feat(auth): add Google OAuth and admin email settings
refactor(auth): swap AuthTkt for SessionAuthenticationPolicy
feat(db): migrate users to Google identity and widen entry authors
feat(auth): add Google OAuth client with state, PKCE and nonce
feat(auth): add authorize and callback views
feat(auth): gate sign-in on a database allowlist
fix(auth): require manage on user_edit and fix user deletion
refactor(auth): replace literal admin checks with effective_principals
refactor(auth): remove password forms and password storage
test(auth): cover the Google OAuth flow without network access
docs: update AGENTS.md for the Google OAuth migration

Restore path for ctp_blogr.sqlite (established in todo 2, required before todo 6):

Branch: feat/google-oauth, created from master at 3dcb88e. No commits, no pushes.
Verified backup (outside the working tree): /var/folders/cm/x5cx1bfd749gxd1x_zt6bvp40000gn/T/opencode/ctp_blogr-backup-20261005-202240.sqlite — PRAGMA integrity_check returns ok.
Restore the pre-migration database: cp /var/folders/cm/x5cx1bfd749gxd1x_zt6bvp40000gn/T/opencode/ctp_blogr-backup-20261005-202240.sqlite ctp_blogr.sqlite
Restore the committed database: git checkout -- ctp_blogr.sqlite
Abort the whole migration: git checkout master

The database diff is a real tracked change and must be inspected before any commit.

Success criteria

Signing in with phuoc@caotek.fr yields a session whose authenticated_userid is that email and which carries the group:administrators principal; the /users page is reachable.
Signing in with ctphuoc@gmail.com yields a session WITHOUT manage; /users does not expose the user list.
Signing in with any other Google account produces an explicit 403 with a French message and no password input on the page, creates no database row, and makes no group lookup.
The password column, passlib, the password form fields, and the local login form are gone from the codebase; no route, template or form field still references them, and the app still boots.
No source line compares an identity to the literal 'admin'; the admin mapping is the auth.admin_emails setting.
No session key matching state|code_verifier|nonce survives a completed sign-in.
A replayed or mismatched state, and a mismatched nonce, each reach the token endpoint zero times and leave no session identity.
python -m pytest passes with the new tests, no test performs network I/O, and no test asserts a bare 200 as proof of access.
pserve development.ini serves /, and /login presents a single "Se connecter avec Google" control instead of a credential form.
AGENTS.md is updated: its "Architecture / wiring" and "Known-broken" sections still describe reality after the migration (it currently documents the AuthTkt + groupfinder wiring, the 200-not-403 behaviour, and the delete(...) NameError that this plan fixes).

56 KiB Raw Blame History