bharos commented on PR #16056: URL: https://github.com/apache/iceberg/pull/16056#issuecomment-4307391190
> This PR likely fixes one of the biggest client authentication issues in the current auth manager. I briefly alluded to it in the auth manager v2 design doc: > > https://docs.google.com/document/d/1Hxw-t8Maa7wZFmrlSujm7LRawKsFP3Q31tET_3aRnQU/edit?tab=t.0#bookmark=id.td3wbcmg3va2 > > But, it's a behavioral regression. I'm not sure it's safe to introduce it without placing it behind a feature flag. > > \cc @nastra Thanks for the feedback @adutra! I'd like to understand better whether this truly needs a feature flag. The parent Bearer token has no semantic role in the client_credentials grant — it's leaked from the parent session, and all other fetchToken call sites already pass Map.of(). In terms of real-world impact: IDPs that currently reject the extra Authorization: Bearer header (CAS, AWS Cognito) will start working correctly. IDPs that currently accept it do so by silently ignoring the extra header — removing it won't change anything for them since the actual authentication (client_id/client_secret) is sent separately in the request body. So what's broken today gets fixed, and what works today should continue to work. Given that, do you still think a feature flag is necessary here? -- 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]
