fix(dev): read RPID from the environment in the dev stack - #237
Merged
Conversation
docker-compose.dev.yml hardcoded RPID: localhost while every neighbouring value takes an override. Compose gives environment: precedence over env_file, so setting RPID in .env had no effect and the only way to change it was editing the compose file. Changing the relying party for mobile work then surfaced as an Android "RP ID cannot be validated", which points nowhere near the cause. Also documents the case that made this hard to diagnose: a row edited directly in the database keeps updatedBy NULL, so it still counts as env-driven and the next boot reverts it. The value reads back correctly until the process restarts. The issue also asked whether admin writes should invalidate the config cache. They already do, in both the whole-config PATCH and the per-provider routes, each with a test. The docs said a write "should" invalidate; they now say the write paths do, and that a direct database edit does not.
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.
Closes #142.
The bug
docker-compose.dev.ymlsetRPID: localhostas a literal while every neighbouring value takes an override (ORIGINS,FRONTEND_URL,APP_ORIGINS). Compose givesenvironment:precedence overenv_file:, so settingRPIDin.envwas silently ignored and editing the compose file was the only way through. Now${RPID:-localhost}.Verified both directions:
RPID=example.test docker compose configresolves toexample.test, and unset still resolves tolocalhost.The two other bullets
Both turned out to be already handled, so this reports rather than changes them.
Cache invalidation on admin writes already happens.
invalidateSystemConfigCache()is called by the whole-config PATCH (systemConfig.ts) and by the per-provider routes (oauthProviders.ts), and there are tests asserting it on both. No change needed.The re-seed is already documented in
docs/configuration.mdunder Environment vs system_config, including theupdatedBy IS NULLprecedence and thewarnlog on overwrite.What that section did not cover is the case the issue actually hit, so this adds it: a row edited directly in the database keeps
updatedByNULL, so it still counts as env-driven and the next boot reverts it to the environment variable. The value reads back correctly from the database right up until the process restarts, which is why this looks like the edit silently failed. Direct edits also miss the cache invalidation the admin routes do, so they are additionally invisible for up to five minutes.Also tightened the closing line of that section, which said a write "should" invalidate the cache. The write paths do; a direct database edit does not.
Verification
docker compose configinterpolation checked both ways. Full gate green via the pre-commit hook: lint, format, typecheck, 1194 tests, coverage, build. Local dev only, no runtime source change, so no changeset.