You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@polaris.apache.org by "vigneshio (via GitHub)" <gi...@apache.org> on 2026/07/07 09:20:56 UTC
[PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
vigneshio opened a new pull request, #4992:
URL: https://github.com/apache/polaris/pull/4992
This PR adds the realm/tenant identifier to the OPA authorization input so policies can properly enforce isolation across realms.
Previously the input sent to OPA was essentially identical for the same principal/role/resource names in different realms (only the random request_id differed), unlike the Ranger authorizer which deliberately scopes by realm.
### Changes
- Added optional `realm` field to the OPA `Context` model
- Thread realm from `RealmContext` through `OpaPolarisAuthorizerFactory` (same pattern as Ranger)
- Updated `OpaPolarisAuthorizer` to include it in `buildContext()\)
This closes the cross-realm authorization gap when using `polaris.authorization.type=opa` with multiple realms pointing at the same policy.
## Checklist
- [ ] Don't disclose security issues!
- [x] Clearly explained why
- [x] Added/updated tests
- [x] Updated CHANGELOG.md
- [ ] Updated site docs (if user-facing)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "gracechen09 (via GitHub)" <gi...@apache.org>.
gracechen09 commented on PR #4992:
URL: https://github.com/apache/polaris/pull/4992#issuecomment-4917542806
Looks good to me, thanks @vigneshio for putting together the PR! This change makes sense and enables OPA authorizer to perform realm-based authorization
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3551319265
##########
extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactoryTest.java:
##########
@@ -62,8 +63,9 @@ public void testFactoryWithStaticTokenConfiguration() {
.build();
try (JavaPoolAsyncExec asyncExec = new JavaPoolAsyncExec()) {
+ RealmContext realmContext = () -> "test-realm";
OpaPolarisAuthorizerFactory factory =
- new OpaPolarisAuthorizerFactory(opaConfig, Clock.systemUTC(), asyncExec);
+ new OpaPolarisAuthorizerFactory(opaConfig, Clock.systemUTC(), asyncExec, realmContext);
Review Comment:
@flyrain Good point - the factory tests were only asserting non-null, never actually exercising the authorizer. Fixed that with two tests in `OpaPolarisAuthorizerFactoryTest:`
- `testFactoryPassesRealmToAuthorizerContext()` runs the full RealmContext → factory → context.realm path against a mock OPA server and verifies the emitted realm matches.
- `testFactoryUsesDistinctRealmValues()` uses a different realm to confirm isolation.
Covered the end-to-end path independently from `OpaPolarisAuthorizerTest.` Also removed the Ranger comparison from CHANGELOG.md..
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3540157365
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Context.java:
##########
@@ -36,4 +37,13 @@
public interface Context {
/** A unique identifier for correlating this request with OPA server logs. */
String requestId();
+
+ /**
+ * The realm (tenant) identifier for this authorization request, if known.
Review Comment:
When would it _not_ be known? 🤔
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/README.md:
##########
@@ -66,6 +66,7 @@ Individual resource with hierarchical context:
### Context
Request metadata:
- `request_id`: Unique correlation ID for logging
+- `realm`: Realm/tenant identifier (when available; allows OPA policies to scope decisions by tenant)
Review Comment:
"tenant" is not a well-defined term in Polaris. It's probably best to avoid it.
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java:
##########
@@ -55,6 +56,8 @@ class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory {
private CloseableHttpClient httpClient;
private BearerTokenProvider bearerTokenProvider;
+ @Inject private RealmContext realmContext;
Review Comment:
Other parameters are injected via the constructors. Let's follow that pattern for `realmContext` too.
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java:
##########
@@ -102,11 +103,31 @@ public OpaPolarisAuthorizer(
@NonNull CloseableHttpClient httpClient,
@NonNull ObjectMapper objectMapper,
@Nullable BearerTokenProvider tokenProvider) {
+ this(policyUri, httpClient, objectMapper, tokenProvider, null);
Review Comment:
`OpaPolarisAuthorizer` is meant to implement a Polaris SPI. It is not meant to be a primary class for reuse downstream. It is ok to alter its method signatures by adding new required parameters.
Cf. https://polaris.apache.org/releases/1.5.0/evolution/
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3543569142
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizer.java:
##########
@@ -102,11 +103,31 @@ public OpaPolarisAuthorizer(
@NonNull CloseableHttpClient httpClient,
@NonNull ObjectMapper objectMapper,
@Nullable BearerTokenProvider tokenProvider) {
+ this(policyUri, httpClient, objectMapper, tokenProvider, null);
Review Comment:
Donee.. dropped the 4-arg constructor, kept the 5-arg with a required @NonNull String realm. This also let me clean up `buildContext()` to always set the realm instead of branching.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3581320504
##########
CHANGELOG.md:
##########
@@ -56,42 +90,26 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- Added support for `register table` overwrite semantics in the Iceberg REST catalog flow (`overwrite=true`) for internal Polaris catalogs. With overwrite enabled, existing table pointers can be updated to a new metadata location while preserving default behavior for `overwrite=false`.
- Added `REGISTER_TABLE_OVERWRITE` authorization operation mapped to `TABLE_FULL_METADATA` for deterministic overwrite authorization.
- Added Polaris Spark 4.0 client.
-- Added `maintenance` support in Helm chart.
-- Python CLI: added `--catalog-url` to specify a custom base URL for the Iceberg REST Catalog (IRC) API. Allows use with deployments that map a path (e.g. `/server1`) directly to the catalog root instead of the standard `/api/catalog`. See #4927.
-- Added support for publishing histogram buckets for HTTP server request duration as configured SLO boundaries.
-- Added an OpenTelemetry event listener for emitting Polaris audit events as OpenTelemetry log records.
-- Added optional `sessionPolicy` field to `SigV4AuthenticationParameters` for catalog federation. When set, the IAM session policy JSON is attached to the STS AssumeRole request, allowing administrators to restrict vended credentials to only the required AWS services and actions (Principle of Least Privilege).
-- Added opt-in idempotency for `createTable` in the Iceberg REST catalog. When enabled via `polaris.idempotency.enabled=true` (default `false`), a client-supplied `Idempotency-Key` header is embedded into the new table entity and committed in the same transaction; a retry carrying the same key within the TTL window (`polaris.idempotency.ttl`, default `PT5M`) replays the original success instead of failing with `AlreadyExists`.
### Changes
-- The admin tool's `bootstrap` command is now idempotent: bootstrapping a realm that is already
- bootstrapped is reported as "Realm '<realm>' is already bootstrapped; skipping." and no longer
- fails the command, making automated bootstrap jobs safe to re-run. Correspondingly,
- `PolarisMetaStoreManager.bootstrapPolarisService` now returns a result with
- `ReturnStatus.ENTITY_ALREADY_EXISTS` instead of throwing `IllegalArgumentException` when the
- root principal already exists. Existing credentials are never returned or altered.
+
- Added REPL support to Polaris CLI.
- The `nosql maintenance-run` admin command now rejects a new run when the latest recorded maintenance run is still unfinished, unless the operator explicitly passes `--supersede-run=<run-id>`.
- Added version option to Polaris CLI.
- The token broker now builds the JWT `Algorithm` and `JWTVerifier` once per realm in the `TokenBrokerFactory` and reuses them across requests, instead of rebuilding them on every `verify()`/`sign()` call on the request-scoped broker. For deployments using file-based symmetric secrets, the secret is now read once per realm (at first use) rather than on every JWT operation; rotating the on-disk secret requires a restart.
-- Authorization failure messages (HTTP 403 / `ForbiddenException` from `PolarisAuthorizerImpl`) now log the specific missing privilege(s) and the entity each was checked against server-side (at `INFO` level), e.g. `missing TABLE_CREATE on NAMESPACE 'ns1'`. The client-facing 403 response remains a generic message to avoid leaking authorization metadata to untrusted clients. Operators can correlate client errors to server logs using the existing `X-Request-ID` header (present in default log MDC as `requestId`).
-
-### Deprecations
### Fixes
- OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms.
Review Comment:
This is under the `1.6.0` section, but the PR targets `main`... This should probably be in the `Unreleased` section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b merged PR #4992:
URL: https://github.com/apache/polaris/pull/4992
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3543553702
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/README.md:
##########
@@ -66,6 +66,7 @@ Individual resource with hierarchical context:
### Context
Request metadata:
- `request_id`: Unique correlation ID for logging
+- `realm`: Realm/tenant identifier (when available; allows OPA policies to scope decisions by tenant)
Review Comment:
Updated - description now uses **"realm identifier"** instead of "tenant", scoped to OPA decisions by realm.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3581320504
##########
CHANGELOG.md:
##########
@@ -56,42 +90,26 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- Added support for `register table` overwrite semantics in the Iceberg REST catalog flow (`overwrite=true`) for internal Polaris catalogs. With overwrite enabled, existing table pointers can be updated to a new metadata location while preserving default behavior for `overwrite=false`.
- Added `REGISTER_TABLE_OVERWRITE` authorization operation mapped to `TABLE_FULL_METADATA` for deterministic overwrite authorization.
- Added Polaris Spark 4.0 client.
-- Added `maintenance` support in Helm chart.
-- Python CLI: added `--catalog-url` to specify a custom base URL for the Iceberg REST Catalog (IRC) API. Allows use with deployments that map a path (e.g. `/server1`) directly to the catalog root instead of the standard `/api/catalog`. See #4927.
-- Added support for publishing histogram buckets for HTTP server request duration as configured SLO boundaries.
-- Added an OpenTelemetry event listener for emitting Polaris audit events as OpenTelemetry log records.
-- Added optional `sessionPolicy` field to `SigV4AuthenticationParameters` for catalog federation. When set, the IAM session policy JSON is attached to the STS AssumeRole request, allowing administrators to restrict vended credentials to only the required AWS services and actions (Principle of Least Privilege).
-- Added opt-in idempotency for `createTable` in the Iceberg REST catalog. When enabled via `polaris.idempotency.enabled=true` (default `false`), a client-supplied `Idempotency-Key` header is embedded into the new table entity and committed in the same transaction; a retry carrying the same key within the TTL window (`polaris.idempotency.ttl`, default `PT5M`) replays the original success instead of failing with `AlreadyExists`.
### Changes
-- The admin tool's `bootstrap` command is now idempotent: bootstrapping a realm that is already
- bootstrapped is reported as "Realm '<realm>' is already bootstrapped; skipping." and no longer
- fails the command, making automated bootstrap jobs safe to re-run. Correspondingly,
- `PolarisMetaStoreManager.bootstrapPolarisService` now returns a result with
- `ReturnStatus.ENTITY_ALREADY_EXISTS` instead of throwing `IllegalArgumentException` when the
- root principal already exists. Existing credentials are never returned or altered.
+
- Added REPL support to Polaris CLI.
- The `nosql maintenance-run` admin command now rejects a new run when the latest recorded maintenance run is still unfinished, unless the operator explicitly passes `--supersede-run=<run-id>`.
- Added version option to Polaris CLI.
- The token broker now builds the JWT `Algorithm` and `JWTVerifier` once per realm in the `TokenBrokerFactory` and reuses them across requests, instead of rebuilding them on every `verify()`/`sign()` call on the request-scoped broker. For deployments using file-based symmetric secrets, the secret is now read once per realm (at first use) rather than on every JWT operation; rotating the on-disk secret requires a restart.
-- Authorization failure messages (HTTP 403 / `ForbiddenException` from `PolarisAuthorizerImpl`) now log the specific missing privilege(s) and the entity each was checked against server-side (at `INFO` level), e.g. `missing TABLE_CREATE on NAMESPACE 'ns1'`. The client-facing 403 response remains a generic message to avoid leaking authorization metadata to untrusted clients. Operators can correlate client errors to server logs using the existing `X-Request-ID` header (present in default log MDC as `requestId`).
-
-### Deprecations
### Fixes
- OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms.
Review Comment:
This is under the `1.6.0` section, but the PR targets `main`... This should probably be in the `Unreleased` section.
##########
CHANGELOG.md:
##########
@@ -99,6 +99,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
- The token broker now builds the JWT `Algorithm` and `JWTVerifier` once per realm in the `TokenBrokerFactory` and reuses them across requests, instead of rebuilding them on every `verify()`/`sign()` call on the request-scoped broker. For deployments using file-based symmetric secrets, the secret is now read once per realm (at first use) rather than on every JWT operation; rotating the on-disk secret requires a restart.
### Fixes
+- OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms.
Review Comment:
This is under the `1.6.0` section, but the PR targets `main`... This should probably be in the `Unreleased` section.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3543580911
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactory.java:
##########
@@ -55,6 +56,8 @@ class OpaPolarisAuthorizerFactory implements PolarisAuthorizerFactory {
private CloseableHttpClient httpClient;
private BearerTokenProvider bearerTokenProvider;
+ @Inject private RealmContext realmContext;
Review Comment:
Makes sense fixed.. RealmContext is now constructor-injected (aligned with other factory dependencies) and passed directly to the authorizer...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "flyrain (via GitHub)" <gi...@apache.org>.
flyrain commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3547930621
##########
extensions/auth/opa/src/test/java/org/apache/polaris/extension/auth/opa/OpaPolarisAuthorizerFactoryTest.java:
##########
@@ -62,8 +63,9 @@ public void testFactoryWithStaticTokenConfiguration() {
.build();
try (JavaPoolAsyncExec asyncExec = new JavaPoolAsyncExec()) {
+ RealmContext realmContext = () -> "test-realm";
OpaPolarisAuthorizerFactory factory =
- new OpaPolarisAuthorizerFactory(opaConfig, Clock.systemUTC(), asyncExec);
+ new OpaPolarisAuthorizerFactory(opaConfig, Clock.systemUTC(), asyncExec, realmContext);
Review Comment:
This builds the factory with a `RealmContext` lambda but the test only asserts the authorizer is non-null, it never invokes it or inspects the OPA request. Combined with `OpaPolarisAuthorizerTest`, which always constructs the authorizer directly with a hardcoded realm string, the `RealmContext -> factory -> context.realm` path is never verified end-to-end. Could you add an assertion here (or a dedicated test) that runs the produced authorizer and checks the emitted `input.context.realm` matches the injected realm? A two-realm variant would also lock in the isolation guarantee this PR is about.
##########
CHANGELOG.md:
##########
@@ -69,6 +69,7 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti
### Deprecations
### Fixes
+- OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms (previously the OPA input was identical for the same principal/role/resource names in different realms, unlike the Ranger authorizer).
Review Comment:
Nit: I think the wording `unlike the Ranger authorizer` is unnecessary and maybe a bit misleading.
```suggestion
- OPA authorizer now includes the realm identifier in the authorization context sent to OPA (`input.context.realm`). This ensures OPA policies can enforce tenant isolation across realms, preventing potential collisions if identical principal or resource names exist in different realms..
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "vigneshio (via GitHub)" <gi...@apache.org>.
vigneshio commented on code in PR #4992:
URL: https://github.com/apache/polaris/pull/4992#discussion_r3543545767
##########
extensions/auth/opa/src/main/java/org/apache/polaris/extension/auth/opa/model/Context.java:
##########
@@ -36,4 +37,13 @@
public interface Context {
/** A unique identifier for correlating this request with OPA server logs. */
String requestId();
+
+ /**
+ * The realm (tenant) identifier for this authorization request, if known.
Review Comment:
Agreed 💯 - realm is always present in practice. Updated Context to require it (removed `@Nullable`), `buildContext()` now always sets it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4992:
URL: https://github.com/apache/polaris/pull/4992#issuecomment-4960563714
@vigneshio : there are odd CI failures... Could you try rebasing, please?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(authz): include realm identifier in OPA authorization input for tenant isolation [polaris]
Posted by "dimas-b (via GitHub)" <gi...@apache.org>.
dimas-b commented on PR #4992:
URL: https://github.com/apache/polaris/pull/4992#issuecomment-4909772533
FYI: @gracechen09
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@polaris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org