bharos commented on code in PR #15999:
URL: https://github.com/apache/iceberg/pull/15999#discussion_r3102629301


##########
core/src/test/java/org/apache/iceberg/rest/auth/TestOAuth2Util.java:
##########
@@ -135,4 +138,113 @@ public void testCredentialFlowForSessionRefresh() throws 
IOException {
               any());
     }
   }
+
+  @Test
+  void fromTokenResponseUsesChildTokenExpiry() {
+    // parent token expires at epoch second 7200 (= 7200000 ms)
+    String parentToken = tokenWithExp(7200);
+    // child token expires at epoch second 300 (= 300000 ms)
+    String childToken = tokenWithExp(300);
+
+    AuthConfig parentConfig =
+        AuthConfig.builder()
+            .token(parentToken)
+            .tokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+            .keepRefreshed(false)
+            .build();
+    OAuth2Util.AuthSession parent = new OAuth2Util.AuthSession(Map.of(), 
parentConfig);
+    
assertThat(parent.expiresAtMillis()).isEqualTo(TimeUnit.SECONDS.toMillis(7200));
+
+    OAuthTokenResponse response =
+        OAuthTokenResponse.builder()
+            .withToken(childToken)
+            .withTokenType(BEARER)
+            .withIssuedTokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+            .setExpirationInSeconds(300)
+            .build();
+
+    // pass null executor to skip scheduling
+    OAuth2Util.AuthSession child =
+        OAuth2Util.AuthSession.fromTokenResponse(
+            null, null, response, System.currentTimeMillis(), parent);
+    assertThat(child.expiresAtMillis())
+        .as("Child session should use the child token's exp, not the parent's")
+        .isEqualTo(TimeUnit.SECONDS.toMillis(300));
+  }
+
+  @Test
+  void fromTokenResponseOpaqueTokenDoesNotInheritParentExpiry() {
+    // parent token with JWT exp claim
+    String parentToken = tokenWithExp(7200);
+    // child token with no exp claim (opaque)
+    String childToken = "opaque-access-token";
+
+    AuthConfig parentConfig =
+        AuthConfig.builder()
+            .token(parentToken)
+            .tokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+            .keepRefreshed(false)
+            .build();
+    OAuth2Util.AuthSession parent = new OAuth2Util.AuthSession(Map.of(), 
parentConfig);
+    
assertThat(parent.expiresAtMillis()).isEqualTo(TimeUnit.SECONDS.toMillis(7200));
+
+    OAuthTokenResponse response =
+        OAuthTokenResponse.builder()
+            .withToken(childToken)
+            .withTokenType(BEARER)
+            .withIssuedTokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+            .setExpirationInSeconds(600)
+            .build();
+
+    OAuth2Util.AuthSession child =
+        OAuth2Util.AuthSession.fromTokenResponse(
+            null, null, response, System.currentTimeMillis(), parent);
+
+    // child token is opaque (no exp claim), so expiresAtMillis should be null,
+    // NOT inherited from parent. The response's expiresInSeconds is used
+    // only for scheduling, not stored in the session config.
+    assertThat(child.expiresAtMillis())
+        .as("Child session with opaque token should not inherit parent's 
expiresAtMillis")
+        .isNull();
+  }
+
+  @Test
+  void fromAccessTokenUsesChildTokenExpiry() {
+    // parent token expires at epoch second 7200
+    String parentToken = tokenWithExp(7200);
+    // child token expires at epoch second 300
+    String childToken = tokenWithExp(300);
+
+    AuthConfig parentConfig =
+        AuthConfig.builder()
+            .token(parentToken)
+            .tokenType(OAuth2Properties.ACCESS_TOKEN_TYPE)
+            .keepRefreshed(false)
+            .build();
+    OAuth2Util.AuthSession parent = new OAuth2Util.AuthSession(Map.of(), 
parentConfig);
+    
assertThat(parent.expiresAtMillis()).isEqualTo(TimeUnit.SECONDS.toMillis(7200));
+
+    // pass null executor to skip scheduling
+    OAuth2Util.AuthSession child =
+        OAuth2Util.AuthSession.fromAccessToken(null, null, childToken, null, 
parent);
+    assertThat(child.expiresAtMillis())
+        .as("Child session should use the child token's exp, not the parent's")
+        .isEqualTo(TimeUnit.SECONDS.toMillis(300));
+  }
+
+  /**
+   * Creates a JWT token with the given expiration time in seconds since 
epoch. The token is not
+   * cryptographically signed but is structurally valid for {@link 
OAuth2Util#expiresAtMillis}
+   * parsing.
+   */
+  private static String tokenWithExp(long expSeconds) {
+    String header = "{\"alg\":\"none\",\"typ\":\"JWT\"}";
+    String payload = "{\"sub\":\"test\",\"exp\":" + expSeconds + "}";
+    Base64.Encoder encoder = Base64.getUrlEncoder().withoutPadding();
+    return encoder.encodeToString(header.getBytes(StandardCharsets.UTF_8))
+        + "."
+        + encoder.encodeToString(payload.getBytes(StandardCharsets.UTF_8))
+        + "."
+        + encoder.encodeToString("fakesig".getBytes(StandardCharsets.UTF_8));
+  }

Review Comment:
   Good point — replaced manual Base64 JWT construction with Nimbus PlainJWT



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to