Opened 5 years ago

Closed 5 years ago

#3924 defect closed fixed (fixed)

HTTPAuthSessionWrapper doesn't accept anonymous requests

Reported by: esteve Owned by:
Priority: normal Milestone:
Component: web Keywords:
Cc: Branch: branches/anonymous-guard-3924
(diff, github, buildbot, log)
Author: esteve Launchpad Bug:

Description

HTTPAuthSessionWrapper won't accept anonymous requests, when an instance of twisted.cred.checkers.AllowAnonymousAccess (or any other checker that supports credentials.IAnonymous) is registered in a Portal.

Attachments (1)

3924-anonymous-checkers-httpauthsessionwrapper.patch (5.7 KB) - added by esteve 5 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 5 years ago by esteve

I've created a branch in Launchpad (lp:~esteve/twisted/3924-support-anonymous-checkers-in-httpauthsessionwrapper) with a patch. I'm writing a test right now :-)

comment:2 Changed 5 years ago by esteve

  • Keywords review added
  • Owner jknight deleted

My patch changes HTTPAuthSessionWrapper#getChildWithDefault to return a util.DeferredResource, (which delegates authentication to the portal) if the HTTP request doesn't have an "authorization" header, instead of returning an instance of UnauthorizedResource. This makes it possible to use HTTPAuthSessionWrapper with a checker that handles IAnonymous credentials.

comment:3 Changed 5 years ago by mwh

This looks sane enough to me on a quick look.

This highly non-obvious URL may help someone else review the code:

http://bazaar.launchpad.net/~esteve/twisted/3924-support-anonymous-checkers-in-httpauthsessionwrapper/revision/15264?remember=15262&compare_revid=15262

(you could also 'propose a merge' in Launchpad, that would generate a diff for people to review).

comment:4 Changed 5 years ago by exarkun

  • Author set to exarkun
  • Branch set to branches/anonymous-guard-3924

(In [27187]) Branching to 'anonymous-guard-3924'

comment:5 Changed 5 years ago by exarkun

(In [27188]) Apply 3924-anonymous-checkers-httpauthsessionwrapper.patch

refs #3924

comment:6 Changed 5 years ago by exarkun

  • Author changed from exarkun to esteve
  • Keywords review removed
  • Owner set to exarkun

Patch looks really good. There are a couple minor issues that I'm going to fix, then I'll merge it:

  1. I like to try to avoid using \-style line continuation. It's not against the Twisted coding standard, and there are a lot of old examples of it in the codebase, but I dislike it. :)
  2. The whitespace change at the top of wrapper.py is just noise.
  3. The wrapper.py module docstring is now slightly wrong, it talks about how unauthenticated requests result in a 401; they are now delegated to the realm.
  4. test_anonymousAccess should be separated from the preceding test method by two blank lines, not one.
  5. assertEquals is preferred over assertEqual, for new code.

comment:7 Changed 5 years ago by exarkun

  • Resolution set to fixed
  • Status changed from new to closed

(In [27192]) Merge anonymous-guard-3924

Author: esteve
Reviewer: exarkun
Fixes: #3924

Add support for anonymous sessions to twisted.web.guard by having the
wrapper attempt to log in to the portal using anonymous credentials
when a request without an authorization header is received.

comment:8 Changed 3 years ago by <automation>

  • Owner exarkun deleted
Note: See TracTickets for help on using tickets.