Skip to content

deploy-uc.sh rejects the external-url path that the #21 cookie fix depends on #22

Closed

Description

@hujincalrin41

Follow-up to #21 (which fixes #20). The server-side fix is correct, but it cannot be reached through this repo's own deployment path, and every piece of documentation steers operators away from the one input that activates it. This issue covers the deployment and documentation side, which lives entirely in deploy/ and is ours — nothing here is asked of the contributor of #21.

The conflict

deploy/deploy-uc.sh:124 rejects any UC_EXTERNAL_URL containing a path:

if [ -n "$UC_EXTERNAL_URL" ] && ! [[ "$UC_EXTERNAL_URL" =~ ^https?://[^/]+$ ]]; then
  echo "ERROR: UC_EXTERNAL_URL must be scheme://host[:port] with no path, e.g. https://uc.example.com"
  echo "       got: $UC_EXTERNAL_URL"
  exit 1
fi

AuthService.browserAuthPath() (added by #21) differs from authMountPath() only when that value has a path. The two conditions are mutually exclusive, so for anyone deploying through deploy-uc.sh the fix never takes effect — the deployment fails before the server starts. This is unconditional, not an edge case.

(Operators deploying with their own tooling are unaffected, which is why #21 works for the reporter.)

Why relaxing the check is the right direction

The rejection is deliberate. deploy/deploy-uc.sh:120-123:

UC_EXTERNAL_URL overrides what the server would otherwise derive, and two things depend on it: the redirect_uri handed to the identity provider, which must equal the registered one byte for byte, and whether the session cookie is marked Secure. A path here silently breaks the first; the property's own validator only checks that the value parses as a URL, so check the shape.

That reasoning holds when the server sits at the root of its host. Behind a gateway that serves it under a prefix and strips the prefix before forwarding, the redirect_uri registered with the identity provider is the prefixed one, so a path is required rather than a mistake.

Two things were verified against the current code, so the implementer does not need to re-derive them:

  • The Secure flag is unaffected by a path. AuthService.browserUsesHttps() only does base.regionMatches(true, 0, "https://", 0, 8). The second concern in the :120-123 comment does not apply.
  • The host-only case stays byte-identical. ServerProperties.getExternalUrl() strips trailing slashes (replaceAll("/+$", "")), and URI.create(base).getRawPath() returns "" for a host-only URL, so browserAuthPath() == authMountPath() whenever no path is configured. Relaxing the check cannot regress existing deployments.

Changes needed

Five locations. All are identical on main and on the #21 branch — #21 touches none of them.

# Location Current Change
1 deploy/deploy-uc.sh:124 ^https?://[^/]+$, then exit 1 Accept a path; keep rejecting trailing slash, query and fragment
2 deploy/deploy-uc.sh:120-123 Comment says a path "silently breaks" the redirect_uri Rewrite: a path is required when a gateway serves UC under a prefix; note the Secure flag is unaffected
3 deploy/deploy-uc.sh:125 ERROR: UC_EXTERNAL_URL must be scheme://host[:port] with no path, … Restate against the new accepted shape
4 server/src/main/java/io/unitycatalog/server/utils/ServerProperties.java:199-201 // Base URL at which browsers reach this server (scheme + host [+ port]), … Drop (scheme + host [+ port]); say the path of this value is also used to scope the OAuth state cookie
5 deploy/uc.env.example:86-88, deploy/README.md:107, deploy/README.zh-CN.md:101 All show https://uc.example.com; uc.env.example:86-87 documents the rejection explicitly ("deploy-uc.sh rejects a path outright") Add the behind-a-prefix case with a worked example; keep EN and zh-CN in step

Shape the check should accept

Value Expected
https://uc.example.com accept
https://uc.example.com:8443 accept
https://api.example.com/unitycatalog accept (today: rejected)
https://api.example.com/a/b accept (today: rejected)
https://uc.example.com/ reject (trailing slash)
https://api.example.com//uc reject (empty segment)
https://uc.example.com?x=1 reject (query)
https://uc.example.com#f reject (fragment)
uc.example.com reject (no scheme)

The server tolerates a trailing slash on its own (getExternalUrl() strips it), but rejecting it at deploy time keeps the error early and explicit — matching how this check already behaves.

Test coverage

Nothing exercises the deploy-uc.sh validation, which is why this conflict passes CI unnoticed. Worth adding a small shell-level case over the table above; at minimum, walk it manually before merging.

AuthServiceHostedLoginBehindPrefixTest (added by #21) covers the server side and is confirmed to have run and passed in CI — no further Java test is needed for this change.

Release ordering

Do not add this fix to features.md or cut a tag until this issue lands. Between #21 merging and this change, main carries a fix that is inert through the standard deploy path, and a release note claiming it would be wrong. Per the release process, features.md is updated first and the tag is cut on that commit — one entry should cover #21 and this change together.

Out of scope

AuthService.java:358 returns Location: returnTo with a root-relative path, so after a successful callback the browser lands at the gateway root rather than back under the prefix. This is pre-existing, not introduced by #21, and touches the UI's whole same-origin assumption (UC_AUTH_API_PREFIX is root-relative in auth-providers.ts, authz.ts and user.ts). Tracking separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions