Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Loading ENV vars still not working #305

Closed
pike1212 opened this issue Dec 5, 2019 · 18 comments · Fixed by #320
Closed

Loading ENV vars still not working #305

pike1212 opened this issue Dec 5, 2019 · 18 comments · Fixed by #320
Assignees
Labels
bug Something is not working.

Comments

@pike1212
Copy link
Contributor

pike1212 commented Dec 5, 2019

Describe the bug

Setting env var of say AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_PRE_AUTHORIZATION_CLIENT_ID doesn't load the config

I think the ory/x dependency just needs updated to 0.0.85

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2019

Did you set the enabled flag to true?

@aeneasr
Copy link
Member

aeneasr commented Dec 6, 2019

Version 0.0.80 includes the changes required. There might still be a bug somewhere, but it should work. Maybe double check that enabled flag is set to true (for both the authentiactor and pre auth) and also post the log message you're seeing here!

@pike1212
Copy link
Contributor Author

pike1212 commented Dec 6, 2019

export AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_PRE_AUTHORIZATION_CLIENT_ID=myclientid

INFO[0000] Config file loaded successfully. path=../../security/oathkeeper/config.yaml
FATA[0000] The configuration is invalid and could not be loaded. error="must validate one and only one schema (oneOf)" validation_error[0]="authenticators.oauth2_introspection: Must validate one and only one schema (oneOf)" validation_error[1]="authenticators.oauth2_introspection.config.pre_authorization: Must validate one and only one schema (oneOf)" validation_error[2]="authenticators.oauth2_introspection.config.pre_authorization.client_id: client_id is required"

@pike1212
Copy link
Contributor Author

pike1212 commented Dec 6, 2019

Using ory/x 0.0.82 seems to get rid of that error

@pike1212
Copy link
Contributor Author

pike1212 commented Dec 6, 2019

Also, regardless of what version of ory/x, if I set the env var

AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_INTROSPECTION_URL

it is still using the value from the config.yaml

@aeneasr
Copy link
Member

aeneasr commented Dec 9, 2019

Using ory/x 0.0.82 seems to get rid of that error

Let's bump ory/x then :) Would you be up for a PR?

@pike1212
Copy link
Contributor Author

Yes, but I still can't get the INTROSPECTION_URL env var to work

@aeneasr
Copy link
Member

aeneasr commented Dec 10, 2019

Could you provide a reproducible case & configuration? That would help a lot!

@pike1212
Copy link
Contributor Author

config.yaml

  oauth2_introspection:
    enabled: true
    config:
      introspection_url: http://badurl

rules.json

      "authenticators": [
        {
          "handler": "jwt"
        },
        {
          "handler": "oauth2_introspection"
        },
        {
          "handler": "anonymous"
        }
      ]

Run the following commands

export AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_INTROSPECTION_URL=http://goodurl
./oathkeeper serve -c config.yaml

Error I get when calling the proxy

{
    "error": {
        "code": 500,
        "message": "Post https://badurl: net/http: request canceled while waiting for connection (Client.Timeout exceeded while awaiting headers)"
    }
}

@aeneasr
Copy link
Member

aeneasr commented Dec 10, 2019

Ok thanks, I'll look into it

@aeneasr aeneasr self-assigned this Dec 10, 2019
@aeneasr aeneasr added the bug Something is not working. label Dec 10, 2019
@aeneasr aeneasr modified the milestones: v1.0.0, v0.19.0 Dec 10, 2019
@aeneasr
Copy link
Member

aeneasr commented Dec 10, 2019

Is it a workaround for you to remove the setting from the config.yaml for now?

@pike1212
Copy link
Contributor Author

I think issue may be that GetStringMap doesn't replace the values with the env vars here

https://github.com/ory/oathkeeper/blob/master/driver/configuration/provider_viper.go#L188

@aeneasr
Copy link
Member

aeneasr commented Dec 11, 2019

So I checked the code and as far as I can tell, it's set up correctly: https://github.com/ory/x/blob/master/viperx/bind_env.go#L37-L39

Basically, we're using BindEnv to bind env vars. It does seem however that GetStringMap doesn't use these env vars if they're already defined.

I was under the impression that env vars take precedence over config file values. Reading through the viper code, it also appears that that's the case: https://github.com/ory/viper/blob/master/viper.go#L1148-L1176

Basically GetStringMap boils down to the referenced lines.

I'm really not sure what's causing this, it would be awesome if you could help debug this! I'm unfortunately very busy with other porojects atm :/

@pike1212
Copy link
Contributor Author

I've been looking into this...

GetStringMap is passed authenticators.oauth2_introspection.config and I don't see where in viper it would look for all the sub keys env vars (i.e. authenticators.oauth2_introspection.config.introspection_url)

@aeneasr
Copy link
Member

aeneasr commented Dec 11, 2019

We're extracting all of the configuration keys from the configuration json schema and then do some transformation to add them to viper as env vars. You can find the code here: https://github.com/ory/x/blob/master/viperx/bind_env.go#L19-L51

@pike1212
Copy link
Contributor Author

pike1212 commented Dec 11, 2019

The extracting works fine, so viper.GetString("authenticators.oauth2_introspection.config.introspection_url") will work, but viper.GetStringMap("authenticators.oauth2_introspection.config") isn't going to iterate through the schema of config and do GetString on everything it finds.

@aeneasr
Copy link
Member

aeneasr commented Dec 13, 2019

Ah I see - nice find! I also saw the PR. I think we can solve it a bit more generic so that this also works on other levels and with over keys not named config!

@pike1212
Copy link
Contributor Author

Good call, PR is updated

aeneasr added a commit that referenced this issue Dec 18, 2019
Previously, some keys did not respect the values set in the environment variables.

Closes #305
Closes #317
aeneasr added a commit that referenced this issue Dec 18, 2019
Previously, some keys did not respect the values set in the environment variables.

Closes #305
Closes #317
pike1212 pushed a commit to pike1212/oathkeeper that referenced this issue Dec 18, 2019
Previously, some keys did not respect the values set in the environment variables.

Closes ory#305
Closes ory#317
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is not working.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants