feat(identity): calendar connections can carry an OAuth refresh token, encrypted at rest - #1653
Open
DmitriyG228 wants to merge 1 commit into
Open
DmitriyG228 wants to merge 1 commit into
DmitriyG228 wants to merge 1 commit into
Conversation
…, encrypted at rest A connection now names its provider. 'ics' is the original — a pasted secret feed URL. 'google' and 'microsoft' carry a refresh token instead, encrypted through field_crypto and never written in the clear: no KEK configured is a 503 refusal to create the connection, never a plaintext fallback. A feed URL in the clear exposes one calendar's contents; a refresh token in the clear is ongoing, silent read access until somebody revokes it. masked_connection is now an ALLOWLIST rather than a copy-and-delete. A denylist leaks by default — the next field anyone adds is published until somebody remembers to remove it — and a test proves an unknown credential added to a connection cannot reach the user-facing shape. internal_connections takes the CIPHER, not an already-opened box, and that is the whole point of the signature. Opening the box unwraps the user's data key, which is itself a decrypt and so is itself a place that throws; taking a box would have moved that failure to the config sweep, where ONE user's rotated KEK blanks the whole list and stops every other user's calendar. Caught here, that user's connection surfaces as unreadable and everyone else syncs. Found by the test that asserts it, which failed on the first shape. Adds SecretCipher.open_user: the read path, which never mints a DEK and returns None instead of raising. for_user stays the write path. Contract change: a LIVE /internal/calendar-configs entry now carries a `provider` key, so the sweep never infers 'no provider means a feed URL'. test_calendar_config.py's exact-shape assertion is updated to match; tombstones are unchanged. [41 passed of the container-free admin-api tests; 14 new]
Member
Author
|
|
🃏 Merge card — #1653
Not mergeable yet — every row above must be accepted before merge (choke point 1). Fill in what's ❌ above, then this clears automatically. How a PR reaches merge: the merge bar. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1644 (the secret envelope) — base that
first, or the diff here reads as two changes. Together with
#1636 this is the storage half of calendar OAuth; the
browser-facing authorize/callback endpoint is still unwritten.
COMMITMENTS IN THIS PR
None. No published terms, no dates, no rights, no spend. One internal-contract change is
described below.
What changes
A calendar connection now names its
provider:calendar_ics_urlkeysics(default, unchanged)google/microsoftOAuth connections are deliberately not mirrored:
calendar_ics_urlmeans a feed URL to everyreader that predates this, and handing them a token they cannot fetch would be worse than absence.
Three decisions worth review
No KEK means no OAuth calendar — a 503, never a plaintext fallback. This is why #876 came first.
A feed URL in the clear exposes one calendar's contents; a refresh token in the clear is ongoing,
silent read access until somebody revokes it. The two do not deserve the same treatment, so the
create path refuses rather than degrades.
masked_connectionis now an allowlist, not copy-and-delete. A denylist leaks by default: thenext field anyone adds is published until somebody remembers to remove it. There is a test that adds
an unknown credential to a connection and asserts it cannot reach the user-facing shape — that
regression matters more than any single field.
internal_connectionstakes the cipher, not an already-opened box — and this is the one I gotwrong first. Opening the box unwraps the user's data key, which is itself a decrypt and so is itself
a place that throws. Taking a box would have moved that failure up into the config sweep, where
one user's rotated KEK blanks the whole list and stops every user's calendar. The test that
asserts otherwise failed on the first shape and is what found it. Caught inside the function, that
one connection surfaces as
unreadableand everyone else syncs.Also adds
SecretCipher.open_user: the read path — never mints a DEK, returnsNoneinstead ofraising.
for_userremains the write path.Contract change
A live
/internal/calendar-configsentry now carriesprovider, so the consuming sweep neverhas to infer "no provider means a feed URL".
test_calendar_config.py's exact-shape assertion isupdated to match. Tombstones are unchanged — a retired connection retires the same way whatever it
was read from. The extra key is additive for existing consumers.
Tests
14 new in
tests/test_calendar_oauth_connections.py, weighted toward the leak questions: can thetoken be stored in the clear (no), reach a user-facing response (no), or take down everybody else's
sync when unreadable (no).
Not run:
test_calendar_config.py. Its conftest starts Postgres and Redis throughtestcontainers, and container workloads do not run on the founder's laptop. I read its assertions
statically instead, which is how the exact-shape break above was found before CI rather than after.
CI is the check on that file.
Contribution rights
under Apache-2.0, and it is not owned or controlled by an employer, client, or other entity.
may control this contribution. I am requesting Vexa's private corporate-authorization process.
Every commit must also carry the contributor's own DCO
Signed-off-byline.