Comment 5 for bug 1489749

Revision history for this message
clayg (clay-gerrard) wrote :

So static-web is definitely supposed to be after auth in the pipeline - if it appears anywhere else it's a configuration bug (it sucks that pipeline configuration is so hard, but thems the knocks).

If we are able to validate there is security implications of this mis-configuration we could:

a) document them

b) think harder about making middleware capable of describing ordering requirements like "after auth"

... but I'm dubious of any effort to make static-web "work" in some sort of off-the-rails configuration? Does this *only* effect pipelines with staticweb in the wrong place?

@Christian

The patch isn't immediately obvious to me. The next if branch after your patch (that checks if it's an as of yet unauthenticated request with a token) checks to see if it *is* an authenticated request and unless web-mode is true - just call into the app directly. That seems like it should be enough to prevent staticweb from messing with the output of the container listing when called from python-swiftclient. We should understand why that is not sufficient - even if adding your check seems to make the undesirable behavior go away.

The bug report also says "static web does not check acls" - and that may be true, but it's not a bug? staticweb should *not* be in the business of authorizing requests. It should let the normal authorization do that - and on authorized responses perform translations/redirects only - if it starts to build up it's own model for auth outside of "this request will not work unless the user could have made this request anyway" - then we're asking for more bugs.

I don't understand exactly what's broken here - how is a `curl http://saio:8080/v1/AUTH_test/container/` working with an acl - how is static web *breaking* the normal authorization checks in the pipeline? Is someone not installing a default deny authorize callback like they should? Is a sub-request causing the authorize callback to be removed prematurely?

Can you validate your observed behavior on d4409c0a^ and see if maybe we accidentally broke something while fixing the other?