Comment 6 for bug 1489749

> 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).

Yes, it's a configuration bug - however, I think we can do better and in case of
a misconfiguration issue a warning at least and possibly skip staticweb.

> 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.

The if branch added by me is only required to fix the issue if staticweb is in
the pipeline before auth. If staticweb is configured as described in the
documentation, it works as expected and doesn't return html on authenticated
requests.

> 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 should have described this differently: "Read ACLs are not checked if
staticweb is used". staticweb uses make_pre_authed_env, thus there is no check
at all and a read_acl in the form of ".r:*, .rlistings" is not required, though
mentioned in the docs. Thus either the documentation needs a fix, or the code -
I would prefer the later, enforcing the use of set ACLs and not allowing
staticweb to pass around them.

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

This is nothing new, the described behavior applies to older Swift releases as
well.

I updated the patch, and now instead of make_pre_authed_env it just uses
make_env. However, this might break deployments because it is now required to
use ".r:*,.rlistings" to make staticweb work; the docs mentioned only ".r:*"
before.

Apart from the questionable behavior, openstack/puppet-swift does put
staticweb before keystone; though there are probably quite a few
deployments out there that see this issue. Granted, that's the problem
of openstack/puppet-swift, and I'd like to submit a fix to openstack/puppet-swift
soon. IMO it's only fixing the client issues, not the acl thing; thus I think
it's safe to do this right now without any OSSA. Objections?