danielcweeks commented on code in PR #12197:
URL: https://github.com/apache/iceberg/pull/12197#discussion_r1990291751
##########
aws/src/main/java/org/apache/iceberg/aws/s3/signer/S3V4RestSignerClient.java:
##########
@@ -200,86 +151,40 @@ private RESTClient httpClient() {
return httpClient;
}
- private AuthSession authSession() {
- String token = token().get();
- if (null != token) {
- return authSessionCache()
- .get(
- token,
- id -> {
- // this client will be reused for token refreshes; it must
contain an empty auth
- // session in order to avoid interfering with refreshed tokens
- RESTClient refreshClient =
-
httpClient().withAuthSession(org.apache.iceberg.rest.auth.AuthSession.EMPTY);
- return AuthSession.fromAccessToken(
- refreshClient,
- tokenRefreshExecutor(),
- token,
- expiresAtMillis(properties()),
- new AuthSession(
- ImmutableMap.of(),
- AuthConfig.builder()
- .token(token)
- .credential(credential())
- .scope(SCOPE)
- .oauth2ServerUri(oauth2ServerUri())
- .optionalOAuthParams(optionalOAuthParams())
- .build()));
- });
- }
-
- if (credentialProvided()) {
- return authSessionCache()
- .get(
- credential(),
- id -> {
- AuthSession session =
- new AuthSession(
- ImmutableMap.of(),
- AuthConfig.builder()
- .credential(credential())
- .scope(SCOPE)
- .oauth2ServerUri(oauth2ServerUri())
- .optionalOAuthParams(optionalOAuthParams())
- .build());
- long startTimeMillis = System.currentTimeMillis();
- // this client will be reused for token refreshes; it must
contain an empty auth
- // session in order to avoid interfering with refreshed tokens
- RESTClient refreshClient =
-
httpClient().withAuthSession(org.apache.iceberg.rest.auth.AuthSession.EMPTY);
- OAuthTokenResponse authResponse =
- OAuth2Util.fetchToken(
- refreshClient,
- session.headers(),
- credential(),
- SCOPE,
- oauth2ServerUri(),
- optionalOAuthParams());
- return AuthSession.fromTokenResponse(
- refreshClient, tokenRefreshExecutor(), authResponse,
startTimeMillis, session);
- });
+ @VisibleForTesting
+ AuthSession authSession() {
+ if (null == authSession) {
+ synchronized (S3V4RestSignerClient.class) {
+ if (null == authSession) {
+ authManager = AuthManagers.loadAuthManager("s3-signer",
properties());
+ ImmutableMap.Builder<String, String> properties =
+ ImmutableMap.<String, String>builder()
+ .putAll(properties())
+ .putAll(optionalOAuthParams())
+ .put(OAuth2Properties.OAUTH2_SERVER_URI, oauth2ServerUri())
+ .put(OAuth2Properties.TOKEN_REFRESH_ENABLED,
String.valueOf(keepTokenRefreshed()))
+ .put(OAuth2Properties.SCOPE, SCOPE);
+ String token = token().get();
+ if (null != token) {
+ properties.put(OAuth2Properties.TOKEN, token);
+ }
+
+ if (credentialProvided()) {
+ properties.put(OAuth2Properties.CREDENTIAL, credential());
+ }
+
+ authSession = authManager.catalogSession(httpClient(),
properties.buildKeepingLast());
Review Comment:
All Table access is via a table's FileIO. That is scoped to a table, not a
catalog. This is a pretty core concept to how we think about how access flows
from a table. There are a number of reasons for this which largely overlap
with resource isolation and access controls. Signing requests for a specific
table falls into that same category. We're creating this built based on table
properties from the table load, it's aligned with table access, but we're
calling it a catalog session.
I think the discussion we had around naming is still correct. There are
specific session scopes for init/catalog/table that all have context and
meaning.
The issue here is that it feels like we're trying to justify the
implementation because it doesn't align with the signatures of the auth session.
--
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-unsubscr...@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org
