Let a service opt out of the HTTPS proxy - #445
Open
krokicki wants to merge 1 commit into
Open
Conversation
Some services cannot work behind the per-job subdomain at all. One whose OAuth callback is registered against a fixed host and port is the concrete case: republished at job-<id>.<zone> it fails the callback no matter what the proxy does. There was no way to say so, so enabling the proxy broke such an app with no recourse short of unregistering it. An entry point can now set `service_proxy: false`. The flag is snapshotted onto the job row at submit time so editing the manifest cannot change the decision under a running job, and it is enforced in both places rather than only at publish time: the job detail endpoint publishes the direct URL, and the resolve endpoint refuses the hostname. The label is unguessable but derivable by anyone holding the signing key, so leaving resolution open would make the opt-out advisory rather than real. Refusals count as `refused_proxy_disabled` beside the existing reasons, so an operator can see an app opting out rather than inferring it from traffic that never arrives. Validated as service-only, matching auto_url. The check keys on the value being false rather than on the field being set, because model_dump writes the True default onto every entry point and a round-tripped job manifest would otherwise fail to revalidate. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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.
Adds
service_proxy: falseon a service entry point, for a service that cannot work when Fileglancer republishes it at a per-job HTTPS subdomain.Docs half: JaneliaSciComp/fileglancer-docs#25. Independent — each is useful without the other.
Why
The proxy rewrites every running service's URL to
https://job-<id>-<mac>.<zone>/.... Most services do not notice. One registers its GitHub OAuth callback against a fixed host and port, so at a rewritten address the callback does not match and the app is unusable. There was no way for a manifest to say so, so enablingapps.service_proxy_domainbroke that app with no recourse short of unregistering it.This is the narrow fix for that, and only that. It is not a route to running services unencrypted by preference — the docs and the field description both say so explicitly.
What's in it
service_proxy: bool = TrueonAppEntryPoint, snapshotted onto the job row at submit time, enforced in two places.Snapshotted rather than read from the manifest. Same reasoning as
commandandcontaineralready: the decision must not change under a job that is already running, and the resolve endpoint must not have to parse a manifest to answer.Enforced at resolve as well as at publish. Suppressing the published URL alone would make the opt-out advisory. The
job-<id>-<mac>label is unguessable but derivable by anyone holdingsession_secret_key, so/api/apps/resolverefuses an opted-out job outright and counts it asrefused_proxy_disabled, beside the existing refusal reasons rather than folded into them.Validated service-only, matching
auto_url. The check keys on the value beingfalse, not on the field being set:model_dumpwrites theTruedefault onto every entry point as manifests round-trip worker → server → DB cache, so a round-trippedtype: jobmanifest would otherwise fail to revalidate. There is a test pinning exactly that.Migration. One
add_column, non-null with asa.true()server default so existing rows backfill rather than leaving a nullable tri-state that every read site has to interpret. Verified on a scratch database: applies on top ofc3e9b7f41a28, backfills a pre-existing row to true, and downgrades cleanly.No new settings. No frontend changes — the rewrite already happens server-side in the job detail endpoint.
Testing
pixi run -e test pytest tests/: 1026 passed.New coverage: the field defaults true, is rejected on a
type: jobentry point, survives amodel_dumpround trip, keeps the direct URL published on a server where the proxy is on, and is refused by/api/apps/resolvewith the right counter label.Migration checked by hand against a scratch SQLite database, both directions.
@StephanPreibisch @JaneliaSciComp/fileglancer