adutra commented on PR #10256: URL: https://github.com/apache/iceberg/pull/10256#issuecomment-2112020920
@rdblue I have no other intention than the one of bringing the spec in line with the current implementation. There are two areas where spec and implementation differ with respect to authentication: 1. Authentication properties from the config endpoint. The OpenAPI Spec for the config endpoint currently states: > Catalog configuration is constructed by setting the defaults, then client-provided configuration, and finally overrides. The final property set is then used to configure the catalog. As we've seen, this is currently not true for many authentication properties, including `credential` and `oauth2-server-uri`, to name a few. But it holds true for `token`. 2. Authentication properties returned in a `LoadTableResult` structure. The OpenAPI Spec for the structure currently states: > The `config` map returns table-specific configuration for the table's resources [..] The following configurations should be respected by clients: [..] `token`. Note that `credential`, for instance, is not listed there, and yet _it is currently honored by the client_. IOW, if the server sends a `credential` in the `config` map, [the client will use it to authenticate](https://github.com/apache/iceberg/blob/7caedf85954c78fb3212414b924ff29302990b2a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java#L945). There is even a test for that: `TestRESTCatalog.testTableCredential`. All of this tends to suggest that authentication is not handled consistently across the board. The `token` property is generally respected, but `credential` is not. The `oauth2-server-uri` property is not respected at all. Such inconsistencies are confusing and make it hard to reason about the system. My intention was to bring the spec in line with the current implementation by honoring `credential` and `oauth2-server-uri`, among others, from the config endpoint. Let me finish by addressing the security concerns you raised. While I would generally agree with you that authentication properties should not be shared between client and resource server, I think we are past that point. The client already happily accepts whatever `token` the resource server gives it, and in some cases, it accepts even the resource server's `credential` too, and uses those to authenticate. _This is in complete contradiction with OAuth 2.0 principles_. Moreover, this behavior is validated by tests, so it's not something that we can change now. This brings me to the tacit conclusion that client and server must form a sort of trusted pair, functioning in a trusted environment. In that situation, I don't see why the client could not obtain all of its authentication configuration from the server, rather than just some portions of it. -- 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: issues-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org
