new HTTP auth code throws away 1st segment of path

[philmayers]

The code here:

http://twistedmatrix.com/trac/browser/trunk/twisted/web/_auth/wrapper.py#L98

...seems to have a bug. Specifically the getChildWithDefault does nothing with "path" so the 1st segment of the URL under the HTTPAuthSessionWrapper is throw away. I think this line:

http://twistedmatrix.com/trac/browser/trunk/twisted/web/_auth/wrapper.py#L121

...should read:

d = util.DeferredResource(self._login(credentials)) d.addCallback(lambda res: res.getChildWithDefault(path)) return d

...the symptoms of this bug are that with this:

class root(resource.Resource):

# render code

class child(resource.Resource):

# render code

root.putChild('foo', child())

...if I hit  http://thesite/foo I just get another copy of the root page, but if I hit  http://thesite/anyrandomstring/foo I get a copy of the child resource.

[exarkun]

(In [26504]) Branching to 'httpauth-wrapper-child-3679'

[exarkun]

Throwing away that first segment was indeed bad. :(

Another bad thing was that the tests should have been catching this. I decided that they were poorly written due to the use of direct calls to getChildWithDefault. I replaced these with getChildForRequest calls and things started to break. Fixing the wrapper to not discard the first path segment fixed them again.

[dreid]

Yeah this is a reasonable solution to this. It's unfortunate that there isn't a better way to do this than mucking around with request, but I guess if twisted.web ever grows locateChild we can change this a little.

Awesome, merge it.

[philmayers]

For what it's worth, I'd solved it locally with a subclass like this, without fiddling with the request:

class HTTPAuthSessionWrapper(guard.HTTPAuthSessionWrapper):

"""This class fixes the bug in Twisted 8.2 with getChildWithDefault"""

def getChildWithDefault(self, path, request):

authheader = request.getHeader('authorization') if not authheader:

return UnauthorizedResource(self._credentialFactories)

factory, respString = self._selectParseHeader(authheader) if factory is None:

return UnauthorizedResource(self._credentialFactories)

try:

credentials = factory.decode(respString, request)

except credError.LoginFailed:

return UnauthorizedResource(self._credentialFactories)

except:

log.err(None, "Unexpected failure from credentials factory") return ErrorPage(500, None, None)

else:

d = self._login(credentials) d.addCallback(self._gcwd, path, request) return util.DeferredResource(d)

def _gcwd(self, authed, path, request):

return authed.getChildWithDefault(path, request)

No idea if that passes the test.

You're right, it is easy with locateChild - I wrote a nevow-ised version of the the new HTTP stuff:

class HTTPAuthNevowWrapper:

implements(inevow.IResource)

def init(self, portal, credentialFactories):

self._portal = portal self._credentialFactories = credentialFactories

def renderHTTP(self, ctx):

raise NotImplementedError

def locateChild(self, ctx, segments):

# get the request object from the context request = inevow.IRequest(ctx)

authheader = request.getHeader('authorization') if not authheader:

return UnauthorizedResource(self._credentialFactories), ()

factory, respString = self._selectParseHeader(authheader) if factory is None:

return UnauthorizedResource(self._credentialFactories), ()

try:

credentials = factory.decode(respString, request)

except credError.LoginFailed:

return UnauthorizedResource(self._credentialFactories), ()

except:

log.err(None, "Unexpected failure from credentials factory") return ErrorPage(500, None, None), ()

d = self._portal.login(credentials, None, inevow.IResource) d.addCallback(self._locateChild, ctx, segments) return d

def _locateChild(self, (iface, avatar, logout), ctx, segments):

request = inevow.IRequest(ctx) request.notifyFinish().addCallback(lambda ign: logout())

return avatar.locateChild(ctx, segments)

def _selectParseHeader(self, header):

""" Choose an C{ICredentialFactory} from C{_credentialFactories} suitable to use to decode the given I{Authenticate} header.

@return: A two-tuple of a factory and the remaining portion of the

header value to be decoded or a two-tuple of C{None} if no factory can decode the header value.

""" elements = header.split(' ') scheme = elements[0].lower() for fact in self._credentialFactories:

if fact.scheme == scheme:

return (fact, ' '.join(elements[1:]))

return (None, None)

[philmayers]

Bah, sorry. My version:

class HTTPAuthSessionWrapper(guard.HTTPAuthSessionWrapper):
    """This class fixes the bug in Twisted 8.2 with getChildWithDefault"""

    def getChildWithDefault(self, path, request):
        authheader = request.getHeader('authorization')
        if not authheader:
            return UnauthorizedResource(self._credentialFactories)

        factory, respString = self._selectParseHeader(authheader)
        if factory is None:
            return UnauthorizedResource(self._credentialFactories)
        try:
            credentials = factory.decode(respString, request)
        except credError.LoginFailed:
            return UnauthorizedResource(self._credentialFactories)
        except:
            log.err(None, "Unexpected failure from credentials factory")
            return ErrorPage(500, None, None)
        else:
            d = self._login(credentials)
            d.addCallback(self._gcwd, path, request)
            return util.DeferredResource(d)


    def _gcwd(self, authed, path, request):
        return authed.getChildWithDefault(path, request)

...and for purely reasons of interest, the nevow one:

class HTTPAuthNevowWrapper:

    implements(inevow.IResource)

    def __init__(self, portal, credentialFactories):
        self._portal = portal
        self._credentialFactories = credentialFactories

    def renderHTTP(self, ctx):
        raise NotImplementedError

    def locateChild(self, ctx, segments):
        # get the request object from the context
        request = inevow.IRequest(ctx)

        authheader = request.getHeader('authorization')
        if not authheader:
            return UnauthorizedResource(self._credentialFactories), ()

        factory, respString = self._selectParseHeader(authheader)
        if factory is None:
            return UnauthorizedResource(self._credentialFactories), ()
        try:
            credentials = factory.decode(respString, request)
        except credError.LoginFailed:
            return UnauthorizedResource(self._credentialFactories), ()
        except:
            log.err(None, "Unexpected failure from credentials factory")
            return ErrorPage(500, None, None), ()

        
        d = self._portal.login(credentials, None, inevow.IResource)
        d.addCallback(self._locateChild, ctx, segments)
        return d

    def _locateChild(self, (iface, avatar, logout), ctx, segments):
        request = inevow.IRequest(ctx)
        request.notifyFinish().addCallback(lambda ign: logout())

        return avatar.locateChild(ctx, segments)

    def _selectParseHeader(self, header):
        """
        Choose an C{ICredentialFactory} from C{_credentialFactories}
        suitable to use to decode the given I{Authenticate} header.

        @return: A two-tuple of a factory and the remaining portion of the
            header value to be decoded or a two-tuple of C{None} if no
            factory can decode the header value.
        """
        elements = header.split(' ')
        scheme = elements[0].lower()
        for fact in self._credentialFactories:
            if fact.scheme == scheme:
                return (fact, ' '.join(elements[1:]))
        return (None, None)

[exarkun]

(In [26540]) Merge httpauth-wrapper-child-3679

Author: exarkun Reviewer: dreid Fixes: #3679

Fix a bug in the HTTP auth resource wrapper which caused it to disregard the first segment of the request after the segment representing the wrapper.

Also change the unit tests to use the real resource traversal API instead of bugfully imitating it.

[exarkun]

#4018 was a duplicate of this.