Demonstrate authentication using bearer token #704

cgpoh · 2025-01-11T14:48:42Z

Add PolarisContextResolver to demonstrate authentication using bearer token instead of the default custom bearer-token format principal:data-engineer;password:test;realm:acct123

# Conflicts: # build.gradle.kts # dropwizard/service/src/main/resources/META-INF/services/io.dropwizard.logging.common.layout.DiscoverableLayoutFactory # polaris-server.yml # service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java # tools/version/src/main/version/polaris-version.properties

…polaris-server.yml

# Conflicts: # polaris-server.yml

adutra

Hi @cgpoh I'm not sure I understand the purpose of this PR, but you seem to be proposing a new realm context resolver that resolves the realm from a claim in the JWT token, rather than from an adhoc header.

While I'm not opposed to the idea, I think you PR contains some unrelated changes (the Postgres Eclipselink configuration). It would be better to clean that up. Thanks!

adutra · 2025-01-13T10:46:39Z

service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java

+                  .build();
+          try {
+            DecodedJWT decodedJWT = verifier.verify(token);
+            String realm = decodedJWT.getClaim(ISS_PROPERTY_KEY).asString();


I don't think you can assume that the iss claim will always contain the realm identifier. For example, Polaris itself sets the iss claim to polaris:

polaris/service/common/src/main/java/org/apache/polaris/service/auth/JWTBroker.java

Line 44 in 343aaaa

private static final String ISSUER_KEY = "polaris";

In fact, Polaris does not include the realm in the JWT tokens it generates.

So, I think this needs to be configurable.

Thanks, I think I just need the verify method to check for valid token. Will remove the getClaim method

adutra · 2025-01-13T10:46:59Z

service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java

+            String principalId = decodedJWT.getClaim(PRINCIPAL_PROPERTY_KEY).asString();
+
+            parsedProperties.put(REALM_PROPERTY_KEY, realm);
+            parsedProperties.put(PRINCIPAL_PROPERTY_KEY, principalId);


Unused property.

Will remove these unused properties

adutra · 2025-01-13T10:47:29Z

service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java

+import org.slf4j.LoggerFactory;
+
+@JsonTypeName("polaris")
+public class PolarisContextResolver implements RealmContextResolver {


I would rather call this BearerTokenRealmContextResolver.

adutra · 2025-01-13T10:48:02Z

extension/persistence/eclipselink/src/main/resources/META-INF/persistence.xml

@@ -44,4 +44,27 @@
      <property name="eclipselink.persistence-context.flush-mode" value="auto"/>
    </properties>
  </persistence-unit>
+  <persistence-unit name="polaris-postgresql" transaction-type="RESOURCE_LOCAL">


This change seems unrelated.

Thanks, will remove that.

eric-maynard · 2025-01-13T18:05:42Z

polaris-server.yml

This also looks unrelated

Thanks, will revert the changes

dimas-b

This looks like an interesting proposal, but I think it might be worth discussing it on the dev mailing list first (due to implications for federated identities, which are already being discussed in dev)?

dimas-b · 2025-01-13T22:48:23Z

service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java

+                  .build();
+          try {
+            DecodedJWT decodedJWT = verifier.verify(token);
+            String realm = decodedJWT.getClaim(ISS_PROPERTY_KEY).asString();


Obtaining real from access tokens looks interesting, but would you mind showing an end-to-end use case, @cgpoh ?

One rough edge seems to be that in order to obtain a token from Polaris, the user must provide a realm, so the access token cannot be an exclusive source of realm information.

If you mean tokens provided by external IdP, that feature is yet not available in Polaris (unless I missed it) 🤔

Thanks @dimas-b. For my use case, I followed the Security section in the configuring-polaris-for-production guide. I deployed Polaris in our Kubernetes POC environment and bootstrapped it using PostgreSQL, setting the environment variables
POLARIS_BOOTSTRAP_POLARIS_ROOT_CLIENT_ID and POLARIS_BOOTSTRAP_POLARIS_ROOT_CLIENT_SECRET.

In the subsection, I noticed Polaris lacks a service to resolve a realm from bearer tokens. To address this, I created the PolarisContextResolver class. With this implementation, we can now create Polaris RBAC and catalogs using the bearer token retrieved from api/catalog/v1/oauth/tokens.

Thanks for the clarification, @cgpoh !

From my POV, I'd question the current text in the callContextResolver & realmIdResolver doc section, actually.

Like I said, getting realm ID from a token is interesting, but it works well only for Polaris own tokens. If a token is from an external IdP (a feature being discussed on the dev mailing list), validating the token may not be as trivial. I can imagine that validation may require some knowledge about the IdP, which would naturally be part of the "realm" definition.

I'd propose to alter the approach a bit and perhaps try to refactor authenticators to also be able to produce realm information. This way, IMHO, the runtime flow will be more coherent: 1) the authenticator receives a "raw" request; 2) the authenticator uses headers (including Bearer token) to make sure that the request is valid (authentic) possibly taking both the realm header and JWT into account; 3) the authenticator declares that the request is for a) some realm, b) some specific principal.

WDYT?

For my POC demonstration, I aimed to use a real token instead of a sequence of key/value pairs, like those in TestInlineBearerTokenPolarisAuthenticator, which led to the creation of the BearerTokenRealmContextResolver class. With the new Quarkus update, it seems TestInlineBearerTokenPolarisAuthenticator is no longer in use, so I believe I can close this PR for now.

I’m intrigued by your approach and will take some time to study the implementation further. Hopefully, I can raise a new PR to address this in the future! 😊

dimas-b · 2025-01-13T22:49:55Z

service/common/src/main/java/org/apache/polaris/service/context/PolarisContextResolver.java

+            parsedProperties.put(REALM_PROPERTY_KEY, realm);
+            parsedProperties.put(PRINCIPAL_PROPERTY_KEY, principalId);
+          } catch (Throwable t) {
+            throw new JWTVerificationException(t.getMessage(), t);


I'm not sure we can assume that all bearer tokens are JWTs. When external IdP support is added, it will be quite possible that access tokens are opaque.

I’m thinking JWT for a start and maybe add more support when Quarkus migration is done, what do you think?

cgpoh · 2025-01-14T04:24:56Z

Hi @cgpoh I'm not sure I understand the purpose of this PR, but you seem to be proposing a new realm context resolver that resolves the realm from a claim in the JWT token, rather than from an adhoc header.

While I'm not opposed to the idea, I think you PR contains some unrelated changes (the Postgres Eclipselink configuration). It would be better to clean that up. Thanks!

Thanks @adutra . Apologies for not being clear about the purpose of this PR. I’ve provided an explanation here

…ContextResolver to BearerTokenRealmContextResolver

dimas-b · 2025-01-15T23:40:17Z

...common/src/main/java/org/apache/polaris/service/context/BearerTokenRealmContextResolver.java

+import org.slf4j.LoggerFactory;
+
+@JsonTypeName("polaris")
+public class BearerTokenRealmContextResolver implements RealmContextResolver {


FYI: RealmContextResolver has been renamed. Please rebase.

collado-mike · 2025-01-16T01:15:28Z

...common/src/main/java/org/apache/polaris/service/context/BearerTokenRealmContextResolver.java

+          JWTVerifier verifier =
+              JWT.require(Algorithm.HMAC256("polaris"))
+                  .withIssuer(ISS_PROPERTY_DEFAULT_VALUE)
+                  .build();


This will never be usable in production if you hardcode the secret here. The default token broker signs the JWT with a secret that is defined in the configuration - could be anything. Is it expected that this resolver is only used for test cases? Or is it intended to work in conjunction with the JWTBroker-generated tokens in prod?

Agree. For now, is more of a POC. I intend to define a secret in a configuration without hardcoding when working with the JWTBroker in prod.

cgpoh added 16 commits November 2, 2024 14:09

feat: add polaris context resolver

de5a5f4

Merge branch 'main' into test

6dd5fcd

chore: revert eclipse link changes

fd45796

chore: use postgreSQL

1e60445

chore: bump eclipselink version to 4.0.5

c90bc37

Merge branch 'fix-optimisticLockException' into polaris-dev

c453acc

fix: fix JsonMappingException when defaultRealm property is added in …

eddcb5f

…polaris-server.yml

Merge branch 'main' into polaris-dev

e49b28e

Merge branch 'update-default-realm-property' into polaris-dev

bd11c87

Merge branch 'polaris-dev' into polaris-demo

d266df0

# Conflicts: # polaris-server.yml

chore: remove queryParams

706da1e

Merge branch 'main' into polaris-demo

4090aee

chore: add persistence unit for PostgreSQL

491d7be

chore: default to in-memory for metastore

293acaa

chore: remove sdkmanrc

a957556

cgpoh requested review from adutra, ashvina, dimas-b, eric-maynard, jackye1995, jbonofre, vvcephei, collado-mike, snazy, RussellSpitzer, takidau, flyrain and ebyhr as code owners January 11, 2025 14:48

Merge branch 'main' into polaris-demo

541c02f

adutra reviewed Jan 13, 2025

View reviewed changes

eric-maynard reviewed Jan 13, 2025

View reviewed changes

dimas-b reviewed Jan 13, 2025

View reviewed changes

cgpoh added 2 commits January 14, 2025 15:15

chore: revert persistence.xml and polaris-server.yaml, rename Polaris…

5c6e754

…ContextResolver to BearerTokenRealmContextResolver

chore: remove unused properties

eaf336d

dimas-b reviewed Jan 15, 2025

View reviewed changes

collado-mike reviewed Jan 16, 2025

View reviewed changes

cgpoh closed this Jan 16, 2025

dimas-b mentioned this pull request Jan 16, 2025

Throw NotAuthorizedException when realm is unknown #805

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Demonstrate authentication using bearer token #704

Demonstrate authentication using bearer token #704

cgpoh commented Jan 11, 2025

adutra left a comment

adutra Jan 13, 2025

cgpoh Jan 14, 2025

adutra Jan 13, 2025

cgpoh Jan 14, 2025

adutra Jan 13, 2025

adutra Jan 13, 2025

cgpoh Jan 14, 2025

eric-maynard Jan 13, 2025

cgpoh Jan 14, 2025

dimas-b left a comment

dimas-b Jan 13, 2025

cgpoh Jan 14, 2025

dimas-b Jan 15, 2025

cgpoh Jan 16, 2025

dimas-b Jan 13, 2025

cgpoh Jan 14, 2025

cgpoh commented Jan 14, 2025

dimas-b Jan 15, 2025

collado-mike Jan 16, 2025

cgpoh Jan 16, 2025

Demonstrate authentication using bearer token #704

Demonstrate authentication using bearer token #704

Conversation

cgpoh commented Jan 11, 2025

adutra left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dimas-b left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgpoh commented Jan 14, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment