Feat/verisafe token broker - #58
Merged
Merged
Conversation
- Store OAuth access/refresh tokens and external user IDs - Track token expiration and validate with is_expired() method - Store granted scopes with has_scope() helper for permission checks - Enforce unique provider per user with unique_together constraint - Add indexes on (user, provider) and expires_at for query performance
The testcontainers dependency is no longer in requirements.txt, so the custom runner failed to import and broke every test invocation. Remove the runner and its TEST_RUNNER setting; tests now use Django's default DiscoverRunner against the Postgres database configured via DB_*.
Verisafe brokers provider access tokens. Its integration guide is explicit that a consuming service never stores Google credentials and never persists an access token, yet OauthToken held access tokens, refresh tokens, and provider secrets. Remove the model, its service layer, and its tests. Migration 0005 drops the table. BREAKING CHANGE: users_oauthtoken is dropped. Any environment holding rows loses them, so confirm the table is empty before deploying.
Verisafe brokers short-lived provider access tokens so this service
holds no user credentials of its own. Add a client for
POST /oauth/{provider}/token and the pre-flight GET /oauth/grants,
mapping each documented response onto a typed exception:
NeedsAuthorization when the user must grant access, ProviderDown for
transient failures worth retrying, and BrokerCredentialsRejected when
our own api key or bot role is at fault.
Name all three providers with their capabilities. Apple carries an empty
capability set rather than being left out, because it is a supported
sign-in provider that brokers nothing.
Base url, api key, timeout, and retry budget come from settings with no
hardcoded default, so QA and production differ only by the environment
they load. Document every variable in .env.example.
Tokens are never cached, persisted, or logged. BrokeredToken redacts the
credential from its repr so it cannot leak through a traceback.
The request had no timeout, so an unresponsive Verisafe could hold a Django worker thread indefinitely. Apply the shared VERISAFE_TIMEOUT and read the base url and api key from settings rather than reaching for the environment at call time.
Local writes stay authoritative and a Celery worker pushes them to Google Tasks afterwards: creates, edits, and soft deletes, for both tasks and lists. Work is queued on transaction commit so a worker cannot read a row before it is durable, and every run takes a fresh access token from the Verisafe broker rather than storing one. Wire the celery app into keep_up/__init__.py. Without it, .delay() from a Django process bound to Celery's default app and published to amqp://guest@localhost rather than the configured broker, so every enqueue would have been dropped in silence. Add SyncStatus.SKIPPED for owners who have not linked Google. That is a normal outcome rather than an error, so it stays out of the failed count, and a nightly sweep revisits skipped records to backfill anyone who links later. Classify failures by who can act on them. A rejected service token leaves the record pending and backs off hard, because the deployment is at fault rather than the row. A malformed request fails outright. A stale remote id is cleared and recreated. Rate limits and outages back off with jitter. Worker and beat process types are declared in the Procfile, but the deployment still has to run them.
The deploy step rewrote only deployment.yaml, so the new worker and beat Deployments would have stayed pinned to the tag they were created with and run stale code through every later deploy. Rewrite all three manifests in a loop. Run the worker with --pool=solo. A prefork parent plus two children each load the full Django app registry, which OOM killed the equivalent chirp worker at 512Mi. Give beat an explicit schedule path so it does not depend on the working directory being writable. Collapse the branch logic duplicated across the path and commit steps into a single directory output, and skip the commit when the manifests already point at the built image rather than failing on an empty commit.
Closes the loop on the relay, which only ever pushed. Edits made in the Google Tasks app now land locally, tasks and lists created there are imported, and tasks deleted there are soft-deleted here. The Tasks API offers no sync tokens and no push notifications, so pulls poll with updatedMin against a per-user watermark held in the new GoogleSyncState. The watermark is the time the pull started rather than the time it finished, or every change made while a pull was running would be lost, and it re-reads a short overlap window to absorb clock skew between Google and this database. A record left pending keeps its local value, because it has an unpushed edit that the queued push will carry to Google. Everything else takes the remote value. Pull writes go through the model rather than the service layer: routing them through update_task would enqueue a push and bounce every imported change straight back to Google. Task deletions are reported by the API, list deletions are not, so a list that stops appearing is inferred to be gone. That inference only runs after a complete enumeration, never touches pending lists, and never runs on a first pull. Add a partial unique constraint on owner_id and external_id for both models, since the pull upserts on exactly that pair and overlapping pulls could otherwise both miss and both insert.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
No description provided.