From 3527ee6940a7c66d7ce2baad9d00818095482379 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 18 Sep 2019 15:24:50 -0700 Subject: [PATCH 1/6] Add support for listenAddress parameter if OIDC provisioners. Fixes smallstep/cli#150 --- authority/provisioner/oidc.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index 12d1d1e0..e2c5e8cd 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -4,6 +4,7 @@ import ( "context" "crypto/x509" "encoding/json" + "net" "net/http" "strings" "time" @@ -55,6 +56,7 @@ type OIDC struct { Admins []string `json:"admins,omitempty"` Domains []string `json:"domains,omitempty"` Groups []string `json:"groups,omitempty"` + ListenAddress string `json:"listenAddress,omitempty"` Claims *Claims `json:"claims,omitempty"` configuration openIDConfiguration keyStore *keyStore @@ -133,6 +135,13 @@ func (o *OIDC) Init(config Config) (err error) { return errors.New("configurationEndpoint cannot be empty") } + // Validate listenAddress if given + if o.ListenAddress != "" { + if _, _, err := net.SplitHostPort(o.ListenAddress); err != nil { + return errors.Wrap(err, "error parsing listenAddress") + } + } + // Update claims with global ones if o.claimer, err = NewClaimer(o.Claims, config.Claims); err != nil { return err From 6c4abfabbb671e10c45bae5b249471e1177ff41e Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 18 Sep 2019 15:54:10 -0700 Subject: [PATCH 2/6] Make /.well-known/openid-configuration optional --- authority/provisioner/oidc.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/authority/provisioner/oidc.go b/authority/provisioner/oidc.go index e2c5e8cd..d4937470 100644 --- a/authority/provisioner/oidc.go +++ b/authority/provisioner/oidc.go @@ -6,6 +6,8 @@ import ( "encoding/json" "net" "net/http" + "net/url" + "path" "strings" "time" @@ -148,7 +150,14 @@ func (o *OIDC) Init(config Config) (err error) { } // Decode and validate openid-configuration endpoint - if err := getAndDecode(o.ConfigurationEndpoint, &o.configuration); err != nil { + u, err := url.Parse(o.ConfigurationEndpoint) + if err != nil { + return errors.Wrapf(err, "error parsing %s", o.ConfigurationEndpoint) + } + if !strings.Contains(u.Path, "/.well-known/openid-configuration") { + u.Path = path.Join(u.Path, "/.well-known/openid-configuration") + } + if err := getAndDecode(u.String(), &o.configuration); err != nil { return err } if err := o.configuration.Validate(); err != nil { From a16b2125bc44bd77ac23c1727f78d6521786df31 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 18 Sep 2019 16:04:43 -0700 Subject: [PATCH 3/6] Fix tests. --- authority/provisioner/oidc_test.go | 18 +++++++++--------- authority/provisioner/utils_test.go | 2 +- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index 7d149076..f18b96ff 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -91,16 +91,16 @@ func TestOIDC_Init(t *testing.T) { args args wantErr bool }{ - {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, false}, - {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, []string{"foo@smallstep.com"}, nil}, args{config}, false}, - {"ok-domains", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, []string{"smallstep.com"}}, args{config}, false}, - {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, false}, - {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, - {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, - {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL + "/openid-configuration", nil, nil, nil}, args{config}, true}, + {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, false}, + {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", nil, []string{"foo@smallstep.com"}, nil}, args{config}, false}, + {"ok-domains", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, []string{"smallstep.com"}}, args{config}, false}, + {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL, nil, nil, nil}, args{config}, false}, + {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, + {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, + {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil, nil}, args{config}, true}, - {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, - {"bad-claims", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/openid-configuration", badClaims, nil, nil}, args{config}, true}, + {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/random", nil, nil, nil}, args{config}, true}, + {"bad-claims", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", badClaims, nil, nil}, args{config}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/authority/provisioner/utils_test.go b/authority/provisioner/utils_test.go index 91e67f02..2760a16b 100644 --- a/authority/provisioner/utils_test.go +++ b/authority/provisioner/utils_test.go @@ -709,7 +709,7 @@ func generateJWKServer(n int) *httptest.Server { http.Error(w, http.StatusText(http.StatusBadRequest), http.StatusBadRequest) case "/hits": writeJSON(w, hits) - case "/openid-configuration", "/.well-known/openid-configuration": + case "/.well-known/openid-configuration": writeJSON(w, openIDConfiguration{Issuer: "the-issuer", JWKSetURI: srv.URL + "/jwks_uri"}) case "/random": keySet := must(generateJSONWebKeySet(n))[0].(jose.JSONWebKeySet) From b7045f27a9cb9f0c48c38e0a54b2ced32570056e Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 18 Sep 2019 17:13:58 -0700 Subject: [PATCH 4/6] Increase coverage. --- authority/provisioner/oidc_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index f18b96ff..daa878c4 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -101,6 +101,8 @@ func TestOIDC_Init(t *testing.T) { {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil, nil}, args{config}, true}, {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/random", nil, nil, nil}, args{config}, true}, {"bad-claims", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", badClaims, nil, nil}, args{config}, true}, + {"bad-parse-url", fields{"oidc", "name", "client-id", "client-secret", ":", nil, nil, nil}, args{config}, true}, + {"bad-get-url", fields{"oidc", "name", "client-id", "client-secret", "https://", nil, nil, nil}, args{config}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -114,6 +116,7 @@ func TestOIDC_Init(t *testing.T) { } if err := p.Init(tt.args.config); (err != nil) != tt.wantErr { t.Errorf("OIDC.Init() error = %v, wantErr %v", err, tt.wantErr) + return } if tt.wantErr == false { assert.Len(t, 2, p.keyStore.keySet.Keys) From 72f1a61f06ee6eccc47582a398b8a3570d98d412 Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Wed, 18 Sep 2019 18:08:26 -0700 Subject: [PATCH 5/6] Increase coverage. --- authority/provisioner/oidc_test.go | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/authority/provisioner/oidc_test.go b/authority/provisioner/oidc_test.go index daa878c4..6e99a0fb 100644 --- a/authority/provisioner/oidc_test.go +++ b/authority/provisioner/oidc_test.go @@ -81,6 +81,7 @@ func TestOIDC_Init(t *testing.T) { Claims *Claims Admins []string Domains []string + ListenAddress string } type args struct { config Config @@ -91,18 +92,21 @@ func TestOIDC_Init(t *testing.T) { args args wantErr bool }{ - {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, false}, - {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", nil, []string{"foo@smallstep.com"}, nil}, args{config}, false}, - {"ok-domains", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, []string{"smallstep.com"}}, args{config}, false}, - {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL, nil, nil, nil}, args{config}, false}, - {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, - {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, - {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL, nil, nil, nil}, args{config}, true}, - {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil, nil}, args{config}, true}, - {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/random", nil, nil, nil}, args{config}, true}, - {"bad-claims", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", badClaims, nil, nil}, args{config}, true}, - {"bad-parse-url", fields{"oidc", "name", "client-id", "client-secret", ":", nil, nil, nil}, args{config}, true}, - {"bad-get-url", fields{"oidc", "name", "client-id", "client-secret", "https://", nil, nil, nil}, args{config}, true}, + {"ok", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil, ""}, args{config}, false}, + {"ok-admins", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", nil, []string{"foo@smallstep.com"}, nil, ""}, args{config}, false}, + {"ok-domains", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, []string{"smallstep.com"}, ""}, args{config}, false}, + {"ok-listen-port", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil, ":10000"}, args{config}, false}, + {"ok-listen-host-port", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil, "127.0.0.1:10000"}, args{config}, false}, + {"ok-no-secret", fields{"oidc", "name", "client-id", "", srv.URL, nil, nil, nil, ""}, args{config}, false}, + {"no-name", fields{"oidc", "", "client-id", "client-secret", srv.URL, nil, nil, nil, ""}, args{config}, true}, + {"no-type", fields{"", "name", "client-id", "client-secret", srv.URL, nil, nil, nil, ""}, args{config}, true}, + {"no-client-id", fields{"oidc", "name", "", "client-secret", srv.URL, nil, nil, nil, ""}, args{config}, true}, + {"no-configuration", fields{"oidc", "name", "client-id", "client-secret", "", nil, nil, nil, ""}, args{config}, true}, + {"bad-configuration", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/random", nil, nil, nil, ""}, args{config}, true}, + {"bad-claims", fields{"oidc", "name", "client-id", "client-secret", srv.URL + "/.well-known/openid-configuration", badClaims, nil, nil, ""}, args{config}, true}, + {"bad-parse-url", fields{"oidc", "name", "client-id", "client-secret", ":", nil, nil, nil, ""}, args{config}, true}, + {"bad-get-url", fields{"oidc", "name", "client-id", "client-secret", "https://", nil, nil, nil, ""}, args{config}, true}, + {"bad-listen-address", fields{"oidc", "name", "client-id", "client-secret", srv.URL, nil, nil, nil, "127.0.0.1"}, args{config}, true}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -113,6 +117,8 @@ func TestOIDC_Init(t *testing.T) { ConfigurationEndpoint: tt.fields.ConfigurationEndpoint, Claims: tt.fields.Claims, Admins: tt.fields.Admins, + Domains: tt.fields.Domains, + ListenAddress: tt.fields.ListenAddress, } if err := p.Init(tt.args.config); (err != nil) != tt.wantErr { t.Errorf("OIDC.Init() error = %v, wantErr %v", err, tt.wantErr) From fa7273d4f54f6b856d655d250f07ca48d87c5b5a Mon Sep 17 00:00:00 2001 From: Mariano Cano Date: Thu, 19 Sep 2019 10:20:41 -0700 Subject: [PATCH 6/6] Add docs on listenAddress. --- docs/provisioners.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/provisioners.md b/docs/provisioners.md index 2b2f8c5e..100ddb58 100644 --- a/docs/provisioners.md +++ b/docs/provisioners.md @@ -111,6 +111,7 @@ is G-Suite. "configurationEndpoint": "https://accounts.google.com/.well-known/openid-configuration", "admins": ["you@smallstep.com"], "domains": ["smallstep.com"], + "listenAddress": ":10000", "claims": { "maxTLSCertDuration": "8h", "defaultTLSCertDuration": "2h", @@ -141,6 +142,12 @@ is G-Suite. * `domains` (optional): is the list of domains valid. If provided only the emails with the provided domains will be able to authenticate. +* `listenAddress` (optional): is the loopback address (`:port` or `host:port`) + where the authorization server will redirect to complete the authorization + flow. If it's not defined `step` will use `127.0.0.1` with a random port. This + configuration is only required if the authorization server doesn't allow any + port to be specified at the time of the request for loopback IP redirect URIs. + * `claims` (optional): overwrites the default claims set in the authority, see the [JWK](#jwk) section for all the options.