amogh-jahagirdar commented on code in PR #14241:
URL: https://github.com/apache/iceberg/pull/14241#discussion_r2402241751
##########
core/src/main/java/org/apache/iceberg/rest/auth/AuthSession.java:
##########
@@ -46,6 +49,30 @@ public void close() {}
*/
HTTPRequest authenticate(HTTPRequest request);
+ /**
+ * Called when the request was challenged (the server returned a 401
response).
+ *
+ * <p>Implementations may choose to return a new request with updated
authentication data, or null
+ * if the request should not be retried. The default implementation returns
null.
+ *
+ * <p>If this method returns null, the 401 response will be surfaced to the
caller as a {@link
+ * org.apache.iceberg.exceptions.NotAuthorizedException}.
+ *
+ * @param restClient the REST client that sent the request
+ * @param request the original request that caused the authentication failure
+ * @param challenge the authentication challenge
+ * @param retryAttempt the retry attempt number, starting with 1
+ * @return a new request with updated authentication headers, or null if the
request should not be
+ * retried
+ * @see <a
href="https://datatracker.ietf.org/doc/html/rfc7235#section-2.1";>RFC 7235
Section
+ * 2.1</a>
+ */
+ @Nullable
+ default HTTPRequest challenge(
Review Comment:
`handleChallenge` or `processChallenge`? Just challenge makes it seem like
the client session is challenging but it's really how to handle the server's
response
##########
core/src/main/java/org/apache/iceberg/rest/HTTPClient.java:
##########
@@ -360,6 +391,37 @@ protected <T extends RESTResponse> T execute(
}
}
+ @Nullable
+ private HTTPRequest processChallenge(
+ HTTPRequest request,
+ CloseableHttpResponse response,
+ HttpClientContext context,
+ int retryAttempt) {
+
+ Map<String, AuthChallenge> challengeMap =
+ authenticationHandler.extractChallengeMap(ChallengeType.TARGET,
response, context);
Review Comment:
Had a question, I see there's a ChallengeType.Proxy, when is the expectation
for clients to use that?
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Util.java:
##########
@@ -432,6 +434,22 @@ public HTTPRequest authenticate(HTTPRequest request) {
:
ImmutableHTTPRequest.builder().from(request).headers(newHeaders).build();
}
+ @Nullable
+ @Override
+ public HTTPRequest challenge(
+ RESTClient restClient, HTTPRequest request, HTTPChallenge challenge,
int retryAttempt) {
+ if (retryAttempt > 1
+ || !"Bearer".equals(challenge.scheme())
+ || !"invalid_token".equals(challenge.params().get("error"))) {
Review Comment:
Nit: Constants for these?
--
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]
