[Proposal/Discussion] New Credentials Issuers

[zikes]

I would like to see two new Issuer types: Headers and Cookies.

Headers

As a baseline proposal for Headers, I'd like to start with Go text/template based configuration and proceed from there. An example rule might look like this, with the template root object being the proxy.AuthenticationSession:

{
    "id": "some-id",
    "upstream": {"url": "http://my-backend-service"},
    "match": { },
    "authenticators": [ ],
    "authorizer": { },
    "credentials_issuer": {
        "handler": "headers",
        "config": {
            "X-User": "{{ .Subject }}",
            "X-Audience": "{{ .Extra[\"aud\"] }}",
            "X-Issuer": "{{ .Extra[\"iss\"] }}"
        }
    }
}

Some concerns I have with the above would be the performance and unnecessary complexity of text/template. The same might be accomplished with a Sprintf, but with the open-ended nature of the Extra map field of the AuthenticationSession struct, I can think of no good way to ensure all map fields are passed into Sprintf reliably. This assumes that the oauth2_introspection Authenticator is eventually extended to allow for "extension" fields for arbitrary introspection data (which I would also love to see).

Cookies

Using text/template as well for the same reasons as above:

{
    "id": "some-id",
    "upstream": {"url": "http://my-backend-service"},
    "match": { },
    "authenticators": [ ],
    "authorizer": { },
    "credentials_issuer": {
        "handler": "cookies",
        "config": {
            "cookie-name": {
                "user": "{{ .Subject }}",
                "audience": "{{ .Extra[\"aud\"] }}",
                "issuer": "{{ .Extra[\"iss\"] }}"
            }
        }
    }
}

Security Concerns

Baseline security would require that the above Headers and Cookies are stripped from incoming requests to the specified upstream, to prevent someone being able to "log in" by running a simple curl -H "X-User: admin" http://example.com

Other Implementations

Kong provides upstream header data in its gateway service, however the headers are fixed: https://docs.konghq.com/plugins/oauth2-authentication/#upstream-headers

This would certainly help with performance, but I think it would be unnecessarily restrictive when considering that OAuth2 introspection could allow for a great deal more upstream context, and that some out-of-the-box applications may have specific requirements for receiving authentication information. I think a good middle ground would be to have a set of default headers which would be used if config is left blank, making the rules simpler and easier to write in many cases.

It's my understanding that AWS API Gateway has a similar feature, but I've been unable to locate the documentation for it. If anyone could link the relevant docs here for review that would be great.

[zikes]

I've got a headers issuer started at https://github.com/zikes/oathkeeper/tree/issuer-header/proxy with a passing test, however I still need to implement incoming header scrubbing and fill in more complex test cases. Let me know if you feel like that's on the right track.

[aeneasr]

That's a pretty cool idea! I think you can build and cache templates so it shouldn't be too straining on performance. I think the Go template syntax needs getting used to but it's a pretty good place for it. I'd love to see a PR!

[zikes]

@arekkas Some good news and bad news about the Go templating: the good news is that it turns out you can access maps via e.g. {{ .Extra.iss }} or {{ .Extra.aud }}. If the key doesn't exist in the map, it will fill in the zero value for the map value's type. The bad news is that the zero value for interface{} is considered unprintable by the text/template package, and it will always print <no value> in such cases even if missingkey=zero is set for the template.

Currently my fix for this is to use a lookup FuncMap function, turning the syntax into {{ lookup .Extra \"iss\" }}, which looks even worse than the original {{ .Extra[\"iss\"] }} in my opinion 😬

Would it be possible to change the Extra field of AuthenticationSession to a map[string]string? Alternatively I can pretty easily convert the map to a map[string]string within the Issuer with fmt.Sprintf, but I don't know what you have planned for that Extra field so I don't know how it would behave down the road.

[zikes]

I went ahead and implemented the map conversion method, in favor of having a better interface to present to the users. If Extra is going to potentially contain more complex data we can figure out a better solution. You can have a look at the current implementation at https://github.com/zikes/oathkeeper/tree/issuer-header/proxy

[aeneasr]

Extra can have arbitrary data, for OAuth 2.0 Access Tokens it's - for example - the metadata associated with the access token. Depending on the server this can be float64, int, string, bool or a nested structure. It should therefore not be map[string]string

[aeneasr]

How about .String(key), .Int(key) ...?

[zikes]

I think I've worked out a FuncMap function that keeps it fairly clean and simple: https://play.golang.org/p/SyzsYJnaepW

I was mostly worried about having to create a complex key lookup system that mimics what text/template does for nesting maps, but with this function I can fall back on text/template for the lookup and override how it prints that value. Oathkeeper can specify that the print FuncMap function is the "safe" way to access keys and should be used for all values.