Merge lp:~leonardr/launchpad/bug-271029 into lp:launchpad

Proposed by Leonard Richardson
Status: Merged
Approved by: Curtis Hovey
Approved revision: no longer in the source branch.
Merged at revision: 12604
Proposed branch: lp:~leonardr/launchpad/bug-271029
Merge into: lp:launchpad
Prerequisite: lp:~leonardr/launchpad/bug-106338
Diff against target: 0 lines
To merge this branch: bzr merge lp:~leonardr/launchpad/bug-271029
Reviewer Review Type Date Requested Status
Curtis Hovey (community) code Approve
Review via email: mp+52423@code.launchpad.net

Commit message

All Unauthorized and ForbiddenAttribute exceptions that happen in the web service should result in 401 ("Unauthorized").

Description of the change

This branch builds on my fix to bug 106338, and fixes bug 271029 in the same way. I 'slam' the 401 error code onto the Unauthorized and ForbiddenAttribute exceptions, so that whenever they occur in a web service context, the result is a 401 response code instead of an OOPS.

This code is not as straightforward as it may appear. For one thing, the Unauthorized bit may not be necessary. We have a special lookup in lazr.restful that maps Zope's Unauthorized exception to a 401 response code. So I may take that out.

Second, it's not absolutely guaranteed that ForbiddenAttribute means 401. As seen in 267888, it might mean 400, when the user tries to modify a read-only field. Bug 267888 was a very early lazr.restful bug, and it was fixed by adding checks in lazr.restful for attempts to modify a read-only field, but in theory it could still happen if a read-only field is explicitly published through the web service as read-write.

To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

Got rid of the Unauthorized stuff; even if it were necessary (I don't think it is), it's not part of this bug.

Revision history for this message
Leonard Richardson (leonardr) wrote :

The diff doesn't show up because I accidentally pushed the changes to the branch this branch depends on. Here's the diff:

http://pastebin.ubuntu.com/577052/

Revision history for this message
Curtis Hovey (sinzui) wrote :

Thank you for fixing this. I look at this same bug last week and pondered what the fix would look like. This is great.

You may want to look at bug 579602, I think you fixed it :)

review: Approve (code)

Preview Diff

Empty