From 0d8beefe7d156a5408c2699cf0bf6f83a40703a1 Mon Sep 17 00:00:00 2001 From: huyang Date: Wed, 23 Sep 2026 04:52:27 +0000 Subject: [PATCH 1/2] feat: browser sign-in for the UI (Microsoft, administrator password, access token) The UI had no way to sign anyone in: the UI server put an administrator token on every API call it forwarded, so opening its address was enough to be admin, and there was no login page to speak of. Give each way in an address of its own. /login is the default page and carries Sign in with Microsoft, through an OAuth flow the server hosts end to end, so the client secret never reaches the browser. /login/admin takes the password from server.admin-password, and /login/token takes an access token this server issued, for whoever runs it. None is linked from another, the application itself is no longer an entry point -- without a session it redirects to /login -- and the UI server adds no credentials of its own. Which ways exist is asked of the server at runtime, so one UI build follows any deployment's configuration. Accepting an identity provider's tokens needed the server side to change too: - the static JWKS file and OIDC discovery now coexist, routed by whether the issuer is registered in the file, so a Relyt instance and Entra ID are both trusted at once - the caller resolves through an ordered claim chain (email, preferred_username, upn, sub), because Entra omits email unless it is configured as an optional claim - discovered providers are cached per issuer and every call to the provider is bounded; an unreachable one answers 401 with its name rather than a bodyless 500 - Entra's {tenantid} issuer template is compared segment-wise, which multi-tenant endpoints advertise literally - the session cookie is one a browser keeps: Secure only when the browser is on HTTPS, read the way the callback URL is read, and SameSite=Lax, since the browser returns from the provider on a cross-site navigation - the URL transcoder passes a redirect back instead of following it, and ends a bodyless response on its headers Deployment follows: deploy-uc.sh derives the audience and, where the authorization URL states it unambiguously, the issuer; it starts the UI alongside the server, and warns when it is about to render secrets over a file that git tracks. The Entra setup, including what the client secret's expiry does and how to rotate it without losing a sign-in, is documented in both deployment guides. --- api/control.yaml | 160 +++++ bin/start-uc-with-ui.sh | 11 +- deploy/README.md | 188 +++++- deploy/README.zh-CN.md | 161 ++++- deploy/deploy-uc.sh | 152 ++++- deploy/server.properties.template | 16 +- deploy/uc.env.example | 80 ++- .../server/URLTranscoderVerticle.java | 9 +- .../server/UnityCatalogServer.java | 11 + .../server/security/JwtClaim.java | 5 + .../server/security/SecurityContext.java | 9 + .../server/service/AuthService.java | 595 ++++++++++++++++-- .../server/utils/JwksOperations.java | 182 ++++-- .../server/utils/ServerProperties.java | 53 ++ .../server/base/auth/BaseAuthCRUDTest.java | 58 ++ .../server/base/auth/JwksTestUtils.java | 36 ++ .../AuthServiceAdminLoginDisabledTest.java | 85 +++ .../service/AuthServiceAdminLoginTest.java | 168 +++++ .../service/AuthServiceAuthProvidersTest.java | 41 ++ .../AuthServiceDiscoveryCachingTest.java | 114 ++++ .../AuthServiceDualTrustChainTest.java | 138 ++++ .../AuthServiceHostedLoginDisabledTest.java | 64 ++ .../service/AuthServiceHostedLoginTest.java | 288 +++++++++ .../AuthServiceIdentityResolutionTest.java | 138 ++++ .../service/AuthServiceTokenLoginTest.java | 176 ++++++ .../utils/JwksOperationsIssuerMatchTest.java | 80 +++ .../server/utils/JwksOperationsTest.java | 11 +- ui/.env | 6 +- ui/server.js | 46 +- ui/src/App.test.tsx | 131 ++++ ui/src/App.tsx | 116 +++- .../components/login/EntraAuthButton.test.tsx | 54 ++ ui/src/components/login/EntraAuthButton.tsx | 46 ++ ui/src/context/auth-context.tsx | 60 +- ui/src/hooks/auth-providers.ts | 52 ++ ui/src/hooks/user.ts | 84 +++ ui/src/pages/AdminLogin.test.tsx | 73 +++ ui/src/pages/AdminLogin.tsx | 138 ++++ ui/src/pages/Login.test.tsx | 74 +++ ui/src/pages/Login.tsx | 28 +- ui/src/pages/TokenLogin.test.tsx | 63 ++ ui/src/pages/TokenLogin.tsx | 130 ++++ ui/src/test-utils/render.tsx | 10 +- ui/src/types/api/control.gen.ts | 283 +++++++++ 44 files changed, 4246 insertions(+), 177 deletions(-) create mode 100644 server/src/test/java/io/unitycatalog/server/base/auth/JwksTestUtils.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginDisabledTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceAuthProvidersTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceDiscoveryCachingTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceDualTrustChainTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginDisabledTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceIdentityResolutionTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/service/AuthServiceTokenLoginTest.java create mode 100644 server/src/test/java/io/unitycatalog/server/utils/JwksOperationsIssuerMatchTest.java create mode 100644 ui/src/App.test.tsx create mode 100644 ui/src/components/login/EntraAuthButton.test.tsx create mode 100644 ui/src/components/login/EntraAuthButton.tsx create mode 100644 ui/src/hooks/auth-providers.ts create mode 100644 ui/src/pages/AdminLogin.test.tsx create mode 100644 ui/src/pages/AdminLogin.tsx create mode 100644 ui/src/pages/Login.test.tsx create mode 100644 ui/src/pages/TokenLogin.test.tsx create mode 100644 ui/src/pages/TokenLogin.tsx diff --git a/api/control.yaml b/api/control.yaml index 56c6ad65..cc7a5519 100644 --- a/api/control.yaml +++ b/api/control.yaml @@ -150,6 +150,154 @@ paths: application/json: schema: $ref: '#/components/schemas/SelfCapabilities' + /auth/login: + get: + tags: + - OAuth2 + operationId: login + summary: Start the server-hosted login flow + description: | + Redirects the browser to the configured identity provider's authorization endpoint + (server.authorization-url) with a one-time state bound to a short-lived cookie. The + identity provider sends the browser back to /auth/callback. Only available when the + OAuth client properties are configured on the server; the client secret never leaves it. + parameters: + - in: query + name: redirect + required: false + description: Path within the UI to return to after login. Must be a relative path. + schema: + type: string + responses: + '302': + description: Redirect to the identity provider. + '400': + description: The hosted login flow is not configured on this server. + /auth/callback: + get: + tags: + - OAuth2 + operationId: loginCallback + summary: Complete the server-hosted login flow + description: | + Receives the authorization code from the identity provider, exchanges it for an identity + token using the server-held client secret, verifies that token exactly like the token + exchange endpoint does, and issues a Unity Catalog access token as the UC_TOKEN cookie + before redirecting back into the UI. + parameters: + - in: query + name: code + required: false + schema: + type: string + - in: query + name: state + required: false + schema: + type: string + - in: query + name: error + required: false + schema: + type: string + - in: query + name: error_description + required: false + schema: + type: string + responses: + '302': + description: Login succeeded; the UC_TOKEN cookie is set and the browser is sent back to the UI. + '400': + description: Missing or mismatched state, an error reported by the identity provider, or an unknown user. + /auth/providers: + get: + tags: + - OAuth2 + operationId: getAuthProviders + summary: Which sign-in methods this server offers + description: | + Public (no token required). Lets a UI decide at runtime whether sign-in is required at all + and which entry points to show, so one UI build follows the server's configuration instead + of baking the choice in. + responses: + '200': + description: The available sign-in methods. + content: + application/json: + schema: + $ref: '#/components/schemas/AuthProviders' + /auth/admin/login: + post: + tags: + - OAuth2 + operationId: adminLogin + summary: Sign in as the built-in administrator with a password + description: | + Exchanges the administrator password configured on the server (server.admin-password) for + a Unity Catalog access token, returned in the body and set as the UC_TOKEN cookie. Serves + the UI's administrator entry point; unavailable unless the password is configured. + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: + type: object + required: + - username + - password + properties: + username: + type: string + password: + type: string + format: password + responses: + '200': + description: Signed in; the UC_TOKEN cookie is set. + content: + application/json: + schema: + $ref: '#/components/schemas/OAuthTokenExchangeInfo' + '400': + description: Administrator login is not configured on this server. + '401': + description: Wrong username or password. + /auth/token/login: + post: + tags: + - OAuth2 + operationId: tokenLogin + summary: Open a browser session with an access token already held + description: | + Accepts a Unity Catalog access token issued by this server (the one in etc/conf/token.txt, + for instance) and sets it as the UC_TOKEN cookie, so an operator can reach the UI with the + token instead of a password. The token is verified as any request's token is -- signature, + expiry and an enabled user -- and is handed back unchanged, so the session cannot outlive + it. Tokens from an identity provider go to /auth/tokens instead. + requestBody: + required: true + content: + application/x-www-form-urlencoded: + schema: + type: object + required: + - token + properties: + token: + type: string + format: password + responses: + '200': + description: Signed in; the UC_TOKEN cookie is set. + content: + application/json: + schema: + $ref: '#/components/schemas/OAuthTokenExchangeInfo' + '400': + description: No token supplied. + '401': + description: The token is not valid for sign-in. /auth/tokens: post: tags: @@ -321,6 +469,18 @@ components: type: string lastModified: type: string + AuthProviders: + type: object + properties: + authorization_enabled: + type: boolean + description: Whether requests must carry a token at all (server.authorization). + hosted_login: + type: boolean + description: Whether the server-hosted OAuth sign-in (/auth/login) is configured. + admin_login: + type: boolean + description: Whether the administrator password sign-in (/auth/admin/login) is configured. SelfCapabilities: type: object properties: diff --git a/bin/start-uc-with-ui.sh b/bin/start-uc-with-ui.sh index af879b39..2b68f025 100755 --- a/bin/start-uc-with-ui.sh +++ b/bin/start-uc-with-ui.sh @@ -23,14 +23,15 @@ cd "$UC_HOME" # --- UI disabled: original single-process behaviour (exec = server is PID) --- case "${UC_ENABLE_UI:-}" in 1 | true | TRUE | yes | on) ;; - *) exec bin/start-uc-server --port "$UC_PORT" ;; + *) exec bin/start-uc-server --port "$UC_PORT" "$@" ;; esac # --- UI enabled: two processes in one container --- -setsid bin/start-uc-server --port "$UC_PORT" & +setsid bin/start-uc-server --port "$UC_PORT" "$@" & UC_PID=$! -# The server writes its admin token on first start; wait for it (bounded). +# The server writes its admin token on first start. The UI does not use it -- an operator +# signs in with it at /login/token -- but its arrival is the signal that the server is up. TOKEN_FILE="$UC_HOME/etc/conf/token.txt" for _ in $(seq 1 90); do [ -s "$TOKEN_FILE" ] && break @@ -46,7 +47,9 @@ done exit 1 } -UC_TARGET="http://localhost:${UC_PORT}" UC_TOKEN_FILE="$TOKEN_FILE" \ +# The UI talks to the Armeria server directly, on UC_PORT+1: UC_PORT itself is the URL +# transcoder, which exists so dotted names survive a path and which the UI never needs. +UC_TARGET="http://localhost:$((UC_PORT + 1))" \ PORT="$UI_PORT" HOST=0.0.0.0 node ui/server.js & UI_PID=$! echo "[uc-ui] UI on :${UI_PORT} -> UC :${UC_PORT}" diff --git a/deploy/README.md b/deploy/README.md index 84cd3ac3..103fda65 100644 --- a/deploy/README.md +++ b/deploy/README.md @@ -57,7 +57,7 @@ authorization is on. `200` means authorization is **off** — check `UC_AUTHORIZ ## Running the UI `deploy-uc.sh` starts the **server only**. The bundled web UI is served by `ui/server.js`, which -proxies `/api` to the UC server and injects the admin token so the browser needs no login. +proxies `/api` to the UC server. It adds no credentials: every visitor signs in for themselves. The container image entrypoint [`bin/start-uc-with-ui.sh`](../bin/start-uc-with-ui.sh) runs both processes when `UC_ENABLE_UI` is truthy (`1`/`true`/`yes`/`on`); otherwise it runs the server alone: @@ -71,13 +71,12 @@ and start the UI server yourself: ```bash cd ui && yarn install && yarn build && cd .. -UC_TARGET=http://localhost:8088 \ -UC_TOKEN_FILE="$UC_HOME/etc/conf/token.txt" \ +UC_TARGET=http://localhost:8089 \ PORT=3000 HOST=0.0.0.0 node ui/server.js ``` -> ⚠️ **The UI injects the admin token into every API call, so anyone who can reach it is a Unity -> Catalog admin with no login.** Keep port `3000` on a trusted network; never expose it publicly. +> ⚠️ **With authorization off, the server asks for nothing and neither does the UI.** Keep port +> `3000` on a trusted network; never expose it publicly. ## Files @@ -100,7 +99,13 @@ PORT=3000 HOST=0.0.0.0 node ui/server.js | `UC_AUTHORIZATION` | no | `enable` (default) / `disable` | | `UC_ALLOWED_ISSUERS` | no | **optional** extra trusted issuers (comma-separated). Trusted issuers are derived primarily from the JWKS file (each key's `issuer` member, hot-reloaded); this list is a **union** on top, only for issuers **not** in the local JWKS (e.g. OIDC discovery). **Leave empty for Relyt** — the JWKS alone governs trust, and onboarding a DWSU means appending a key (hot-reloaded, no restart, no change here) | | `UC_AUDIENCES` | **yes** | audience of the subject token, e.g. `unitycatalog-server` (must match the signer's `unity.audience`) | -| `UC_ACCESS_TOKEN_TTL` | no | lifetime of exchanged tokens (ISO-8601, e.g. `PT1H`). A **blank** value means **no expiry**; `uc.env.example` ships `PT1H` deliberately — keep it unless you have a reason not to | +| `UC_ACCESS_TOKEN_TTL` | no | lifetime of exchanged tokens, **ISO-8601** (`PT12H`, `PT30M`; `12h` is rejected). Blank = **no expiry**. One value for every caller — the coordinator and the UI. `uc.env.example` ships `PT12H`, which is the value to deploy: the coordinator's exp-based refresh path had not run in production when this was chosen, so the default leaves headroom. Shorten it only once that path has been observed in your own deployment | +| `UC_AUTHORIZATION_URL` | no | provider's OAuth authorization endpoint, e.g. `https://login.microsoftonline.com//oauth2/v2.0/authorize`. **All four** login variables blank = hosted login off (see [Microsoft Entra ID sign-in](#microsoft-entra-id-sign-in-optional)) | +| `UC_TOKEN_URL` | no | provider's OAuth token endpoint, e.g. `https://login.microsoftonline.com//oauth2/v2.0/token` | +| `UC_CLIENT_ID` | no | client id of the application registered for the UI login; must also be listed in `UC_AUDIENCES` | +| `UC_CLIENT_SECRET` | no | that application's client secret — stays on the server, never sent to the browser | +| `UC_EXTERNAL_URL` | no | browser-facing base URL of UC (`https://uc.example.com`) when a proxy in front does not send `X-Forwarded-Proto`/`X-Forwarded-Host`; blank = derived from the request | +| `UC_ADMIN_PASSWORD` | no | password of the built-in `admin` for the UI's administrator sign-in at `/login/admin`. Blank = that entry point off. See [UI sign-in](#ui-sign-in) | | `ALIYUN_REGION` | **yes** | e.g. `cn-hangzhou` | | `ALIYUN_ACCESS_KEY` | **yes** | master RAM user AK (used for STS `AssumeRole`) | | `ALIYUN_SECRET_KEY` | **yes** | master RAM user SK | @@ -267,3 +272,174 @@ Notes: [Onboarding a new DWSU](#onboarding-a-new-dwsu-hot-no-restart)): `./uc_add_jwks_key.sh $UC_EXTERNAL_JWKS_FILE ` (the third argument binds the issuer to that key, so UC only uses it to verify tokens from that issuer). + + +## UI sign-in + +The UI requires sign-in. Each way in has an address of its own; the application's address is not one +of them, and without a session it redirects to `/login`. Which ways exist is decided by the +server's configuration at runtime — the UI asks `GET /auth/providers` on load, so one UI build serves +every deployment: + +| Address | Who | Enabled by | +|---|---|---| +| **`/login`** — the default page, offering **Sign in with Microsoft** | employees, with their M365 account | the four `UC_AUTHORIZATION_URL` … `UC_CLIENT_SECRET` settings | +| **`/login/admin`** — a password form, deliberately not linked from the login page | whoever runs the server | `UC_ADMIN_PASSWORD` | +| **`/login/token`** — paste an access token this server issued | whoever runs the server | always available | + +All three end in the same `UC_TOKEN` cookie, so nothing downstream distinguishes them. After sign-in the +catalog / schema / table lists are filtered to what that user has been granted (`filterCatalogs` and +friends run server-side); the administrator, as metastore owner, sees everything. + +`deploy-uc.sh` warns when authorization is on but neither entry point is configured, since nobody +could sign in. + +**Signing in with an access token.** Whoever runs the server can sign in with the token it writes +to `etc/conf/token.txt` at `/login/token`, pasting it into the form or opening +`/login/token?token=`; the page drops the token from the address bar once it is used. +The server verifies the token the way it verifies any request and hands it back as the session +cookie, so the session ends when the token does. + +Earlier versions had the UI server inject that token into every API call instead, which made the +UI's own address a way into an authorized server with no sign-in at all. The UI server now adds no +credentials of its own. + +### Microsoft Entra ID (M365) + +#### 1. Entra side + +Register one application for the UI: + +| Item | | +|---|---| +| Application registration → `tenant id`, `client id`, `client secret`. The first two sit side by side on the app's overview page and are **different** GUIDs: `Directory (tenant) ID` identifies the organization and goes into the two URLs; `Application (client) ID` identifies this app and goes into `UC_CLIENT_ID` | ✅ | +| **`email` optional claim** — without it accounts cannot be matched to UC users | ✅ | +| Redirect URI: `/api/1.0/unity-control/auth/callback` | ✅ | + +Who can do this: a member of the tenant can register an application and create its secret while the +tenant leaves "users can register applications" and user consent on, which many organizations turn +off. The scopes UC asks for — `openid`, `profile`, `email` — need no administrator consent by +themselves, but an application that requires assignment does. Ask the tenant administrator unless +the customer says otherwise; it is a ten-minute job, and it also settles who owns the application +and therefore who rotates its secret. + +The tenant id and both endpoint URLs are public. Given the customer's email domain you can read them +without signing in to anything: + +```bash +curl -s https://login.microsoftonline.com//v2.0/.well-known/openid-configuration +``` + +The `issuer` field states the tenant GUID, and `authorization_endpoint` / `token_endpoint` are the +two URLs below. + +#### 2. UC side (`uc.env`) + +```bash +UC_AUTHORIZATION_URL=https://login.microsoftonline.com//oauth2/v2.0/authorize +UC_TOKEN_URL=https://login.microsoftonline.com//oauth2/v2.0/token +UC_CLIENT_ID= +UC_CLIENT_SECRET= +UC_ACCESS_TOKEN_TTL=PT12H +``` + +`deploy-uc.sh` derives the rest from those four and prints each addition, leaving whatever is +already configured in place: + +| Derived | From | When | +|---|---|---| +| `UC_AUDIENCES` += the client id | `UC_CLIENT_ID` | always; an id_token is addressed to the application | +| `UC_ALLOWED_ISSUERS` += `https://login.microsoftonline.com//v2.0` | `UC_AUTHORIZATION_URL` | only when it names a tenant GUID on the v2.0 endpoint | + +A v1.0 endpoint issues as `https://sts.windows.net//`, a tenant named by domain does not +state its GUID, and `common` / `organizations` issue per tenant. The script does not guess those: it +warns and points at the provider's `.well-known/openid-configuration`, whose `issuer` field is the +value to put in `UC_ALLOWED_ISSUERS`. + +The static JWKS for Relyt instances keeps working alongside this: an issuer with a key in the JWKS +file is verified from the file, any other trusted issuer through OIDC discovery. + +Multi-tenant (`common` / `organizations`) endpoints advertise the literal issuer template +`https://login.microsoftonline.com/{tenantid}/v2.0`; UC accepts the real tenant GUID in that position. +`UC_ALLOWED_ISSUERS` is still an exact match, so list each tenant's issuer. + +The UI proxy forwards `X-Forwarded-Proto`/`X-Forwarded-Host` so UC can build the callback URL the +browser actually uses; set `UC_EXTERNAL_URL` if another proxy in front strips them. + +#### 3. Provisioning users + +There is no just-in-time provisioning: every employee who should be able to sign in must exist in UC +first (`POST /scim2/Users`) with their **real Microsoft email, byte-for-byte equal to the token's +`email` claim**. UC matches a token by `email`, then `preferred_username` / `upn` (Entra's sign-in +name), then `sub` — all against the user's email. Grants are then given to that UC user like any +other. Deleting and recreating a user drops every grant (grants are stored against the user's UUID). + +#### 4. The client secret expires + +Entra issues client secrets with an end date — 6, 12 or 24 months, whichever the application's owner +picked — and says nothing when it approaches. Know the blast radius before the day arrives. + +**What stops, and what does not.** The secret is used in exactly one request: the server redeeming +the authorization code for an id_token inside `/auth/callback`. Everything else is untouched: + +| Path | On an expired secret | +|---|---| +| Programs reading and writing through Relyt | unaffected — the coordinator authenticates with its instance key from the local JWKS file, never through Entra | +| Spark reading metadata with a UC token | unaffected, same reason | +| People already signed in to the UI | keep working until their token expires (`UC_ACCESS_TOKEN_TTL`, `PT12H` by default): `UC_TOKEN` is issued and verified by UC itself, statelessly | +| Signing in again with a Microsoft account | **fails** | +| `/login/admin` and `/login/token` | unaffected — neither touches Entra, so whoever runs the server is never locked out | + +**What it looks like, and the trap in diagnosing it.** The failure lands *after* Entra has +authenticated the person: they sign in at Microsoft successfully and the error appears on the way +back. The browser shows + +``` +Identity provider rejected the authorization code: 401 Unauthorized (invalid_client, AADSTS7000222) +``` + +The code in brackets is what separates the three causes that all fail here with the same status: a +lapsed secret (`AADSTS7000222`), a redirect URI that no longer matches the registration +(`AADSTS50011`), and an authorization code already used (`AADSTS54005`). The provider's full +description, with its correlation id, is logged at `WARN` — read the server log before touching the +configuration. + +**Rotating, without interrupting a single sign-in.** An application may hold two valid secrets at +once, so the order matters more than the timing: + +1. In Entra, under *Certificates & secrets*, create the new secret **before the old one lapses**. Both are now valid. +2. Put it in `UC_CLIENT_SECRET` and re-render (`deploy-uc.sh`). +3. **Restart the UC server.** This step cannot be skipped: `ServerProperties` reads the file once at startup and never reloads it, so an edited `server.properties` does nothing until the process restarts. +4. Sign in with a Microsoft account to confirm the new secret works. +5. Delete the old secret in Entra. + +The restart is invisible to people already signed in — their cookie is verified statelessly and +survives it. Only new sign-ins are refused, for as long as the process takes to come back. + +**Operational advice.** Choose the maximum lifetime Entra offers (24 months) and record the end date +somewhere that gets read, because nothing will remind you. For a real alert, poll the application's +`passwordCredentials[].endDateTime` through the Graph API and warn a month ahead. + +Note the current limit: UC supports a client secret only. There is no configuration for a +certificate or a federated credential, and the code exchange always sends a `client_secret` form +field. If your security policy forbids long-lived static secrets, that is a code change worth +tracking as its own issue rather than a configuration option waiting to be found. + +#### 5. Verifying the permission filter + +Sign in as two Microsoft accounts with different grants and confirm the catalog / schema / table +lists differ. The filter is server-side, so no UI configuration is involved. + +### Administrator sign-in + +Set `UC_ADMIN_PASSWORD` and open `/login/admin`. The username is the built-in `admin`. The +password is stored in the rendered `server.properties` alongside the other secrets and compared in +constant time; a failed attempt is delayed by a second, which slows but does not stop guessing — use +a long random value and keep the UI behind your usual network controls. + +### How programs reach UC + +Customer programs do not sign in with Microsoft identities. They go through Relyt, whose coordinator +authenticates to UC with its instance key registered in the static JWKS file (see [Onboarding a new +DWSU](#onboarding-a-new-dwsu-hot-no-restart)); Spark reads metadata directly with a UC token. The +Microsoft integration above is for people using the UI. diff --git a/deploy/README.zh-CN.md b/deploy/README.zh-CN.md index db1e7c94..db103edb 100644 --- a/deploy/README.zh-CN.md +++ b/deploy/README.zh-CN.md @@ -65,13 +65,12 @@ UC_ENABLE_UI=true UC_PORT=8088 UI_PORT=3000 bin/start-uc-with-ui.sh ```bash cd ui && yarn install && yarn build && cd .. -UC_TARGET=http://localhost:8088 \ -UC_TOKEN_FILE="$UC_HOME/etc/conf/token.txt" \ +UC_TARGET=http://localhost:8089 \ PORT=3000 HOST=0.0.0.0 node ui/server.js ``` -> ⚠️ **UI 会把 admin token 注入到每个接口请求，因此任何能访问它的人都等同于 Unity Catalog 管理员， -> 且无需登录。** 请把 `3000` 端口限制在可信网络内，绝不要暴露公网。 +> ⚠️ **授权关闭时服务端不校验身份，UI 同样不会要求登录。** 请把 `3000` 端口限制在可信网络内， +> 绝不要暴露公网。 ## 文件 @@ -94,7 +93,13 @@ PORT=3000 HOST=0.0.0.0 node ui/server.js | `UC_AUTHORIZATION` | 否 | `enable`（默认，启用鉴权）/ `disable` | | `UC_ALLOWED_ISSUERS` | 否 | **可选**的额外受信 issuer（逗号分隔）。受信 issuer 主要由 JWKS 文件派生（各 key 的 `issuer` 成员，热加载），此项与之取**并集**，仅用于信任**不在本地 JWKS** 的 issuer（如 OIDC discovery）。**Relyt 部署留空即可**：完全以 JWKS 为准；接入新 DWSU = 往 JWKS 追加公钥、热加载零重启、无需改此项 | | `UC_AUDIENCES` | **是** | subject_token 的 audience，如 `unitycatalog-server`（须与签发端 `unity.audience` 一致） | -| `UC_ACCESS_TOKEN_TTL` | 否 | 换发 token 有效期（ISO-8601，如 `PT1H`）。**留空 = 不过期**；`uc.env.example` 特意预置了 `PT1H`，无特殊理由请保留 | +| `UC_ACCESS_TOKEN_TTL` | 否 | 换发 token 有效期（ISO-8601，如 `PT12H`、`PT30M`；`12h` 会被拒绝）。**留空 = 不过期**。这一个值对所有调用方生效 —— coordinator 和 UI 共用。`uc.env.example` 预置 `PT12H`，部署就用这个值：选它是因为 coordinator 基于 `exp` 的刷新路径当时还没在生产跑过，默认值先留足余量；要调短，等那条路径在你自己的环境里被观察过之后再说 | +| `UC_AUTHORIZATION_URL` | 否 | IdP 的 OAuth 授权端点，如 `https://login.microsoftonline.com/<租户id>/oauth2/v2.0/authorize`。**四项登录变量全空 = 关闭托管登录**，见 [UI 登录](#ui-登录) | +| `UC_TOKEN_URL` | 否 | IdP 的 OAuth 令牌端点，如 `https://login.microsoftonline.com/<租户id>/oauth2/v2.0/token` | +| `UC_CLIENT_ID` | 否 | 为 UI 登录注册的应用的 client id；`deploy-uc.sh` 会自动把它追加进 `UC_AUDIENCES` | +| `UC_CLIENT_SECRET` | 否 | 该应用的 client secret，**只留在服务端**，不下发浏览器。有有效期，到期要轮换 | +| `UC_EXTERNAL_URL` | 否 | 浏览器访问 UC 的基地址（如 `https://uc.example.com`），当前置代理不传 `X-Forwarded-Proto`/`X-Forwarded-Host` 时需要；留空则按请求推导 | +| `UC_ADMIN_PASSWORD` | 否 | 内置 `admin` 在 `/login/admin` 的登录密码。留空 = 关闭该入口，见 [UI 登录](#ui-登录) | | `ALIYUN_REGION` | **是** | 如 `cn-hangzhou` | | `ALIYUN_ACCESS_KEY` | **是** | master RAM 用户 AK（用于 STS `AssumeRole`） | | `ALIYUN_SECRET_KEY` | **是** | master RAM 用户 SK | @@ -225,6 +230,152 @@ ConfigMap 只是"改这个文件"的一种投递方式。 | 读表时 OSS 拒绝访问 | 凭证里的 role 缺 bucket 上的 `oss:ListObjects`（配 `oss:Prefix` 条件），只给 `oss:GetObject` 不够 | | 换 tag 升级后跑的仍是旧版本 | 整个 `UC_HOME` 被挂成了卷，遮蔽了镜像里的二进制；只挂 `etc/conf` 和 `etc/db` | + +## UI 登录 + +UI 必须登录。**每个入口都有自己的地址，应用地址不是入口**：没有会话时应用地址会跳到 `/login`。 +有哪些入口由服务端配置在运行时决定 —— 页面加载时读 `GET /auth/providers`，所以一份 UI 构建适用于所有 +部署，改配置不用重新打镜像。 + +| 地址 | 给谁用 | 由什么开启 | +|---|---|---| +| **`/login`** —— 默认页，提供 **Sign in with Microsoft** | 员工，用自己的 M365 账号 | `UC_AUTHORIZATION_URL` 等四项 | +| **`/login/admin`** —— 密码表单，刻意不从登录页链接 | 运维方 | `UC_ADMIN_PASSWORD` | +| **`/login/token`** —— 粘贴本服务器签发的访问令牌 | 运维方 | 始终可用 | + +三者最终都落到同一个 `UC_TOKEN` cookie，下游不区分来源。登录后 catalog / schema / 表列表按该用户被授予 +的权限过滤（过滤发生在服务端），管理员作为 metastore owner 看得到全部。 + +`deploy-uc.sh` 会在"授权开着但一个登录入口都没配"时告警，因为那样没人能登录。 + +**用访问令牌登录**：运维方可以拿服务端写在 `etc/conf/token.txt` 里的令牌，在 `/login/token` 粘贴， +或直接打开 `/login/token?token=<令牌>`，页面用完会立即把令牌从地址栏抹掉。服务端按校验普通请求的 +同一套规则校验它，然后**原样**作为会话 cookie 返回，所以会话不会比令牌活得久。 + +早期版本是由 UI server 把这个令牌注入到每个接口请求里，等于让 UI 的地址本身成为一条无需登录的入口。 +现在 UI server 不再携带任何凭据。 + +### 接 Microsoft Entra ID（M365） + +#### 1. Entra 侧 + +为 UI 注册一个应用： + +| 项 | | +|---|---| +| 应用注册 → `租户 id`、`client id`、`client secret`。前两个在应用概览页上下并排、都是 GUID，但**是两样东西**：`目录(租户) ID` 标识这个组织，填进两个 URL；`应用程序(客户端) ID` 标识这个应用，填进 `UC_CLIENT_ID` | ✅ | +| **`email` 可选声明** —— 不配的话账号匹配不到 UC 用户 | ✅ | +| 回调地址：`<浏览器访问 UC 的地址>/api/1.0/unity-control/auth/callback` | ✅ | + +谁能做：租户的普通成员在"允许用户注册应用"和用户同意都开着时可以自己注册并建密钥，但多数企业会关掉 +这两个开关。UC 申请的 `openid`、`profile`、`email` 三个权限本身不需要管理员同意；若应用被设为"需要分配 +用户"，则要管理员分配。除非客户另有说明，直接找租户管理员，十分钟的事，顺带把应用归属和密钥轮换责任 +定下来。 + +租户 id 和两个端点地址是公开信息，知道客户的邮箱域名就能查，不用登录： + +```bash +curl -s https://login.microsoftonline.com/<客户域名>/v2.0/.well-known/openid-configuration +``` + +返回里的 `issuer` 带着租户 GUID，`authorization_endpoint` 和 `token_endpoint` 就是下面要填的两项。 + +#### 2. UC 侧（`uc.env`） + +```bash +UC_AUTHORIZATION_URL=https://login.microsoftonline.com/<租户id>/oauth2/v2.0/authorize +UC_TOKEN_URL=https://login.microsoftonline.com/<租户id>/oauth2/v2.0/token +UC_CLIENT_ID= +UC_CLIENT_SECRET= +UC_ACCESS_TOKEN_TTL=PT12H +``` + +`deploy-uc.sh` 会从这四项派生出令牌校验需要的另外两项，逐条打印，且只做**追加**，不覆盖已有值： + +| 派生项 | 来源 | 条件 | +|---|---|---| +| `UC_AUDIENCES` += client id | `UC_CLIENT_ID` | 总是。id_token 的 `aud` 就是应用本身 | +| `UC_ALLOWED_ISSUERS` += `https://login.microsoftonline.com/<租户id>/v2.0` | `UC_AUTHORIZATION_URL` | 仅当授权地址是 v2.0 端点且租户写成 GUID | + +v1.0 端点的签发方是 `https://sts.windows.net/<租户id>/`，按域名写的租户不体现 GUID，`common` / +`organizations` 按实际租户签发。这三类脚本不猜，只告警并指向 `.well-known/openid-configuration` 的 +`issuer` 字段，那就是要填进 `UC_ALLOWED_ISSUERS` 的值。 + +Relyt 实例那条本地 JWKS 链路不受影响：issuer 在 JWKS 文件里有 key 的走文件验签，其余受信 issuer 走 +OIDC discovery。 + +UI 代理会转发 `X-Forwarded-Proto` / `X-Forwarded-Host`，UC 据此拼出浏览器实际使用的回调地址；若前面还有 +一层代理把这两个头去掉了，就填 `UC_EXTERNAL_URL`。 + +#### 3. 建用户 + +**没有自动建号**：每个要登录的员工必须先在 UC 里存在（`POST /scim2/Users`），邮箱要与令牌里的 `email` +声明**逐字节一致**。UC 依次用 `email`、`preferred_username` / `upn`（Entra 的登录名）、`sub` 去匹配，全部 +按邮箱查。授权照常授给这个 UC 用户。**删号重建会丢掉该用户的全部授权**，因为授权按用户 UUID 存。 + +#### 4. 客户端密钥会过期 + +Entra 的 client secret 带有效期，6、12 或 24 个月，取决于创建时的选择，临近到期它不会主动提醒。到期前 +先把影响面看清楚，免得当天过度紧张。 + +**哪些会停，哪些不会。** 这个密钥在整个系统里只用于一次请求：`/auth/callback` 里服务端拿授权码换 +id_token。其余全都不经过它： + +| 链路 | 密钥过期后 | +|---|---| +| 客户程序经 Relyt 读写 | 不受影响 —— coordinator 用本地 JWKS 文件里的实例密钥认证，不走 Entra | +| Spark 持 UC token 读元数据 | 不受影响，同上 | +| 已登录 UI 的人 | 在令牌有效期内照常使用（`UC_ACCESS_TOKEN_TTL`，默认 `PT12H`）：`UC_TOKEN` 由 UC 自己签发、自己无状态验签 | +| 用 M365 账号**重新登录** | **失败** | +| `/login/admin` 与 `/login/token` | 不受影响 —— 两者都不碰 Entra，运维方不会被锁在门外 | + +**现象与排查陷阱。** 失败发生在 Entra 已经认证成功**之后**：用户在微软那边登录明明成功了，跳回来却报 +错。浏览器上看到的是 + +``` +Identity provider rejected the authorization code: 401 Unauthorized (invalid_client, AADSTS7000222) +``` + +括号里那个码是区分三种同样报 401 的原因的唯一依据：密钥过期是 `AADSTS7000222`，回调地址与注册不符是 +`AADSTS50011`，授权码被重复使用是 `AADSTS54005`。IdP 返回的完整描述（含 correlation id）记在服务端日志 +的 `WARN` 级别 —— **先看日志再改配置**。 + +**轮换：可以做到零登录中断。** 一个应用允许同时挂两条有效密钥，所以顺序比时机更重要： + +1. 在 Entra 的「证书和机密」里，**趁旧密钥尚未过期**新建一条，此时新旧并存且都有效 +2. 把新值填进 `UC_CLIENT_SECRET`，重新渲染配置（`deploy-uc.sh`） +3. **重启 UC server。这一步省不掉**：`ServerProperties` 只在启动时读一次文件，没有热加载，改完渲染后的 `server.properties` 不重启不生效 +4. 用 M365 账号登录一次，确认新密钥可用 +5. 回 Entra 删除旧的那条 + +重启对已登录的人无感，他们的 cookie 是无状态校验的，重启后继续有效；受影响的只有重启这段时间内无法 +完成新登录。 + +**运维建议。** 创建时直接选 Entra 允许的最长有效期（24 个月），并把到期日记进运维台账或日历，因为没有 +任何东西会提醒你。想要更稳的话，用 Graph API 定期查应用的 `passwordCredentials[].endDateTime`，提前一个 +月告警。 + +还要说明当前实现的边界：UC 只支持 client secret，没有证书凭据或联合凭据的配置项，换取令牌时固定拼 +`client_secret` 表单字段。如果安全规范不允许落地长期静态密钥，那是一处需要改代码的增强点，建议单独开 +issue 跟踪，不要在配置项里找。 + +#### 5. 验证权限过滤 + +用两个授权不同的 M365 账号分别登录，确认看到的 catalog / schema / 表列表不同。过滤在服务端完成，UI 侧 +没有任何配置参与。 + +### 管理员登录 + +配 `UC_ADMIN_PASSWORD`，打开 `/login/admin`，用户名是内置的 `admin`。密码与其它密钥一同存在渲染后的 +`server.properties` 里，比对是常量时间的，失败会延迟一秒 —— 这能减缓但挡不住暴力猜测，请用足够长的随机 +值，并把 UI 放在既有的网络管控之后。 + +### 程序怎么访问 UC + +客户程序不以 M365 身份访问 UC。它们经 Relyt：coordinator 用登记在本地 JWKS 文件里的实例密钥向 UC 认证 +（见[接入新 DWSU](#接入新-dwsu在线热生效无需重启)）；Spark 则直接持 UC 令牌读元数据。上面这套微软集成 +是给**用 UI 的人**准备的。 + ## 安全要点 - **`uc.env` 已被 `.gitignore` 忽略**，真实密钥不会被提交。 diff --git a/deploy/deploy-uc.sh b/deploy/deploy-uc.sh index 4726a5f3..1d0ffc99 100755 --- a/deploy/deploy-uc.sh +++ b/deploy/deploy-uc.sh @@ -1,8 +1,12 @@ #!/usr/bin/env bash -# Render UC config from templates + uc.env, then start the UC server. +# Render UC config from templates + uc.env, then start the UC server and the browser UI. # # cp uc.env.example uc.env # then fill in real values (mainly UC_HOME + Aliyun creds) -# ./deploy-uc.sh # render + start (foreground) +# ./deploy-uc.sh # render + start both (foreground) +# +# Both processes come up by default; a deployment nobody can open is not a deployment. The UI needs +# ui/build, which is not in the repo: this script builds it once if it is missing and yarn is +# available. Set UC_ENABLE_UI=false for a server-only run, and UI_PORT to move the UI off 3000. # # Everything stateful lives UNDER UC_HOME: the rendered server.properties / hibernate.properties, # the JWKS file, and the H2 metastore DB. Point UC_HOME at a persistent (cloud-disk) path and @@ -38,12 +42,43 @@ set -a; . "$ENV_FILE"; set +a : "${UC_ACCESS_TOKEN_TTL:=}" # blank = no expiry (opt-in) : "${UC_AUTHORIZATION:=enable}" # enable = require auth; disable = no auth : "${UC_ALLOWED_ISSUERS:=}" # blank = trust only the issuers derived from the JWKS +: "${UC_AUTHORIZATION_URL:=}" # hosted login (issue #15): all four blank = off +: "${UC_TOKEN_URL:=}" +: "${UC_CLIENT_ID:=}" +: "${UC_CLIENT_SECRET:=}" +: "${UC_EXTERNAL_URL:=}" # blank = derive redirect_uri from the request +: "${UC_ADMIN_PASSWORD:=}" # blank = administrator UI sign-in off +: "${ALIYUN_MASTER_ROLE_ARN:=}" # blank = advertise nothing; see the WARN below : "${UC_PORT:=8088}" # matches bin/start-uc-with-ui.sh and the e2e suites +: "${UC_ENABLE_UI:=true}" # on by default; anything else = server only +: "${UI_PORT:=3000}" # browser-facing UI port, only read when UI is on + +# Accept exactly the spellings bin/start-uc-with-ui.sh accepts, so uc.env means the same thing here +# and in the container image. (The image itself still defaults to OFF: other consumers of it expect +# a lone server. A deployment driven from here is meant to be usable, so it defaults to ON.) +ui_on="" +case "$UC_ENABLE_UI" in 1 | true | TRUE | yes | on) ui_on=1 ;; esac + +# The UI is served from ui/build, which the repo does not carry. Build it once rather than starting +# half a deployment: the UI process would exit on the missing directory and take the server with it. +if [ -n "$ui_on" ] && [ ! -f "$UC_HOME/ui/build/index.html" ]; then + if command -v yarn >/dev/null 2>&1; then + echo "UI assets not built yet; building once (a few minutes) ..." + (cd "$UC_HOME/ui" && yarn install --frozen-lockfile && yarn build) || { + echo "ERROR: building the UI failed. Fix it, or set UC_ENABLE_UI=false to start the server alone." + exit 1 + } + else + echo "ERROR: the UI needs $UC_HOME/ui/build, which is missing, and yarn is not installed." + echo " Build it on a machine that has yarn (cd ui && yarn install && yarn build) and copy" + echo " ui/build across, or set UC_ENABLE_UI=false to start the server alone." + exit 1 + fi +fi # Validate required values (paths are derived, so only real config/secrets are required). missing=0 -for v in UC_HOME ALIYUN_REGION ALIYUN_ACCESS_KEY ALIYUN_SECRET_KEY ALIYUN_MASTER_ROLE_ARN \ - UC_AUDIENCES; do +for v in UC_HOME ALIYUN_REGION ALIYUN_ACCESS_KEY ALIYUN_SECRET_KEY UC_AUDIENCES; do if [ -z "${!v:-}" ]; then echo "ERROR: required variable not set: $v"; missing=1; fi done [ "$missing" -eq 0 ] || { echo "Fill the missing variables in $ENV_FILE and re-run."; exit 1; } @@ -51,9 +86,86 @@ done [ -d "$UC_HOME" ] || { echo "ERROR: UC_HOME does not exist: $UC_HOME"; exit 1; } [ -x "$UC_HOME/bin/start-uc-server" ] || { echo "ERROR: $UC_HOME/bin/start-uc-server not found"; exit 1; } [ -f "$UC_EXTERNAL_JWKS_FILE" ] || echo "WARN: JWKS file not found yet: $UC_EXTERNAL_JWKS_FILE (register keys before serving per-user traffic)" +# Not required to start or to vend credentials: the server only echoes it back as a credential's +# unity_catalog_ram_arn, so the caller knows which principal to trust in their own RAM role. +[ -n "$ALIYUN_MASTER_ROLE_ARN" ] || \ + echo "WARN: ALIYUN_MASTER_ROLE_ARN is blank -- credentials will not advertise which RAM principal UC calls AssumeRole as" + +# Check what the UI needs before anything starts. Worth failing early: start-uc-with-ui.sh ties the +# two processes together, so a UI that cannot boot takes the server down with it and the visible +# symptom is a server that exits seconds after coming up. +if [ -n "$ui_on" ]; then + [ -f "$UC_HOME/ui/build/index.html" ] || { + echo "ERROR: the UI is enabled but its build is missing: $UC_HOME/ui/build" + echo " Build it once (ui/build is deliberately not in the repo):" + echo " cd '$UC_HOME/ui' && yarn install && yarn build" + echo " Or set UC_ENABLE_UI=false in $ENV_FILE to run the server on its own." + exit 1 + } + command -v node >/dev/null 2>&1 || { + echo "ERROR: the UI is enabled but 'node' is not on PATH; the UI server is a Node process." + echo " Install Node, or set UC_ENABLE_UI=false in $ENV_FILE to run the server on its own." + exit 1 + } +fi +login_set=0; login_blank=0 +for v in UC_AUTHORIZATION_URL UC_TOKEN_URL UC_CLIENT_ID UC_CLIENT_SECRET; do + if [ -n "${!v}" ]; then login_set=1; else login_blank=1; fi +done +[ "$login_set" -eq 1 ] && [ "$login_blank" -eq 1 ] && \ + echo "WARN: hosted login needs all of UC_AUTHORIZATION_URL/UC_TOKEN_URL/UC_CLIENT_ID/UC_CLIENT_SECRET; only some are set, so it stays OFF" +[ "$UC_AUTHORIZATION" = "enable" ] && [ "$login_set" -eq 0 ] && [ -z "$UC_ADMIN_PASSWORD" ] && \ + echo "WARN: authorization is on but neither Microsoft sign-in nor UC_ADMIN_PASSWORD is set: nobody can sign in to the UI" + +# 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. +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 +[ -n "$ui_on" ] && [ "$UC_AUTHORIZATION" != "enable" ] && \ + echo "WARN: authorization is off and the UI is on, so the UI asks nobody to sign in. Keep port $UI_PORT on a trusted network -- never expose it to the internet." + +# An identity provider's tokens are only accepted when its issuer is trusted and the token's +# audience is expected. Both are lists that the Relyt chain already uses, so the provider's values +# are appended to what is configured rather than replacing it, and each addition is printed. +append_to_list() { # append_to_list VAR value + local var="$1" value="$2" current existing + current="${!var}" # assigned separately: `local` does not have `var` in scope for ${!var} yet + local IFS=',' + for existing in $current; do + if [ "${existing// /}" = "$value" ]; then + return 0 # already configured, by us or by whoever wrote the file + fi + done + printf -v "$var" '%s' "${current:+$current,}$value" + echo " $var += $value" +} + +if [ "$login_set" -eq 1 ] && [ "$login_blank" -eq 0 ]; then + echo "Microsoft sign-in is configured; deriving what the token check needs:" + # The id_token is addressed to the application, so the client id is the audience to expect. + append_to_list UC_AUDIENCES "$UC_CLIENT_ID" + # The issuer is only derived where the authorization URL states it unambiguously: a tenant GUID + # on the v2.0 endpoint. A v1.0 endpoint issues as sts.windows.net, a tenant named by domain does + # not name its GUID, and common/organizations issue a per-tenant value -- guessing any of those + # would put a value nobody wrote into the configuration and fail later as an untrusted issuer. + if [[ "$UC_AUTHORIZATION_URL" =~ ^https://login\.microsoftonline\.com/([0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12})/oauth2/v2\.0/authorize/?$ ]]; then + append_to_list UC_ALLOWED_ISSUERS "https://login.microsoftonline.com/${BASH_REMATCH[1]}/v2.0" + else + echo "WARN: cannot derive the issuer from UC_AUTHORIZATION_URL ($UC_AUTHORIZATION_URL)." + echo " Add the provider's issuer to UC_ALLOWED_ISSUERS yourself. It is the \"issuer\" field of" + echo " /.well-known/openid-configuration, e.g." + echo " https://login.microsoftonline.com//v2.0 for the Microsoft v2.0 endpoint." + fi +fi # Substitute ${VAR} placeholders in a template -> output file (only the known keys). export UC_AUTHORIZATION UC_ALLOWED_ISSUERS UC_EXTERNAL_JWKS_FILE UC_AUDIENCES UC_ACCESS_TOKEN_TTL \ + UC_AUTHORIZATION_URL UC_TOKEN_URL UC_CLIENT_ID UC_CLIENT_SECRET UC_EXTERNAL_URL UC_ADMIN_PASSWORD \ ALIYUN_REGION ALIYUN_ACCESS_KEY ALIYUN_SECRET_KEY ALIYUN_MASTER_ROLE_ARN UC_DB_FILE render() { local tpl="$1" out="$2" @@ -62,8 +174,9 @@ render() { import os, sys tpl, out = sys.argv[1], sys.argv[2] keys = ["UC_AUTHORIZATION", "UC_ALLOWED_ISSUERS", "UC_EXTERNAL_JWKS_FILE", "UC_AUDIENCES", - "UC_ACCESS_TOKEN_TTL", "ALIYUN_REGION", "ALIYUN_ACCESS_KEY", "ALIYUN_SECRET_KEY", - "ALIYUN_MASTER_ROLE_ARN", "UC_DB_FILE"] + "UC_ACCESS_TOKEN_TTL", "UC_AUTHORIZATION_URL", "UC_TOKEN_URL", "UC_CLIENT_ID", + "UC_CLIENT_SECRET", "UC_EXTERNAL_URL", "UC_ADMIN_PASSWORD", "ALIYUN_REGION", "ALIYUN_ACCESS_KEY", + "ALIYUN_SECRET_KEY", "ALIYUN_MASTER_ROLE_ARN", "UC_DB_FILE"] s = open(tpl, encoding="utf-8").read() for k in keys: s = s.replace("${%s}" % k, os.environ.get(k, "")) @@ -71,6 +184,18 @@ open(out, "w", encoding="utf-8").write(s) PY } +# Rendering writes real secrets. With UC_HOME left at the install root, that is the checked-out +# copy of etc/conf, and the next `git add` picks it up: say so rather than leaving it to be noticed +# in a diff. A real deployment points UC_HOME at a persistent path outside any work tree, where +# this check finds nothing. +if command -v git >/dev/null 2>&1 \ + && git -C "$(dirname "$UC_SERVER_PROPERTIES")" ls-files --error-unmatch \ + "$UC_SERVER_PROPERTIES" >/dev/null 2>&1; then + echo "WARN: $UC_SERVER_PROPERTIES is tracked by git, and it is about to hold real secrets." + echo " Point UC_HOME at a path outside the repository, or restore the file when you are done:" + echo " git checkout -- etc/conf/server.properties etc/conf/hibernate.properties" +fi + render "$SP_TEMPLATE" "$UC_SERVER_PROPERTIES" render "$HB_TEMPLATE" "$UC_HIBERNATE_PROPERTIES" mkdir -p "$(dirname "$UC_DB_FILE")" # ensure the H2 dir exists under UC_HOME @@ -85,6 +210,21 @@ port_given=0 for a in "$@"; do case "$a" in -p|--port|--port=*) port_given=1; break ;; esac done +if [ -n "$ui_on" ]; then + # bin/start-uc-with-ui.sh owns both processes: it waits for the server to write its admin token, + # starts the UI against the server's own port (UC_PORT+1, the Armeria port the transcoder fronts), + # and takes one down when the other exits. It reads the port from UC_PORT, so a --port in "$@" + # would leave the UI proxying to a server that is not there -- refuse instead of half-starting. + [ "$port_given" -eq 0 ] || { + echo "ERROR: with the UI enabled, set the port through UC_PORT, not --port." + exit 1 + } + echo "Starting Unity Catalog server + UI from $UC_HOME ..." + echo " server on port $UC_PORT, UI on port $UI_PORT -- browse to the UI" + exec env UC_ENABLE_UI=true UC_HOME="$UC_HOME" UC_PORT="$UC_PORT" UI_PORT="$UI_PORT" \ + bin/start-uc-with-ui.sh "$@" +fi + echo "Starting Unity Catalog server from $UC_HOME ..." if [ "$port_given" -eq 1 ]; then exec bin/start-uc-server "$@" diff --git a/deploy/server.properties.template b/deploy/server.properties.template index b275ed73..da2da5d9 100644 --- a/deploy/server.properties.template +++ b/deploy/server.properties.template @@ -4,11 +4,19 @@ server.env=dev ## Authorization (enable for per-user token-exchange; disable to turn auth off) server.authorization=${UC_AUTHORIZATION} -server.authorization-url= -server.token-url= -server.client-id= -server.client-secret= +# Server-hosted browser login (issue #15). Leave all four blank to turn the flow off; set all four +# to enable "Sign in with Microsoft" in the UI. The client secret never leaves the server. +server.authorization-url=${UC_AUTHORIZATION_URL} +server.token-url=${UC_TOKEN_URL} +server.client-id=${UC_CLIENT_ID} +server.client-secret=${UC_CLIENT_SECRET} server.redirect-port= +# Browser-facing base URL of this server, used to build the OAuth redirect_uri. Optional: when +# blank it is derived from X-Forwarded-Proto/Host (set by the UI proxy) or the request's Host. +server.external-url=${UC_EXTERNAL_URL} +# Password for the built-in "admin" account's UI sign-in (issue #15). Blank = administrator sign-in +# off. The UI's entry point is /login/admin; it is deliberately not linked from the login page. +server.admin-password=${UC_ADMIN_PASSWORD} # OPTIONAL extra trusted issuers (comma-separated). Trusted issuers are derived primarily from the # JWKS file below (each key's "issuer" member, hot-reloaded); this list is a UNION on top, only for diff --git a/deploy/uc.env.example b/deploy/uc.env.example index a7d3046e..a8bb47f2 100644 --- a/deploy/uc.env.example +++ b/deploy/uc.env.example @@ -10,8 +10,8 @@ UC_HOME= # --- Listen port --- -# Port the UC server listens on. 8088 matches bin/start-uc-with-ui.sh and the phoenix/presto e2e -# suites; change it only if you have a conflict. An explicit --port passed to deploy-uc.sh wins. +# Port the UC server listens on. 8088 is what the phoenix/presto e2e suites expect; change it only +# if you have a conflict. An explicit --port passed to deploy-uc.sh wins. UC_PORT=8088 # --- Auth / token-exchange --- @@ -26,8 +26,67 @@ UC_ALLOWED_ISSUERS= # Expected audience of the subject tokens (must match the signer's unity.audience). UC_AUDIENCES=unitycatalog-server # Exchanged access-token TTL (ISO-8601). The server treats a blank value as "never expires"; -# this sample ships PT1H (1 hour) on purpose — keep it unless you have a reason to opt out. -UC_ACCESS_TOKEN_TTL=PT1H +# this sample ships PT12H (12 hours) on purpose — keep it unless you have a reason to opt out. +UC_ACCESS_TOKEN_TTL=PT12H + +# --- UI sign-in (issue #15) --- +# The UI requires sign-in. Two entry points, both decided by these settings at runtime (no UI +# rebuild): "Sign in with Microsoft" on the login page, and the administrator password at +# /login/admin (not linked from the login page). + +# Administrator password. Blank = administrator sign-in off. Keep it strong: it is compared in +# constant time but not rate-limited beyond a short delay per failure. +UC_ADMIN_PASSWORD= + +# Microsoft Entra ID sign-in, optional. Leave all four blank to offer only the administrator +# sign-in. To enable, register an application in Entra (redirect URI = +# /api/1.0/unity-control/auth/callback), then: +# - configure the "email" optional claim, or accounts cannot be matched to UC users +# - create each employee in UC (POST /scim2/Users) with their Microsoft email; grants decide +# which catalogs / schemas / tables they see +# deploy-uc.sh derives what the token check needs from these four and prints each addition: the +# client id joins UC_AUDIENCES (an id_token is addressed to the application), and the issuer joins +# UC_ALLOWED_ISSUERS when the authorization URL names a tenant GUID on the v2.0 endpoint. Anything +# else -- a v1.0 endpoint, a tenant named by domain, common/organizations -- issues under a +# different name, so the script says so and leaves the issuer to you. +# +# Shape of the four values: +# UC_AUTHORIZATION_URL=https://login.microsoftonline.com/TENANTID/oauth2/v2.0/authorize +# UC_TOKEN_URL=https://login.microsoftonline.com/TENANTID/oauth2/v2.0/token +# UC_CLIENT_ID=CLIENTID +# UC_CLIENT_SECRET=CLIENTSECRET +# TENANTID and CLIENTID are both GUIDs but name different things -- never paste one for the other: +# TENANTID the customer's Entra tenant, i.e. the organization. Appears only inside the two +# URLs above. Public: read it, and both endpoints, off the customer's email domain +# without signing in -- authorization_endpoint, token_endpoint and the GUID inside +# issuer, from +# curl -s https://login.microsoftonline.com//v2.0/.well-known/openid-configuration +# It must be that GUID: a domain name in its place still signs people in, but stops +# the issuer from being derived (see above). +# CLIENTID the application registered inside that tenant -- "Application (client) ID" in the +# portal. One tenant holds many; it exists only once someone registers the app. The +# id_token is addressed to it, which is why deploy-uc.sh appends it to UC_AUDIENCES. +# CLIENTSECRET that same application's secret VALUE, shown once at creation -- not the Secret ID +# displayed next to it. +# +# Leave the four below empty to keep Microsoft sign-in off. Setting only some of them does not +# half-enable it: deploy-uc.sh warns and the flow stays off until all four are set. +UC_AUTHORIZATION_URL= +UC_TOKEN_URL= +UC_CLIENT_ID= +UC_CLIENT_SECRET= +# Browser-facing base URL of UC (scheme://host[:port], no path). It builds the OAuth redirect_uri +# and decides whether the session cookie is marked Secure. NORMALLY LEAVE BLANK: the server derives +# the browser-facing origin from X-Forwarded-Proto/Host, which the deployment puts there. Set it +# only when those headers do not reach the server -- a proxy that strips them, or a TLS terminator +# that never adds them. Two ways to get it wrong, both silent: +# - a scheme that does not match reality: https here over plain HTTP makes the browser drop the +# session cookie (sign-in appears to succeed and lands back on the login page); http here over +# HTTPS leaves the cookie without Secure +# - anything after the host: the redirect_uri no longer equals the one registered in Entra, which +# answers AADSTS50011 -- deploy-uc.sh rejects a path outright +# UC_EXTERNAL_URL=https://uc.example.com +UC_EXTERNAL_URL= # --- Aliyun OSS (master RAM identity for STS AssumeRole) --- ALIYUN_REGION=cn-hangzhou @@ -39,8 +98,15 @@ ALIYUN_SECRET_KEY= ALIYUN_MASTER_ROLE_ARN=acs:ram:::user/ # --- Path overrides (optional) --- -# All default to under UC_HOME; set only to relocate an individual file off UC_HOME. -# UC_SERVER_PROPERTIES=$UC_HOME/etc/conf/server.properties -# UC_HIBERNATE_PROPERTIES=$UC_HOME/etc/conf/hibernate.properties +# These two are genuinely relocatable: their paths are recorded as values inside the rendered +# config, so the server reads them wherever you point them. # UC_EXTERNAL_JWKS_FILE=$UC_HOME/etc/conf/relyt_jwks.json # UC_DB_FILE=$UC_HOME/etc/db/h2db +# +# These two are NOT. The server opens "etc/conf/server.properties" and "etc/conf/hibernate.properties" +# relative to its working directory, which is UC_HOME — the paths are compiled in, not configurable. +# The variables below only move where deploy-uc.sh WRITES the rendered files, so setting them +# renders the config somewhere the server never looks and it starts with the old one. Move UC_HOME +# instead; they are listed here only to say why not to use them. +# UC_SERVER_PROPERTIES=$UC_HOME/etc/conf/server.properties +# UC_HIBERNATE_PROPERTIES=$UC_HOME/etc/conf/hibernate.properties diff --git a/server/src/main/java/io/unitycatalog/server/URLTranscoderVerticle.java b/server/src/main/java/io/unitycatalog/server/URLTranscoderVerticle.java index aac2c6d4..4d7ab304 100644 --- a/server/src/main/java/io/unitycatalog/server/URLTranscoderVerticle.java +++ b/server/src/main/java/io/unitycatalog/server/URLTranscoderVerticle.java @@ -40,6 +40,10 @@ public void start() { String path = transcodeRequest.path().replace("%1F", "."); HttpRequest serviceRequest = client.request(method, servicePort, host, path); + // A redirect is an answer to pass back, not one to chase: the sign-in flow + // sends the browser to an identity provider, and following it here would + // return that provider's page to the browser instead of the redirect. + serviceRequest.followRedirects(false); serviceRequest.putHeaders(transcodeRequest.headers()); for (Map.Entry entry : transcodeRequest.params()) { serviceRequest.addQueryParam( @@ -54,7 +58,10 @@ public void start() { for (Map.Entry entry : resp.headers()) { transcodeResp.putHeader(entry.getKey(), entry.getValue()); } - return transcodeResp.end(resp.bodyAsBuffer()); + // A redirect, a 304 or an answer to HEAD carries no body, and ending with a + // null buffer leaves the caller waiting forever. The headers are the answer. + Buffer body = resp.bodyAsBuffer(); + return body == null ? transcodeResp.end() : transcodeResp.end(body); }); }); diff --git a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java index 7771a6e6..0479de8f 100644 --- a/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java +++ b/server/src/main/java/io/unitycatalog/server/UnityCatalogServer.java @@ -320,6 +320,12 @@ private void addSecurityDecorators( .routeDecorator() .pathPrefix(CONTROL_PATH) .exclude(CONTROL_PATH + "auth/tokens") + // The hosted login flow is what produces a token; it cannot require one. + .exclude(CONTROL_PATH + "auth/login") + .exclude(CONTROL_PATH + "auth/callback") + .exclude(CONTROL_PATH + "auth/admin/login") + .exclude(CONTROL_PATH + "auth/token/login") + .exclude(CONTROL_PATH + "auth/providers") .build(accessDecorator); AuthDecorator authDecorator = new AuthDecorator(securityContext, repositories); @@ -328,6 +334,11 @@ private void addSecurityDecorators( .routeDecorator() .pathPrefix(CONTROL_PATH) .exclude(CONTROL_PATH + "auth/tokens") + .exclude(CONTROL_PATH + "auth/login") + .exclude(CONTROL_PATH + "auth/callback") + .exclude(CONTROL_PATH + "auth/admin/login") + .exclude(CONTROL_PATH + "auth/token/login") + .exclude(CONTROL_PATH + "auth/providers") .build(authDecorator); ExceptionHandlingDecorator exceptionDecorator = diff --git a/server/src/main/java/io/unitycatalog/server/security/JwtClaim.java b/server/src/main/java/io/unitycatalog/server/security/JwtClaim.java index 680461c8..b593bde8 100644 --- a/server/src/main/java/io/unitycatalog/server/security/JwtClaim.java +++ b/server/src/main/java/io/unitycatalog/server/security/JwtClaim.java @@ -5,6 +5,11 @@ public enum JwtClaim { ISSUER("iss"), SUBJECT("sub"), EMAIL("email"), + // Sign-in name claims consulted when resolving the caller to a local user (see + // AuthService#resolvePrincipal): Entra ID does not send "email" unless it is configured as an + // optional claim, so "preferred_username" / "upn" serve as fallbacks. + PREFERRED_USERNAME("preferred_username"), + UPN("upn"), EXPIRATION("exp"), ISSUED_AT("iat"), JWT_ID("jti"), diff --git a/server/src/main/java/io/unitycatalog/server/security/SecurityContext.java b/server/src/main/java/io/unitycatalog/server/security/SecurityContext.java index 37177ee5..460f98ad 100644 --- a/server/src/main/java/io/unitycatalog/server/security/SecurityContext.java +++ b/server/src/main/java/io/unitycatalog/server/security/SecurityContext.java @@ -79,7 +79,16 @@ public String createAccessToken(DecodedJWT decodedJWT, java.time.Duration ttl) { .getClaims() .getOrDefault(JwtClaim.EMAIL.key(), decodedJWT.getClaim(JwtClaim.SUBJECT.key())) .asString(); + return createAccessToken(subject, ttl); + } + /** + * Issues an access token whose {@code sub} is the given principal. The token exchange resolves + * the incoming identity to a local user first and passes that user's email here, so {@code sub} + * is always the key the request path looks users up by (AuthDecorator, UserRepository) regardless + * of which claim the identity provider identified the caller with. + */ + public String createAccessToken(String subject, java.time.Duration ttl) { Date now = new Date(); var builder = JWT.create() diff --git a/server/src/main/java/io/unitycatalog/server/service/AuthService.java b/server/src/main/java/io/unitycatalog/server/service/AuthService.java index ed845cb1..0d69ce88 100644 --- a/server/src/main/java/io/unitycatalog/server/service/AuthService.java +++ b/server/src/main/java/io/unitycatalog/server/service/AuthService.java @@ -5,21 +5,26 @@ import com.auth0.jwt.exceptions.JWTDecodeException; import com.auth0.jwt.exceptions.JWTVerificationException; import com.auth0.jwt.interfaces.DecodedJWT; +import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; import com.linecorp.armeria.common.AggregatedHttpRequest; import com.linecorp.armeria.common.Cookie; import com.linecorp.armeria.common.HttpData; import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; import com.linecorp.armeria.common.HttpRequest; import com.linecorp.armeria.common.HttpResponse; import com.linecorp.armeria.common.HttpStatus; import com.linecorp.armeria.common.MediaType; import com.linecorp.armeria.common.QueryParams; +import com.linecorp.armeria.common.RequestHeaders; import com.linecorp.armeria.common.ResponseHeaders; import com.linecorp.armeria.common.ResponseHeadersBuilder; import com.linecorp.armeria.common.annotation.Nullable; import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.ExceptionHandler; +import com.linecorp.armeria.server.annotation.Get; import com.linecorp.armeria.server.annotation.Param; import com.linecorp.armeria.server.annotation.Post; import com.linecorp.armeria.server.annotation.RequestConverter; @@ -42,13 +47,18 @@ import io.unitycatalog.server.utils.ServerProperties; import io.unitycatalog.server.utils.ServerProperties.Property; import java.lang.reflect.ParameterizedType; +import java.net.URLEncoder; import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; import java.time.Duration; +import java.time.Instant; import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.UUID; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -64,6 +74,23 @@ public class AuthService { private static final String EMPTY_RESPONSE = "{}"; + /** Binds the browser's /login round-trip to its /callback: "|". */ + private static final String OAUTH_STATE_COOKIE = "UC_OAUTH_STATE"; + + private static final String OAUTH_STATE_TTL = "PT10M"; + private static final String OAUTH_SCOPE = "openid profile email"; + private static final Duration IDP_TIMEOUT = Duration.ofSeconds(10); + private static final ObjectMapper JSON = new ObjectMapper(); + + /** The only account the password sign-in accepts; it is the bootstrap administrator's email. */ + private static final String ADMIN_USERNAME = "admin"; + + /** Slows password guessing without a lockout table; the handler is @Blocking. */ + private static final Duration FAILED_LOGIN_DELAY = Duration.ofSeconds(1); + + private final WebClient idpClient = + WebClient.builder().responseTimeout(IDP_TIMEOUT).writeTimeout(IDP_TIMEOUT).build(); + public AuthService( SecurityContext securityContext, ServerProperties serverProperties, @@ -106,6 +133,7 @@ public AuthService( @Post("/tokens") @com.linecorp.armeria.server.annotation.Blocking public HttpResponse grantToken( + ServiceRequestContext ctx, @Param("ext") Optional ext, @RequestConverter(ToOAuthTokenExchangeFormConverter.class) OAuthTokenExchangeForm form) { LOGGER.debug("Got token: {}", form); @@ -131,6 +159,36 @@ public HttpResponse grantToken( ErrorCode.INVALID_ARGUMENT, "Actor tokens not currently supported"); } + String accessToken = exchangeSubjectToken(form.getSubjectToken()); + + OAuthTokenExchangeInfo tokenExchangeInfo = + new OAuthTokenExchangeInfo() + .accessToken(accessToken) + .issuedTokenType(TokenType.ACCESS_TOKEN) + .tokenType(AccessTokenType.BEARER); + + // Set token as cookie if ext param is set to cookie + ResponseHeadersBuilder responseHeaders = ResponseHeaders.builder(HttpStatus.OK); + ext.ifPresent( + e -> { + if (e.equals(TokenEndpointExtensionType.COOKIE)) { + // Set cookie timeout to 5 days by default if not present in server.properties + String cookieTimeout = this.serverProperties.get(Property.COOKIE_TIMEOUT); + Cookie cookie = + createCookie(ctx, AuthDecorator.UC_TOKEN_KEY, accessToken, "/", cookieTimeout); + responseHeaders.add(HttpHeaderNames.SET_COOKIE, cookie.toSetCookieHeader()); + } + }); + + return HttpResponse.ofJson(responseHeaders.build(), tokenExchangeInfo); + } + + /** + * Verifies an identity token from a trusted issuer and issues a UC access token for the local + * user it names. Shared by the token-exchange endpoint and the server-hosted login callback, so + * both admit callers under exactly the same rules. + */ + private String exchangeSubjectToken(String subjectToken) { boolean authorizationEnabled = this.serverProperties.isAuthorizationEnabled(); if (!authorizationEnabled) { throw new OAuthInvalidRequestException( @@ -164,7 +222,7 @@ public HttpResponse grantToken( DecodedJWT decodedJWT; try { - decodedJWT = JWT.decode(form.getSubjectToken()); + decodedJWT = JWT.decode(subjectToken); } catch (JWTDecodeException e) { LOGGER.debug("Token rejected: malformed token", e); throw new OAuthInvalidRequestException( @@ -195,43 +253,446 @@ public HttpResponse grantToken( ErrorCode.UNAUTHENTICATED, "Token verification failed: " + e.getMessage(), e); } - verifyPrincipal(decodedJWT); + User user = resolvePrincipal(decodedJWT); LOGGER.debug("Validated. Creating access token."); - String accessToken = - securityContext.createAccessToken(decodedJWT, serverProperties.getAccessTokenTtl()); + // The exchanged token carries the LOCAL user's email as sub, whatever claim identified the + // caller. Everything downstream keys off that email, so the request path is untouched by + // which claim matched. + return securityContext.createAccessToken(user.getEmail(), serverProperties.getAccessTokenTtl()); + } - OAuthTokenExchangeInfo tokenExchangeInfo = + /** + * Server-hosted OAuth login, step one: send the browser to the identity provider (issue #15). + * + *

The browser is redirected -- the server never contacts the provider here. A one-time state + * is bound to a short-lived cookie so the callback can tell this login apart from a forged one. + * The client secret stays on the server; the UI only needs a link to this endpoint. + * + * @param redirect UI path to return to after login; anything but a relative path falls back to / + */ + @Get("/login") + public HttpResponse login( + ServiceRequestContext ctx, + HttpRequest request, + @Param("redirect") Optional redirect) { + requireHostedLoginConfigured(); + + String state = UUID.randomUUID().toString(); + String returnTo = safeReturnPath(redirect.orElse(null)); + String authorizationUrl = serverProperties.getAuthorizationUrl(); + String location = + authorizationUrl + + (authorizationUrl.contains("?") ? "&" : "?") + + "client_id=" + + urlEncode(serverProperties.getClientId()) + + "&response_type=code&response_mode=query" + + "&redirect_uri=" + + urlEncode(callbackUrl(ctx, request)) + + "&scope=" + + urlEncode(OAUTH_SCOPE) + + "&state=" + + urlEncode(state); + + Cookie stateCookie = + createCookie( + ctx, OAUTH_STATE_COOKIE, state + "|" + returnTo, authMountPath(ctx), OAUTH_STATE_TTL); + return HttpResponse.of( + ResponseHeaders.builder(HttpStatus.FOUND) + .add(HttpHeaderNames.LOCATION, location) + .add(HttpHeaderNames.SET_COOKIE, stateCookie.toSetCookieHeader()) + .build()); + } + + /** + * Server-hosted OAuth login, step two: the provider has sent the browser back with a code. This + * is the one place the server itself calls the provider -- redeeming the code with the client + * secret -- after which the resulting id_token goes through the same verification as a token + * exchange and the UC access token is set as the session cookie. + */ + @Get("/callback") + @com.linecorp.armeria.server.annotation.Blocking + public HttpResponse loginCallback( + ServiceRequestContext ctx, + HttpRequest request, + @Param("code") Optional code, + @Param("state") Optional state, + @Param("error") Optional error, + @Param("error_description") Optional errorDescription) { + requireHostedLoginConfigured(); + + if (error.isPresent()) { + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, + "Identity provider returned " + error.get() + ": " + errorDescription.orElse("")); + } + + String stateCookie = readCookie(request, OAUTH_STATE_COOKIE); + if (stateCookie == null) { + throw new OAuthInvalidRequestException( + ErrorCode.INVALID_ARGUMENT, "Missing login state; start again from /login"); + } + int separator = stateCookie.indexOf('|'); + String expectedState = separator < 0 ? stateCookie : stateCookie.substring(0, separator); + String returnTo = separator < 0 ? "/" : safeReturnPath(stateCookie.substring(separator + 1)); + if (state.isEmpty() || !expectedState.equals(state.get())) { + throw new OAuthInvalidRequestException(ErrorCode.INVALID_ARGUMENT, "Login state mismatch"); + } + if (code.isEmpty() || code.get().isBlank()) { + throw new OAuthInvalidRequestException( + ErrorCode.INVALID_ARGUMENT, "Missing authorization code"); + } + + String idToken = redeemAuthorizationCode(code.get(), callbackUrl(ctx, request)); + String accessToken = exchangeSubjectToken(idToken); + + String cookieTimeout = this.serverProperties.get(Property.COOKIE_TIMEOUT); + Cookie sessionCookie = + createCookie(ctx, AuthDecorator.UC_TOKEN_KEY, accessToken, "/", cookieTimeout); + Cookie clearedState = createCookie(ctx, OAUTH_STATE_COOKIE, "", authMountPath(ctx), "PT0S"); + return HttpResponse.of( + ResponseHeaders.builder(HttpStatus.FOUND) + .add(HttpHeaderNames.LOCATION, returnTo) + .add(HttpHeaderNames.SET_COOKIE, sessionCookie.toSetCookieHeader()) + .add(HttpHeaderNames.SET_COOKIE, clearedState.toSetCookieHeader()) + .build()); + } + + private void requireHostedLoginConfigured() { + if (!serverProperties.isHostedLoginConfigured()) { + throw new OAuthInvalidRequestException( + ErrorCode.INVALID_ARGUMENT, + "Hosted login is not configured: set server.authorization-url, server.token-url, " + + "server.client-id and server.client-secret"); + } + } + + /** Exchanges the authorization code at the provider's token endpoint for an id_token. */ + private String redeemAuthorizationCode(String code, String redirectUri) { + String form = + "grant_type=authorization_code" + + "&code=" + + urlEncode(code) + + "&redirect_uri=" + + urlEncode(redirectUri) + + "&client_id=" + + urlEncode(serverProperties.getClientId()) + + "&client_secret=" + + urlEncode(serverProperties.getClientSecret()) + + "&scope=" + + urlEncode(OAUTH_SCOPE); + RequestHeaders headers = + RequestHeaders.of( + HttpMethod.POST, + serverProperties.getTokenUrl(), + HttpHeaderNames.CONTENT_TYPE, + MediaType.FORM_DATA); + var response = idpClient.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + if (!response.status().isSuccess()) { + // This is where an expired client secret surfaces, and it surfaces late: the person has + // already signed in at the provider. Name the provider's own error code, or a lapsed secret, + // a redirect URI that no longer matches and a replayed code all read as the same failure. + String reason = oauthErrorCode(response.contentUtf8()); + LOGGER.warn( + "Code redemption failed: {} {}", response.status(), abbreviate(response.contentUtf8())); + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, + "Identity provider rejected the authorization code: " + + response.status() + + (reason.isEmpty() ? "" : " (" + reason + ")")); + } + try { + JsonNode body = JSON.readTree(response.contentUtf8()); + JsonNode idToken = body.get("id_token"); + if (idToken == null || idToken.asText().isBlank()) { + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, + "Identity provider did not return an id_token; is the openid scope granted?"); + } + return idToken.asText(); + } catch (java.io.IOException e) { + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, "Unreadable response from the identity provider", e); + } + } + + /** + * The provider's own name for the failure, from the OAuth error response: its {@code error} field + * plus the first provider-specific code in {@code error_description} (Entra ID states + * AADSTS7000222 for an expired client secret, AADSTS50011 for a redirect URI that does not + * match). The description itself is left to the log: it runs to several lines and carries a + * correlation id, and the browser only needs enough to tell the causes apart. + */ + private static String oauthErrorCode(String body) { + if (body == null || body.isBlank()) { + return ""; + } + try { + JsonNode json = JSON.readTree(body); + String error = json.path("error").asText(""); + java.util.regex.Matcher m = + java.util.regex.Pattern.compile("[A-Z]{2,}\\d{4,}") + .matcher(json.path("error_description").asText("")); + String providerCode = m.find() ? m.group() : ""; + return Stream.of(error, providerCode) + .filter(v -> !v.isBlank()) + .collect(Collectors.joining(", ")); + } catch (java.io.IOException e) { + return ""; + } + } + + private static String abbreviate(String body) { + if (body == null) { + return ""; + } + String oneLine = body.replaceAll("\\s+", " ").trim(); + return oneLine.length() <= 500 ? oneLine : oneLine.substring(0, 500) + "…"; + } + + /** + * The absolute URL of /callback as the BROWSER must reach it. Behind the UI's reverse proxy the + * server sees the proxy's host, not the one in the address bar, so honour server.external-url + * first, then X-Forwarded-Proto/Host, and only then the request's own scheme and Host. + */ + private String callbackUrl(ServiceRequestContext ctx, HttpRequest request) { + String base = serverProperties.getExternalUrl(); + if (base == null) { + String forwardedProto = firstValue(request.headers().get(HttpHeaderNames.X_FORWARDED_PROTO)); + String forwardedHost = firstValue(request.headers().get(HttpHeaderNames.X_FORWARDED_HOST)); + String scheme = + forwardedProto != null + ? forwardedProto + : (ctx.sessionProtocol().isTls() ? "https" : "http"); + String host = forwardedHost != null ? forwardedHost : request.headers().authority(); + base = scheme + "://" + host; + } + return base + authMountPath(ctx) + "/callback"; + } + + /** The path this service is mounted at (e.g. /api/1.0/unity-control/auth), from the request. */ + private static String authMountPath(ServiceRequestContext ctx) { + String path = ctx.path(); + int lastSlash = path.lastIndexOf('/'); + return lastSlash > 0 ? path.substring(0, lastSlash) : ""; + } + + /** Only a relative in-app path may be returned to after login; anything else goes to /. */ + private static String safeReturnPath(String candidate) { + if (candidate == null || candidate.isBlank()) { + return "/"; + } + // "//host" is scheme-relative and would leave the site; so would a scheme or backslashes. + if (!candidate.startsWith("/") || candidate.startsWith("//") || candidate.contains("\\")) { + return "/"; + } + return candidate; + } + + private static String firstValue(String headerValue) { + if (headerValue == null || headerValue.isBlank()) { + return null; + } + int comma = headerValue.indexOf(','); + return (comma < 0 ? headerValue : headerValue.substring(0, comma)).trim(); + } + + private static String readCookie(HttpRequest request, String name) { + return request.headers().cookies().stream() + .filter(c -> c.name().equals(name)) + .map(Cookie::value) + .findFirst() + .orElse(null); + } + + private static String urlEncode(String value) { + return URLEncoder.encode(value, StandardCharsets.UTF_8); + } + + /** + * Which sign-in methods this server offers. Public: the UI reads it before anyone has signed in, + * to decide whether to show a login page at all and which entry points to put on it, so a single + * UI build follows the server's configuration. Nothing here is secret -- three booleans saying + * which documented features are switched on. + */ + @Get("/providers") + public HttpResponse authProviders() { + return HttpResponse.ofJson( + Map.of( + "authorization_enabled", serverProperties.isAuthorizationEnabled(), + "hosted_login", serverProperties.isHostedLoginConfigured(), + "admin_login", serverProperties.getAdminPassword() != null)); + } + + /** + * The administrator's password sign-in for the UI (issue #15). Only the built-in "admin" account + * signs in this way; everyone else uses their identity provider. The password from + * server.properties is compared in constant time and a failure is delayed. Success ends exactly + * where a Microsoft sign-in does -- a UC access token in the UC_TOKEN cookie -- so nothing + * downstream distinguishes the two sessions. + */ + @Post("/admin/login") + @com.linecorp.armeria.server.annotation.Blocking + public HttpResponse adminLogin(ServiceRequestContext ctx, AggregatedHttpRequest request) { + String configured = serverProperties.getAdminPassword(); + if (configured == null) { + throw new OAuthInvalidRequestException( + ErrorCode.INVALID_ARGUMENT, + "Administrator sign-in is not configured: set server.admin-password"); + } + + Map form = formFields(request); + String username = form.getOrDefault("username", ""); + String password = form.getOrDefault("password", ""); + + // Both comparisons always run so timing does not reveal which one failed. + boolean usernameMatches = constantTimeEquals(ADMIN_USERNAME, username); + boolean passwordMatches = constantTimeEquals(configured, password); + if (!usernameMatches || !passwordMatches) { + LOGGER.warn("Administrator sign-in rejected for username '{}'", username); + sleepQuietly(FAILED_LOGIN_DELAY); + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, "Wrong username or password"); + } + + String accessToken = + securityContext.createAccessToken(ADMIN_USERNAME, serverProperties.getAccessTokenTtl()); + OAuthTokenExchangeInfo info = new OAuthTokenExchangeInfo() .accessToken(accessToken) .issuedTokenType(TokenType.ACCESS_TOKEN) .tokenType(AccessTokenType.BEARER); + String cookieTimeout = this.serverProperties.get(Property.COOKIE_TIMEOUT); + Cookie session = createCookie(ctx, AuthDecorator.UC_TOKEN_KEY, accessToken, "/", cookieTimeout); + return HttpResponse.ofJson( + ResponseHeaders.builder(HttpStatus.OK) + .add(HttpHeaderNames.SET_COOKIE, session.toSetCookieHeader()) + .build(), + info); + } - // Set token as cookie if ext param is set to cookie - ResponseHeadersBuilder responseHeaders = ResponseHeaders.builder(HttpStatus.OK); - ext.ifPresent( - e -> { - if (e.equals(TokenEndpointExtensionType.COOKIE)) { - // Set cookie timeout to 5 days by default if not present in server.properties - String cookieTimeout = this.serverProperties.get(Property.COOKIE_TIMEOUT); - Cookie cookie = - createCookie(AuthDecorator.UC_TOKEN_KEY, accessToken, "/", cookieTimeout); - responseHeaders.add(HttpHeaderNames.SET_COOKIE, cookie.toSetCookieHeader()); - } - }); + /** + * Signs a browser in with a Unity Catalog access token the operator already holds -- the one in + * etc/conf/token.txt, typically. It replaces the proxy's blanket token injection, which made the + * application's own address an unauthenticated way in: the token now has to be presented at an + * address of its own, and it buys exactly the identity the token names. + * + *

The token is checked the way every request is checked -- issued by this server, signature + * valid, not expired, naming an enabled user -- and is then handed back as the session cookie + * itself. Nothing is minted here, so the session cannot outlive the token it came from. + */ + @Post("/token/login") + @com.linecorp.armeria.server.annotation.Blocking + public HttpResponse tokenLogin(ServiceRequestContext ctx, AggregatedHttpRequest request) { + String token = formFields(request).getOrDefault("token", "").trim(); + if (token.isEmpty()) { + throw new OAuthInvalidRequestException(ErrorCode.INVALID_ARGUMENT, "No token supplied"); + } - return HttpResponse.ofJson(responseHeaders.build(), tokenExchangeInfo); + DecodedJWT decodedJWT; + try { + decodedJWT = JWT.decode(token); + } catch (JWTDecodeException e) { + throw rejectToken("not a token", e); + } + + // Only this server's own tokens open a session. A token from an identity provider goes through + // /auth/tokens instead, which is where issuer and audience are checked. + if (!SecurityContext.Issuers.INTERNAL.equals(decodedJWT.getIssuer())) { + throw rejectToken("issuer '" + decodedJWT.getIssuer() + "' is not this server", null); + } + + try { + JWTVerifier verifier = + jwksOperations.verifierForIssuerAndKey( + SecurityContext.Issuers.INTERNAL, + decodedJWT.getKeyId(), + decodedJWT.getAlgorithm(), + List.of()); + decodedJWT = verifier.verify(decodedJWT); + } catch (JWTVerificationException e) { + throw rejectToken("verification failed", e); + } + + String subject = decodedJWT.getSubject(); + User user = enabledUserOrNull(() -> userRepository.getUserByEmail(subject)); + if (user == null) { + throw rejectToken("no enabled user named '" + subject + "'", null); + } + + LOGGER.info("Token sign-in accepted for {}", user.getEmail()); + OAuthTokenExchangeInfo info = + new OAuthTokenExchangeInfo() + .accessToken(token) + .issuedTokenType(TokenType.ACCESS_TOKEN) + .tokenType(AccessTokenType.BEARER); + Cookie session = + createCookie(ctx, AuthDecorator.UC_TOKEN_KEY, token, "/", sessionSeconds(decodedJWT)); + return HttpResponse.ofJson( + ResponseHeaders.builder(HttpStatus.OK) + .add(HttpHeaderNames.SET_COOKIE, session.toSetCookieHeader()) + .build(), + info); + } + + /** + * One rejection for every way a presented token can be wrong, so a caller cannot tell them apart; + * the reason is logged rather than returned. Delayed like a wrong password: this endpoint is + * public and the token is the only secret it asks for. + */ + private OAuthInvalidRequestException rejectToken(String reason, Exception cause) { + LOGGER.warn("Token sign-in rejected: {}", reason); + sleepQuietly(FAILED_LOGIN_DELAY); + String message = "Token is not valid for sign-in"; + return cause == null + ? new OAuthInvalidRequestException(ErrorCode.UNAUTHENTICATED, message) + : new OAuthInvalidRequestException(ErrorCode.UNAUTHENTICATED, message, cause); + } + + /** The session may last as long as the configured cookie timeout, but never past the token. */ + private long sessionSeconds(DecodedJWT decodedJWT) { + long configured = Duration.parse(serverProperties.get(Property.COOKIE_TIMEOUT)).getSeconds(); + if (decodedJWT.getExpiresAt() == null) { + return configured; + } + long untilExpiry = + Duration.between(Instant.now(), decodedJWT.getExpiresAt().toInstant()).getSeconds(); + return Math.max(0, Math.min(configured, untilExpiry)); + } + + private static Map formFields(AggregatedHttpRequest request) { + MediaType contentType = request.contentType(); + if (contentType == null || !contentType.belongsTo(MediaType.FORM_DATA)) { + return Map.of(); + } + return QueryParams.fromQueryString(request.content(contentType.charset(StandardCharsets.UTF_8))) + .stream() + .collect( + Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (first, second) -> first)); + } + + private static boolean constantTimeEquals(String expected, String actual) { + return MessageDigest.isEqual( + expected.getBytes(StandardCharsets.UTF_8), actual.getBytes(StandardCharsets.UTF_8)); + } + + private static void sleepQuietly(Duration duration) { + try { + Thread.sleep(duration.toMillis()); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } } @Post("/logout") - public HttpResponse logout(HttpRequest request) { + public HttpResponse logout(ServiceRequestContext ctx, HttpRequest request) { return request.headers().cookies().stream() .filter(c -> c.name().equals(AuthDecorator.UC_TOKEN_KEY)) .findFirst() .map( authorizationCookie -> { - Cookie expiredCookie = createCookie(AuthDecorator.UC_TOKEN_KEY, "", "/", "PT0S"); + Cookie expiredCookie = createCookie(ctx, AuthDecorator.UC_TOKEN_KEY, "", "/", "PT0S"); ResponseHeaders headers = ResponseHeaders.builder() .status(HttpStatus.OK) @@ -244,41 +705,91 @@ public HttpResponse logout(HttpRequest request) { .orElse(HttpResponse.of(HttpStatus.OK, MediaType.JSON, EMPTY_RESPONSE)); } - private void verifyPrincipal(DecodedJWT decodedJWT) { - String subject = - decodedJWT - .getClaims() - .getOrDefault(JwtClaim.EMAIL.key(), decodedJWT.getClaim(JwtClaim.SUBJECT.key())) - .asString(); + /** + * Maps the verified identity token to a local, enabled user. Claims are tried in order and the + * first that names a user wins: {@code email}; then {@code preferred_username} and {@code upn}, + * because Entra ID omits {@code email} unless it is configured as an optional claim and the + * sign-in name is what identifies the account; then {@code sub}, which is where Relyt instances + * and the bootstrap admin carry the principal. Every lookup is by the user's email. There is no + * just-in-time provisioning: the user must already exist. + */ + private User resolvePrincipal(DecodedJWT decodedJWT) { + String identifier = null; + for (JwtClaim claim : + List.of(JwtClaim.EMAIL, JwtClaim.PREFERRED_USERNAME, JwtClaim.UPN, JwtClaim.SUBJECT)) { + String value = claimAsString(decodedJWT, claim); + if (value == null) { + continue; + } + identifier = identifier == null ? value : identifier; + User user = enabledUserOrNull(() -> userRepository.getUserByEmail(value)); + if (user != null) { + LOGGER.debug("Principal resolved by {}: {}", claim.key(), value); + return user; + } + } - LOGGER.debug("Validating principal: {}", subject); + throw new OAuthInvalidRequestException( + ErrorCode.INVALID_ARGUMENT, "User not allowed: " + identifier); + } - if (subject.equals("admin")) { - LOGGER.debug("admin always allowed"); - return; - } + private static String claimAsString(DecodedJWT decodedJWT, JwtClaim claim) { + String value = decodedJWT.getClaim(claim.key()).asString(); + return value == null || value.isBlank() ? null : value; + } + /** The user if the lookup succeeds and the account is enabled; null otherwise. */ + private static User enabledUserOrNull(java.util.function.Supplier lookup) { try { - User user = userRepository.getUserByEmail(subject); - if (user != null && user.getState() == User.StateEnum.ENABLED) { - LOGGER.debug("Principal {} is enabled", subject); - return; - } + User user = lookup.get(); + return user != null && user.getState() == User.StateEnum.ENABLED ? user : null; } catch (Exception e) { - // IGNORE + return null; } + } - throw new OAuthInvalidRequestException( - ErrorCode.INVALID_ARGUMENT, "User not allowed: " + subject); + private Cookie createCookie( + ServiceRequestContext ctx, String key, String value, String path, String maxAge) { + return createCookie(ctx, key, value, path, Duration.parse(maxAge).getSeconds()); } - private Cookie createCookie(String key, String value, String path, String maxAge) { - return Cookie.secureBuilder(key, value) + /** + * A session cookie the browser will actually keep and send back. + * + *

Secure follows the address bar rather than being asserted unconditionally: a browser drops a + * Secure cookie arriving over plain HTTP, which is how the UI is reached in a local or + * internal-network deployment, and the sign-in then appears to succeed while leaving no session. + * + *

SameSite is Lax rather than Strict because the browser comes back from the identity provider + * on a cross-site navigation: a Strict cookie is withheld on exactly that request, so the state + * cookie would be missing at /callback and the new session cookie would not be sent to the page + * the callback redirects to. Lax still withholds it from cross-site subrequests. + */ + private Cookie createCookie( + ServiceRequestContext ctx, String key, String value, String path, long maxAgeSeconds) { + return Cookie.builder(key, value) .path(path) - .maxAge(Duration.parse(maxAge).getSeconds()) + .maxAge(maxAgeSeconds) + .httpOnly(true) + .secure(browserUsesHttps(ctx)) + .sameSite("Lax") .build(); } + /** Whether the browser reached this server over HTTPS, read the same way callbackUrl reads it. */ + private boolean browserUsesHttps(ServiceRequestContext ctx) { + String base = serverProperties.getExternalUrl(); + if (base != null) { + return base.regionMatches(true, 0, "https://", 0, "https://".length()); + } + String forwardedProto = + firstValue(ctx.request().headers().get(HttpHeaderNames.X_FORWARDED_PROTO)); + if (forwardedProto != null) { + return forwardedProto.equalsIgnoreCase("https"); + } + return ctx.sessionProtocol().isTls(); + } + // NOTE: // When specifying `application/x-www-form-urlencoded` as the content type in the OpenAPI schema, // the OpenAPI Generator does not create request models from the schema. diff --git a/server/src/main/java/io/unitycatalog/server/utils/JwksOperations.java b/server/src/main/java/io/unitycatalog/server/utils/JwksOperations.java index 0cfa92b4..910651d5 100644 --- a/server/src/main/java/io/unitycatalog/server/utils/JwksOperations.java +++ b/server/src/main/java/io/unitycatalog/server/utils/JwksOperations.java @@ -25,18 +25,32 @@ import java.nio.file.Path; import java.security.interfaces.ECPublicKey; import java.security.interfaces.RSAPublicKey; +import java.time.Duration; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.TimeUnit; import lombok.SneakyThrows; import org.slf4j.Logger; import org.slf4j.LoggerFactory; public class JwksOperations { - private final WebClient webClient = WebClient.builder().build(); + // Discovery and JWKS fetches go to a third party over the public internet; without a bound a + // slow or unreachable identity provider would pin every token exchange for as long as the + // socket stays open. + private static final Duration IDP_TIMEOUT = Duration.ofSeconds(10); + + private final WebClient webClient = + WebClient.builder().responseTimeout(IDP_TIMEOUT).writeTimeout(IDP_TIMEOUT).build(); private static final ObjectMapper mapper = new ObjectMapper(); + // One provider per OIDC issuer, resolved through discovery once and then reused; the provider + // itself caches keys by kid and re-fetches on an unseen kid, so key rotation still works. A + // changed jwks_uri needs a restart, the same as server.allowed-issuers. + private final ConcurrentMap discoveredProviders = new ConcurrentHashMap<>(); private final SecurityContext securityContext; private final ServerProperties serverProperties; @@ -99,70 +113,136 @@ public JwkProvider loadJwkProvider(String issuer) { Path certsFile = securityContext.getCertsFile(); return new JwkProviderBuilder(certsFile.toUri().toURL()).cached(false).build(); } else { - // Trusted external issuers (e.g. Relyt instances doing token-exchange) are bare - // identifiers, not OIDC providers: their public keys are registered locally in a static - // JWKS file instead of being discovered over the network. Because a single file can hold - // keys for multiple issuers, each JWK must carry an "issuer" member and a key is only - // accepted for the issuer it was registered to (see IssuerScopedJwkProvider) — otherwise - // one registered instance could sign tokens accepted as another allowlisted issuer. - String externalJwksFile = - serverProperties != null ? serverProperties.getExternalJwksFile() : null; - if (externalJwksFile != null && !externalJwksFile.isBlank()) { - Path jwksPath = Path.of(externalJwksFile); - if (Files.exists(jwksPath)) { - LOGGER.debug("Using static external JWKS file '{}' for issuer '{}'", jwksPath, issuer); - JwkProvider fileProvider = - new JwkProviderBuilder(jwksPath.toUri().toURL()).cached(false).build(); - return new IssuerScopedJwkProvider(fileProvider, issuer); - } - LOGGER.warn("Configured external JWKS file '{}' does not exist", jwksPath); + // Two trust chains share this method (issue #15). Relyt instances are bare identifiers whose + // public keys are registered in the static external JWKS file; OIDC providers such as Entra + // ID publish theirs through discovery. The file used to win unconditionally whenever it was + // configured -- which the deployment template does by default -- so an OIDC token never + // reached discovery and failed with "key not registered for issuer". Route on whether the + // issuer has a key registered in the file instead: registered -> file, scoped to that + // issuer (see IssuerScopedJwkProvider, which keeps one instance's key from vouching for + // another allowlisted issuer); otherwise -> discovery. + if (knownIssuers().contains(issuer)) { + Path jwksPath = Path.of(serverProperties.getExternalJwksFile()); + LOGGER.debug("Using static external JWKS file '{}' for issuer '{}'", jwksPath, issuer); + JwkProvider fileProvider = + new JwkProviderBuilder(jwksPath.toUri().toURL()).cached(false).build(); + return new IssuerScopedJwkProvider(fileProvider, issuer); } - // Get the JWKS from the OIDC well-known location described here - // https://openid.net/specs/openid-connect-discovery-1_0-21.html#ProviderConfig + return discoveredProviders.computeIfAbsent(issuer, this::discoverJwkProvider); + } + } - if (!issuer.startsWith("https://") && !issuer.startsWith("http://")) { - issuer = "https://" + issuer; - } + /** + * Resolves an OIDC issuer's JWKS through its well-known configuration. Called once per issuer + * (see {@link #discoveredProviders}); the returned provider caches keys and rate-limits fetches. + */ + @SneakyThrows + private JwkProvider discoverJwkProvider(String issuer) { + // Get the JWKS from the OIDC well-known location described here + // https://openid.net/specs/openid-connect-discovery-1_0-21.html#ProviderConfig - String wellKnownConfigUrl = issuer; + if (!issuer.startsWith("https://") && !issuer.startsWith("http://")) { + issuer = "https://" + issuer; + } - if (!wellKnownConfigUrl.endsWith("/")) { - wellKnownConfigUrl += "/"; - } + String wellKnownConfigUrl = issuer; - var path = wellKnownConfigUrl + ".well-known/openid-configuration"; - LOGGER.debug("path: {}", path); + if (!wellKnownConfigUrl.endsWith("/")) { + wellKnownConfigUrl += "/"; + } - String response = webClient - .get(path) - .aggregate() - .join() - .contentUtf8(); + var path = wellKnownConfigUrl + ".well-known/openid-configuration"; + LOGGER.debug("path: {}", path); - // TODO: We should cache this. No need to fetch it each time. - Map configMap = mapper.readValue(response, new TypeReference<>() {}); + // A provider that is down, unreachable or slow (see IDP_TIMEOUT) is an authentication + // failure for this token, not a server fault: surface it as a 401 with a JSON body rather than + // letting the client's CompletionException escape as a bodyless 500. + String response; + try { + response = webClient.get(path).aggregate().join().contentUtf8(); + } catch (RuntimeException e) { + Throwable cause = e.getCause() != null ? e.getCause() : e; + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, + String.format( + "Could not fetch the OIDC configuration of issuer '%s': %s", + issuer, cause.getMessage()), + e); + } - if (configMap == null || configMap.isEmpty()) { - throw new OAuthInvalidRequestException(ErrorCode.ABORTED, - "Could not get issuer configuration"); - } + Map configMap; + try { + configMap = mapper.readValue(response, new TypeReference<>() {}); + } catch (IOException e) { + throw new OAuthInvalidRequestException( + ErrorCode.UNAUTHENTICATED, + String.format("Unreadable OIDC configuration from issuer '%s'", issuer), + e); + } - String configIssuer = (String) configMap.get("issuer"); - String configJwksUri = (String) configMap.get("jwks_uri"); + if (configMap == null || configMap.isEmpty()) { + throw new OAuthInvalidRequestException( + ErrorCode.ABORTED, "Could not get issuer configuration"); + } - if (!configIssuer.equals(issuer)) { - throw new OAuthInvalidRequestException(ErrorCode.ABORTED, - "Issuer doesn't match configuration"); - } + String configIssuer = (String) configMap.get("issuer"); + String configJwksUri = (String) configMap.get("jwks_uri"); - if (configJwksUri == null) { - throw new OAuthInvalidRequestException(ErrorCode.ABORTED, "JWKS configuration missing"); - } + if (!issuerMatchesConfiguration(configIssuer, issuer)) { + throw new OAuthInvalidRequestException( + ErrorCode.ABORTED, "Issuer doesn't match configuration"); + } - // TODO: Or maybe just cache the provider for reuse. - return new JwkProviderBuilder(URI.create(configJwksUri).toURL()).cached(false).build(); + if (configJwksUri == null) { + throw new OAuthInvalidRequestException(ErrorCode.ABORTED, "JWKS configuration missing"); + } + + // Keys are cached by kid; an unseen kid (rotation) triggers a fetch, bounded by the rate + // limiter so a stream of bogus kids cannot hammer the provider. + return new JwkProviderBuilder(URI.create(configJwksUri).toURL()) + .cached(10, 24, TimeUnit.HOURS) + .rateLimited(10, 1, TimeUnit.MINUTES) + .build(); + } + + /** + * Whether the issuer advertised by a discovery document is the one the token claims. + * + *

Single-tenant providers advertise their exact issuer and a plain comparison suffices. Entra + * ID's multi-tenant endpoints ({@code common}, {@code organizations}) instead advertise the + * literal template {@code https://login.microsoftonline.com/{tenantid}/v2.0} while tokens carry + * the real tenant GUID in that position, so the two never compare equal. A {@code {tenantid}} + * path segment therefore matches any single segment; every other segment, and the scheme and + * host, must still match exactly. Exact-issuer configurations are unaffected. + */ + static boolean issuerMatchesConfiguration(String configIssuer, String issuer) { + if (configIssuer == null || issuer == null) { + return false; + } + if (configIssuer.equals(issuer)) { + return true; + } + if (!configIssuer.contains("{tenantid}")) { + return false; + } + String[] expected = configIssuer.split("/"); + String[] actual = issuer.split("/"); + if (expected.length != actual.length) { + return false; + } + for (int i = 0; i < expected.length; i++) { + if (expected[i].equals("{tenantid}")) { + if (actual[i].isEmpty()) { + return false; + } + continue; + } + if (!expected[i].equals(actual[i])) { + return false; + } } + return true; } /** diff --git a/server/src/main/java/io/unitycatalog/server/utils/ServerProperties.java b/server/src/main/java/io/unitycatalog/server/utils/ServerProperties.java index 0f7900a1..e7e2e2a0 100644 --- a/server/src/main/java/io/unitycatalog/server/utils/ServerProperties.java +++ b/server/src/main/java/io/unitycatalog/server/utils/ServerProperties.java @@ -196,6 +196,12 @@ public enum Property { CLIENT_ID("server.client-id"), CLIENT_SECRET("server.client-secret"), REDIRECT_PORT("server.redirect-port", POSITIVE_INTEGER_VALIDATOR), + // Base URL at which browsers reach this server (scheme + host [+ port]), used to build the + // OAuth redirect_uri for the server-hosted login flow. Optional: when unset it is derived + // from X-Forwarded-Proto / X-Forwarded-Host or the Host header of the incoming request. + EXTERNAL_URL("server.external-url", URL_VALIDATOR), + // Password for signing the built-in "admin" account into the UI (issue #15). Blank = off. + ADMIN_PASSWORD("server.admin-password"), COOKIE_TIMEOUT("server.cookie-timeout", "P5D", DURATION_VALIDATOR), MANAGED_TABLE_ENABLED("server.managed-table.enabled", "false", BOOLEAN_VALIDATOR), // `storage-root.*` are replaced by managed storage locations of catalog and schema. @@ -523,6 +529,53 @@ public List getAudiences() { return getCommaSeparatedList("server.audiences"); } + /** OAuth authorization endpoint of the identity provider behind the server-hosted login. */ + public String getAuthorizationUrl() { + return blankToNull(get(Property.AUTHORIZATION_URL)); + } + + /** OAuth token endpoint the server exchanges the authorization code at. */ + public String getTokenUrl() { + return blankToNull(get(Property.TOKEN_URL)); + } + + public String getClientId() { + return blankToNull(get(Property.CLIENT_ID)); + } + + public String getClientSecret() { + return blankToNull(get(Property.CLIENT_SECRET)); + } + + /** + * Password of the built-in administrator for the UI's password sign-in, or null when that entry + * point is switched off. Compared in constant time by the login endpoint. + */ + public String getAdminPassword() { + return blankToNull(get(Property.ADMIN_PASSWORD)); + } + + /** Browser-facing base URL of this server, or null to derive it from the request. */ + public String getExternalUrl() { + String value = blankToNull(get(Property.EXTERNAL_URL)); + return value == null ? null : value.replaceAll("/+$", ""); + } + + /** + * Whether the server-hosted login flow (/auth/login, /auth/callback) is fully configured: the + * deployment template ships the four OAuth properties blank, which turns the flow off. + */ + public boolean isHostedLoginConfigured() { + return getAuthorizationUrl() != null + && getTokenUrl() != null + && getClientId() != null + && getClientSecret() != null; + } + + private static String blankToNull(String value) { + return value == null || value.isBlank() ? null : value; + } + /** * Path of a local JWKS file holding the public keys of trusted external issuers (e.g. Relyt * instances doing token-exchange). Lets issuers that are bare identifiers rather than OIDC diff --git a/server/src/test/java/io/unitycatalog/server/base/auth/BaseAuthCRUDTest.java b/server/src/test/java/io/unitycatalog/server/base/auth/BaseAuthCRUDTest.java index ad325487..b9db3a13 100644 --- a/server/src/test/java/io/unitycatalog/server/base/auth/BaseAuthCRUDTest.java +++ b/server/src/test/java/io/unitycatalog/server/base/auth/BaseAuthCRUDTest.java @@ -2,6 +2,7 @@ import static io.unitycatalog.server.security.SecurityContext.Issuers.INTERNAL; +import com.auth0.jwt.JWT; import com.auth0.jwt.algorithms.Algorithm; import com.sun.net.httpserver.HttpServer; import io.unitycatalog.server.base.BaseServerTest; @@ -16,8 +17,10 @@ import java.security.interfaces.RSAPrivateKey; import java.security.interfaces.RSAPublicKey; import java.util.Base64; +import java.util.Date; import java.util.UUID; import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -30,6 +33,28 @@ public abstract class BaseAuthCRUDTest extends BaseServerTest { // Test identity provider keypair and mock OIDC server private HttpServer mockOidcServer; + /** How many times the mock IdP served its discovery document / JWKS (for caching assertions). */ + protected final AtomicInteger discoveryRequests = new AtomicInteger(); + + protected final AtomicInteger jwksRequests = new AtomicInteger(); + + /** + * The mock IdP also plays the OAuth token endpoint ({@code /token}) for the server-hosted login + * flow: it records the form it received and answers with an id_token signed by the test key for + * {@link #tokenEndpointSubject}, addressed to {@link #tokenEndpointAudience}. + */ + protected volatile String tokenEndpointSubject = "admin"; + + protected volatile String tokenEndpointAudience = TEST_AUDIENCE; + protected volatile String lastTokenRequestBody; + + /** + * Set to an OAuth error body (with {@link #tokenEndpointStatus}) to make the token endpoint + * refuse the code, the way a provider does when the client secret has lapsed. + */ + protected volatile String tokenEndpointErrorBody; + + protected volatile int tokenEndpointStatus = 200; protected String testIssuer; protected Algorithm testIssuerAlgorithm; protected String testIssuerKeyId; @@ -87,6 +112,7 @@ private void startMockOidcServer() throws Exception { mockOidcServer.createContext( "/.well-known/openid-configuration", exchange -> { + discoveryRequests.incrementAndGet(); String discoveryDoc = String.format("{\"issuer\":\"%s\",\"jwks_uri\":\"%s/jwks\"}", testIssuer, testIssuer); byte[] body = discoveryDoc.getBytes(StandardCharsets.UTF_8); @@ -99,6 +125,7 @@ private void startMockOidcServer() throws Exception { mockOidcServer.createContext( "/jwks", exchange -> { + jwksRequests.incrementAndGet(); byte[] body = jwksJson.getBytes(StandardCharsets.UTF_8); exchange.getResponseHeaders().set("Content-Type", "application/json"); exchange.sendResponseHeaders(200, body.length); @@ -106,6 +133,37 @@ private void startMockOidcServer() throws Exception { exchange.close(); }); + mockOidcServer.createContext( + "/token", + exchange -> { + lastTokenRequestBody = + new String(exchange.getRequestBody().readAllBytes(), StandardCharsets.UTF_8); + if (tokenEndpointErrorBody != null) { + byte[] error = tokenEndpointErrorBody.getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(tokenEndpointStatus, error.length); + exchange.getResponseBody().write(error); + exchange.close(); + return; + } + String idToken = + JWT.create() + .withSubject(tokenEndpointSubject) + .withIssuer(testIssuer) + .withAudience(tokenEndpointAudience) + .withIssuedAt(new Date()) + .withKeyId(testIssuerKeyId) + .withJWTId(UUID.randomUUID().toString()) + .sign(testIssuerAlgorithm); + byte[] body = + String.format("{\"id_token\":\"%s\",\"token_type\":\"Bearer\"}", idToken) + .getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().set("Content-Type", "application/json"); + exchange.sendResponseHeaders(200, body.length); + exchange.getResponseBody().write(body); + exchange.close(); + }); + mockOidcServer.start(); int port = mockOidcServer.getAddress().getPort(); diff --git a/server/src/test/java/io/unitycatalog/server/base/auth/JwksTestUtils.java b/server/src/test/java/io/unitycatalog/server/base/auth/JwksTestUtils.java new file mode 100644 index 00000000..6a4ec5b9 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/base/auth/JwksTestUtils.java @@ -0,0 +1,36 @@ +package io.unitycatalog.server.base.auth; + +import java.math.BigInteger; +import java.security.interfaces.RSAPublicKey; +import java.util.Base64; + +/** Builds JWKS documents for tests that stand up their own signing keys. */ +public final class JwksTestUtils { + + private JwksTestUtils() {} + + /** + * A single-key RSA JWKS carrying an {@code issuer} member, the shape the server's static external + * JWKS file expects (each key is bound to the issuer it was registered for). + */ + public static String issuerBoundJwks(RSAPublicKey publicKey, String keyId, String issuer) { + Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding(); + String n = encoder.encodeToString(toUnsignedBytes(publicKey.getModulus())); + String e = encoder.encodeToString(toUnsignedBytes(publicKey.getPublicExponent())); + return String.format( + "{\"keys\":[{\"kty\":\"RSA\",\"use\":\"sig\",\"alg\":\"RS512\",\"kid\":\"%s\"," + + "\"n\":\"%s\",\"e\":\"%s\",\"issuer\":\"%s\"}]}", + keyId, n, e, issuer); + } + + /** Unsigned big-endian bytes of a BigInteger (no sign-padding zero byte). */ + private static byte[] toUnsignedBytes(BigInteger value) { + byte[] bytes = value.toByteArray(); + if (bytes[0] == 0) { + byte[] trimmed = new byte[bytes.length - 1]; + System.arraycopy(bytes, 1, trimmed, 0, trimmed.length); + return trimmed; + } + return bytes; + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginDisabledTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginDisabledTest.java new file mode 100644 index 00000000..187e58a9 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginDisabledTest.java @@ -0,0 +1,85 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import java.io.IOException; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The deployment template ships server.admin-password blank, which is how the administrator entry + * point is switched off. It must then refuse every attempt -- including one that guesses an empty + * password -- and say so through the providers endpoint so the UI does not offer the form. + */ +public class AuthServiceAdminLoginDisabledTest extends BaseAuthCRUDTest { + + private static final String AUTH_PATH = "/api/1.0/unity-control/auth"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private WebClient client; + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void signInIsRefusedWhenNoPasswordIsConfigured() { + AggregatedHttpResponse response = login("admin", "anything"); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("Administrator sign-in is not configured"); + assertThat(response.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith(AuthDecorator.UC_TOKEN_KEY + "=")); + } + + /** A blank property must not become a blank password that anyone can send. */ + @Test + public void anEmptyPasswordDoesNotOpenASession() { + AggregatedHttpResponse response = login("admin", ""); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith(AuthDecorator.UC_TOKEN_KEY + "=")); + } + + @Test + public void providersReportAdminLoginOff() throws IOException { + AggregatedHttpResponse response = + client + .execute(RequestHeaders.of(HttpMethod.GET, AUTH_PATH + "/providers")) + .aggregate() + .join(); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + JsonNode body = MAPPER.readTree(response.contentUtf8()); + assertThat(body.get("authorization_enabled").asBoolean()).isTrue(); + assertThat(body.get("admin_login").asBoolean()).isFalse(); + } + + private AggregatedHttpResponse login(String username, String password) { + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(AUTH_PATH + "/admin/login") + .contentType(MediaType.FORM_DATA) + .build(); + return client + .execute(headers, HttpData.ofUtf8("username=" + username + "&password=" + password)) + .aggregate() + .join(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginTest.java new file mode 100644 index 00000000..4225cd5c --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAdminLoginTest.java @@ -0,0 +1,168 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import java.io.IOException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The administrator's password sign-in for the UI (issue #15): a hidden entry point that turns the + * password configured in server.properties into the same UC_TOKEN cookie a Microsoft sign-in ends + * with, so the rest of the UI does not care how the session was opened. + */ +public class AuthServiceAdminLoginTest extends BaseAuthCRUDTest { + + private static final String AUTH_PATH = "/api/1.0/unity-control/auth"; + private static final String PASSWORD = "correct horse battery staple"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private WebClient client; + + @Override + protected void setUpProperties() { + super.setUpProperties(); + serverProperties.setProperty("server.admin-password", PASSWORD); + } + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void correctPasswordOpensAnAdminSession() throws IOException { + AggregatedHttpResponse response = login("admin", PASSWORD); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + JsonNode body = MAPPER.readTree(response.contentUtf8()); + assertThat(body.get("access_token").asText()).isNotEmpty(); + + String session = setCookieValue(response, AuthDecorator.UC_TOKEN_KEY); + assertThat(JWT.decode(session).getSubject()).isEqualTo("admin"); + + // The cookie alone must open a protected endpoint as the administrator. + AggregatedHttpResponse me = + client + .execute( + RequestHeaders.builder(HttpMethod.GET, "/api/1.0/unity-control/scim2/Me") + .add(HttpHeaderNames.COOKIE, AuthDecorator.UC_TOKEN_KEY + "=" + session) + .build()) + .aggregate() + .join(); + assertThat(me.status()).isEqualTo(HttpStatus.OK); + assertThat(MAPPER.readTree(me.contentUtf8()).get("userName").asText()).isEqualTo("admin"); + } + + /** + * A browser silently drops a Secure cookie that arrives over plain HTTP, which is how the UI is + * reached in a local or internal-network deployment: the sign-in would look like it worked and + * leave no session. Strict would be dropped on the way back from an identity provider. + */ + @Test + public void theSessionCookieSurvivesAPlainHttpDeployment() { + AggregatedHttpResponse response = login("admin", PASSWORD); + + String setCookie = setCookieHeader(response, AuthDecorator.UC_TOKEN_KEY); + assertThat(setCookie).doesNotContain("Secure").contains("SameSite=Lax").contains("HTTPOnly"); + } + + /** Behind a TLS-terminating proxy the cookie must be Secure again. */ + @Test + public void theSessionCookieIsSecureWhenTheBrowserIsOnHttps() { + String form = "username=admin&password=" + URLEncoder.encode(PASSWORD, StandardCharsets.UTF_8); + AggregatedHttpResponse response = + client + .execute( + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(AUTH_PATH + "/admin/login") + .contentType(MediaType.FORM_DATA) + .add(HttpHeaderNames.X_FORWARDED_PROTO, "https") + .build(), + HttpData.ofUtf8(form)) + .aggregate() + .join(); + + assertThat(setCookieHeader(response, AuthDecorator.UC_TOKEN_KEY)).contains("Secure"); + } + + @Test + public void wrongPasswordIsRejectedWithoutASession() { + AggregatedHttpResponse response = login("admin", "not the password"); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(response.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith(AuthDecorator.UC_TOKEN_KEY + "=")); + } + + /** Only the built-in administrator signs in this way; other accounts use their provider. */ + @Test + public void otherUsernamesAreRejectedEvenWithTheRightPassword() { + AggregatedHttpResponse response = login("alice@example.com", PASSWORD); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + @Test + public void providersAdvertiseAdminLoginOnly() throws IOException { + AggregatedHttpResponse response = + client + .execute(RequestHeaders.of(HttpMethod.GET, AUTH_PATH + "/providers")) + .aggregate() + .join(); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + JsonNode body = MAPPER.readTree(response.contentUtf8()); + assertThat(body.get("authorization_enabled").asBoolean()).isTrue(); + assertThat(body.get("admin_login").asBoolean()).isTrue(); + assertThat(body.get("hosted_login").asBoolean()).isFalse(); + } + + private AggregatedHttpResponse login(String username, String password) { + String form = + "username=" + + URLEncoder.encode(username, StandardCharsets.UTF_8) + + "&password=" + + URLEncoder.encode(password, StandardCharsets.UTF_8); + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(AUTH_PATH + "/admin/login") + .contentType(MediaType.FORM_DATA) + .build(); + return client.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + } + + private static String setCookieHeader(AggregatedHttpResponse response, String name) { + List cookies = response.headers().getAll(HttpHeaderNames.SET_COOKIE); + return cookies.stream() + .filter(c -> c.startsWith(name + "=")) + .findFirst() + .orElseThrow(() -> new AssertionError("no Set-Cookie for " + name + " in " + cookies)); + } + + private static String setCookieValue(AggregatedHttpResponse response, String name) { + String header = setCookieHeader(response, name); + String afterName = header.substring(name.length() + 1); + int semicolon = afterName.indexOf(';'); + return semicolon < 0 ? afterName : afterName.substring(0, semicolon); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceAuthProvidersTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAuthProvidersTest.java new file mode 100644 index 00000000..077bfedc --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceAuthProvidersTest.java @@ -0,0 +1,41 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.BaseServerTest; +import java.io.IOException; +import org.junit.jupiter.api.Test; + +/** + * With authorization switched off the UI must not put itself behind a login page it cannot get + * past: the providers endpoint tells it so, and is reachable without any token. The enabled-side + * combinations are asserted in the admin-login and hosted-login test classes. + */ +public class AuthServiceAuthProvidersTest extends BaseServerTest { + + private static final ObjectMapper MAPPER = new ObjectMapper(); + + @Test + public void reportsAuthorizationDisabledWithNoSignInMethods() throws IOException { + WebClient client = WebClient.builder(serverConfig.getServerUrl()).build(); + + AggregatedHttpResponse response = + client + .execute(RequestHeaders.of(HttpMethod.GET, "/api/1.0/unity-control/auth/providers")) + .aggregate() + .join(); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + JsonNode body = MAPPER.readTree(response.contentUtf8()); + assertThat(body.get("authorization_enabled").asBoolean()).isFalse(); + assertThat(body.get("hosted_login").asBoolean()).isFalse(); + assertThat(body.get("admin_login").asBoolean()).isFalse(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceDiscoveryCachingTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceDiscoveryCachingTest.java new file mode 100644 index 00000000..5ba363be --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceDiscoveryCachingTest.java @@ -0,0 +1,114 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import java.util.Date; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Every token exchange used to fetch the issuer's discovery document and JWKS afresh -- two + * round-trips to a third party per login (issue #15). Both are now resolved once per issuer and + * reused, while an unseen kid still triggers a JWKS fetch so key rotation keeps working. + */ +public class AuthServiceDiscoveryCachingTest extends BaseAuthCRUDTest { + + private static final String TOKEN_ENDPOINT = "/api/1.0/unity-control/auth/tokens"; + /** Trusted, but nothing listens there: connection refused, immediately. */ + private static final String UNREACHABLE_ISSUER = "http://127.0.0.1:1"; + + private WebClient client; + + @Override + protected void setUpProperties() { + super.setUpProperties(); + serverProperties.setProperty("server.allowed-issuers", testIssuer + "," + UNREACHABLE_ISSUER); + } + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void discoveryAndJwksAreFetchedOnceAcrossExchanges() { + assertThat(exchange(testIssuerKeyId).status()).isEqualTo(HttpStatus.OK); + assertThat(exchange(testIssuerKeyId).status()).isEqualTo(HttpStatus.OK); + assertThat(exchange(testIssuerKeyId).status()).isEqualTo(HttpStatus.OK); + + assertThat(discoveryRequests.get()).isEqualTo(1); + assertThat(jwksRequests.get()).isEqualTo(1); + } + + /** Rotation: a kid the cache has not seen must reach the JWKS endpoint again. */ + @Test + public void unseenKidRefetchesTheJwks() { + assertThat(exchange(testIssuerKeyId).status()).isEqualTo(HttpStatus.OK); + assertThat(jwksRequests.get()).isEqualTo(1); + + assertThat(exchange(UUID.randomUUID().toString()).status()).isEqualTo(HttpStatus.UNAUTHORIZED); + + assertThat(jwksRequests.get()).isEqualTo(2); + // The discovery document itself is not re-read for a kid miss. + assertThat(discoveryRequests.get()).isEqualTo(1); + } + + /** + * A trusted issuer that cannot be reached is an authentication failure for that token, not a + * server fault: the client must get a 401 with a JSON error, never a bodyless 500 from an escaped + * CompletionException. + */ + @Test + public void unreachableIssuerIsRejectedAsUnauthenticated() throws java.io.IOException { + AggregatedHttpResponse response = exchange(UNREACHABLE_ISSUER, testIssuerKeyId); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + com.fasterxml.jackson.databind.JsonNode body = + new com.fasterxml.jackson.databind.ObjectMapper().readTree(response.contentUtf8()); + assertThat(body.get("error_code").asText()).isEqualTo("UNAUTHENTICATED"); + assertThat(body.get("message").asText()).contains(UNREACHABLE_ISSUER); + // Nothing was cached for the failed issuer; a later attempt would try again. + assertThat(discoveryRequests.get()).isEqualTo(0); + } + + private AggregatedHttpResponse exchange(String keyId) { + return exchange(testIssuer, keyId); + } + + private AggregatedHttpResponse exchange(String issuer, String keyId) { + String token = + JWT.create() + .withSubject("admin") + .withIssuer(issuer) + .withAudience(TEST_AUDIENCE) + .withIssuedAt(new Date()) + .withKeyId(keyId) + .withJWTId(UUID.randomUUID().toString()) + .sign(testIssuerAlgorithm); + String form = + "grant_type=urn:ietf:params:oauth:grant-type:token-exchange" + + "&requested_token_type=urn:ietf:params:oauth:token-type:access_token" + + "&subject_token_type=urn:ietf:params:oauth:token-type:id_token" + + "&subject_token=" + + token; + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(TOKEN_ENDPOINT) + .contentType(MediaType.FORM_DATA) + .build(); + return client.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceDualTrustChainTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceDualTrustChainTest.java new file mode 100644 index 00000000..01fe09e8 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceDualTrustChainTest.java @@ -0,0 +1,138 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.algorithms.Algorithm; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import io.unitycatalog.server.base.auth.JwksTestUtils; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import java.security.KeyPairGenerator; +import java.security.interfaces.RSAPrivateKey; +import java.security.interfaces.RSAPublicKey; +import java.util.Date; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Both trust chains configured on one server (issue #15): a Relyt instance whose key sits in the + * static external JWKS file, and an OIDC provider reached through discovery (the base class's mock + * IdP, listed in {@code server.allowed-issuers}). The file used to be consulted unconditionally + * whenever it was configured, so the OIDC token never reached discovery. Each chain must verify its + * own tokens and neither chain's key may vouch for the other's issuer. + */ +public class AuthServiceDualTrustChainTest extends BaseAuthCRUDTest { + + private static final String TOKEN_ENDPOINT = "/api/1.0/unity-control/auth/tokens"; + private static final String LOCAL_ISSUER = "relyt-instance-local"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private WebClient client; + private Algorithm localIssuerAlgorithm; + private String localIssuerKeyId; + + @Override + protected void setUpProperties() { + // Mock OIDC issuer in allowed-issuers + audiences. + super.setUpProperties(); + // Plus a bare-identifier issuer registered in the static external JWKS file -- the default + // shape of a Relyt deployment. + try { + KeyPairGenerator generator = KeyPairGenerator.getInstance("RSA"); + generator.initialize(2048); + var keyPair = generator.generateKeyPair(); + RSAPublicKey publicKey = (RSAPublicKey) keyPair.getPublic(); + localIssuerAlgorithm = Algorithm.RSA512(publicKey, (RSAPrivateKey) keyPair.getPrivate()); + localIssuerKeyId = UUID.randomUUID().toString(); + + Path jwksFile = Files.createTempFile("uc-dual-jwks", ".json"); + Files.writeString( + jwksFile, JwksTestUtils.issuerBoundJwks(publicKey, localIssuerKeyId, LOCAL_ISSUER)); + serverProperties.setProperty("server.external-jwks-file", jwksFile.toString()); + } catch (Exception e) { + throw new RuntimeException("Failed to set up external JWKS file", e); + } + } + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void localIssuerIsVerifiedFromTheFile() throws IOException { + AggregatedHttpResponse response = + exchangeToken(signedToken(LOCAL_ISSUER, localIssuerAlgorithm, localIssuerKeyId)); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(MAPPER.readTree(response.contentUtf8()).get("access_token").asText()).isNotEmpty(); + } + + /** The regression: with the file configured, an OIDC issuer must still reach discovery. */ + @Test + public void oidcIssuerIsVerifiedViaDiscoveryWhileTheFileIsConfigured() throws IOException { + AggregatedHttpResponse response = + exchangeToken(signedToken(testIssuer, testIssuerAlgorithm, testIssuerKeyId)); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(MAPPER.readTree(response.contentUtf8()).get("access_token").asText()).isNotEmpty(); + } + + /** A key registered for the local issuer must not sign tokens claiming the OIDC issuer. */ + @Test + public void localKeyDoesNotVouchForTheOidcIssuer() { + AggregatedHttpResponse response = + exchangeToken(signedToken(testIssuer, localIssuerAlgorithm, localIssuerKeyId)); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + /** Nor may the OIDC provider's key sign tokens claiming the local issuer. */ + @Test + public void oidcKeyDoesNotVouchForTheLocalIssuer() { + AggregatedHttpResponse response = + exchangeToken(signedToken(LOCAL_ISSUER, testIssuerAlgorithm, testIssuerKeyId)); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + private static String signedToken(String issuer, Algorithm algorithm, String keyId) { + return JWT.create() + .withSubject("admin") + .withIssuer(issuer) + .withAudience(TEST_AUDIENCE) + .withIssuedAt(new Date()) + .withKeyId(keyId) + .withJWTId(UUID.randomUUID().toString()) + .sign(algorithm); + } + + private AggregatedHttpResponse exchangeToken(String subjectToken) { + String form = + "grant_type=urn:ietf:params:oauth:grant-type:token-exchange" + + "&requested_token_type=urn:ietf:params:oauth:token-type:access_token" + + "&subject_token_type=urn:ietf:params:oauth:token-type:id_token" + + "&subject_token=" + + subjectToken; + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(TOKEN_ENDPOINT) + .contentType(MediaType.FORM_DATA) + .build(); + return client.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginDisabledTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginDisabledTest.java new file mode 100644 index 00000000..287d39f2 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginDisabledTest.java @@ -0,0 +1,64 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The deployment template ships the four OAuth client properties blank, which is how the hosted + * login flow is switched off. Its endpoints must then refuse clearly rather than redirect to an + * empty URL or redeem a code against nothing. + */ +public class AuthServiceHostedLoginDisabledTest extends BaseAuthCRUDTest { + + private static final String AUTH_PATH = "/api/1.0/unity-control/auth"; + + private WebClient client; + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void loginIsRefusedWhenNotConfigured() { + AggregatedHttpResponse response = + client.execute(RequestHeaders.of(HttpMethod.GET, AUTH_PATH + "/login")).aggregate().join(); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("Hosted login is not configured"); + } + + @Test + public void callbackIsRefusedWhenNotConfigured() { + AggregatedHttpResponse response = + client + .execute(RequestHeaders.of(HttpMethod.GET, AUTH_PATH + "/callback?code=x&state=y")) + .aggregate() + .join(); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + } + + /** The token exchange endpoint is unaffected by the hosted flow being off. */ + @Test + public void tokenExchangeStillWorks() { + // Nothing to assert beyond reachability here; AuthServiceTest covers the exchange itself. + AggregatedHttpResponse response = + client + .execute(RequestHeaders.of(HttpMethod.POST, AUTH_PATH + "/tokens")) + .aggregate() + .join(); + + assertThat(response.status()).isNotEqualTo(HttpStatus.NOT_FOUND); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginTest.java new file mode 100644 index 00000000..05f0d1fc --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceHostedLoginTest.java @@ -0,0 +1,288 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.linecorp.armeria.client.ClientFactory; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import java.net.URI; +import java.net.URLDecoder; +import java.nio.charset.StandardCharsets; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * The server-hosted login flow (issue #15): /login sends the browser to the provider, /callback + * redeems the code with the server-held client secret and turns the id_token into the UC session + * cookie. The base class's mock IdP plays both the discovery/JWKS host and the token endpoint. + */ +public class AuthServiceHostedLoginTest extends BaseAuthCRUDTest { + + private static final String AUTH_PATH = "/api/1.0/unity-control/auth"; + private static final String CLIENT_ID = "uc-ui-client"; + private static final String CLIENT_SECRET = "s3cret-kept-on-the-server"; + + private WebClient client; + + @Override + protected void setUpProperties() { + super.setUpProperties(); + serverProperties.setProperty("server.authorization-url", testIssuer + "/authorize"); + serverProperties.setProperty("server.token-url", testIssuer + "/token"); + serverProperties.setProperty("server.client-id", CLIENT_ID); + serverProperties.setProperty("server.client-secret", CLIENT_SECRET); + // Entra addresses id_tokens to the application's client id; audiences must list it. + serverProperties.setProperty("server.audiences", TEST_AUDIENCE + "," + CLIENT_ID); + } + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + tokenEndpointAudience = CLIENT_ID; + tokenEndpointSubject = "admin"; + // No automatic redirect following: the 302s ARE the behaviour under test. + client = + WebClient.builder(serverConfig.getServerUrl()).factory(ClientFactory.ofDefault()).build(); + } + + @Test + public void loginRedirectsToTheProviderWithStateAndServerCallback() { + AggregatedHttpResponse response = get(AUTH_PATH + "/login?redirect=/data/demo", null); + + assertThat(response.status()).isEqualTo(HttpStatus.FOUND); + String location = response.headers().get(HttpHeaderNames.LOCATION); + assertThat(location).startsWith(testIssuer + "/authorize?"); + + Map query = queryOf(location); + assertThat(query.get("client_id")).isEqualTo(CLIENT_ID); + assertThat(query.get("response_type")).isEqualTo("code"); + assertThat(query.get("scope")).isEqualTo("openid profile email"); + assertThat(query.get("state")).isNotBlank(); + // Derived from the request's Host: the callback lives on THIS server. + assertThat(query.get("redirect_uri")) + .isEqualTo(serverConfig.getServerUrl() + AUTH_PATH + "/callback"); + + // The state is bound to a cookie scoped to the auth mount path, so /callback receives it. + String stateCookie = setCookieValue(response, "UC_OAUTH_STATE"); + assertThat(stateCookie).startsWith(query.get("state") + "|/data/demo"); + assertThat(setCookieHeader(response, "UC_OAUTH_STATE")).contains("Path=" + AUTH_PATH); + // Lax, not Strict: the browser returns from the identity provider on a cross-site + // navigation, and a Strict cookie is withheld on exactly that request -- /callback would + // then find no state and refuse a sign-in that had in fact succeeded. + assertThat(setCookieHeader(response, "UC_OAUTH_STATE")).contains("SameSite=Lax"); + // The client secret must never appear in anything sent to the browser. + assertThat(location).doesNotContain(CLIENT_SECRET); + } + + /** + * An expired client secret fails here and nowhere else, after the person has already signed in at + * the provider. Naming the provider's code is what tells it apart from a redirect URI that no + * longer matches or a replayed code, which fail at the same place with the same status. + */ + @Test + public void callbackNamesWhyTheProviderRefusedTheCode() { + tokenEndpointStatus = 401; + tokenEndpointErrorBody = + "{\"error\":\"invalid_client\",\"error_description\":\"AADSTS7000222: The provided client" + + " secret keys for app '" + + CLIENT_ID + + "' are expired.\\r\\nTrace ID: 00000000-0000-0000-0000-000000000000\"}"; + try { + AggregatedHttpResponse login = get(AUTH_PATH + "/login?redirect=/", null); + String state = queryOf(login.headers().get(HttpHeaderNames.LOCATION)).get("state"); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get( + AUTH_PATH + "/callback?code=the-code&state=" + state, + "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(callback.contentUtf8()).contains("invalid_client").contains("AADSTS7000222"); + // The description itself, with its correlation id, belongs in the log rather than the + // browser. + assertThat(callback.contentUtf8()).doesNotContain("Trace ID"); + assertThat(callback.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith("UC_TOKEN=")); + } finally { + tokenEndpointErrorBody = null; + tokenEndpointStatus = 200; + } + } + + @Test + public void callbackRedeemsTheCodeAndSetsTheSessionCookie() { + AggregatedHttpResponse login = get(AUTH_PATH + "/login?redirect=/data/demo", null); + String state = queryOf(login.headers().get(HttpHeaderNames.LOCATION)).get("state"); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get(AUTH_PATH + "/callback?code=the-code&state=" + state, "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.status()).isEqualTo(HttpStatus.FOUND); + assertThat(callback.headers().get(HttpHeaderNames.LOCATION)).isEqualTo("/data/demo"); + + // The code was redeemed server-side with the client secret, against the registered callback. + assertThat(lastTokenRequestBody).contains("grant_type=authorization_code"); + assertThat(lastTokenRequestBody).contains("code=the-code"); + assertThat(lastTokenRequestBody).contains("client_id=" + CLIENT_ID); + assertThat(lastTokenRequestBody).contains("client_secret=" + CLIENT_SECRET); + assertThat(URLDecoder.decode(lastTokenRequestBody, StandardCharsets.UTF_8)) + .contains("redirect_uri=" + serverConfig.getServerUrl() + AUTH_PATH + "/callback"); + + // The session cookie is a UC access token for the resolved local user ... + String session = setCookieValue(callback, "UC_TOKEN"); + assertThat(JWT.decode(session).getSubject()).isEqualTo("admin"); + // ... and the one-time state cookie is cleared. + assertThat(setCookieHeader(callback, "UC_OAUTH_STATE")).contains("Max-Age=0"); + } + + @Test + public void callbackRejectsAMismatchedState() { + AggregatedHttpResponse login = get(AUTH_PATH + "/login", null); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get( + AUTH_PATH + "/callback?code=the-code&state=someone-elses-state", + "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(callback.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith("UC_TOKEN=")); + } + + @Test + public void callbackWithoutTheStateCookieIsRejected() { + AggregatedHttpResponse callback = get(AUTH_PATH + "/callback?code=x&state=y", null); + + assertThat(callback.status()).isEqualTo(HttpStatus.BAD_REQUEST); + } + + /** The id_token names a user UC does not know: same rule as the token exchange. */ + @Test + public void callbackRejectsAnUnknownUser() { + tokenEndpointSubject = "nobody@example.com"; + AggregatedHttpResponse login = get(AUTH_PATH + "/login", null); + String state = queryOf(login.headers().get(HttpHeaderNames.LOCATION)).get("state"); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get(AUTH_PATH + "/callback?code=c&state=" + state, "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(callback.contentUtf8()).contains("User not allowed: nobody@example.com"); + } + + @Test + public void callbackSurfacesAProviderError() { + AggregatedHttpResponse callback = + get(AUTH_PATH + "/callback?error=access_denied&error_description=MFA+required", null); + + assertThat(callback.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(callback.contentUtf8()).contains("access_denied"); + } + + /** An absolute redirect would let a crafted login link send the user off-site. */ + @Test + public void absoluteRedirectFallsBackToRoot() { + AggregatedHttpResponse login = get(AUTH_PATH + "/login?redirect=https://evil.example/", null); + String state = queryOf(login.headers().get(HttpHeaderNames.LOCATION)).get("state"); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get(AUTH_PATH + "/callback?code=c&state=" + state, "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.status()).isEqualTo(HttpStatus.FOUND); + assertThat(callback.headers().get(HttpHeaderNames.LOCATION)).isEqualTo("/"); + } + + @Test + public void schemeRelativeRedirectFallsBackToRoot() { + AggregatedHttpResponse login = get(AUTH_PATH + "/login?redirect=//evil.example/x", null); + String state = queryOf(login.headers().get(HttpHeaderNames.LOCATION)).get("state"); + String stateCookie = setCookieValue(login, "UC_OAUTH_STATE"); + + AggregatedHttpResponse callback = + get(AUTH_PATH + "/callback?code=c&state=" + state, "UC_OAUTH_STATE=" + stateCookie); + + assertThat(callback.headers().get(HttpHeaderNames.LOCATION)).isEqualTo("/"); + } + + /** Behind the UI's reverse proxy the browser-facing origin comes from X-Forwarded-*. */ + @Test + public void callbackUrlHonoursForwardedHeaders() { + RequestHeaders headers = + RequestHeaders.builder(HttpMethod.GET, AUTH_PATH + "/login") + .add(HttpHeaderNames.X_FORWARDED_PROTO, "https") + .add(HttpHeaderNames.X_FORWARDED_HOST, "uc.example.com") + .build(); + AggregatedHttpResponse response = client.execute(headers).aggregate().join(); + + String redirectUri = + queryOf(response.headers().get(HttpHeaderNames.LOCATION)).get("redirect_uri"); + assertThat(redirectUri).isEqualTo("https://uc.example.com" + AUTH_PATH + "/callback"); + } + + /** The UI shows the Microsoft button only because the server says the flow is configured. */ + @Test + public void providersAdvertiseHostedLoginOnly() throws Exception { + AggregatedHttpResponse response = get(AUTH_PATH + "/providers", null); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + com.fasterxml.jackson.databind.JsonNode body = + new com.fasterxml.jackson.databind.ObjectMapper().readTree(response.contentUtf8()); + assertThat(body.get("authorization_enabled").asBoolean()).isTrue(); + assertThat(body.get("hosted_login").asBoolean()).isTrue(); + assertThat(body.get("admin_login").asBoolean()).isFalse(); + } + + private AggregatedHttpResponse get(String path, String cookieHeader) { + var builder = RequestHeaders.builder(HttpMethod.GET, path); + if (cookieHeader != null) { + builder.add(HttpHeaderNames.COOKIE, cookieHeader); + } + return client.execute(builder.build()).aggregate().join(); + } + + private static Map queryOf(String url) { + Map query = new HashMap<>(); + String raw = URI.create(url).getRawQuery(); + if (raw == null) { + return query; + } + for (String pair : raw.split("&")) { + int eq = pair.indexOf('='); + String key = URLDecoder.decode(eq < 0 ? pair : pair.substring(0, eq), StandardCharsets.UTF_8); + String value = + eq < 0 ? "" : URLDecoder.decode(pair.substring(eq + 1), StandardCharsets.UTF_8); + query.put(key, value); + } + return query; + } + + private static String setCookieHeader(AggregatedHttpResponse response, String name) { + List cookies = response.headers().getAll(HttpHeaderNames.SET_COOKIE); + return cookies.stream() + .filter(c -> c.startsWith(name + "=")) + .findFirst() + .orElseThrow(() -> new AssertionError("no Set-Cookie for " + name + " in " + cookies)); + } + + private static String setCookieValue(AggregatedHttpResponse response, String name) { + String header = setCookieHeader(response, name); + String afterName = header.substring(name.length() + 1); + int semicolon = afterName.indexOf(';'); + return semicolon < 0 ? afterName : afterName.substring(0, semicolon); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceIdentityResolutionTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceIdentityResolutionTest.java new file mode 100644 index 00000000..e27057de --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceIdentityResolutionTest.java @@ -0,0 +1,138 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.auth0.jwt.JWTCreator; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import io.unitycatalog.server.persist.Repositories; +import io.unitycatalog.server.persist.model.CreateUser; +import io.unitycatalog.server.utils.ServerProperties; +import java.io.IOException; +import java.util.Date; +import java.util.UUID; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * How the token exchange maps an identity provider's claims to a local user (issue #15). Entra ID + * omits {@code email} unless configured as an optional claim, so the sign-in name claims must also + * identify the account. Whatever claim matched, the exchanged token must carry the LOCAL user's + * email as {@code sub}, because that is the only key the request path looks users up by. + */ +public class AuthServiceIdentityResolutionTest extends BaseAuthCRUDTest { + + private static final String TOKEN_ENDPOINT = "/api/1.0/unity-control/auth/tokens"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private static final String USER_EMAIL = "alice@example.com"; + + private WebClient client; + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + + Repositories repositories = + new Repositories( + hibernateConfigurator.getSessionFactory(), new ServerProperties(serverProperties)); + repositories + .getUserRepository() + .createUser(CreateUser.builder().name("Alice").email(USER_EMAIL).build()); + } + + @Test + public void emailClaimResolvesTheUser() throws IOException { + AggregatedHttpResponse response = + exchange(token().withClaim("email", USER_EMAIL).withSubject(UUID.randomUUID().toString())); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(exchangedSubject(response)).isEqualTo(USER_EMAIL); + } + + /** Entra ID without the optional email claim: the sign-in name still identifies the user. */ + @Test + public void preferredUsernameResolvesTheUserWhenEmailIsAbsent() throws IOException { + AggregatedHttpResponse response = + exchange( + token() + .withClaim("preferred_username", USER_EMAIL) + .withSubject(UUID.randomUUID().toString())); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(exchangedSubject(response)).isEqualTo(USER_EMAIL); + } + + @Test + public void upnResolvesTheUserWhenEmailIsAbsent() throws IOException { + AggregatedHttpResponse response = + exchange(token().withClaim("upn", USER_EMAIL).withSubject(UUID.randomUUID().toString())); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(exchangedSubject(response)).isEqualTo(USER_EMAIL); + } + + /** Relyt instances and the bootstrap admin carry the principal in sub alone. */ + @Test + public void subAsEmailStillWorks() throws IOException { + AggregatedHttpResponse response = exchange(token().withSubject("admin")); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + assertThat(exchangedSubject(response)).isEqualTo("admin"); + } + + /** No just-in-time provisioning: an identity with no matching user is rejected. */ + @Test + public void unknownIdentityIsRejected() { + AggregatedHttpResponse response = + exchange( + token() + .withClaim("email", "nobody@example.com") + .withSubject(UUID.randomUUID().toString())); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + assertThat(response.contentUtf8()).contains("User not allowed: nobody@example.com"); + } + + private JWTCreator.Builder token() { + return JWT.create() + .withIssuer(testIssuer) + .withAudience(TEST_AUDIENCE) + .withIssuedAt(new Date()) + .withKeyId(testIssuerKeyId) + .withJWTId(UUID.randomUUID().toString()); + } + + private AggregatedHttpResponse exchange(JWTCreator.Builder token) { + String form = + "grant_type=urn:ietf:params:oauth:grant-type:token-exchange" + + "&requested_token_type=urn:ietf:params:oauth:token-type:access_token" + + "&subject_token_type=urn:ietf:params:oauth:token-type:id_token" + + "&subject_token=" + + token.sign(testIssuerAlgorithm); + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(TOKEN_ENDPOINT) + .contentType(MediaType.FORM_DATA) + .build(); + return client.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + } + + /** The {@code sub} of the UC access token the exchange handed back. */ + private static String exchangedSubject(AggregatedHttpResponse response) throws IOException { + JsonNode body = MAPPER.readTree(response.contentUtf8()); + return JWT.decode(body.get("access_token").asText()).getSubject(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/service/AuthServiceTokenLoginTest.java b/server/src/test/java/io/unitycatalog/server/service/AuthServiceTokenLoginTest.java new file mode 100644 index 00000000..b0a8892c --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/service/AuthServiceTokenLoginTest.java @@ -0,0 +1,176 @@ +package io.unitycatalog.server.service; + +import static org.assertj.core.api.Assertions.assertThat; + +import com.auth0.jwt.JWT; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import com.linecorp.armeria.client.WebClient; +import com.linecorp.armeria.common.AggregatedHttpResponse; +import com.linecorp.armeria.common.HttpData; +import com.linecorp.armeria.common.HttpHeaderNames; +import com.linecorp.armeria.common.HttpMethod; +import com.linecorp.armeria.common.HttpStatus; +import com.linecorp.armeria.common.MediaType; +import com.linecorp.armeria.common.RequestHeaders; +import io.unitycatalog.server.base.auth.BaseAuthCRUDTest; +import java.io.IOException; +import java.net.URLEncoder; +import java.nio.charset.StandardCharsets; +import java.util.Date; +import java.util.List; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Signing in with an access token this server issued (issue #15). It is how whoever runs the server + * reaches the UI with the token from etc/conf/token.txt, and it replaced the UI proxy's blanket + * token injection -- so what matters here is that it accepts only this server's own valid tokens, + * and that the session it opens is the token itself rather than a fresh one. + */ +public class AuthServiceTokenLoginTest extends BaseAuthCRUDTest { + + private static final String AUTH_PATH = "/api/1.0/unity-control/auth"; + private static final String PASSWORD = "correct horse battery staple"; + private static final ObjectMapper MAPPER = new ObjectMapper(); + + private WebClient client; + + @Override + protected void setUpProperties() { + super.setUpProperties(); + // Only to obtain a genuine token to present; the sign-in under test needs no password. + serverProperties.setProperty("server.admin-password", PASSWORD); + } + + @BeforeEach + @Override + public void setUp() { + super.setUp(); + client = WebClient.builder(serverConfig.getServerUrl()).build(); + } + + @Test + public void tokenIssuedByThisServerOpensASession() throws IOException { + String token = anAdminToken(); + + AggregatedHttpResponse response = signIn(token); + + assertThat(response.status()).isEqualTo(HttpStatus.OK); + // The session is the presented token, not a new one: it cannot outlive what was presented. + assertThat(MAPPER.readTree(response.contentUtf8()).get("access_token").asText()) + .isEqualTo(token); + assertThat(setCookieValue(response, AuthDecorator.UC_TOKEN_KEY)).isEqualTo(token); + + AggregatedHttpResponse me = + client + .execute( + RequestHeaders.builder(HttpMethod.GET, "/api/1.0/unity-control/scim2/Me") + .add(HttpHeaderNames.COOKIE, AuthDecorator.UC_TOKEN_KEY + "=" + token) + .build()) + .aggregate() + .join(); + assertThat(me.status()).isEqualTo(HttpStatus.OK); + assertThat(MAPPER.readTree(me.contentUtf8()).get("userName").asText()).isEqualTo("admin"); + } + + /** A token whose payload was edited no longer matches its signature. */ + @Test + public void anAlteredTokenIsRejected() throws IOException { + String token = anAdminToken(); + String[] parts = token.split("\\."); + String tampered = parts[0] + "." + parts[1] + "." + alterInTheMiddle(parts[2]); + + AggregatedHttpResponse response = signIn(tampered); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + assertThat(response.headers().getAll(HttpHeaderNames.SET_COOKIE)) + .noneMatch(c -> c.startsWith(AuthDecorator.UC_TOKEN_KEY + "=")); + } + + /** + * An identity provider's token is not a session here even when the provider is trusted: it goes + * to /auth/tokens, which checks issuer and audience and exchanges it. + */ + @Test + public void tokenFromAnotherIssuerIsRejected() { + String foreign = + JWT.create() + .withIssuer(testIssuer) + .withSubject("admin") + .withAudience(TEST_AUDIENCE) + .withKeyId(testIssuerKeyId) + .withExpiresAt(new Date(System.currentTimeMillis() + 60_000)) + .sign(testIssuerAlgorithm); + + AggregatedHttpResponse response = signIn(foreign); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + @Test + public void gibberishIsRejected() { + AggregatedHttpResponse response = signIn("not-a-token"); + + assertThat(response.status()).isEqualTo(HttpStatus.UNAUTHORIZED); + } + + @Test + public void anEmptyTokenIsRefused() { + AggregatedHttpResponse response = signIn(""); + + assertThat(response.status()).isEqualTo(HttpStatus.BAD_REQUEST); + } + + /** The administrator password sign-in is only a convenient source of a real token here. */ + private String anAdminToken() throws IOException { + String form = "username=admin&password=" + URLEncoder.encode(PASSWORD, StandardCharsets.UTF_8); + AggregatedHttpResponse response = + client + .execute( + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(AUTH_PATH + "/admin/login") + .contentType(MediaType.FORM_DATA) + .build(), + HttpData.ofUtf8(form)) + .aggregate() + .join(); + assertThat(response.status()).isEqualTo(HttpStatus.OK); + JsonNode body = MAPPER.readTree(response.contentUtf8()); + return body.get("access_token").asText(); + } + + private AggregatedHttpResponse signIn(String token) { + RequestHeaders headers = + RequestHeaders.builder() + .method(HttpMethod.POST) + .path(AUTH_PATH + "/token/login") + .contentType(MediaType.FORM_DATA) + .build(); + String form = "token=" + URLEncoder.encode(token, StandardCharsets.UTF_8); + return client.execute(headers, HttpData.ofUtf8(form)).aggregate().join(); + } + + /** + * Alters a character in the middle: base64url's final character carries only the leftover bits of + * the signature, some of which are discarded, so changing it can leave the same bytes behind. + */ + private static String alterInTheMiddle(String value) { + int at = value.length() / 2; + char replacement = value.charAt(at) == 'A' ? 'B' : 'A'; + return value.substring(0, at) + replacement + value.substring(at + 1); + } + + private static String setCookieValue(AggregatedHttpResponse response, String name) { + List cookies = response.headers().getAll(HttpHeaderNames.SET_COOKIE); + String header = + cookies.stream() + .filter(c -> c.startsWith(name + "=")) + .findFirst() + .orElseThrow(() -> new AssertionError("no Set-Cookie for " + name + " in " + cookies)); + String afterName = header.substring(name.length() + 1); + int semicolon = afterName.indexOf(';'); + return semicolon < 0 ? afterName : afterName.substring(0, semicolon); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsIssuerMatchTest.java b/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsIssuerMatchTest.java new file mode 100644 index 00000000..8279a9d7 --- /dev/null +++ b/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsIssuerMatchTest.java @@ -0,0 +1,80 @@ +package io.unitycatalog.server.utils; + +import static io.unitycatalog.server.utils.JwksOperations.issuerMatchesConfiguration; +import static org.assertj.core.api.Assertions.assertThat; + +import org.junit.jupiter.api.Test; + +/** + * Entra ID's multi-tenant discovery documents advertise {@code {tenantid}} where the token carries + * the real tenant GUID (issue #15). The comparison must accept exactly that substitution and + * nothing looser, and must leave single-tenant (exact) configurations untouched. + */ +public class JwksOperationsIssuerMatchTest { + + private static final String TENANT = "72f988bf-86f1-41af-91ab-2d7cd011db47"; + private static final String TEMPLATE = "https://login.microsoftonline.com/{tenantid}/v2.0"; + private static final String TENANT_ISSUER = + "https://login.microsoftonline.com/" + TENANT + "/v2.0"; + + @Test + public void exactIssuerStillMatchesExactly() { + assertThat(issuerMatchesConfiguration(TENANT_ISSUER, TENANT_ISSUER)).isTrue(); + assertThat( + issuerMatchesConfiguration( + "https://accounts.google.com", "https://accounts.google.com")) + .isTrue(); + } + + @Test + public void exactIssuerRejectsAnyDifference() { + assertThat( + issuerMatchesConfiguration( + TENANT_ISSUER, "https://login.microsoftonline.com/other-tenant/v2.0")) + .isFalse(); + assertThat( + issuerMatchesConfiguration( + "https://accounts.google.com", "https://accounts.google.com/")) + .isFalse(); + } + + @Test + public void tenantTemplateMatchesTheRealTenantGuid() { + assertThat(issuerMatchesConfiguration(TEMPLATE, TENANT_ISSUER)).isTrue(); + } + + @Test + public void tenantTemplateOnlyReplacesThatSegment() { + // Different host: the template must not turn into a wildcard for the whole URL. + assertThat(issuerMatchesConfiguration(TEMPLATE, "https://evil.example.com/" + TENANT + "/v2.0")) + .isFalse(); + // Different version segment. + assertThat( + issuerMatchesConfiguration( + TEMPLATE, "https://login.microsoftonline.com/" + TENANT + "/v1.0")) + .isFalse(); + // Different scheme. + assertThat( + issuerMatchesConfiguration( + TEMPLATE, "http://login.microsoftonline.com/" + TENANT + "/v2.0")) + .isFalse(); + } + + @Test + public void tenantTemplateRejectsMissingOrExtraSegments() { + assertThat(issuerMatchesConfiguration(TEMPLATE, "https://login.microsoftonline.com//v2.0")) + .isFalse(); + assertThat( + issuerMatchesConfiguration( + TEMPLATE, "https://login.microsoftonline.com/" + TENANT + "/extra/v2.0")) + .isFalse(); + assertThat(issuerMatchesConfiguration(TEMPLATE, "https://login.microsoftonline.com/v2.0")) + .isFalse(); + } + + @Test + public void nullsNeverMatch() { + assertThat(issuerMatchesConfiguration(null, TENANT_ISSUER)).isFalse(); + assertThat(issuerMatchesConfiguration(TEMPLATE, null)).isFalse(); + } +} diff --git a/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsTest.java b/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsTest.java index 51dc81b0..ed428102 100644 --- a/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsTest.java +++ b/server/src/test/java/io/unitycatalog/server/utils/JwksOperationsTest.java @@ -64,10 +64,19 @@ public void keyIsUsableOnlyForItsRegisteredIssuer() throws Exception { @Test public void keyWithoutIssuerMemberIsRejected() throws Exception { - String jwks = "{\"keys\":[" + entry("kidNoIssuer", X_A, Y_A, null) + "]}"; + // issuer-a IS registered (via kidA), so the file provider is consulted; the unbound key in the + // same file must still not be handed out for it. (An issuer with no registered key at all + // never reaches the file: it is routed to OIDC discovery, see knownIssuers* below.) + String jwks = + "{\"keys\":[" + + entry("kidA", X_A, Y_A, "issuer-a") + + "," + + entry("kidNoIssuer", X_B, Y_B, null) + + "]}"; JwkProvider provider = providerFor("issuer-a", jwks); + assertThat(provider.get("kidA").getId()).isEqualTo("kidA"); assertThatThrownBy(() -> provider.get("kidNoIssuer")).isInstanceOf(JwkException.class); } diff --git a/ui/.env b/ui/.env index ff6b7567..823c24c3 100644 --- a/ui/.env +++ b/ui/.env @@ -11,4 +11,8 @@ REACT_APP_OKTA_CLIENT_ID= REACT_APP_KEYCLOAK_AUTH_ENABLED=false REACT_APP_KEYCLOAK_URL= REACT_APP_KEYCLOAK_REALM_ID= -REACT_APP_KEYCLOAK_CLIENT_ID= \ No newline at end of file +REACT_APP_KEYCLOAK_CLIENT_ID= + +# Microsoft Entra ID sign-in and the administrator password sign-in need no +# setting here: the UI asks the server (GET /auth/providers) which of them are +# configured and whether sign-in is required at all. diff --git a/ui/server.js b/ui/server.js index 4038d1a6..1bfcb5d7 100644 --- a/ui/server.js +++ b/ui/server.js @@ -1,10 +1,14 @@ // Standalone Unity Catalog UI server. // // Serves the CRA production build (`yarn build` -> ./build) and proxies the -// Unity Catalog REST API, injecting a server-side bearer token so the browser -// needs no OAuth/IdP login. This is the built-bundle counterpart of the -// dev-only src/setupProxy.js (same token-injection trick, but for a real -// process that can run outside `react-scripts start`). +// Unity Catalog REST API. It adds no credentials of its own: every visitor +// signs in and carries their own UC_TOKEN cookie, so who you are in the UI is +// who the server says you are. +// +// It used to inject the admin bearer token into every proxied call, which made +// this address an unauthenticated way into an authorized server -- opening it +// was enough to be admin. The token now has an entry point of its own, +// /login/token, which the server verifies before opening a session. // // Used two ways: // - local "binary package" mode: `yarn build` then `node server.js` @@ -14,8 +18,10 @@ // Env: // PORT listen port (default 3000) // HOST bind address (default 0.0.0.0) -// UC_TARGET Unity Catalog server base URL (default http://localhost:8088) -// UC_TOKEN_FILE path to the UC admin token file (bearer injected into /api/*) — required +// UC_TARGET Unity Catalog server base URL (default http://localhost:8089) +// This is the server's own port, which is the port it is started with plus +// one: the started port runs a URL transcoder that the UI does not need and +// that drops bodyless answers such as the sign-in redirect. const path = require('path'); const fs = require('fs'); const express = require('express'); @@ -23,8 +29,7 @@ const { createProxyMiddleware } = require('http-proxy-middleware'); const PORT = process.env.PORT || 3000; const HOST = process.env.HOST || '0.0.0.0'; -const UC_TARGET = process.env.UC_TARGET || 'http://localhost:8088'; -const TOKEN_FILE = process.env.UC_TOKEN_FILE; +const UC_TARGET = process.env.UC_TARGET || 'http://localhost:8089'; const BUILD_DIR = path.join(__dirname, 'build'); function die(msg) { @@ -32,35 +37,22 @@ function die(msg) { process.exit(1); } -if (!TOKEN_FILE) { - die('UC_TOKEN_FILE is required (path to the UC admin token).'); -} if (!fs.existsSync(path.join(BUILD_DIR, 'index.html'))) { die(`UI build not found at ${BUILD_DIR}. Run "yarn build" first.`); } -let token; -try { - token = fs.readFileSync(TOKEN_FILE, 'utf8').trim(); -} catch (err) { - die(`cannot read UC_TOKEN_FILE (${TOKEN_FILE}): ${err.message}`); -} -if (!token) { - die(`UC_TOKEN_FILE (${TOKEN_FILE}) is empty.`); -} - const app = express(); -// Proxy the UC REST API first (before static/fallback), injecting the bearer -// token. No body parser runs ahead of this, so POST/PATCH bodies stream through -// untouched. +// Proxy the UC REST API first (before static/fallback). Requests go through as +// the browser sent them, cookie included; no body parser runs ahead of this, so +// POST/PATCH bodies stream through untouched. app.use( ['/api/1.0', '/api/2.1'], createProxyMiddleware({ target: UC_TARGET, changeOrigin: true, - onProxyReq: (proxyReq) => { - proxyReq.setHeader('Authorization', 'Bearer ' + token); - }, + // X-Forwarded-Proto/Host let the server build the OAuth callback URL the + // browser actually uses (this origin), not the proxied target's. + xfwd: true, }), ); diff --git a/ui/src/App.test.tsx b/ui/src/App.test.tsx new file mode 100644 index 00000000..2c455228 --- /dev/null +++ b/ui/src/App.test.tsx @@ -0,0 +1,131 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import { createMemoryRouter, RouterProvider } from 'react-router-dom'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { NotificationProvider } from './utils/NotificationContext'; +import { appRoutes } from './App'; +import { useAuth } from './context/auth-context'; +import { useAuthProviders } from './hooks/auth-providers'; + +// The real route table is under test, so only the leaves are replaced: the +// identity hooks, the sign-in buttons that need a live SDK, and the two heavy +// pages behind the gate. +jest.mock('./context/auth-context', () => ({ + AuthProvider: ({ children }: { children: React.ReactNode }) => ( + <>{children} + ), + useAuth: jest.fn(), +})); +jest.mock('./hooks/auth-providers', () => ({ useAuthProviders: jest.fn() })); +jest.mock('./context/client', () => ({ CLIENT: { request: jest.fn() } })); +jest.mock( + './components/login/OktaAuthButton', + () => + function OktaAuthButton() { + return

; + }, +); +jest.mock( + './components/login/GoogleAuthButton', + () => + function GoogleAuthButton() { + return
; + }, +); +jest.mock( + './components/login/KeycloakAuthButton', + () => + function KeycloakAuthButton() { + return
; + }, +); +jest.mock( + './components/SchemaBrowser', + () => + function SchemaBrowser() { + return
; + }, +); +jest.mock( + './pages/CatalogsList', + () => + function CatalogsList() { + return
Catalogs page
; + }, +); + +const mockUseAuth = useAuth as jest.Mock; +const mockUseAuthProviders = useAuthProviders as jest.Mock; + +function signedIn(user: object | null) { + mockUseAuth.mockReturnValue({ + currentUser: user, + currentUserPending: false, + logout: jest.fn(), + }); +} + +/** Mounts the application's own routes at an address. */ +function openAt(path: string) { + const router = createMemoryRouter(appRoutes, { initialEntries: [path] }); + render( + + + + + , + ); + return router; +} + +describe('App routing', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockUseAuthProviders.mockReturnValue({ + data: { + authorization_enabled: true, + hosted_login: true, + admin_login: true, + }, + isPending: false, + }); + }); + + it('sends a visitor without a session away from the application', async () => { + signedIn(null); + + const router = openAt('/'); + + expect(await screen.findByText('Login to Unity Catalog')).toBeVisible(); + await waitFor(() => expect(router.state.location.pathname).toBe('/login')); + }); + + it('keeps the administrator form on an address of its own', async () => { + signedIn(null); + + openAt('/login/admin'); + + expect(await screen.findByText('Administrator sign-in')).toBeVisible(); + }); + + it('keeps the token sign-in on an address of its own', async () => { + signedIn(null); + + openAt('/login/token'); + + expect(await screen.findByText('Sign in with a token')).toBeVisible(); + }); + + /** Once signed in the sign-in pages have nothing to offer. */ + it('sends a signed-in visitor from a sign-in page to the application', async () => { + signedIn({ id: '1', displayName: 'Admin' }); + + const router = openAt('/login/admin'); + + expect(await screen.findByText('Catalogs page')).toBeVisible(); + await waitFor(() => expect(router.state.location.pathname).toBe('/')); + }); +}); diff --git a/ui/src/App.tsx b/ui/src/App.tsx index dca39706..942ab81e 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -12,6 +12,7 @@ import { createBrowserRouter, Navigate, Outlet, + RouteObject, RouterProvider, useLocation, useNavigate, @@ -29,6 +30,9 @@ import SchemaDetails from './pages/SchemaDetails'; import { NotificationProvider } from './utils/NotificationContext'; import ModelDetails from './pages/ModelDetails'; import Login from './pages/Login'; +import AdminLogin from './pages/AdminLogin'; +import TokenLogin from './pages/TokenLogin'; +import { useAuthProviders } from './hooks/auth-providers'; import { AuthProvider, useAuth } from './context/auth-context'; import { UserOutlined } from '@ant-design/icons'; import ModelVersionDetails from './pages/ModelVersionDetails'; @@ -42,9 +46,47 @@ import ResizableSplit from './components/layouts/ResizableSplit'; // As of [19/02/2025], this implementation should be updated once the following PR are merged. // SEE: // https://github.com/unitycatalog/unitycatalog/pull/809 -const authEnabled = process.env.REACT_APP_GOOGLE_AUTH_ENABLED === 'true'; +// Google sign-in is the one provider still decided at build time (it runs an +// OAuth client in the browser). Whether sign-in is required at all, whether +// Microsoft sign-in is offered and whether the administrator password is set +// are asked of the server at runtime (useAuthProviders), so a single UI build +// follows the server's configuration. +const buildTimeAuth = process.env.REACT_APP_GOOGLE_AUTH_ENABLED === 'true'; -const router = createBrowserRouter([ +// The sign-in entry points, each an address of its own. /login is the default +// one people are sent to; the other two are for whoever runs the server and are +// not linked from it. The application itself is never an entry point: without a +// session it redirects here. +export const LOGIN_PATH = '/login'; +export const ADMIN_LOGIN_PATH = '/login/admin'; +export const TOKEN_LOGIN_PATH = '/login/token'; + +/** The route table, exported so tests can mount it on a memory router. */ +export const appRoutes: RouteObject[] = [ + { + path: LOGIN_PATH, + element: ( + + + + ), + }, + { + path: ADMIN_LOGIN_PATH, + element: ( + + + + ), + }, + { + path: TOKEN_LOGIN_PATH, + element: ( + + + + ), + }, { element: , children: [ @@ -106,12 +148,48 @@ const router = createBrowserRouter([ }, ], }, -]); +]; + +const router = createBrowserRouter(appRoutes); + +/** + * Whether a session is needed, whether there is one, and whether either is + * still being established. Both the application and the sign-in pages decide + * from this, so they cannot disagree about who is signed in. + */ +function useSignInState() { + const { currentUser, currentUserPending } = useAuth(); + const { data: providers, isPending: providersPending } = useAuthProviders(); + const loginRequired = + buildTimeAuth || providers?.authorization_enabled === true; + // Until the server has said whether sign-in is required, and then until the + // session has been checked, show neither the login page nor the app: + // flashing either one misleads. + const pending = + (!buildTimeAuth && providersPending) || + (loginRequired && currentUserPending); + return { loginRequired, pending, currentUser }; +} + +/** A sign-in page: pointless once signed in, and when nothing requires a session. */ +function SignInRoute({ children }: { children: React.ReactNode }) { + const { loginRequired, pending, currentUser } = useSignInState(); + + if (pending) { + return

Loading...

; + } + if (currentUser || !loginRequired) { + return ; + } + return <>{children}; +} function AppProvider() { const { logout, currentUser } = useAuth(); const navigate = useNavigate(); - const { pathname } = useLocation(); + const location = useLocation(); + const { pathname } = location; + const { loginRequired, pending } = useSignInState(); const selectedNavKey = pathname.startsWith('/external-data') ? 'external-data' @@ -148,9 +226,21 @@ function AppProvider() { [currentUser, logout, navigate], ); - return authEnabled && !currentUser ? ( - - ) : ( + if (pending) { + return

Loading...

; + } + // No session: the address people land on is the login page, not this one. + if (loginRequired && !currentUser) { + return ( + + ); + } + + return (
- {authEnabled && ( + {loginRequired && (
@@ -281,12 +373,6 @@ function App() { - ) : ( - - - Loading...

} /> - - ); } diff --git a/ui/src/components/login/EntraAuthButton.test.tsx b/ui/src/components/login/EntraAuthButton.test.tsx new file mode 100644 index 00000000..599e3119 --- /dev/null +++ b/ui/src/components/login/EntraAuthButton.test.tsx @@ -0,0 +1,54 @@ +import { fireEvent, render, screen } from '@testing-library/react'; +import EntraAuthButton, { hostedLoginUrl } from './EntraAuthButton'; + +describe('hostedLoginUrl', () => { + it('points at the server-hosted login with the return path encoded', () => { + expect(hostedLoginUrl('/data/demo?tab=tables')).toBe( + '/api/1.0/unity-control/auth/login?redirect=%2Fdata%2Fdemo%3Ftab%3Dtables', + ); + }); + + it('defaults to the root', () => { + expect(hostedLoginUrl(undefined)).toBe( + '/api/1.0/unity-control/auth/login?redirect=%2F', + ); + expect(hostedLoginUrl('')).toBe( + '/api/1.0/unity-control/auth/login?redirect=%2F', + ); + }); + + /** A crafted link must not be able to bounce the user to another site after login. */ + it('refuses absolute and scheme-relative return targets', () => { + expect(hostedLoginUrl('https://evil.example/')).toBe( + '/api/1.0/unity-control/auth/login?redirect=%2F', + ); + expect(hostedLoginUrl('//evil.example/x')).toBe( + '/api/1.0/unity-control/auth/login?redirect=%2F', + ); + }); +}); + +describe('EntraAuthButton', () => { + it('navigates the browser to the hosted login', () => { + const assign = jest.fn(); + const original = window.location; + Object.defineProperty(window, 'location', { + value: { ...original, assign }, + writable: true, + }); + + render(); + fireEvent.click( + screen.getByRole('button', { name: /Sign in with Microsoft/ }), + ); + + expect(assign).toHaveBeenCalledWith( + '/api/1.0/unity-control/auth/login?redirect=%2Fdata%2Fdemo', + ); + + Object.defineProperty(window, 'location', { + value: original, + writable: true, + }); + }); +}); diff --git a/ui/src/components/login/EntraAuthButton.tsx b/ui/src/components/login/EntraAuthButton.tsx new file mode 100644 index 00000000..9b0ca380 --- /dev/null +++ b/ui/src/components/login/EntraAuthButton.tsx @@ -0,0 +1,46 @@ +import { Button } from 'antd'; +import { WindowsOutlined } from '@ant-design/icons'; +import { UC_AUTH_API_PREFIX } from '../../utils/constants'; + +interface EntraAuthButtonProps { + /** In-app path to return to after login. */ + returnTo?: string; +} + +/** + * The server-hosted login entry point. Unlike the Google/Okta buttons this + * runs no OAuth client in the browser: the server redirects to Microsoft, + * redeems the code with a secret that never leaves it, and comes back with the + * UC_TOKEN cookie set. Only a relative path may be handed over as the return + * target; the server enforces the same rule. + */ +export function hostedLoginUrl(returnTo?: string): string { + const redirect = + returnTo && returnTo.startsWith('/') && !returnTo.startsWith('//') + ? returnTo + : '/'; + return `${UC_AUTH_API_PREFIX}/auth/login?redirect=${encodeURIComponent(redirect)}`; +} + +/** Microsoft's own blue, so the button reads as the Microsoft one at a glance. */ +const MICROSOFT_BLUE = '#0078D4'; + +export default function EntraAuthButton({ returnTo }: EntraAuthButtonProps) { + return ( + + ); +} diff --git a/ui/src/context/auth-context.tsx b/ui/src/context/auth-context.tsx index 8a825115..ed6bbeb3 100644 --- a/ui/src/context/auth-context.tsx +++ b/ui/src/context/auth-context.tsx @@ -1,6 +1,8 @@ import React, { useCallback, useMemo } from 'react'; import { useGetCurrentUser, + useLoginWithAccessToken, + useLoginWithPassword, useLoginWithToken, useLogoutCurrentUser, UserInterface, @@ -11,21 +13,36 @@ import { useNotification } from '../utils/NotificationContext'; interface AuthContextProps { accessToken: any; loginWithToken: any; + /** Administrator password sign-in; rejects on a wrong password. */ + loginWithPassword: (username: string, password: string) => Promise; + /** Sign-in with an access token this server issued; rejects on an invalid one. */ + loginWithAccessToken: (token: string) => Promise; logout: any; currentUser: UserInterface | null; + /** True until the first /scim2/Me answer, so callers can avoid flashing a login page. */ + currentUserPending: boolean; } const AuthContext = React.createContext({ accessToken: null, loginWithToken: null, + loginWithPassword: async () => {}, + loginWithAccessToken: async () => {}, logout: null, currentUser: null, + currentUserPending: false, }); AuthContext.displayName = 'AuthContext'; function AuthProvider(props: any) { - const { data: currentUser, refetch } = useGetCurrentUser(); + const { + data: currentUser, + refetch, + isPending: currentUserPending, + } = useGetCurrentUser(); const loginWithTokenMutation = useLoginWithToken(); + const loginWithPasswordMutation = useLoginWithPassword(); + const loginWithAccessTokenMutation = useLoginWithAccessToken(); const logoutUser = useLogoutCurrentUser(); const { setNotification } = useNotification(); @@ -54,6 +71,35 @@ function AuthProvider(props: any) { [loginWithTokenMutation, setNotification, refetch], ); + const loginWithPassword = useCallback( + async (username: string, password: string) => { + try { + await loginWithPasswordMutation.mutateAsync({ username, password }); + } catch (error) { + setNotification( + 'Login failed. Check the username and password.', + 'error', + ); + throw error; + } + await refetch(); + }, + [loginWithPasswordMutation, setNotification, refetch], + ); + + const loginWithAccessToken = useCallback( + async (token: string) => { + try { + await loginWithAccessTokenMutation.mutateAsync({ token }); + } catch (error) { + setNotification('Login failed. Check the access token.', 'error'); + throw error; + } + await refetch(); + }, + [loginWithAccessTokenMutation, setNotification, refetch], + ); + const logout = useCallback(async () => { return logoutUser.mutate( {}, @@ -74,10 +120,20 @@ function AuthProvider(props: any) { const value = useMemo( () => ({ loginWithToken, + loginWithPassword, + loginWithAccessToken, logout, currentUser, + currentUserPending, }), - [loginWithToken, logout, currentUser], + [ + loginWithToken, + loginWithPassword, + loginWithAccessToken, + logout, + currentUser, + currentUserPending, + ], ); return ; diff --git a/ui/src/hooks/auth-providers.ts b/ui/src/hooks/auth-providers.ts new file mode 100644 index 00000000..f6b07177 --- /dev/null +++ b/ui/src/hooks/auth-providers.ts @@ -0,0 +1,52 @@ +import { useQuery } from '@tanstack/react-query'; +import { CLIENT } from '../context/client'; +import { UC_AUTH_API_PREFIX } from '../utils/constants'; +import { route, isError } from '../utils/openapi'; +import type { + paths as ControlApi, + components as ControlComponent, +} from '../types/api/control.gen'; +import type { Model, Route } from '../utils/openapi'; + +export interface AuthProvidersInterface + extends Model {} + +/** + * What the server offers for signing in, asked at runtime rather than baked + * in at build time: whether a token is required at all, whether the + * server-hosted Microsoft sign-in is configured, and whether the administrator + * password entry point is. One UI build thereby follows the server's + * configuration. + * + * The endpoint is public. A server that predates it (404) is treated as + * offering nothing, which leaves the build-time Google switch as the only + * gate -- the behaviour such a server had before. + */ +export function useAuthProviders() { + return useQuery({ + queryKey: ['authProviders'], + staleTime: Infinity, + queryFn: async () => { + try { + const response = await (route as Route)({ + client: CLIENT, + request: { path: '/auth/providers', method: 'get' }, + config: { baseURL: UC_AUTH_API_PREFIX }, + errorMessage: 'Failed to fetch sign-in providers', + }).call(); + if (isError(response)) { + return NONE; + } + return response.data; + } catch { + return NONE; + } + }, + }); +} + +const NONE: AuthProvidersInterface = { + authorization_enabled: false, + hosted_login: false, + admin_login: false, +}; diff --git a/ui/src/hooks/user.ts b/ui/src/hooks/user.ts index c473c050..ed05d5e0 100644 --- a/ui/src/hooks/user.ts +++ b/ui/src/hooks/user.ts @@ -63,6 +63,90 @@ export function useLoginWithToken() { }); } +export interface LoginWithPasswordMutationParams + extends RequestBody< + ControlApi, + '/auth/admin/login', + 'post', + 'application/x-www-form-urlencoded' + > {} + +/** + * The administrator's password sign-in. Like the token exchange it asks for the + * session to be set as a cookie, so the browser is signed in the same way a + * Microsoft sign-in leaves it. + */ +export function useLoginWithPassword() { + return useMutation< + OAuthTokenExchangeInterface, + Error, + LoginWithPasswordMutationParams + >({ + mutationFn: async (params: LoginWithPasswordMutationParams) => { + const response = await (route as Route)({ + client: CLIENT, + request: { + path: '/auth/admin/login', + method: 'post', + params: { body: params }, + }, + config: { + baseURL: UC_AUTH_API_PREFIX, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }, + errorMessage: 'Failed to login', + }).call(); + if (isError(response)) { + return assertNever(response.data.status); + } else { + return response.data; + } + }, + }); +} + +export interface LoginWithAccessTokenMutationParams + extends RequestBody< + ControlApi, + '/auth/token/login', + 'post', + 'application/x-www-form-urlencoded' + > {} + +/** + * Opens a session from an access token this server already issued, which is how + * an operator holding the token in etc/conf/token.txt reaches the UI. The token + * itself becomes the session cookie; the server verifies it first. + */ +export function useLoginWithAccessToken() { + return useMutation< + OAuthTokenExchangeInterface, + Error, + LoginWithAccessTokenMutationParams + >({ + mutationFn: async (params: LoginWithAccessTokenMutationParams) => { + const response = await (route as Route)({ + client: CLIENT, + request: { + path: '/auth/token/login', + method: 'post', + params: { body: params }, + }, + config: { + baseURL: UC_AUTH_API_PREFIX, + headers: { 'Content-Type': 'application/x-www-form-urlencoded' }, + }, + errorMessage: 'Failed to login', + }).call(); + if (isError(response)) { + return assertNever(response.data.status); + } else { + return response.data; + } + }, + }); +} + export interface LogoutCurrentUserMutationParams {} export function useLogoutCurrentUser() { diff --git a/ui/src/pages/AdminLogin.test.tsx b/ui/src/pages/AdminLogin.test.tsx new file mode 100644 index 00000000..3cf171fb --- /dev/null +++ b/ui/src/pages/AdminLogin.test.tsx @@ -0,0 +1,73 @@ +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../test-utils/render'; +import AdminLoginPage from './AdminLogin'; +import { useAuth } from '../context/auth-context'; +import { useAuthProviders } from '../hooks/auth-providers'; + +jest.mock('../context/client', () => ({ CLIENT: { request: jest.fn() } })); +jest.mock('../context/auth-context', () => ({ useAuth: jest.fn() })); +jest.mock('../hooks/auth-providers', () => ({ useAuthProviders: jest.fn() })); + +const mockUseAuth = useAuth as jest.Mock; +const mockUseAuthProviders = useAuthProviders as jest.Mock; + +function providers(admin_login: boolean) { + mockUseAuthProviders.mockReturnValue({ + data: { authorization_enabled: true, hosted_login: false, admin_login }, + isPending: false, + }); +} + +describe('AdminLoginPage', () => { + const loginWithPassword = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + mockUseAuth.mockReturnValue({ loginWithPassword }); + }); + + it('signs in with the entered credentials', async () => { + providers(true); + loginWithPassword.mockResolvedValue(undefined); + renderWithProviders(); + + // The username defaults to the built-in administrator. + expect(screen.getByLabelText('Username')).toHaveValue('admin'); + fireEvent.change(screen.getByLabelText('Password'), { + target: { value: 's3cret' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Sign in' })); + + await waitFor(() => + expect(loginWithPassword).toHaveBeenCalledWith('admin', 's3cret'), + ); + }); + + it('shows a clear failure and no session on a rejected password', async () => { + providers(true); + loginWithPassword.mockRejectedValue(new Error('401')); + renderWithProviders(); + + fireEvent.change(screen.getByLabelText('Password'), { + target: { value: 'wrong' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Sign in' })); + + expect( + await screen.findByText('Wrong username or password'), + ).toBeInTheDocument(); + }); + + /** The page exists whether or not the server has a password; it must say which. */ + it('explains when the server has no administrator password', () => { + providers(false); + renderWithProviders(); + + expect( + screen.getByText( + 'Administrator sign-in is not configured on this server', + ), + ).toBeInTheDocument(); + expect(screen.queryByRole('button', { name: 'Sign in' })).toBeNull(); + }); +}); diff --git a/ui/src/pages/AdminLogin.tsx b/ui/src/pages/AdminLogin.tsx new file mode 100644 index 00000000..53ba097b --- /dev/null +++ b/ui/src/pages/AdminLogin.tsx @@ -0,0 +1,138 @@ +import { Alert, Button, Flex, Form, Input, Layout, Typography } from 'antd'; +import { LockOutlined, UserOutlined } from '@ant-design/icons'; +import { useState } from 'react'; +import { useNavigate } from 'react-router-dom'; +import { useAuth } from '../context/auth-context'; +import { useAuthProviders } from '../hooks/auth-providers'; + +interface AdminLoginFormValues { + username: string; + password: string; +} + +/** + * The administrator's entry point, deliberately not linked from the login + * page: it exists for whoever runs the server, not for the people signing in + * with their Microsoft account. The password lives in server.properties and is + * checked server-side; a successful sign-in ends in the same UC_TOKEN cookie a + * Microsoft sign-in does, so nothing downstream distinguishes the two. + */ +export default function AdminLoginPage() { + const { loginWithPassword } = useAuth(); + const { data: providers } = useAuthProviders(); + const navigate = useNavigate(); + const [submitting, setSubmitting] = useState(false); + const [failed, setFailed] = useState(false); + + const configured = providers?.admin_login === true; + + const onFinish = async ({ username, password }: AdminLoginFormValues) => { + setFailed(false); + setSubmitting(true); + try { + await loginWithPassword(username, password); + navigate('/', { replace: true }); + } catch { + setFailed(true); + } finally { + setSubmitting(false); + } + }; + + return ( + + +
+ uc-logo-horizontal +
+
+ + Administrator sign-in + {providers && !configured ? ( + + ) : ( + + layout="vertical" + style={{ width: 300 }} + initialValues={{ username: 'admin' }} + onFinish={onFinish} + disabled={!configured} + > + + } autoComplete="username" /> + + + } + autoComplete="current-password" + /> + + {failed && ( + + + + )} + + + + + )} + +
+ + + ); +} diff --git a/ui/src/pages/Login.test.tsx b/ui/src/pages/Login.test.tsx new file mode 100644 index 00000000..8fd792ed --- /dev/null +++ b/ui/src/pages/Login.test.tsx @@ -0,0 +1,74 @@ +import { screen } from '@testing-library/react'; +import { renderWithProviders } from '../test-utils/render'; +import LoginPage from './Login'; + +jest.mock('../context/client', () => ({ CLIENT: { request: jest.fn() } })); +// The third-party sign-in widgets touch browser APIs jsdom lacks at import +// time (the Okta widget reads canvas pixel ratios). They are not what this page +// test is about; stub them so the page's provider gating can be exercised. +jest.mock('../components/login/OktaAuthButton', () => () => ( + +)); +jest.mock('../components/login/GoogleAuthButton', () => () => ( + +)); +jest.mock('../hooks/auth-providers', () => ({ useAuthProviders: jest.fn() })); +jest.mock('../context/auth-context', () => ({ + useAuth: () => ({ loginWithToken: jest.fn() }), +})); + +const mockUseAuthProviders = jest.requireMock('../hooks/auth-providers') + .useAuthProviders as jest.Mock; +function serverOffers(hosted_login: boolean) { + mockUseAuthProviders.mockReturnValue({ + data: { authorization_enabled: true, hosted_login, admin_login: true }, + isPending: false, + }); +} + +/** + * The login page shows one button per enabled provider. Microsoft sign-in is + * offered when the server reports hosting it; the administrator entry point is + * never linked from here. + */ +describe('LoginPage', () => { + const env = process.env; + + afterEach(() => { + process.env = env; + }); + + it('offers Microsoft sign-in when the server hosts it', () => { + serverOffers(true); + + renderWithProviders(); + + expect( + screen.getByRole('button', { name: /Sign in with Microsoft/ }), + ).toBeInTheDocument(); + expect( + screen.queryByText('No sign-in provider is configured on this server'), + ).toBeNull(); + // Even with admin login configured, this page does not advertise it. + expect(screen.queryByText(/administrator/i)).toBeNull(); + }); + + it('reports no providers when none is enabled', () => { + serverOffers(false); + process.env = { + ...env, + REACT_APP_GOOGLE_AUTH_ENABLED: 'false', + REACT_APP_OKTA_AUTH_ENABLED: 'false', + REACT_APP_KEYCLOAK_AUTH_ENABLED: 'false', + }; + + renderWithProviders(); + + expect( + screen.getByText('No sign-in provider is configured on this server'), + ).toBeInTheDocument(); + expect( + screen.queryByRole('button', { name: /Sign in with Microsoft/ }), + ).toBeNull(); + }); +}); diff --git a/ui/src/pages/Login.tsx b/ui/src/pages/Login.tsx index 46b447cf..dcee8ba5 100644 --- a/ui/src/pages/Login.tsx +++ b/ui/src/pages/Login.tsx @@ -4,6 +4,8 @@ import GoogleAuthButton from '../components/login/GoogleAuthButton'; import OktaAuthButton from '../components/login/OktaAuthButton'; import { useAuth } from '../context/auth-context'; import KeycloakAuthButton from '../components/login/KeycloakAuthButton'; +import EntraAuthButton from '../components/login/EntraAuthButton'; +import { useAuthProviders } from '../hooks/auth-providers'; import { useNavigate, useLocation } from 'react-router-dom'; export default function LoginPage() { @@ -15,6 +17,9 @@ export default function LoginPage() { const oktaEnabled = process.env.REACT_APP_OKTA_AUTH_ENABLED === 'true'; const keycloakEnabled = process.env.REACT_APP_KEYCLOAK_AUTH_ENABLED === 'true'; + // Offered whenever the server hosts the flow; nothing to configure in the UI. + const { data: providers } = useAuthProviders(); + const entraEnabled = providers?.hosted_login === true; const handleGoogleSignIn = async (idToken: string) => { await loginWithToken(idToken).then(() => navigate(from, { replace: true })); @@ -71,9 +76,26 @@ export default function LoginPage() { /> )} {keycloakEnabled && } - {!googleEnabled && !oktaEnabled && !keycloakEnabled && ( - Auth providers have not been enabled - )} + {entraEnabled && } + {!googleEnabled && + !oktaEnabled && + !keycloakEnabled && + !entraEnabled && ( + + + No sign-in provider is configured on this server + + + Sign in with Microsoft appears here once the server has the + authorization URL, token URL, client id and client secret of + an identity provider. Whoever runs the server signs in with + a password or an access token instead. + + + )}
diff --git a/ui/src/pages/TokenLogin.test.tsx b/ui/src/pages/TokenLogin.test.tsx new file mode 100644 index 00000000..95834c29 --- /dev/null +++ b/ui/src/pages/TokenLogin.test.tsx @@ -0,0 +1,63 @@ +import { fireEvent, screen, waitFor } from '@testing-library/react'; +import { renderWithProviders } from '../test-utils/render'; +import TokenLoginPage from './TokenLogin'; +import { useAuth } from '../context/auth-context'; + +jest.mock('../context/client', () => ({ CLIENT: { request: jest.fn() } })); +jest.mock('../context/auth-context', () => ({ useAuth: jest.fn() })); + +const mockUseAuth = useAuth as jest.Mock; + +describe('TokenLoginPage', () => { + const loginWithAccessToken = jest.fn(); + + beforeEach(() => { + jest.clearAllMocks(); + mockUseAuth.mockReturnValue({ loginWithAccessToken }); + }); + + it('signs in with the pasted token', async () => { + loginWithAccessToken.mockResolvedValue(undefined); + renderWithProviders(); + + fireEvent.change(screen.getByLabelText('Unity Catalog access token'), { + target: { value: ' header.payload.signature ' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Sign in' })); + + await waitFor(() => + expect(loginWithAccessToken).toHaveBeenCalledWith( + 'header.payload.signature', + ), + ); + }); + + it('says so when the server rejects the token', async () => { + loginWithAccessToken.mockRejectedValue(new Error('401')); + renderWithProviders(); + + fireEvent.change(screen.getByLabelText('Unity Catalog access token'), { + target: { value: 'stale.token.here' }, + }); + fireEvent.click(screen.getByRole('button', { name: 'Sign in' })); + + expect( + await screen.findByText('Token is not valid for sign-in'), + ).toBeInTheDocument(); + }); + + /** A bookmarkable link signs in by itself, and the token leaves the address bar. */ + it('uses a token given in the query string exactly once', async () => { + loginWithAccessToken.mockResolvedValue(undefined); + renderWithProviders(, { + route: '/login/token?token=header.payload.signature', + }); + + await waitFor(() => + expect(loginWithAccessToken).toHaveBeenCalledWith( + 'header.payload.signature', + ), + ); + expect(loginWithAccessToken).toHaveBeenCalledTimes(1); + }); +}); diff --git a/ui/src/pages/TokenLogin.tsx b/ui/src/pages/TokenLogin.tsx new file mode 100644 index 00000000..9d8b2d90 --- /dev/null +++ b/ui/src/pages/TokenLogin.tsx @@ -0,0 +1,130 @@ +import { Alert, Button, Flex, Form, Input, Layout, Typography } from 'antd'; +import { useCallback, useEffect, useRef, useState } from 'react'; +import { useNavigate, useSearchParams } from 'react-router-dom'; +import { useAuth } from '../context/auth-context'; + +interface TokenLoginFormValues { + token: string; +} + +/** + * The operator's way in with an access token this server issued -- the one in + * etc/conf/token.txt. It replaces the proxy's blanket token injection, which + * turned the application's own address into an unauthenticated entry point: + * the token is now presented at an address of its own, and buys exactly the + * identity it names. + * + * A token may also be put in the query string for a bookmarkable link. That is + * a convenience with a cost -- the URL reaches browser history, proxy logs and + * Referer headers -- so the page signs in and immediately drops the token from + * the address bar, and the form is the way that leaves no trail. + */ +export default function TokenLoginPage() { + const { loginWithAccessToken } = useAuth(); + const navigate = useNavigate(); + const [searchParams, setSearchParams] = useSearchParams(); + const [submitting, setSubmitting] = useState(false); + const [failed, setFailed] = useState(false); + const attemptedFromUrl = useRef(false); + + const signIn = useCallback( + async (token: string) => { + setFailed(false); + setSubmitting(true); + try { + await loginWithAccessToken(token); + navigate('/', { replace: true }); + } catch { + setFailed(true); + } finally { + setSubmitting(false); + } + }, + [loginWithAccessToken, navigate], + ); + + useEffect(() => { + const fromUrl = searchParams.get('token'); + if (!fromUrl || attemptedFromUrl.current) return; + attemptedFromUrl.current = true; + // Out of the address bar before anything else happens to it. + setSearchParams({}, { replace: true }); + signIn(fromUrl); + }, [searchParams, setSearchParams, signIn]); + + return ( + + +
+ uc-logo-horizontal +
+
+ + Sign in with a token + + layout="vertical" + style={{ width: 320 }} + onFinish={({ token }) => signIn(token.trim())} + > + + + + {failed && ( + + + + )} + + + + + +
+ + + ); +} diff --git a/ui/src/test-utils/render.tsx b/ui/src/test-utils/render.tsx index f49eb31e..9cf68d89 100644 --- a/ui/src/test-utils/render.tsx +++ b/ui/src/test-utils/render.tsx @@ -9,7 +9,11 @@ import { NotificationProvider } from '../utils/NotificationContext'; * retries, an in-memory router and the notification provider) so component * tests exercise the same wiring as the app. */ -export function renderWithProviders(ui: ReactElement) { +export function renderWithProviders( + ui: ReactElement, + /** Address to start at, for components that read the path or query string. */ + options: { route?: string } = {}, +) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false }, @@ -19,7 +23,9 @@ export function renderWithProviders(ui: ReactElement) { return render( - {ui} + + {ui} + , ); diff --git a/ui/src/types/api/control.gen.ts b/ui/src/types/api/control.gen.ts index f2e98f0d..c6a8f48f 100644 --- a/ui/src/types/api/control.gen.ts +++ b/ui/src/types/api/control.gen.ts @@ -102,6 +102,120 @@ export interface paths { patch?: never; trace?: never; }; + '/auth/login': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Start the server-hosted login flow + * @description Redirects the browser to the configured identity provider's authorization endpoint + * (server.authorization-url) with a one-time state bound to a short-lived cookie. The + * identity provider sends the browser back to /auth/callback. Only available when the + * OAuth client properties are configured on the server; the client secret never leaves it. + */ + get: operations['login']; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/auth/callback': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Complete the server-hosted login flow + * @description Receives the authorization code from the identity provider, exchanges it for an identity + * token using the server-held client secret, verifies that token exactly like the token + * exchange endpoint does, and issues a Unity Catalog access token as the UC_TOKEN cookie + * before redirecting back into the UI. + */ + get: operations['loginCallback']; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/auth/providers': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + /** + * Which sign-in methods this server offers + * @description Public (no token required). Lets a UI decide at runtime whether sign-in is required at all + * and which entry points to show, so one UI build follows the server's configuration instead + * of baking the choice in. + */ + get: operations['getAuthProviders']; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/auth/admin/login': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Sign in as the built-in administrator with a password + * @description Exchanges the administrator password configured on the server (server.admin-password) for + * a Unity Catalog access token, returned in the body and set as the UC_TOKEN cookie. Serves + * the UI's administrator entry point; unavailable unless the password is configured. + */ + post: operations['adminLogin']; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + '/auth/token/login': { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get?: never; + put?: never; + /** + * Open a browser session with an access token already held + * @description Accepts a Unity Catalog access token issued by this server (the one in etc/conf/token.txt, + * for instance) and sets it as the UC_TOKEN cookie, so an operator can reach the UI with the + * token instead of a password. The token is verified as any request's token is -- signature, + * expiry and an enabled user -- and is handed back unchanged, so the session cannot outlive + * it. Tokens from an identity provider go to /auth/tokens instead. + */ + post: operations['tokenLogin']; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; '/auth/tokens': { parameters: { query?: never; @@ -203,6 +317,14 @@ export interface components { lastModified?: string; }; }; + AuthProviders: { + /** @description Whether requests must carry a token at all (server.authorization). */ + authorization_enabled?: boolean; + /** @description Whether the server-hosted OAuth sign-in (/auth/login) is configured. */ + hosted_login?: boolean; + /** @description Whether the administrator password sign-in (/auth/admin/login) is configured. */ + admin_login?: boolean; + }; SelfCapabilities: { /** * @description Whether the caller owns the metastore. Metastore owners are authorized for @@ -522,6 +644,167 @@ export interface operations { }; }; }; + login: { + parameters: { + query?: { + /** @description Path within the UI to return to after login. Must be a relative path. */ + redirect?: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Redirect to the identity provider. */ + 302: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description The hosted login flow is not configured on this server. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + loginCallback: { + parameters: { + query?: { + code?: string; + state?: string; + error?: string; + error_description?: string; + }; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description Login succeeded; the UC_TOKEN cookie is set and the browser is sent back to the UI. */ + 302: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Missing or mismatched state, an error reported by the identity provider, or an unknown user. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + getAuthProviders: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description The available sign-in methods. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['AuthProviders']; + }; + }; + }; + }; + adminLogin: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + 'application/x-www-form-urlencoded': { + username: string; + /** Format: password */ + password: string; + }; + }; + }; + responses: { + /** @description Signed in; the UC_TOKEN cookie is set. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['OAuthTokenExchangeInfo']; + }; + }; + /** @description Administrator login is not configured on this server. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Wrong username or password. */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; + tokenLogin: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody: { + content: { + 'application/x-www-form-urlencoded': { + /** Format: password */ + token: string; + }; + }; + }; + responses: { + /** @description Signed in; the UC_TOKEN cookie is set. */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + 'application/json': components['schemas']['OAuthTokenExchangeInfo']; + }; + }; + /** @description No token supplied. */ + 400: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description The token is not valid for sign-in. */ + 401: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + }; + }; getToken: { parameters: { query?: { From a63b2f04dd3aba6c2045b90416a5f3ea3d33fd98 Mon Sep 17 00:00:00 2001 From: huyang Date: Wed, 23 Sep 2026 05:03:06 +0000 Subject: [PATCH 2/2] docs: record #17 in features.md The known-gap note it removes is what this PR closes. --- features.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/features.md b/features.md index 59c5eba2..a837e55f 100644 --- a/features.md +++ b/features.md @@ -52,9 +52,10 @@ Feature history of this fork, by PR merge date. - features: — - bugfix: `deploy-uc.sh` no longer refuses to start when `UC_ALLOWED_ISSUERS` is empty, which is the documented default (trusted issuers come from the external JWKS file). -## ⚠️ Known gap: UI authentication is incomplete +## 2026-09-23 + +#17 (https://github.com/relytcloud/unitycatalog/pull/17) + +- features: UI sign-in, per user. Three entry points, each at an address of its own: `/login` with **Sign in with Microsoft** (Entra ID, through an OAuth flow the server hosts end to end, so the client secret never reaches the browser), `/login/admin` for the administrator password, and `/login/token` for an access token the server issued. The application is no longer an entry point and the UI server injects nothing; what a signed-in person sees is filtered by their grants. On the server: the static JWKS file and OIDC discovery now coexist (routed by whether the issuer is registered in the file), the caller resolves through an ordered claim chain (`email`, `preferred_username`, `upn`, `sub`), discovery is cached per issuer and bounded, and Entra's `{tenantid}` issuer template is matched segment-wise. `deploy-uc.sh` derives the audience and issuer from the Microsoft settings and starts the UI with the server. +- bugfix: The session cookie is now one a browser keeps -- `Secure` only when the browser is on HTTPS, and `SameSite=Lax` rather than `Strict`, which was withheld on the cross-site navigation back from the identity provider. The URL transcoder passes a redirect back instead of following it (it relayed the provider's error page as a 400) and ends a bodyless response on its headers instead of leaving the caller waiting. A refused authorization code now names the provider's error code, which separates an expired client secret from a mismatched redirect URI. -The UI currently has no per-user login: the standalone UI server injects a single -server-side bearer token for all API calls, so everyone using the UI acts as that -one identity. A proper UI login & authentication system (per-user sign-in wired -into the server's token-exchange flow) still needs to be built.