danielcweeks commented on code in PR #10256: URL: https://github.com/apache/iceberg/pull/10256#discussion_r1600407349
########## core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java: ########## @@ -215,6 +215,12 @@ public void initialize(String name, Map<String, String> unresolved) { this.paths = ResourcePaths.forCatalogProperties(mergedProps); String token = mergedProps.get(OAuth2Properties.TOKEN); + // re-resolve these variables in case they were overridden by the config endpoint + credential = mergedProps.get(OAuth2Properties.CREDENTIAL); Review Comment: > seems to [require authentication](https://github.com/apache/iceberg/blob/ea916c1700ef37377d3e8bba1197b3fe32d1a248/open-api/rest-catalog-open-api.yaml#L121-L122). There is no requirement that use of the REST protocol requires auth. For people operating their own internal REST implementations, they can choose to support auth or not. > the config endpoint seems to already return sensitive information The config endpoint may return sensitive information or not, but I won't recommend returning sensitive information if the communication channel is not secure. -- 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