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

Authorisation incorrectly implements 403 Forbidden against RFC 2616 #436

Closed
simonyarde opened this issue Feb 14, 2012 · 24 comments
Closed

Comments

@simonyarde
Copy link

I have provided an explanation of the issue as-I-see-it and an outline for
potential improvements to the cookbook re issue #433 since I believe
they are related, hence the length of the article as I've attempted to capture
both concerns here.

Background

There are two common approaches to authentication: HTTP Authentication re
RFC2617; or so called "custom authentication", also known as "forms
authentication" or "cookie authentication".

With custom authentication, a web application masks 401 Unauthorised responses
with redirection to a custom login-form and uses cookies to persist
authentication information in so-called "auth tickets". Following successful
authentication the user is typically redirected to the protected resource
initially requested, or to a default page if the login request did not originate
from a masked 401. In such designs, 401 responses are never sent because they
are always replaced by 302 Found (since HTTP/1.0), or more appropriate 303 See
Other (since HTTP/1.1). Custom authentication is common-place because it has
been seen to allow developers greater control over the login process,
particularly in relation to presentation, log-out, feedback regarding forgotten
passwords, and the ability to implement the kinds of increased-security features
employed by financial institutions.

HTTP Authorisation is appropriate where a user-agent needs to access a protected
resource without user-interaction such as a private RSS-feed. In this
circumstance custom authentication is not possible because there is no defined
standard. HTTP Authentication is also applicable in many cases where basic
log/pass functionality is required. As browsers improve the user-experience of
login-challenges triggered by 401 responses they may become more accepted. Users
are increasingly accustomed to mobile interfaces that create much greater
consistency between browser and operating-system UI, and indeed less distinction
between web-pages and apps connected to web-services.

Hybrid login systems providing both HTTP Authorisation and custom authorisation
are a likely interim situation for the foreseeable future as described in this
article by Atif Aziz and Scott Mitchell -
http://msdn.microsoft.com/en-us/library/aa479391.aspx - and are especially
relevant as the web becomes more RESTful or web-service-like.

Issue

Pyramid's authorisation raises a 403 Forbidden exception. I believe this is the
wrong exception and response because 403 explicitly states that authorisation
will not help, i.e. the problem has nothing to do with authorisation
whatsoever
:

10.4.4 403 Forbidden

The server understood the request, but is refusing to fulfill it. Authorization
will not help and the request SHOULD NOT be repeated. If the request method was
not HEAD and the server wishes to make public why the request has not been
fulfilled, it SHOULD describe the reason for the refusal in the entity. If the
server does not wish to make this information available to the client, the
status code 404 (Not Found) can be used instead.

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.4

401 is the meaningful exception and correct HTTP response for all matters
relating to authentication and authorisation:

10.4.2 401 Unauthorized

The request requires user authentication. The response MUST include a
WWW-Authenticate header field (section 14.47) containing a challenge applicable
to the requested resource. The client MAY repeat the request with a suitable
Authorization header field (section 14.8). If the request already included
Authorization credentials, then the 401 response indicates that authorization
has been refused for those credentials. If the 401 response contains the same
challenge as the prior response, and the user agent has already attempted
authentication at least once, then the user SHOULD be presented the entity that
was given in the response, since that entity might include relevant diagnostic
information. HTTP access authentication is explained in "HTTP Authentication:
Basic and Digest Access Authentication" [43].

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html#sec10.4.2

The introduction of HTTP standards into Pyramid's exception-language creates an
expectation for correct usage that is disrupted by the misuse of 403 in relation
to authorisation. This is having negative repercussions in the tutorial content,
which might otherwise have gently introduced new developers to web-standards and
to the logic of a custom auth mechanism that is complimentary to HTTP standards
and HTTP Authorisation.

Furthermore, because the 403 Forbidden view is in use by authorisation it is
difficult to devise code that responds to genuine 403 exceptions (see Twitter
example discussed below).

Applications wishing to use HTTP Authorisation currently have to intercept a 403
Forbidden exception and mask it with a 401 response in order to trigger
user-agent login mechanisms; again, it is hard to devise good code and
counterintuitive to communicate in documentation and tutorials.

Regarding permissions, 401 is the correct exception/response where a user does
not have a particular permission because the user should be given a chance to
login with different credentials, and additionally be informed of the reason why
their login failed:

If the request already included Authorization credentials, then the 401
response indicates that authorization has been refused for those
credentials.

RFC2616

and as mentioned by @mmdonc in relation to Issue #433:

.. in reality what the
login-form-invoked-as-the-result-of-an-HTTPForbidden-when-the-user-already-
is-not-just-Everyone could say is "your current credentials do not allow you
to perform the specified operation, but you can try to log in with another
set of credentials if you like, and the operation will then be performed if
those credentials are permitted to perform the operation". This is also the
answer to the second question above.

For contrast I highlight the example of Twitter which responds with 403
Forbidden where the number of requests for a given user has been exceeded; the
issue is temporary and is not related to an authorisation problem (auth will not
help), but has been caused by the user, and so is the correct use of 403. The
corresponding server-error 503 Service Unavailable would have been appropriate
had it been the server that was at fault.

Related Note re Cookbook and Docs

#433

I believe the problem needs to be solved root-and-branch, starting with the
correct use of 401 as discussed above. Otherwise developers need to understand
that 403 Forbidden is a meaningless exception that they will have to
intercept and mask. As mentioned previously it becomes hard to write good code
that clearly separates authentication logic from informational 403 exceptions.

It would be beneficial to highlight the two common approaches to authorisation,
i.e. HTTP Authorisation and custom authorisation, and explain their relationship.

It would probably be best to focus on custom authorisation since the
implementation is easier and a more common-place requirement.

The cookbook could explain briefly that HTTP Authorisation can be integrated
at a later stage to create a hybrid authorisation mechanism to support certain
user-agents, where the 401 response would be returned rather than masked
with a redirect.

Developers need to understand that if they implement an ACL but no means for a
user or user-agent to authenticate, then the user will be presented with the
browser's login challenge or be redirected to a login page (if configured); the
content will remain inaccessible in any case.

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

Wow. You really shouldn't have spent as much time on this as you did. Think of the children! To hell with the children, think of me!! I have to respond now! ;-)

This topic has a long history. You might search the pylons-discuss maillist archives for "HTTPForbidden" and "HTTPNotFound" (and/or "Forbidden" and/or "NotFound").

Long ago, when it was named repoze.bfg, Pyramid used to return response code 401 when a view was found but the invoking user was disallowed from invoking it due to a permission failure and no exception view was registered. But this confused the shit out of people because, as per your description above, it's invalid to return a 401 response without a WWW-Authenticate header and it made no sense at all to do so in cases where basic auth was not desirable. And basic auth is almost never desirable for applications that are driven by a non-expert human.

Pyramid also used to make a distinction between the pyramid.httpexceptions.HTTPForbidden exception (which was a response) and the pyramid.exceptions.Forbidden exception (which was not a response). Pyramid used to raise the latter when view execution was disallowed due to security. But this also confused the shit out of people because they apparently couldn't make the distinction between the two exceptions, so we made pyramid.exceptions.Forbidden an alias for pyramid.httpexceptions.HTTPForbidden to tamp down the whining. We also did this for another pair of exceptions: pyramid.httpexceptions.HTTPNotFound vs. pyramid.exceptions.NotFound. This is where I think you're probably right, and that we might have just as well decided to not conflate the idea of an HTTP forbidden with a Pyramid Forbidden exception. It was a debatable decision, much argued over on the maillist, and I had vague misgivings about it, but it's done now.

In any case, all of this seems moot to me, given the following:

  • pyramid.httpexceptions.HTTPForbidden is a Python exception. It happens, when used as a response,
    to return a 403 Forbidden status, but that doesn't matter when it's not used as a response.

  • The same exception is also importable as pyramid.exceptions.Forbidden (an alias).

  • When treating it as an exception, you needn't think of it as a response. If importing it as
    pyramid.exceptions.Forbidden helps you make this distinction mentally, you can import from there.
    If it's the name that bothers you, alias it in your own code. I'm not real eager to make people go through a firedrill
    deprecation just to rename it "Unauthorized", it's a well-established API; and "Unauthorized" actually
    doesn't make sense here either, because it may not be accessible using any other credentials either.
    The view might be plain inaccessible to anyone.

  • You can register an exception view for pyramid.exceptions.Forbidden which returns a response with
    whatever response code you like (including 401 with WWW-Authenticate). You are actually expected to do this
    in any application that requires reauthentication-as-the-result-of-inability-to-invoke-a-view-due-to-security.
    The default response is never sensible for any application that allows the ability to reauthenticate to
    provide different credentials. This isn't really logically the same as "masking a 403" as you mention above.
    This is registering a view which catches an exception and returns a custom response after doing so; the exception
    it catches isn't identified by its "403-ness" in reality, it's just like any other exception.

    The default response is just too ugly for anyone to use, even if they
    don't want someone to be able to reauthenticate, so it's very unlikely you'll get very far in developing any project
    that does not register a Forbidden exception view anyway.

So I think this criticism, as every good criticism does, boils down to names. It'd take a minor miracle for Pyramid's API to change right now, however, just for the sake of naming, so I'm pretty sure we're going to trade about 50 emails attached to this issue and it will be closed without action. ;-)

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

Note that I agree that it is confusing for people who really do want to raise pyramid.httpexceptions.Forbidden with the expectation that it will be used as a response without being caught by an exception view. But, for better or worse, that ship has sailed (with misgivings noted above). They won't be able to do this:

raise pyramid.httpexceptions.HTTPForbidden()

Instead they'll have to return the value to avoid it being caught by an exception view registered for HTTPForbidden:

return pyramid.httpexceptions.HTTPForbidden()

@simonyarde
Copy link
Author

Ha! Well I may be overzealous but at least you know I care ;) Sorry I missed the previous discussions. Recreational! Pah! :)

and "Unauthorized" actually doesn't make sense here either, because it may not be accessible using any other credentials either. The view might be plain inaccessible to anyone.

I resisted (can you believe it) writing about this, but surely in a web-app a view which is inaccessible to anyone shouldn't be a view? 403 might make sense for a web-server with files that are protected. A view is either accessible to everyone or subject to some authorisation - even if it blocks everyone except the admin?

I've always thought the HTTP way was very elegant and worth adopting, on the basis that you can't know if someone doesn't have credentials until they are authenticated re RFC2616:

If the request already included Authorization credentials, then the 401 response indicates that authorization has been refused for those credentials.

If the user isn't authenticated then we need to authenticate them; if they are authenticated then they must not be authorised; therefore 401 Unauthorised seems to be correct for permission-required or authentication-required type exceptions.

Even as a mental-framework I felt there was merit here to improve the guide re #433 - I'll see what I can do in the coming weeks.

Sure, I understand the distinction of raising exceptions and returning responses. So how do I get Pyramid's auth to raise an exception of my choosing?

I would be happy to raise HTTPUnauthorised, or just Unauthorised for the above reasons. I am comfortable that I would never let an incomplete 401 be served, and would mask this with a redirect 301/3 response for humans to a custom login page; the exception raised is HTTPUnauthorised and the view would either return 401 or return a Found response. To me at least this is a logical design for a hybrid-auth app.

I (feel like I) need to do the above so I can raise an HTTPForbidden and have that picked-up by the real HTTPForbidden view. Otherwise how do I go about customising some decent-looking views if I just return the HTTPForbidden? It's getting messy?

Anyway.. let's keep it brief and recreational! I understood Pyramid was unlikely to change for exactly the reasons you say, and the casualty does seem to be Forbidden views re your last note. I'm always grateful to have had the discussion and put what we find here into the mutual knowledge-bank, but I'd appreciate some guidance in what I'm trying to do. Please go ahead and close if it keeps things tidy :)

@mmerickel
Copy link
Member

I appreciate the concern, but it really does seem to boil down to naming. Until the user demonstrates a willingness to modify the behavior, HTTPForbidden is the best return value for a view that has a permission on it because 401 just doesn't make sense without adding the WWW-Authenticate headers - requiring customization on their part. Now both ways require customization, but the HTTPForbidden works better as a default, unless you think pyramid should just raise a custom internal exception that, until caught, gets turned into a 500 server error.

The HTTPForbidden is used for Pyramid's auth system as a _Python exception. It is intended to be overridden, and from within an exception view it is possible to return any type of response desired, including HTTPUnauthorized (401), or HTTPForbidden (403) or HTTPSeeOther (303) or HTTPFound (302), _OR pyramid.response.Response.

The only change I see needed to Pyramid in this issue is better auth documentation.

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

Ha! Well I may be overzealous but at least you know I care ;) Sorry I missed the previous discussions. Recreational!
Pah! :)

Heh, sorry, having a little fun. ;-) Gotta have some fun.

I resisted (can you believe it) writing about this, but surely in a web-app a view which is inaccessible to anyone
shouldn't be a view?

In a system with very dynamic permission assignments it's not impossible for a view to at least temporarily have no way to be invoked directly via a URL. Since at least in terms of Pyramid default authorization, involving ACLs, principals possess permissions (often only in a certain context), an administrator might change ACLs in such a way that no one possesses that permission in such a context. In such a system, the developer may have no way to ensure that a permission is granted to a set of users or even that any of the ACLs in the system mention a particular permission. The authorization policy in terms of the ACLs granted is in charge of that, and ACLs in highly dynamic systems are owned by the administrator, not the software developer. There's just no way to ensure that every URL can be invoked if the administrator has complete control. He may change it later so that someone can invoke the view, but it's a fluid situation.

403 might make sense for a web-server with files that are protected. A view is either
accessible to everyone or subject to some authorisation - even if it blocks everyone except the admin?

Again, I think you need to mentally divorce "403-resulting-from-rendering-of-HTTPForbidden" from "HTTPForbidden-as-an-exception-meaning-a-view-invocation-was-rejected." In the latter case, the fact that Pyramid raises HTTPForbidden as an exception meaning a view can't be invoked due to security constraints. The fact that it is also a response object and has a 403 status code is not relevant in that situation, particularly if you configure an exception view to catch it and return a substitute response. And if you don't, well, you're going to get an ugly plaintext page with a big 403 on it, which usually just means you haven't actually finished writing your application. This would be the same if we used HTTPUnauthorized to represent the "view-invocation-was-rejected" case unless we also passed a WWW-Authenticate header in the response, and even if we did that, if a basic auth security policy was not configured, the goggles, they would do nothing.

Don't have the energy to respond to the rest of the stuff, I think I'd just be repeating myself anyway.

@mmerickel
Copy link
Member

Otherwise how do I go about customising some decent-looking views if I just return the HTTPForbidden? It's getting messy?

@view_config(context=HTTPForbidden, renderer='forbidden.mako')
def forbidden_view(request):
    request.response.status = 403
    return {
        # params required for rendering forbidden.mako
    }

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

Pedantic: the above view is not invoked when an HTTPForbidden is returned (as per the original question) but it would be invoked when an HTTPForbidden is raised. When an httpexception is returned, it is never caught by any exception view (it is not treated as an exception, just as a response). If you really did want to return a response with a 403 status code, you'd just do it:

@view_config(renderer='mytemplate.mako')
def my_view(request):
    request.response.status = 403
    return {
        # params required for rendering mytemplate.mako
    }

@mmerickel
Copy link
Member

Oops, I thought that we'd already covered "return vs raise" so in my mind it was about raising. Anyway, move along.. when no exception is raised, no exception view is invoked, so the original view is responsible for invoking a renderer itself and returning it's own response.

@simonyarde
Copy link
Author

Thanks Michael... yes, I was asking about 'raising' an HTTPForbidden exception to be handled by a Forbidden view that 'returns' 403. But my point was, with that in place how does one create a Forbidden view for authentication purposes?

http://docs.pylonsproject.org/projects/pyramid/en/1.3-branch/narr/hooks.html#changing-the-forbidden-view

How can one distinguish between Forbidden and Forbidden-when-we-really-mean-unauthorised exceptions?

@simonyarde
Copy link
Author

@mcdonc You did already answer the above;

Instead they'll have to return the value to avoid it being caught by an exception view registered for HTTPForbidden:

So can I alter the exception generated by authorisation instead so I can distinguish between them?

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

So can I alter the exception generated by authorisation ...

No.

instead so I can distinguish between them?

Which "them" are you referring to?

@mmerickel
Copy link
Member

Maybe you need to give a specific example. Pyramid's forbidden view is telling you that authorization failed. What you do in that view is up to you. My typical forbidden view boils down to what's been posted before in the mailing list. If there is no authenticated user, you can determine they just need to be challenged to login. If they are already logged in, you can make some custom decision there to challenge them further, or just return a 403.

@view_config(context=HTTPForbidden, renderer='forbidden.mako')
def forbidden_view(request):
    if authenticated_userid(request) is None:
        location = request.route_url('login', _query={'came_from': request.path})
        return HTTPSeeOther(location)

    request.response.status = 403
    return {
        # params required for rendering mytemplate.mako
    }

@simonyarde
Copy link
Author

Which "them" are you referring to?

Two views, one catching Forbidden, one catching Forbidden as raised by auth.

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

For the record, you can also work around it like so:

from pyramid.httpexceptions import WSGIHTTPException

class MyHTTPForbiddenException(WSGIHTTPException):
    code = 403
    title = 'Forbidden'
    explanation = 'Access was denied to this resource'

def myview(request):
    raise MyHTTPForbiddenException()

@mmerickel
Copy link
Member

Pyramid uses HTTPForbidden as the exception for it's authorization system. So that's what you use in the exception view for "raised by auth". If you need another exception also, you'll have to define one yourself.

class ReallyForbidden(HTTPForbidden):
    """ A custom exception for someone who's really forbidden, independent of further challenges."""

@view_config(context=ReallyForbidden)
def really_forbidden_view(request):
    return request.context

def some_other_view(request):
    raise ReallyForbidden()

@simonyarde
Copy link
Author

Is it now possible to raise an exception from within a view? I'm sure I asked about this before and the answer was no?

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

Sure, you can raise any exception you like within a view. If no exception view is registered for that class or any interface attached to the class, it will bubble up all the way to the WSGI server, which will usually result in a 500 response being sent to the browser by the server.

On the other hand, if one or more exception views are registered for the raised exception, the most specific exception view found (let's put how that decision is made aside for now) will receive a request and is expected to return a Pyramid response. The Pyramid response will then be turned into a WSGI response.

Pyramid, by default, registers an exception view for an interface named pyramid.interfaces.IExceptionResponse. All exceptionresponse classes in the pyramid.wsgihttpexceptions module (e.g. HTTPFound, HTTPUnauthorized, HTTPNotFound, etc) inherit from WSGIHTTPException. ANd WSGIHTTPException implements the "IExceptionResponse" interface. The exception view registered for IExceptionResponse just turns around and returns the object that is passed to it as request.context. So any class that inherits from WSGIHTTPException, when raised, will be caught by this exception view and the default body of the exception will be rendered to the server.

@mcdonc
Copy link
Member

mcdonc commented Feb 14, 2012

I'm sure that didn't actually answer your question, but it answered some question. ;-)

@simonyarde
Copy link
Author

Well, I guess it did!

So, HTTPForbidden is something to do with Unauthorised, HTTPReallyForbidden is HTTPForbidden, HTTPUnauthorised is unused... or something like that.. well good luck getting that into a tutorial ;) This editor is really going to have his work cut out!

@mmerickel
Copy link
Member

I think you have a point that Pyramid should have a completely separate Exception class that is raised for its auth system. It should not be HTTPForbidden and it should not be HTTPUnauthorized, but should be pyramid.exceptions.Forbidden. The damage is already done though, and in reality since Forbidden is aliased to HTTPForbidden you do get effectively that behavior. The big difference being that the user must understand the special meaning of HTTPForbidden when raising it from within their own views.

@mcdonc
Copy link
Member

mcdonc commented Feb 15, 2012

In any case, the current situation is tenable, if regrettable. To be honest, I'm actually glad it is the way it is now, because previously when we had distinct exceptions for pyramid.exceptions.Forbidden and pyramid.httpexceptions.HTTPForbidden it caused lots of confusion. The intersection of people who want to 1) raise HTTPForbidden by hand to indicate what, in your example, twitter uses it for 2) don't expect it to invoke an exception view registered as a "login view" (or other "credentials required" or "sorry you cant see that" view) and 3) can't cope with the workarounds that we mentioned in this thread is a fairly minimal amount of people, compared to the number of people who couldn't work out the difference between pyramid.exceptions.Forbidden and pyramid.httpexceptions.HTTPForbidden. So all in all, while it will be confusing for a small minority, I think it's the better choice of the two we had at the time when we aliased them to mean the same thing, because people got all kinds of axle-wrapped before we did it.

I'm closing this, as there's really nothing we can do about it now.

@mcdonc mcdonc closed this as completed Feb 15, 2012
@simonyarde
Copy link
Author

Thanks @mcdonc and @mmerickel for taking the time to understand my concerns and provide the feedback you have. Really, it is appreciated, as always, and is a mark of class on Pyramid's part.

@mmerickel I'm with you on the auth exception - the exception needs to be distinct to be really useful and the handler/view should in turn raise whatever HTTPExceptions the designer deems fit.

The ability to raise HTTPExceptions is extremely elegant design on Pyramids part - it's just that the use of HTTPForbidden by auth is the 'exception' that breaks the rule ;)

I'm running a number of small patches, that for reasons I totally understand are unlikely to easily find a home in Pyramid in the near-term, and I will make these available for curiosity value.

@simonyarde
Copy link
Author

Sure, you can raise any exception you like within a view.

Please can you let me know why this doesn't work?

class MyException(Exception):
    """ """

@view_config(context=MyException)
def myexception(request):
    return Reponse('My Exception')

@view_config(context=HTTPNotFound)
def http_not_found(request):
    raise MyException()

MyException falls-through to 500 Internal Server Error. I get the same problem when raising an exception from within a custom exception view.

@mmerickel
Copy link
Member

Sorry, you only get one level of exception handling from Pyramid. No raising exceptions from exception views.

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

No branches or pull requests

3 participants