Skip to content

Commit b4498df

Browse files
authored
The mint seam takes a grant, not two callables (#290)
mint_access_token(key=..., grant=...) now takes one object carrying the two grant verbs: load_refresh_token() and save_refresh_token(rotated). The engine's real dependency is a grant it can read fresh under the refresh lock and rotate durably; two loose callables atomized that noun and let a loader for one row pair silently with a saver for another. The lock and commit contracts now live on the grant parameter's documentation. McpOauthGrant carries the two verbs itself; the MCP mint wrapper passes the row. Redis keys, lock and cache semantics, and all observable MCP behavior are unchanged. The engine tests use small grant stubs, keep every behavioral assertion, and add one the seam now affords: an error raised by a grant's own verb surfaces unmasked.
1 parent d815557 commit b4498df

5 files changed

Lines changed: 133 additions & 125 deletions

File tree

‎backend/druks/mcp/models.py‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,18 +2,22 @@
22
from datetime import datetime
33
from typing import Any
44

5-
from sqlalchemy import Boolean, ForeignKey, String, UniqueConstraint, select
5+
from sqlalchemy import Boolean, ForeignKey, String, UniqueConstraint, select, update
66
from sqlalchemy.dialects.postgresql import JSONB
77
from sqlalchemy.dialects.postgresql import insert as pg_insert
88
from sqlalchemy.orm import Mapped, mapped_column
99

1010
from druks.accounts.constants import SYSTEM_ACCOUNT_ID
1111
from druks.core.models import Uuid7Pk
12-
from druks.database import db_session
12+
from druks.database import db_session, get_session
1313
from druks.extensions.registry import mcp_servers
1414
from druks.mcp.constants import NAME_PATTERN
1515
from druks.mcp.enums import IdentityMode, TokenSource
16-
from druks.mcp.exceptions import InvalidServerNameError, UnresolvedGrantAccountError
16+
from druks.mcp.exceptions import (
17+
InvalidServerNameError,
18+
MissingGrantError,
19+
UnresolvedGrantAccountError,
20+
)
1721
from druks.models import Base
1822
from druks.secrets.fields import EncryptedJsonField, EncryptedTextField, Secret
1923

@@ -258,6 +262,40 @@ def store(
258262
).returning(cls)
259263
return session.scalars(statement, execution_options={"populate_existing": True}).one()
260264

265+
def load_refresh_token(self) -> str:
266+
# Under the refresh lock: another process may have rotated and
267+
# committed, and this transaction may already hold the row —
268+
# populate_existing re-reads it past the identity map.
269+
fresh = (
270+
db_session()
271+
.scalars(
272+
select(McpOauthGrant)
273+
.where(McpOauthGrant.id == self.id)
274+
.execution_options(populate_existing=True)
275+
)
276+
.one_or_none()
277+
)
278+
if not fresh:
279+
raise MissingGrantError(self.server_name, self.account_id)
280+
# The grant's secret halves are ciphertext at rest; the plaintext
281+
# exists only in the refresh request body.
282+
return fresh.refresh_token.decrypt()
283+
284+
def save_refresh_token(self, rotated: str) -> None:
285+
# The provider invalidated the old token the moment it rotated, so
286+
# the write commits on its own session, never the enclosing
287+
# transaction — a step that rolls back later must not brick the grant.
288+
with get_session(db_session().get_bind()) as session:
289+
session.execute(
290+
update(McpOauthGrant)
291+
.where(McpOauthGrant.id == self.id)
292+
.values(refresh_token=rotated)
293+
)
294+
session.commit()
295+
# Keep the enclosing transaction's copy true as well.
296+
self.refresh_token = rotated
297+
db_session().flush()
298+
261299
def delete(self) -> None:
262300
session = db_session()
263301
session.delete(self)

‎backend/druks/mcp/oauth.py‎

Lines changed: 2 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
from sqlalchemy import select, update
55
from sqlalchemy.dialects.postgresql import insert as pg_insert
66

7-
from druks.database import db_session, get_session
7+
from druks.database import db_session
88
from druks.mcp.constants import OAUTH_CALLBACK_PATH, OAUTH_PROVIDER
99
from druks.mcp.enums import IdentityMode
1010
from druks.mcp.exceptions import GrantRefreshError, MissingGrantError, OauthConnectError
@@ -244,41 +244,6 @@ async def mint_access_token(name: str, account_id: str) -> str:
244244
grant = McpOauthGrant.get_for_account(name, account_id)
245245
if not grant:
246246
raise MissingGrantError(name, account_id)
247-
248-
def load_refresh_token() -> str:
249-
# Under the refresh lock: another process may have rotated and
250-
# committed, and this transaction may already hold the row —
251-
# populate_existing re-reads it past the identity map.
252-
fresh = (
253-
db_session()
254-
.scalars(
255-
select(McpOauthGrant)
256-
.where(McpOauthGrant.id == grant.id)
257-
.execution_options(populate_existing=True)
258-
)
259-
.one_or_none()
260-
)
261-
if not fresh:
262-
raise MissingGrantError(name, account_id)
263-
# The grant's secret halves are ciphertext at rest; the plaintext
264-
# exists only in the refresh request body.
265-
return fresh.refresh_token.decrypt()
266-
267-
def save_refresh_token(refresh_token: str) -> None:
268-
# The provider invalidated the old token the moment it rotated, so
269-
# the write commits on its own session, never the enclosing
270-
# transaction — a step that rolls back later must not brick the grant.
271-
with get_session(db_session().get_bind()) as session:
272-
session.execute(
273-
update(McpOauthGrant)
274-
.where(McpOauthGrant.id == grant.id)
275-
.values(refresh_token=refresh_token)
276-
)
277-
session.commit()
278-
# Keep the enclosing transaction's copy true as well.
279-
grant.refresh_token = refresh_token
280-
db_session().flush()
281-
282247
client = OauthClient(
283248
provider=OAUTH_PROVIDER,
284249
token_endpoint=grant.token_endpoint,
@@ -292,10 +257,6 @@ def save_refresh_token(refresh_token: str) -> None:
292257
http_factory=_http,
293258
)
294259
try:
295-
return await client.mint_access_token(
296-
key=f"{name}:{account_id}",
297-
load_refresh_token=load_refresh_token,
298-
save_refresh_token=save_refresh_token,
299-
)
260+
return await client.mint_access_token(key=f"{name}:{account_id}", grant=grant)
300261
except OauthRefreshError as error:
301262
raise GrantRefreshError(name, error.reason) from error

‎backend/druks/services/oauth.py‎

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ class OauthClient:
6161
``begin_connect`` returns the consent URL to open; ``complete_connect``
6262
consumes the callback's single-use state and exchanges the code;
6363
``mint_access_token`` serves delivery from the Redis token cache, electing
64-
one refresher per grant. Grants live on the caller's own rows, reached
65-
through the two callables mint takes. A ``Service`` with declared OAuth
64+
one refresher per grant. Grants live on the caller's own rows — mint takes
65+
the grant object that owns them. A ``Service`` with declared OAuth
6666
endpoints hands back a configured client via ``get_oauth_client()`` —
6767
construct directly only when no service holds the client credentials.
6868
@@ -213,13 +213,7 @@ async def complete_connect(self, *, state: str, code: str) -> tuple[dict, dict]:
213213
)
214214
return tokens, pending
215215

216-
async def mint_access_token(
217-
self,
218-
*,
219-
key: str,
220-
load_refresh_token: Callable[[], str],
221-
save_refresh_token: Callable[[str], None],
222-
) -> str:
216+
async def mint_access_token(self, *, key: str, grant) -> str:
223217
"""The delivery-side token for one grant: the cached access token
224218
while it lives, else one refreshed through the grant's refresh token.
225219
The provider may rotate the refresh token on use — two concurrent
@@ -228,16 +222,19 @@ async def mint_access_token(
228222
backstop a live refresh cannot outlive). Losers poll for the winner's
229223
cache fill, for about one token-endpoint round trip, then fail loudly.
230224
231-
``load_refresh_token()`` runs under the refresh lock and must observe
232-
rotations other processes committed — a naive re-select can return a
233-
row this transaction already identity-mapped, so read with
225+
``grant`` is the caller's own object — typically the row the grant
226+
lives on — carrying two verbs:
227+
228+
``grant.load_refresh_token()`` runs under the refresh lock and must
229+
observe rotations other processes committed — a naive re-select can
230+
return a row this transaction already identity-mapped, so read with
234231
``populate_existing`` or on a fresh session.
235232
236-
``save_refresh_token(token)`` receives a rotated refresh token and
237-
must have committed it before returning: the provider has already
238-
invalidated the old token, so the write cannot ride an enclosing
239-
transaction that may later roll back. The cache fills only after it
240-
returns."""
233+
``grant.save_refresh_token(rotated)`` receives a rotated refresh
234+
token and must have committed it before returning: the provider has
235+
already invalidated the old token, so the write cannot ride an
236+
enclosing transaction that may later roll back. The cache fills only
237+
after it returns."""
241238
redis = get_client()
242239
token_key = f"{self.provider}:access_token:{key}"
243240
lock_key = f"{self.provider}:refresh_lock:{key}"
@@ -255,7 +252,7 @@ async def mint_access_token(
255252
try:
256253
data = {
257254
"grant_type": "refresh_token",
258-
"refresh_token": load_refresh_token(),
255+
"refresh_token": grant.load_refresh_token(),
259256
**self.extra_token_params,
260257
}
261258
async with self._http() as http:
@@ -286,7 +283,7 @@ async def mint_access_token(
286283
self.provider, "the token endpoint returned no access token"
287284
)
288285
if tokens.get("refresh_token"):
289-
save_refresh_token(tokens["refresh_token"])
286+
grant.save_refresh_token(tokens["refresh_token"])
290287
try:
291288
ttl = int(tokens.get("expires_in", 3600)) - OAUTH_TOKEN_TTL_SKEW_SECONDS
292289
except (TypeError, ValueError) as error:

‎backend/tests/test_oauth_client.py‎

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -48,35 +48,47 @@ def _client(token_endpoint: FakeTokenEndpoint, **overrides) -> OauthClient:
4848
return OauthClient(**kwargs)
4949

5050

51-
def _fail_save(token: str) -> None:
52-
pytest.fail("nothing rotated")
51+
class GrantStub:
52+
"""A grant with the two mint verbs over an in-memory refresh token."""
53+
54+
def __init__(self) -> None:
55+
self.refresh_token = "rt-old"
56+
self.saved: list[str] = []
57+
58+
def load_refresh_token(self) -> str:
59+
return self.refresh_token
60+
61+
def save_refresh_token(self, rotated: str) -> None:
62+
self.saved.append(rotated)
63+
64+
65+
class UntouchedGrant(GrantStub):
66+
"""A grant the mint under test must never read or rotate."""
67+
68+
def load_refresh_token(self) -> str:
69+
pytest.fail("this mint reads no grant")
70+
71+
def save_refresh_token(self, rotated: str) -> None:
72+
pytest.fail("nothing rotated")
5373

5474

5575
async def test_mint_serves_the_cache_without_a_refresh(token_endpoint):
5676
await get_client().set(_TOKEN_KEY, "at-cached")
5777

58-
token = await _client(token_endpoint).mint_access_token(
59-
key="grant-1",
60-
load_refresh_token=lambda: pytest.fail("cache hit reads no grant"),
61-
save_refresh_token=_fail_save,
62-
)
78+
token = await _client(token_endpoint).mint_access_token(key="grant-1", grant=UntouchedGrant())
6379

6480
assert token == "at-cached"
6581
assert not token_endpoint.requests
6682

6783

6884
async def test_mint_refreshes_persists_rotation_and_fills_with_skewed_ttl(token_endpoint):
6985
token_endpoint.response = {"access_token": "at-2", "refresh_token": "rt-new", "expires_in": 300}
70-
saved: list[str] = []
86+
grant = GrantStub()
7187

72-
token = await _client(token_endpoint).mint_access_token(
73-
key="grant-1",
74-
load_refresh_token=lambda: "rt-old",
75-
save_refresh_token=saved.append,
76-
)
88+
token = await _client(token_endpoint).mint_access_token(key="grant-1", grant=grant)
7789

7890
assert token == "at-2"
79-
assert saved == ["rt-new"]
91+
assert grant.saved == ["rt-new"]
8092
refresh = token_endpoint.requests[0]
8193
assert refresh["grant_type"] == "refresh_token"
8294
assert refresh["refresh_token"] == "rt-old"
@@ -92,21 +104,30 @@ async def test_mint_refreshes_persists_rotation_and_fills_with_skewed_ttl(token_
92104
async def test_mint_fills_the_cache_only_after_the_rotation_is_saved(token_endpoint):
93105
token_endpoint.response = {"access_token": "at-2", "refresh_token": "rt-new", "expires_in": 300}
94106

95-
def save_refresh_token(token: str) -> None:
96-
raise RuntimeError("rotation write failed")
107+
class UnsavableGrant(GrantStub):
108+
def save_refresh_token(self, rotated: str) -> None:
109+
raise RuntimeError("rotation write failed")
97110

98111
with pytest.raises(RuntimeError, match="rotation write failed"):
99-
await _client(token_endpoint).mint_access_token(
100-
key="grant-1",
101-
load_refresh_token=lambda: "rt-old",
102-
save_refresh_token=save_refresh_token,
103-
)
112+
await _client(token_endpoint).mint_access_token(key="grant-1", grant=UnsavableGrant())
104113

105114
redis = get_client()
106115
assert not await redis.get(_TOKEN_KEY)
107116
assert not await redis.get(_LOCK_KEY)
108117

109118

119+
async def test_mint_surfaces_the_grants_own_load_error(token_endpoint):
120+
class GoneGrant(GrantStub):
121+
def load_refresh_token(self) -> str:
122+
raise LookupError("the grant row is gone")
123+
124+
with pytest.raises(LookupError, match="the grant row is gone"):
125+
await _client(token_endpoint).mint_access_token(key="grant-1", grant=GoneGrant())
126+
127+
assert not token_endpoint.requests
128+
assert not await get_client().get(_LOCK_KEY)
129+
130+
110131
async def test_mint_losing_the_lock_polls_for_the_winners_token(token_endpoint):
111132
redis = get_client()
112133
await redis.set(_LOCK_KEY, "1")
@@ -116,11 +137,7 @@ async def _winner_finishes():
116137
await redis.delete(_LOCK_KEY)
117138

118139
winner = asyncio.create_task(_winner_finishes())
119-
token = await _client(token_endpoint).mint_access_token(
120-
key="grant-1",
121-
load_refresh_token=lambda: "rt-old",
122-
save_refresh_token=_fail_save,
123-
)
140+
token = await _client(token_endpoint).mint_access_token(key="grant-1", grant=UntouchedGrant())
124141
await winner
125142

126143
assert token == "at-winner"
@@ -132,21 +149,18 @@ async def test_mint_times_out_loudly_when_the_lock_never_frees(token_endpoint):
132149

133150
with pytest.raises(OauthRefreshError, match="concurrent refresh"):
134151
await _client(token_endpoint, mint_wait_attempts=3).mint_access_token(
135-
key="grant-1",
136-
load_refresh_token=lambda: "rt-old",
137-
save_refresh_token=_fail_save,
152+
key="grant-1", grant=UntouchedGrant()
138153
)
139154

140155

141156
async def test_mint_refresh_rejection_evicts_and_raises(token_endpoint):
142157
token_endpoint.status = 400
143158

159+
grant = GrantStub()
144160
with pytest.raises(OauthRefreshError, match="HTTP 400"):
145-
await _client(token_endpoint).mint_access_token(
146-
key="grant-1",
147-
load_refresh_token=lambda: "rt-old",
148-
save_refresh_token=_fail_save,
149-
)
161+
await _client(token_endpoint).mint_access_token(key="grant-1", grant=grant)
162+
163+
assert not grant.saved
150164

151165
redis = get_client()
152166
assert not await redis.get(_TOKEN_KEY)
@@ -155,9 +169,7 @@ async def test_mint_refresh_rejection_evicts_and_raises(token_endpoint):
155169

156170
async def test_mint_refresh_uses_basic_auth(token_endpoint):
157171
await _client(token_endpoint, basic_auth=True).mint_access_token(
158-
key="grant-1",
159-
load_refresh_token=lambda: "rt-old",
160-
save_refresh_token=lambda token: None,
172+
key="grant-1", grant=GrantStub()
161173
)
162174

163175
assert token_endpoint.authorizations[0].startswith("Basic ")

0 commit comments

Comments
 (0)