From 8701f3dd59cde2678c7d36728ac8ece00e6c76ed Mon Sep 17 00:00:00 2001 From: Paulo Date: Sat, 22 Aug 2026 11:31:15 +0200 Subject: [PATCH 1/2] A declared identity key matches a fresh sign-in to its existing connection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Service may name the identity fact that identifies the provider account (identity_key = "sub"). Declared, a fresh sign-in whose identity matches an existing connection for the same owner lands on that row — a live row's tokens are replaced, a revoked row comes back to life under its old id — instead of creating a sibling. A live match outranks revoked history. Undeclared, every fresh sign-in stays a new connection. --- backend/druks/services/base.py | 6 + backend/druks/services/models.py | 21 ++++ backend/druks/services/routes.py | 18 ++- backend/tests/test_services.py | 201 ++++++++++++++++++++++++++++++- docs/writing-an-extension.md | 31 +++-- 5 files changed, 257 insertions(+), 20 deletions(-) diff --git a/backend/druks/services/base.py b/backend/druks/services/base.py index 3b61e903..55b597ef 100644 --- a/backend/druks/services/base.py +++ b/backend/druks/services/base.py @@ -118,6 +118,12 @@ class Service: # identity_scopes join the consent ask. identity_endpoint: ClassVar[str] = "" identity_scopes: ClassVar[tuple[str, ...]] = () + # The identity fact that names the provider account (Google's "sub", + # GitHub's "id"). Declared, a fresh sign-in whose identity matches an + # existing connection for the same owner lands on that row — live or + # revoked — instead of creating a sibling. Undeclared, every fresh + # sign-in is a new connection. + identity_key: ClassVar[str] = "" def __init_subclass__(cls, **kwargs: Any) -> None: super().__init_subclass__(**kwargs) diff --git a/backend/druks/services/models.py b/backend/druks/services/models.py index b9d3b5fd..76158249 100644 --- a/backend/druks/services/models.py +++ b/backend/druks/services/models.py @@ -115,6 +115,27 @@ def list_for_account(cls, provider: str, account_id: str) -> "list[OauthConnecti ) ) + @classmethod + def get_for_identity( + cls, provider: str, account_id: str, key: str, value: Any + ) -> "OauthConnection | None": + # A live grant outranks revoked history; among revoked, the latest + # consent. + return ( + db_session() + .scalars( + select(cls) + .where( + cls.provider == provider, + cls.account_id == account_id, + cls.identity[key].astext == str(value), + ) + .order_by(cls.revoked_at.is_(None).desc(), cls.connected_at.desc()) + .limit(1) + ) + .first() + ) + @classmethod def list_for_provider( cls, provider: str, *, include_revoked: bool = False diff --git a/backend/druks/services/routes.py b/backend/druks/services/routes.py index 26320f89..0681534f 100644 --- a/backend/druks/services/routes.py +++ b/backend/druks/services/routes.py @@ -126,15 +126,23 @@ async def oauth_callback(state: str = "", code: str = "", error: str = "") -> Re raise HTTPException(status_code=400, detail=f"No OAuth service {provider!r}.") granted = tokens.get("scope", "").split() or pending["scopes"] identity = await service.get_identity(tokens["access_token"]) - reconsent = bool(pending["connection_id"]) - if reconsent: - row = OauthConnection.get(pending["connection_id"]) + connection_id = pending["connection_id"] + # Two doors land on an existing row: reconsent names it by id, and a + # declared identity key matches a fresh sign-in to it. Either returns a + # revoked row to life. + row = None + if connection_id: + row = OauthConnection.get(connection_id) if not row: raise HTTPException( status_code=400, detail="The connection was removed while consent was open." ) - # Reconsent names the row, so it also returns a revoked one to life. - # A fresh sign-in never does — it always creates a new connection. + elif service.identity_key and (value := identity.get(service.identity_key)): + row = OauthConnection.get_for_identity( + provider, pending["account_id"], service.identity_key, value + ) + reconsent = bool(row) + if row: row.reconnect(refresh_token=tokens["refresh_token"], scopes=granted, identity=identity) # A token cached before this consent must not serve the new one. await OauthClient(provider=provider).evict_access_token(row.id) diff --git a/backend/tests/test_services.py b/backend/tests/test_services.py index 98a87acd..e2777d42 100644 --- a/backend/tests/test_services.py +++ b/backend/tests/test_services.py @@ -3,14 +3,17 @@ import html import json from types import SimpleNamespace +from urllib.parse import parse_qsl, urlparse import httpx import pytest +from druks.accounts.models import Account from druks.core.apis.github import GitHubClient, get_github_client from druks.core.webhooks.github import GitHubEvents from druks.database import db_session from druks.services.exceptions import ServiceNotConnectedError -from druks.services.models import ServiceIdentity +from druks.services.models import OauthConnection, ServiceIdentity +from druks.services.oauth import OauthClient from druks.testing import make_settings from fastapi import HTTPException from fastapi.testclient import TestClient @@ -575,6 +578,7 @@ class Acme(Service): token_endpoint = "https://acme.test/token" identity_endpoint = "https://acme.test/whoami" identity_scopes = ("openid",) + identity_key = "sub" class Settings(BaseModel): client_id: str @@ -604,6 +608,36 @@ def handler(request: httpx.Request) -> httpx.Response: return Acme +@pytest.fixture +def keyed_acme(acme, monkeypatch): + async def get_identity(service, access_token): + assert service is acme + assert access_token == "at-1" + return {"sub": "account-1", "email": "op@acme.test"} + + monkeypatch.setattr(acme, "get_identity", classmethod(get_identity)) + return acme + + +@pytest.fixture +def oauth_events(monkeypatch): + events = [] + + async def record(name, **kwargs): + events.append((name, kwargs)) + + monkeypatch.setattr("druks.services.routes.publish", record) + return events + + +def _complete_oauth_sign_in(client: TestClient, provider: str = "acme") -> None: + consent = client.get(f"/api/oauth/{provider}/connect", follow_redirects=False) + state = dict(parse_qsl(urlparse(consent.headers["location"]).query))["state"] + response = client.get("/api/oauth/callback", params={"state": state, "code": "c-1"}) + + assert response.status_code == 200 + + def test_oauth_connect_redirects_to_consent_with_the_scope_union( tmp_path, acme, druks_db, monkeypatch ): @@ -723,6 +757,165 @@ def test_oauth_connect_rejects_an_unknown_reconnect_target(tmp_path, acme, druks assert client.get("/api/oauth/acme/connect?connection=zzz").status_code == 404 +def test_fresh_sign_in_with_matching_identity_resurrects_revoked_connection( + tmp_path, keyed_acme, druks_db, oauth_events +): + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + keyed_acme.name, identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + account = Account.get_or_create("op@example.com") + connection = OauthConnection.create( + provider=keyed_acme.name, + account_id=account.id, + refresh_token="rt-old", + scopes=["profile.read"], + identity={"sub": "account-1"}, + ) + connection_id = connection.id + connection.revoke("user") + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + + with TestClient(configure_app_for_test(settings=settings)) as client: + _complete_oauth_sign_in(client, keyed_acme.name) + + db_session().expire_all() + resurrected = OauthConnection.get(connection_id) + assert resurrected + assert not resurrected.revoked_at + assert not resurrected.revoked_reason + assert resurrected.refresh_token.decrypt() == "rt-1" + assert [row.id for row in OauthConnection.list_for_provider(keyed_acme.name)] == [connection_id] + assert len(OauthConnection.list_for_provider(keyed_acme.name, include_revoked=True)) == 1 + assert oauth_events == [ + ( + "oauth.connected", + { + "provider": keyed_acme.name, + "connection_id": connection_id, + "account_id": account.id, + "reconsent": True, + }, + ) + ] + + +def test_matching_fresh_sign_in_lands_on_live_connection_and_evicts_cached_token( + tmp_path, keyed_acme, druks_db, oauth_events, monkeypatch +): + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + keyed_acme.name, identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + account = Account.get_or_create("op@example.com") + connection = OauthConnection.create( + provider=keyed_acme.name, + account_id=account.id, + refresh_token="rt-old", + scopes=["profile.read"], + identity={"sub": "account-1"}, + ) + connection_id = connection.id + evicted_connection_ids = [] + + async def record_eviction(oauth_client, evicted_connection_id): + assert oauth_client.provider == keyed_acme.name + evicted_connection_ids.append(evicted_connection_id) + + monkeypatch.setattr(OauthClient, "evict_access_token", record_eviction) + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + + with TestClient(configure_app_for_test(settings=settings)) as client: + _complete_oauth_sign_in(client, keyed_acme.name) + + db_session().expire_all() + reconnected = OauthConnection.get(connection_id) + assert reconnected + assert reconnected.refresh_token.decrypt() == "rt-1" + assert len(OauthConnection.list_for_provider(keyed_acme.name, include_revoked=True)) == 1 + assert evicted_connection_ids == [connection_id] + assert oauth_events[-1][1]["connection_id"] == connection_id + assert oauth_events[-1][1]["reconsent"] is True + + +def test_fresh_sign_in_with_live_and_revoked_identity_matches_lands_on_live_connection( + tmp_path, keyed_acme, druks_db, oauth_events +): + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + keyed_acme.name, identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + account = Account.get_or_create("op@example.com") + live = OauthConnection.create( + provider=keyed_acme.name, + account_id=account.id, + refresh_token="rt-live-old", + scopes=["profile.read"], + identity={"sub": "account-1"}, + ) + live_id = live.id + revoked = OauthConnection.create( + provider=keyed_acme.name, + account_id=account.id, + refresh_token="rt-revoked-old", + scopes=["profile.read"], + identity={"sub": "account-1"}, + ) + revoked_id = revoked.id + revoked.revoke("user") + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + + with TestClient(configure_app_for_test(settings=settings)) as client: + _complete_oauth_sign_in(client, keyed_acme.name) + + db_session().expire_all() + reconnected = OauthConnection.get(live_id) + still_revoked = OauthConnection.get(revoked_id) + assert reconnected + assert still_revoked + assert reconnected.refresh_token.decrypt() == "rt-1" + assert still_revoked.revoked_at + assert not still_revoked.refresh_token + assert len(OauthConnection.list_for_provider(keyed_acme.name, include_revoked=True)) == 2 + assert oauth_events[-1][1]["connection_id"] == live_id + assert oauth_events[-1][1]["reconsent"] is True + + +def test_fresh_sign_in_without_the_declared_identity_fact_creates_a_new_connection( + tmp_path, acme, druks_db, oauth_events +): + from druks.testing import configure_app_for_test + + ServiceIdentity.connect( + acme.name, identity={"client_id": "id-1"}, secrets={"client_secret": "sec-1"} + ) + account = Account.get_or_create("op@example.com") + revoked = OauthConnection.create( + provider=acme.name, + account_id=account.id, + refresh_token="rt-old", + scopes=["profile.read"], + identity={"sub": "account-1"}, + ) + revoked_id = revoked.id + revoked.revoke("user") + settings = make_settings(tmp_path, urls={"endpoint": "https://druks.example"}) + + with TestClient(configure_app_for_test(settings=settings)) as client: + _complete_oauth_sign_in(client, acme.name) + + db_session().expire_all() + [created] = OauthConnection.list_for_provider(acme.name) + assert created.id != revoked_id + assert len(OauthConnection.list_for_provider(acme.name, include_revoked=True)) == 2 + assert OauthConnection.get(revoked_id).revoked_at + assert oauth_events[-1][1]["connection_id"] == created.id + assert oauth_events[-1][1]["reconsent"] is False + + def test_fresh_sign_in_after_revoke_creates_a_new_connection(tmp_path, acme, druks_db, monkeypatch): from urllib.parse import parse_qsl, urlparse @@ -755,8 +948,8 @@ def sign_in(): [first] = OauthConnection.list_for_provider("acme") assert client.delete(f"/api/oauth/connections/{first.id}").status_code == 204 - # A fresh sign-in never reuses a row, even for the same provider - # account. The revoked row stays behind as history. + # The provider supplied no declared key fact, so it cannot identify + # the existing account. The revoked row stays behind as history. sign_in() [live] = OauthConnection.list_for_provider("acme") assert live.id != first.id @@ -795,7 +988,7 @@ async def record(name, **kwargs): [connection] = OauthConnection.list_for_provider("acme") assert client.delete(f"/api/oauth/connections/{connection.id}").status_code == 204 - # Reconsent names the row, so it is the one way back to life. + # Reconsent names the row, so it returns this revoked consent to life. reconnect = client.get( f"/api/oauth/acme/connect?connection={connection.id}", follow_redirects=False ) diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 13ac6e44..0a027d59 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -781,6 +781,12 @@ account's facts (email, username, name). Druks calls it once at consent and shows the facts as the connection's label in Settings. `identity_scopes` are the scopes that call needs; they join the consent ask. +Declare `identity_key` when one identity fact names the provider account: +`"sub"` for Google, or `"id"` for GitHub. A fresh sign-in that matches an +existing connection for the same owner lands on that row instead of +creating a sibling — a revoked row comes back to life under its old id. +Without this declaration, every fresh sign-in creates a new connection. + Some providers have no such endpoint, or return the facts in a different shape. Override `get_identity` for them: @@ -805,6 +811,7 @@ class GoogleOauth(Service): extra_authorize_params = {"access_type": "offline", "prompt": "consent"} identity_endpoint = "https://openidconnect.googleapis.com/v1/userinfo" identity_scopes = ("openid", "email") + identity_key = "sub" class Settings(BaseModel): client_id: str = Field(title="Client ID") @@ -851,11 +858,12 @@ identity. Your rows never need tombstone copies of either. Your UI starts a sign-in by opening `/api/oauth/acme/connect` — the platform runs the consent with the union of every installed extension's declared scopes and stores the connection for the signed-in user. A fresh -sign-in always creates a new connection, even for a provider account that -was connected before. To widen an existing connection's scopes, open -`/api/oauth/acme/connect?connection=`; reconsent replaces its tokens. -Reconsent names the row, so it also returns a revoked connection to life -under its old id — the only way a row comes back. +sign-in creates a new connection unless the service's `identity_key` +matches an existing connection for the same owner. To widen an existing +connection's scopes, open `/api/oauth/acme/connect?connection=`; +reconsent replaces its tokens. Reconsent names the row, so it returns a +revoked connection to life under its old id; a fresh sign-in that matches +the service's `identity_key` does the same. Add `?next=/app/night_watch/accounts` to land the user back on your page after consent instead of the generic "connected" page. `next` must be a bare path starting with `/` — a URL with a scheme or host is @@ -864,12 +872,13 @@ rejected, so the door can never redirect off the box. Register serves every service. React to sign-ins with the signal machinery. The platform publishes -`oauth.connected` when a consent completes. `reconsent` is true when it -replaced an existing connection's tokens, including a revoked row that -returned to life. It publishes `oauth.disconnected` when a connection is -revoked — by the user, or because the service's client credentials were -replaced. Revocation is a state, not a deletion: your subscriber can still -read the connection it is told about. Subscribe in `subscribers.py`: +`oauth.connected` when a consent completes. `reconsent` is true when the +consent replaced an existing connection's tokens. This includes explicit +reconsent by id and a keyed fresh sign-in, whether the row was live or +revoked. It publishes `oauth.disconnected` when a connection is revoked — +by the user, or because the service's client credentials were replaced. +Revocation is a state, not a deletion: your subscriber can still read the +connection it is told about. Subscribe in `subscribers.py`: ```python from druks.signals import subscribe From 19a646151b2d5d6a3cd8c46d011a0503384e8a44 Mon Sep 17 00:00:00 2001 From: Paulo Date: Sat, 22 Aug 2026 11:43:31 +0200 Subject: [PATCH 2/2] Comments and docs use Simplified Technical English --- backend/druks/services/base.py | 9 ++++----- backend/druks/services/models.py | 3 +-- backend/druks/services/routes.py | 5 ++--- backend/tests/test_services.py | 6 +++--- docs/writing-an-extension.md | 28 +++++++++++++++------------- 5 files changed, 25 insertions(+), 26 deletions(-) diff --git a/backend/druks/services/base.py b/backend/druks/services/base.py index 55b597ef..dcd49031 100644 --- a/backend/druks/services/base.py +++ b/backend/druks/services/base.py @@ -118,11 +118,10 @@ class Service: # identity_scopes join the consent ask. identity_endpoint: ClassVar[str] = "" identity_scopes: ClassVar[tuple[str, ...]] = () - # The identity fact that names the provider account (Google's "sub", - # GitHub's "id"). Declared, a fresh sign-in whose identity matches an - # existing connection for the same owner lands on that row — live or - # revoked — instead of creating a sibling. Undeclared, every fresh - # sign-in is a new connection. + # The identity fact that names the provider account — "sub" for Google, + # "id" for GitHub. When set, a fresh sign-in that matches an existing + # connection for the same owner updates that row; a revoked row becomes + # live again. When empty, each fresh sign-in creates a new connection. identity_key: ClassVar[str] = "" def __init_subclass__(cls, **kwargs: Any) -> None: diff --git a/backend/druks/services/models.py b/backend/druks/services/models.py index 76158249..2569e3e6 100644 --- a/backend/druks/services/models.py +++ b/backend/druks/services/models.py @@ -119,8 +119,7 @@ def list_for_account(cls, provider: str, account_id: str) -> "list[OauthConnecti def get_for_identity( cls, provider: str, account_id: str, key: str, value: Any ) -> "OauthConnection | None": - # A live grant outranks revoked history; among revoked, the latest - # consent. + # A live match wins; among revoked matches, the latest consent wins. return ( db_session() .scalars( diff --git a/backend/druks/services/routes.py b/backend/druks/services/routes.py index 0681534f..3c0405bc 100644 --- a/backend/druks/services/routes.py +++ b/backend/druks/services/routes.py @@ -127,9 +127,8 @@ async def oauth_callback(state: str = "", code: str = "", error: str = "") -> Re granted = tokens.get("scope", "").split() or pending["scopes"] identity = await service.get_identity(tokens["access_token"]) connection_id = pending["connection_id"] - # Two doors land on an existing row: reconsent names it by id, and a - # declared identity key matches a fresh sign-in to it. Either returns a - # revoked row to life. + # Reconsent names an existing row by id; a declared identity key + # matches a fresh sign-in to one. Both make a revoked row live again. row = None if connection_id: row = OauthConnection.get(connection_id) diff --git a/backend/tests/test_services.py b/backend/tests/test_services.py index e2777d42..a45bd06a 100644 --- a/backend/tests/test_services.py +++ b/backend/tests/test_services.py @@ -948,8 +948,8 @@ def sign_in(): [first] = OauthConnection.list_for_provider("acme") assert client.delete(f"/api/oauth/connections/{first.id}").status_code == 204 - # The provider supplied no declared key fact, so it cannot identify - # the existing account. The revoked row stays behind as history. + # The identity facts do not include "sub", so the sign-in cannot + # match the existing row. The revoked row stays as history. sign_in() [live] = OauthConnection.list_for_provider("acme") assert live.id != first.id @@ -988,7 +988,7 @@ async def record(name, **kwargs): [connection] = OauthConnection.list_for_provider("acme") assert client.delete(f"/api/oauth/connections/{connection.id}").status_code == 204 - # Reconsent names the row, so it returns this revoked consent to life. + # Reconsent names the row and makes the revoked consent live again. reconnect = client.get( f"/api/oauth/acme/connect?connection={connection.id}", follow_redirects=False ) diff --git a/docs/writing-an-extension.md b/docs/writing-an-extension.md index 0a027d59..058be4c3 100644 --- a/docs/writing-an-extension.md +++ b/docs/writing-an-extension.md @@ -782,10 +782,11 @@ shows the facts as the connection's label in Settings. `identity_scopes` are the scopes that call needs; they join the consent ask. Declare `identity_key` when one identity fact names the provider account: -`"sub"` for Google, or `"id"` for GitHub. A fresh sign-in that matches an -existing connection for the same owner lands on that row instead of -creating a sibling — a revoked row comes back to life under its old id. -Without this declaration, every fresh sign-in creates a new connection. +`"sub"` for Google, `"id"` for GitHub. A fresh sign-in that matches an +existing connection for the same owner updates that row instead of +creating a second one. A revoked row that matches becomes live again and +keeps its id. Without the declaration, each fresh sign-in creates a new +connection. Some providers have no such endpoint, or return the facts in a different shape. Override `get_identity` for them: @@ -858,12 +859,13 @@ identity. Your rows never need tombstone copies of either. Your UI starts a sign-in by opening `/api/oauth/acme/connect` — the platform runs the consent with the union of every installed extension's declared scopes and stores the connection for the signed-in user. A fresh -sign-in creates a new connection unless the service's `identity_key` -matches an existing connection for the same owner. To widen an existing -connection's scopes, open `/api/oauth/acme/connect?connection=`; -reconsent replaces its tokens. Reconsent names the row, so it returns a -revoked connection to life under its old id; a fresh sign-in that matches -the service's `identity_key` does the same. +sign-in creates a new connection, unless the service's `identity_key` +matches it to an existing connection for the same owner. To widen an +existing connection's scopes, open +`/api/oauth/acme/connect?connection=`; reconsent replaces its tokens. +Reconsent names the row, so it also makes a revoked connection live again +under its old id. A fresh sign-in that matches the `identity_key` does the +same. Add `?next=/app/night_watch/accounts` to land the user back on your page after consent instead of the generic "connected" page. `next` must be a bare path starting with `/` — a URL with a scheme or host is @@ -873,9 +875,9 @@ serves every service. React to sign-ins with the signal machinery. The platform publishes `oauth.connected` when a consent completes. `reconsent` is true when the -consent replaced an existing connection's tokens. This includes explicit -reconsent by id and a keyed fresh sign-in, whether the row was live or -revoked. It publishes `oauth.disconnected` when a connection is revoked — +consent replaced an existing connection's tokens. This happens on +reconsent by id and on an `identity_key` match, for a live or a revoked +row. It publishes `oauth.disconnected` when a connection is revoked — by the user, or because the service's client credentials were replaced. Revocation is a state, not a deletion: your subscriber can still read the connection it is told about. Subscribe in `subscribers.py`: