Env vars for jwks_url in id_token mutator not working in versions >v0.19.0-beta.1

[kubadz]

Describe the bug

Mutator id_token requires jwks_url to be configured either in global or in per-rule configuration. The global configuration in most cases can be set by env variables. As the version v0.19.0 introduced changes in config (#258) it is logical that also env variables might have been changed.

Previously our configuration was setting MUTATORS_ID_TOKEN_JWKS_URL and it obviously didn't work in the recent oathkeeper version. I set it to MUTATORS_ID_TOKEN_CONFIG_JWKS_URL as it is currently configured on master but it also didn't work.

For both cases Oathkeeper returns error

{"error":{"code":500,"status":"Internal Server Error","request":"7da5f8a3-8db8-45dd-b358-e7eec13cdaaa","reason":"Configuration for mutator \"id_token\" could not be validated: jwks_url is required","message":"mutator matching this route is misconfigured or disabled"}}

when calling a service.

some thoughts on this issue: I suspect that it might be related to the way how config for mutators is validated. Maybe the validation with json schema happens on the config object without jwks_url set to value from env variable. I haven't checked it thoroughly in the code, though.

Reproducing the bug

Steps to reproduce the behavior:

1. Run Oathkeeper with env var MUTATORS_ID_TOKEN_CONFIG_JWKS_URL set to the jwks location, and with id_token mutator in global config set as following:

...
      id_token:
        config:
          issuer_url: https://example.com/
          ttl: 60s
...

2. Create a rule for some service with mutators configured as following:

        "mutators": [
          {
            "handler": "id_token",
            "config": {
              "issuer_url": "https://example.com/v2"
            }
          }
        ]

3. Create a request to that service. The response should contain an error mentioned in the description.

Server logs

time="2019-10-16T09:43:59Z" level=info msg="started handling request" method=GET remote="127.0.0.1:38458" request=/ request_id=98490e37-3176-4905-a407-83a24781921a
time="2019-10-16T09:43:59Z" level=warning msg="Invalid mutator requested" access_url="http://lambda-default.35.187.58.27.xip.io/" authorization_handler=id_token error="mutator matching this route is misconfigured or disabled" granted=false reason_id=invalid_mutation_handler
time="2019-10-16T09:43:59Z" level=warning msg="Access request denied" access_url="http://lambda-default.35.187.58.27.xip.io/" error="mutator matching this route is misconfigured or disabled" granted=false
time="2019-10-16T09:43:59Z" level=error msg="An error occurred while handling a request" code=500 debug= details="map[]" error="mutator matching this route is misconfigured or disabled" reason="Configuration for mutator \"id_token\" could not be validated: jwks_url is required" request-id=98490e37-3176-4905-a407-83a24781921a status=500 trace="Stack trace: \ngithub.com/ory/oathkeeper/driver/configuration.(*ViperProvider).PipelineConfig\n\t/go/src/github.com/ory/oathkeeper/driver/configuration/provider_viper.go:257\ngithub.com/ory/oathkeeper/driver/configuration.(*ViperProvider).MutatorConfig\n\t/go/src/github.com/ory/oathkeeper/driver/configuration/provider_viper.go:284\ngithub.com/ory/oathkeeper/pipeline/mutate.(*MutatorIDToken).Config\n\t/go/src/github.com/ory/oathkeeper/pipeline/mutate/mutator_id_token.go:150\ngithub.com/ory/oathkeeper/pipeline/mutate.(*MutatorIDToken).Validate\n\t/go/src/github.com/ory/oathkeeper/pipeline/mutate/mutator_id_token.go:144\ngithub.com/ory/oathkeeper/proxy.(*RequestHandler).HandleRequest\n\t/go/src/github.com/ory/oathkeeper/proxy/request_handler.go:181\ngithub.com/ory/oathkeeper/proxy.(*Proxy).Director\n\t/go/src/github.com/ory/oathkeeper/proxy/proxy.go:116\nnet/http/httputil.(*ReverseProxy).ServeHTTP\n\t/usr/local/go/src/net/http/httputil/reverseproxy.go:216\ngithub.com/urfave/negroni.Wrap.func1\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:46\ngithub.com/urfave/negroni.HandlerFunc.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:29\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\ngithub.com/meatballhat/negroni-logrus.(*Middleware).ServeHTTP\n\t/go/pkg/mod/github.com/meatballhat/[email protected]/middleware.go:136\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\ngithub.com/ory/x/metricsx.(*Service).ServeHTTP\n\t/go/pkg/mod/github.com/ory/[email protected]/metricsx/middleware.go:261\ngithub.com/urfave/negroni.middleware.ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:38\ngithub.com/urfave/negroni.(*Negroni).ServeHTTP\n\t/go/pkg/mod/github.com/urfave/[email protected]/negroni.go:96\nnet/http.serverHandler.ServeHTTP\n\t/usr/local/go/src/net/http/server.go:2774\nnet/http.(*conn).serve\n\t/usr/local/go/src/net/http/server.go:1878\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1337" writer=JSON
time="2019-10-16T09:43:59Z" level=info msg="completed handling request" measure#oathkeeper-proxy.latency=14988034 method=GET remote="127.0.0.1:38458" request=/ request_id=98490e37-3176-4905-a407-83a24781921a status=500 text_status="Internal Server Error" took=14.988034ms

Expected behavior

I expect the request to pass.

Environment

• Version: >v0.19.0-beta.1
• Environment: Docker, k8s

Additional context

[aeneasr]

This is probably a dupe of #270 right?

[kubadz]

I wouldn't say that it is a duplicate for two reasons:

• as @GuillaumeSmaha notes, the lines for env vars related to Oauth2TokenIntrospection authenticator config was removed: link
  whether a line for specifying JWKS_URL for id_token mutator was preserved: link
• as I wrote in the description of this issue, I would expect that previous configuration MUTATORS_ID_TOKEN_JWKS_URL would be changed to MUTATORS_ID_TOKEN_CONFIG_JWKS_URL, as the configuration was changed. @GuillaumeSmaha mentions that AUTHENTICATORS_OAUTH2_INTROSPECTION_PRE_AUTHORIZATION_CLIENT_ID doesn't work.
  According to this line I would expect that the env var should look like AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_PRE_AUTHORIZATION_CLIENT_ID. Although I haven't actually tried that.

[aeneasr]

While the symptoms are the different, the cause is the same, right? I think if we just add the new keys there this should be resolved right away. We should also document this in the upgrade guide then. Would you be up for that?

[kubadz]

As I said, I don't think that adding proper keys would be sufficient. The proper key for JWKS_URL
setting already exists (link) and doesn't work as expected. At least I think that it is the proper key. And somehow it doesn't work.

I would rather suspect that it is related to the way how viper is loading env variables. If you set a value for the key manually (uncomment this line and remove config for jwks_url) the test is positive.

One thing I was missing when looking to the code is where is the setting for viper, so for a key foo.bar it looks for env FOO_BAR instead of FOO.BAR. I don't have much experience with viper, but I tested it a little, and it doesn't replace . with _ by default.

[aeneasr]

One thing I was missing when looking to the code is where is the setting for viper, so for a key foo.bar it looks for env FOO_BAR instead of FOO.BAR. I don't have much experience with viper, but I tested it a little, and it doesn't replace . with _ by default.

We did that because . is not a valid character for environment variables, hence the replacement.

But you're right, MUTATORS_ID_TOKEN_CONFIG_JWKS_URL should actually be the proper key for this (I have to be honest that I skimmed your OP thinking it was a dupe of the other issue). I have to investigate that. However, I'm really really busy at the moment so any pointers or help you could provide me with would be super duper helpful.

[aeneasr]

I believe I have found the issue, BindEnv is not actually being called, causing viper to not observe those keys, hence, when trying to get the whole config, this fails.

[aeneasr]

I will try to come up with a PR this week (however, we have an internal deadline for another project) and also a test to make sure this doesn't regress in the future.