Skip to content

Commit

Permalink
Properly merge env vars into pipeline configs (ory#320)
Browse files Browse the repository at this point in the history
Previously, some keys did not respect the values set in the environment variables.

Closes ory#305
Closes ory#317
  • Loading branch information
aeneasr authored and pike1212 committed Dec 18, 2019
1 parent 274c19b commit 6b59512
Show file tree
Hide file tree
Showing 9 changed files with 79 additions and 64 deletions.
31 changes: 14 additions & 17 deletions .schemas/config.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -282,11 +282,12 @@
"title": "Token From",
"description": "The location of the token.\n If not configured, the token will be received from a default location - 'Authorization' header.\n One and only one location (header or query) must be specified.",
"oneOf": [
{
"type": "null"
},
{
"type": "object",
"required": [
"header"
],
"additionalProperties": false,
"properties": {
"header": {
"title": "Header",
Expand All @@ -297,9 +298,7 @@
},
{
"type": "object",
"required": [
"query_parameter"
],
"additionalProperties": false,
"properties": {
"query_parameter": {
"title": "Query Parameter",
Expand Down Expand Up @@ -481,12 +480,13 @@
"token_from": {
"title": "Token From",
"description": "The location of the token.\n If not configured, the token will be received from a default location - 'Authorization' header.\n One and only one location (header or query) must be specified.",
"type": "object",
"oneOf": [
{
"required": [
"header"
],
"type": "null"
},
{
"type": "object",
"additionalProperties": false,
"properties": {
"header": {
"title": "Header",
Expand All @@ -496,9 +496,8 @@
}
},
{
"required": [
"query_parameter"
],
"type": "object",
"additionalProperties": false,
"properties": {
"query_parameter": {
"title": "Query Parameter",
Expand All @@ -508,9 +507,8 @@
}
},
{
"required": [
"cookie"
],
"type": "object",
"additionalProperties": false,
"properties": {
"cookie": {
"title": "Cookie",
Expand Down Expand Up @@ -821,7 +819,6 @@
"title": "Repositories",
"description": "Locations (list of URLs) where access rules should be fetched from on boot. It is expected that the documents at those locations return a JSON or YAML Array containing ORY Oathkeeper Access Rules:\n\n- If the URL Scheme is `file://`, the access rules (an array of access rules is expected) will be fetched from the local file system.\n- If the URL Scheme is `inline://`, the access rules (an array of access rules is expected) are expected to be a base64 encoded (with padding!) JSON/YAML string (base64_encode(`[{\"id\":\"foo-rule\",\"authenticators\":[....]}]`)).\n- If the URL Scheme is `http://` or `https://`, the access rules (an array of access rules is expected) will be fetched from the provided HTTP(s) location.",
"type": "array",
"default": [],
"items": {
"type": "string",
"format": "uri"
Expand Down
6 changes: 3 additions & 3 deletions driver/configuration/provider_viper.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import (
"github.com/rs/cors"
"github.com/sirupsen/logrus"

"github.com/ory/go-convenience/stringsx"

"github.com/ory/gojsonschema"
"github.com/ory/x/jsonx"

"github.com/ory/viper"

"github.com/ory/fosite"
"github.com/ory/x/corsx"
"github.com/ory/x/urlx"
Expand Down Expand Up @@ -185,7 +185,7 @@ func (v *ViperProvider) pipelineIsEnabled(prefix, id string) bool {

func (v *ViperProvider) PipelineConfig(prefix, id string, override json.RawMessage, dest interface{}) error {
// we need to create a copy for config otherwise we will accidentally override values
config, err := x.Deepcopy(viper.GetStringMap(fmt.Sprintf("%s.%s.config", prefix, id)))
config, err := x.Deepcopy(viperx.GetStringMapConfig(stringsx.Splitx(fmt.Sprintf("%s.%s.config", prefix, id), ".")...))
if err != nil {
return errors.WithStack(err)
}
Expand Down
21 changes: 18 additions & 3 deletions driver/configuration/provider_viper_public_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"fmt"
"net/url"
"os"
"testing"

"github.com/rs/cors"
Expand Down Expand Up @@ -40,16 +41,30 @@ func TestPipelineConfig(t *testing.T) {

p := NewViperProvider(logrus.New())

t.Run("case=should use config from environment variables", func(t *testing.T) {
var res json.RawMessage
require.NoError(t, os.Setenv("AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_INTROSPECTION_URL", "https://override/path"))

require.NoError(t, p.PipelineConfig("authenticators", "oauth2_introspection", nil, &res))
assert.JSONEq(t, `{"introspection_request_headers":{},"introspection_url":"https://override/path","pre_authorization":{"client_id":"some_id","client_secret":"some_secret","enabled":true,"scope":["foo","bar"],"token_url":"https://my-website.com/oauth2/token"},"required_scope":[],"scope_strategy":"exact","target_audience":[],"trusted_issuers":[]}`, string(res), "%s", res)

require.NoError(t, os.Setenv("AUTHENTICATORS_OAUTH2_INTROSPECTION_CONFIG_INTROSPECTION_URL", ""))

require.NoError(t, p.PipelineConfig("authenticators", "oauth2_introspection", nil, &res))
assert.JSONEq(t, `{"introspection_request_headers":{},"introspection_url":"https://my-website.com/oauth2/introspection","pre_authorization":{"client_id":"some_id","client_secret":"some_secret","enabled":true,"scope":["foo","bar"],"token_url":"https://my-website.com/oauth2/token"},"required_scope":[],"scope_strategy":"exact","target_audience":[],"trusted_issuers":[]}`, string(res), "%s", res)

})

t.Run("case=should fail when invalid value is used in override", func(t *testing.T) {
res := json.RawMessage{}
require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"not-api":"invalid"}`), &res))
assert.JSONEq(t, `{"api":{"url":"https://some-url/"},"not-api":"invalid"}`, string(res))
assert.JSONEq(t, `{"api":{"url":"https://some-url/","retry":{"give_up_after":"1s","max_delay":"100ms"}},"not-api":"invalid"}`, string(res))

require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"api":{"this-key-does-not-exist":true}}`), &res))
assert.JSONEq(t, `{"api":{"url":"https://some-url/","this-key-does-not-exist":true}}`, string(res))
assert.JSONEq(t, `{"api":{"url":"https://some-url/","this-key-does-not-exist":true,"retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))

require.Error(t, p.PipelineConfig("mutators", "hydrator", json.RawMessage(`{"api":{"url":"not-a-url"}}`), &res))
assert.JSONEq(t, `{"api":{"url":"not-a-url"}}`, string(res))
assert.JSONEq(t, `{"api":{"url":"not-a-url","retry":{"give_up_after":"1s","max_delay":"100ms"}}}`, string(res))
})

t.Run("case=should pass and override values", func(t *testing.T) {
Expand Down
10 changes: 2 additions & 8 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ require (
github.com/auth0/go-jwt-middleware v0.0.0-20170425171159-5493cabe49f7
github.com/blang/semver v3.5.1+incompatible
github.com/bxcodec/faker v2.0.1+incompatible
github.com/cenkalti/backoff v2.1.1+incompatible
github.com/dgrijalva/jwt-go v3.2.0+incompatible
github.com/fsnotify/fsnotify v1.4.7
github.com/ghodss/yaml v1.0.0
Expand All @@ -25,7 +24,6 @@ require (
github.com/go-swagger/go-swagger v0.19.0
github.com/go-swagger/scan-repo-boundary v0.0.0-20180623220736-973b3573c013 // indirect
github.com/gobuffalo/packr/v2 v2.0.0-rc.15
github.com/golang/gddo v0.0.0-20190312205958-5a2505f3dbf0 // indirect
github.com/golang/mock v1.3.1
github.com/google/uuid v1.1.1
github.com/gorilla/handlers v1.4.0 // indirect
Expand All @@ -35,11 +33,8 @@ require (
github.com/jessevdk/go-flags v1.4.0 // indirect
github.com/julienschmidt/httprouter v1.2.0
github.com/lib/pq v1.0.0
github.com/luna-duclos/instrumentedsql v0.0.0-20190316074304-ecad98b20aec // indirect
github.com/mattn/goveralls v0.0.3
github.com/meatballhat/negroni-logrus v0.0.0-20170801195057-31067281800f
github.com/opencontainers/runc v1.0.0-rc5 // indirect
github.com/opentracing/opentracing-go v1.1.0 // indirect
github.com/ory/fosite v0.29.2
github.com/ory/go-acc v0.0.0-20181118080137-ddc355013f90
github.com/ory/go-convenience v0.1.0
Expand All @@ -48,7 +43,7 @@ require (
github.com/ory/herodot v0.6.2
github.com/ory/ladon v1.0.1
github.com/ory/viper v1.5.6
github.com/ory/x v0.0.80
github.com/ory/x v0.0.87
github.com/pborman/uuid v1.2.0
github.com/pelletier/go-toml v1.6.0 // indirect
github.com/phayes/freeport v0.0.0-20180830031419-95f893ade6f2
Expand All @@ -60,7 +55,7 @@ require (
github.com/spf13/viper v1.4.0 // indirect
github.com/sqs/goreturns v0.0.0-20181028201513-538ac6014518
github.com/square/go-jose v2.3.1+incompatible
github.com/stretchr/testify v1.3.0
github.com/stretchr/testify v1.4.0
github.com/subosito/gotenv v1.2.0 // indirect
github.com/tidwall/gjson v1.3.2
github.com/tidwall/sjson v1.0.4
Expand All @@ -70,7 +65,6 @@ require (
github.com/xeipuuv/gojsonpointer v0.0.0-20190905194746-02993c407bfb // indirect
golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4
golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
golang.org/x/sys v0.0.0-20191026070338-33540a1f6037 // indirect
golang.org/x/tools v0.0.0-20191026034945-b2104f82a97d
gopkg.in/square/go-jose.v2 v2.3.0
)
Expand Down
Loading

0 comments on commit 6b59512

Please sign in to comment.