Yeah, I don't think that this response was used. We thought that it was needed but it can probably be safely removed as I'm not aware of any implementation that sent or handled it. If that's the right thing to do because there are other more standard mechanisms for sending more information about a 401, then that sounds reasonable to me.
We should make sure the folks that run systems that authenticate using SigV4 are okay with this, too. On Mon, Feb 24, 2025 at 1:15 AM Alex Dutra <alex.du...@dremio.com.invalid> wrote: > Hi all, > > Thanks Sung for your support – I will start a discussion thread soon on > the topic of expected client behavior for both 401 and 419 response codes. > > And out of curiosity, I looked at how the AuthenticationTimeoutResponse > type was currently handled in iceberg-core, and was surprised to see that > it's not even materialized as a RESTResponse. There is no test verifying > this response type either. If we force the server to send back this error > code, the client surfaces a generic error to the user (instead of > NotAuthorizedException for instance): > > org.apache.iceberg.exceptions.RESTException: Unable to process: > Credentials have timed out > > So imho this definitely means that we need a better handling of this error > (and authentication errors in general). > > Thanks, > > Alex > > On Sun, Feb 23, 2025 at 12:30 AM Sung Yun <sungwy...@gmail.com> wrote: > >> Thank you Daniel, Dmitri, Alex and Yufei for sharing your thoughts. >> >> I'm in favor of updating the REST Spec to be more clear about these >> status codes. >> >> Here's a PR[1] that follows the RFC language to make expected and >> recommended behaviors of the server and the client more explicit >> (thanks Dmitri for the suggestion). Please feel free to review and >> make suggested changes. >> >> And Alex, yes that sounds like a great idea. I think codifying the >> expected client behavior will be helpful, as it will help client >> implementations in the different language libraries be more >> consistent. >> >> [1] https://github.com/apache/iceberg/pull/12376 >> >> On Fri, Feb 21, 2025 at 7:25 PM Yufei Gu <flyrain...@gmail.com> wrote: >> > >> > I'm fine with 419 being considered an optional status code, in which >> case: >> > >> > Servers can choose not to implement it. >> > Clients shouldn't rely solely on it for token expiration. >> > >> > However, the current REST spec doesn't clarify this. We should make it >> more explicit. >> > >> > We could try to deprecate it later if needed. >> > >> > Yufei >> > >> > >> > On Fri, Feb 21, 2025 at 5:04 AM Alex Dutra >> <alex.du...@dremio.com.invalid> wrote: >> >> >> >> Hi all, >> >> >> >> I think there are two aspects in this issue that should be discussed >> separately: >> >> >> >> What are the pros and cons of having a custom HTTP response code for >> authentication issues, as opposed to using 401? >> >> How should clients deal with authentication issues, both 401 and 419? >> >> >> >> Regarding aspect 1: I am not sure that I see a good reason for having >> both 419 vs 401. >> >> >> >> First, it's a custom response code, so it requires custom handling for >> clients. Sticking to HTTP standards is way safer imho. >> >> >> >> Secondly, a custom response code would be interesting only if the >> client could take a specific action to fix the issue. But what can a client >> do to "fix" a 419 response? An interactive client would redirect the user >> to a login form. A non-interactive client would probably just fetch new >> credentials and retry. That's exactly what these clients would do when >> receiving 401 as well. >> >> >> >> Moreover, the 401 response code, as defined by RFCs 7235 and 9110 [1], >> already provides a mechanism to tell the client what's wrong with >> authentication and how to fix it: that's the purpose of the >> WWW-Authenticate response header [2]. >> >> >> >> For example, when using OAuth2 and bearer token authentication, an >> error code named invalid_token is defined as follows [3]: >> >> >> >>> The access token provided is expired, revoked, malformed, or invalid >> for other reasons. The resource SHOULD respond with the HTTP 401 >> (Unauthorized) status code. The client MAY request a new access token and >> retry the protected resource request. >> >> >> >> >> >> The above error code and recommended client behavior seems perfectly >> fit for signaling expired OAuth2 credentials to clients. >> >> >> >> When using other authentication schemes, like SigV4, obviously other >> error codes should be used, but the 401 challenge mechanism is by no means >> tied to OAuth2. Thus I do not see a valid reason to also support 419. I >> would prefer if each authentication scheme (OAuth, SigV4 etc.) could >> provide specific guidance as to how to interpret the WWW-Authenticate >> header contents appropriately. I would also love to see if someone in this >> mailing list could confirm how SigV4-enabled servers currently handle >> expired credentials – but I would be surprised if they use anything other >> than 401. >> >> >> >> Regarding aspect 2: as I mentioned above, clients willing to "fix" >> authentication issues should intercept the 401 (and 419) responses and act >> on them by attempting to re-authenticate. It seems that's what the Python >> client is doing for 419 – but not for 401, surprisingly. The Java client, >> OTOH, right now does nothing with these response codes, and throws an error. >> >> >> >> That's arguably not very useful and begs the question: should clients >> have a retry/challenge mechanism for authentication errors? Should the >> client behavior be specified or documented somewhere? Should it be >> normative? >> >> >> >> I have already been thinking about processing authentication >> challenges in the Java client, but that requires the AuthManager API to be >> merged first [4]. This could come in the form of a new method in >> AuthSession: >> >> >> >> Optional<HTTPRequest> onUnauthorized(HTTPRequest request, HTTPHeaders >> responseHeaders, int currentAttempt); >> >> >> >> With this new method, the HTTP client could redirect response codes >> like 401 (or 419) to the AuthSession, which would then decide if it's >> possible to re-authenticate and retry, or if it should give up and surface >> the error to the user. >> >> >> >> If there is interest in moving forward with this idea, I'm happy to >> start a separate discussion thread. >> >> >> >> Thanks, >> >> >> >> Alex >> >> >> >> [1] https://www.rfc-editor.org/rfc/rfc9110#status.401 >> >> [2] https://www.rfc-editor.org/rfc/rfc9110#field.www-authenticate >> >> [3] https://www.rfc-editor.org/rfc/rfc6750.html#section-3.1 >> >> [4] https://github.com/apache/iceberg/pull/12197 >> >> >> >> >> >> On Fri, Feb 21, 2025 at 5:02 AM Dmitri Bourlatchkov <di...@apache.org> >> wrote: >> >>> >> >>> Thank you for your response, Dan! I agree with all your points. >> >>> >> >>> On the other hand, I guess readers of the Catalog REST API spec may >> still be uncertain about when specific error codes MAY or SHOULD occur >> (using the traditional RFC language). >> >>> >> >>> Do you think it might be worth clarifying the REST spec to avoid this >> ambiguity (just in descriptions, without materially changing its OpenAPI >> elements)? >> >>> >> >>> Thanks, >> >>> Dmitri. >> >>> >> >>> On Thu, Feb 20, 2025 at 7:01 PM Daniel Weeks <dwe...@apache.org> >> wrote: >> >>>> >> >>>> Hey Sung, >> >>>> >> >>>> My interpretation is that it's up to the REST Server to decide >> whether to send a 419 or 401 response code (I don't think it's a mandate). >> >>>> >> >>>> The use case for 419 would be that the client has client credentials >> or can re-authenticate via some other mechanism and could reattempt the >> request. >> >>>> >> >>>> This may be more useful with other auth mechanisms (e.g. SigV4), but >> that's not currently part of the reference implementation. >> >>>> >> >>>> So, I don't think it's necessary to remove 419 as the REST Catalog >> is not OAuth specific, but I think it's entirely reasonable (and likely >> correct based on the OAuth RFC) to respond with a 401 when using OAuth. >> >>>> >> >>>> -Dan >> >>>> >> >>>> >> >>>> >> >>>> On Wed, Feb 19, 2025 at 7:06 PM Sung Yun <sungwy...@gmail.com> >> wrote: >> >>>>> >> >>>>> Hi folks, >> >>>>> >> >>>>> While working with the Polaris community on an issue[1], I decided >> to >> >>>>> bring this discussion to the Iceberg community mailing list as I >> >>>>> believe that the status code 419 in the REST Catalog Open API Spec >> may >> >>>>> have become a source of confusion for the community. >> >>>>> >> >>>>> In the Iceberg REST Catalog Open API Spec[2], 419 status code is >> >>>>> reserved for AuthenticationTimeoutResponse, which according to the >> >>>>> description means: >> >>>>> >> >>>>> "Credentials have timed out. If possible, the client should refresh >> >>>>> credentials and retry." >> >>>>> >> >>>>> I believe this description can be interpreted to mean that the 419 >> >>>>> status code should be reserved by the Iceberg REST Catalog for >> >>>>> responding to requests with expired tokens. I think it is important >> to >> >>>>> note that this is contrary to the recommended practice in RFC6750[3] >> >>>>> which mentions that the resource should respond with a 401 if the >> >>>>> access token is expired. This interpretation of the spec has already >> >>>>> resulted in PyIceberg introducing retries with token refreshes on >> 419 >> >>>>> status code[4]. >> >>>>> >> >>>>> So I'd like to ask the community - does the Open API Spec mandate >> that >> >>>>> a REST Catalog Server respond with a 419 status code on token >> >>>>> expiration? If not, would it make sense to remove the 419 status >> code >> >>>>> response from the spec, and instead make a recommendation on using >> 401 >> >>>>> for token expiration with a specific error message? >> >>>>> >> >>>>> In my opinion, having a consistent way of communicating to our >> clients >> >>>>> the rationale for denial of access is good practice, because it >> >>>>> informs the client on the next logical course of action they could >> >>>>> take. >> >>>>> >> >>>>> [1] https://github.com/apache/polaris/issues/791 >> >>>>> [2] >> https://github.com/apache/iceberg/blob/6c546fe1346e81ca0c3f477695340016da891204/open-api/rest-catalog-open-api.yaml#L4571-L4584 >> >>>>> [3] https://www.rfc-editor.org/rfc/rfc6750 >> >>>>> [4] >> https://github.com/apache/iceberg-python/blob/d1fea5c2bdd4cec248c20c0af51cf6c49966b7dd/pyiceberg/catalog/rest.py#L151-L156 >> >
