Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authorization vs Authentication #319

Open
tggreene opened this issue May 3, 2020 · 5 comments
Open

Authorization vs Authentication #319

tggreene opened this issue May 3, 2020 · 5 comments

Comments

@tggreene
Copy link

tggreene commented May 3, 2020

Liberator maps :authorized? to the 401 Unauthorized response, I believe this is wrong according the IETF description of this response code here:

3.1.  401 Unauthorized

The 401 (Unauthorized) status code indicates that the request has not
been applied because it lacks valid authentication credentials for
the target resource.  The server generating a 401 response MUST send
a WWW-Authenticate header field (Section 4.1) containing at least one
challenge applicable to the target resource.

This should rather be :authenticated? and be handled before the contents of the requests are inspected (i.e. before :malformed?/400 Bad Request).

Instead :authorized? should replace :allowed? and map to the 403 Forbidden response, again this is the description according to the IETF here:

The 403 (Forbidden) status code indicates that the server understood
the request but refuses to authorize it.  A server that wishes to
make public why the request has been forbidden can describe that
reason in the response payload (if any).

I can see why you would not want to implement this change at this point, but I wanted to see if there was a reason behind this and to mark it in case other users come across this in the future.

It's reasonable to continue using the framework transposing the meaning of these two handlers (:authorized? => :authenticated?, :allowed? => :authorized?) and accept that the body will be deserialized before a request is authenticated.

An alternative flow diagram illustrating what I believe is the correct order can be seen here.

@ordnungswidrig
Copy link
Member

Hi Tim,

thanks for the feedback and in depth view on things.

I want to disagree that :malformed? should come after :authenticated?. 400 Bad request indicates that the server could not understand the request. How could it determine if the request is authenticated?

But you hit a problem with liberator's design: it has a fixed order of decisions. And while you can argue whether, e.g. exists? should come before authorized? or the other way round, liberator is "opinionated" in this regard and has a static decision tree. (Of course you can always pull ahead the actual implementation of a later decision, store the result in the context and just access that pre-executed result in the actual decision function).

Regarding why liberator's decision order is like it is, the reason is that it started as a clojure port of Erlang's webmachine and inherited it's decision graph. It has deviated from it in the meanwhile though.

Because of backward compatibiliy neither the names nor the order of decisions can be changed.

@ordnungswidrig
Copy link
Member

Regarding malformed? my understanding is that it's for requests broken at the HTTP protocol level, not for errors at the application layers:

6.5.1. 400 Bad Request

The 400 (Bad Request) status code indicates that the server cannot or
will not process the request due to something that is perceived to be
a client error (e.g., malformed request syntax, invalid request
message framing, or deceptive request routing).

Source

@tggreene
Copy link
Author

tggreene commented May 4, 2020

Regarding why liberator's decision order is like it is, the reason is that it started as a clojure port of Erlang's webmachine and inherited it's decision graph. It has deviated from it in the meanwhile though.

Because of backward compatibiliy neither the names nor the order of decisions can be changed.

These are both absolutely reasonable and I don't expect you to make any changes to the framework based on this issue, I merely wanted to pin down the reasoning for future investigators! (In fact please feel free to close this issue following this comment).

You're absolutely correct 400 Bad Request should be returned by the server at the very first oppurtunity if the request is syntactically incorrect (I realize I implied that it's not the case above!). However, where liberator is acting to determine `:malformed?` is after ring has already worked to coerce the request into the best possible representation for the handler, so this has already essentially been dealt with.

The difference between the two RFCs 2616 and 7231 highlights a slight change in meaning of this code from "HTTP syntax" to a broader definition "the server cannot or will not process the request due to something that is perceived to be a client error", there's some more discussion on that here and here.

The problem I suppose is that at the point that the `:malformed?` handler is invoked we're already at the application layer, in a sense the absence of a valid `Authorization` header is just a subset of the conditions we might use to determine whether a request should proceed, it's reasonable to say that we can save ourselves any additional expense validating the request by prioritising this check first. I suppose you could also argue that a badly formed request may have valid authentication.

Anyway, this is probably more analysis than is required, although very interesting (to me at least) looking into the issue. Thanks for your prompt reply and thoughts.

@ordnungswidrig
Copy link
Member

The difference between the two RFCs 2616 and 7231 highlights a slight change in meaning of this code from "HTTP syntax" to a broader definition "the server cannot or will not process the request due to something that is perceived to be a client error", there's some more discussion on that here and here.

Oh I totally forgot about that, and, to be honest, I've seen 400 being applied to application layer client errors in the wild a lot. Using 400 also fills a gap as 422 is only applicable to request with a body.

Deep analysis of the http semantics is core to the development of liberator and while it's kind of in a frozen state regarding the decision graph, improving the understanding of what should be correct will help with either evolving this library or building any future successor.

The diagram you linked from loggly.com was very helpful to me for re-organizing my mental model of http processing.

@tggreene
Copy link
Author

tggreene commented May 5, 2020

I found the date quite surprising on RFC 7231, I always think these things were locked in stone long, long ago!

I'm pleased it was helpful, the same is true for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants