adutra commented on PR #17148: URL: https://github.com/apache/iceberg/pull/17148#issuecomment-4979760950
> If a SessionContext also carries a credential (the pattern used by Trino when iceberg.rest-catalog.session=NONE) the first catalog operation triggers contextualSession() -> fromCredential(), which inherits the catalog parent session headers and forwards existing Authorization header alongside client_id+client_secret in body to the authorization server. These symptoms occur precisely because the current Auth Manager overlays the session context properties on top of the catalog ones, and creates a session based on the merged result. I flagged this behavior as concerning in the [Auth Manager V2 design doc](https://docs.google.com/document/d/1Hxw-t8Maa7wZFmrlSujm7LRawKsFP3Q31tET_3aRnQU/edit?tab=t.0#heading=h.igicsw455u69), precisely because of what we are seeing here: the resulting session properties become non-deterministic / nonsensical. See chapter 5.6.1: "Session Inheritance" in the doc. Unfortunately, solving this problem holistically is difficult, because other properties may also be incorrectly merged: e.g. what if the session context contains an `audience` claim, but the catalog session contains a `resource` claim? Merging the two together may result in conflicting parameters in the token request. That said, at least this PR solves the most common merge issue which is around the `Authorization` header. -- 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]
