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

Reply via email to