adutra commented on code in PR #17148:
URL: https://github.com/apache/iceberg/pull/17148#discussion_r3586636234
##########
core/src/main/java/org/apache/iceberg/rest/auth/OAuth2Properties.java:
##########
@@ -64,6 +64,16 @@ private OAuth2Properties() {}
/** Optional param resource for OAuth2. */
public static final String RESOURCE = "resource";
+ /**
+ * When enabled, the Authorization header inherited from the parent session
is not forwarded when
+ * fetching a new token via the client credentials flow. This prevents
strict identity providers
+ * from rejecting the token request due to using multiple authentication
methods.
+ */
+ public static final String SKIP_INHERITED_AUTH_HEADER_IN_TOKEN_REQUEST =
Review Comment:
An `Authorization` header is **never** permitted when contacting an OAuth2
token endpoint.
Yet, today the Auth Manager may send an `Authorization` header to the token
endpoint in two situations:
1. When the contextual session contains credentials;
2. When refreshing tokens using a non-standard token exchange grant.
IDPs generally reject both.
This PR is addressing the first situation specifically, and introduces a new
boolean flag for it.
For the second situation, we already have a boolean flag:
`TOKEN_EXCHANGE_ENABLED`, that gates both the non-standard token exchange
grant, and the `Authorization` header that comes with it.
Instead of introducing yet another flag, I wonder if we shouldn't merge
these two flags together, since the goal is ultimately the same: make this Auth
Manager compatible with external IDPs.
I cannot see a situation where people would set these flags otherwise than
as below:
```properties
token-exchange-enabled=false
skip-inherited-auth-header-in-token-request=true
```
Since that's the only combination that works with external IDPs, I think one
flag should be enough.
\cc @danielcweeks @nastra
--
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]
