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 >
