From 217b5ca2c7b429ed8667e55bcd4990fb702977a3 Mon Sep 17 00:00:00 2001 From: Phu Kieu Date: Tue, 28 Mar 2017 18:06:49 -0700 Subject: [PATCH 1/2] Add ssoIssuer to fix Response issuer checking Rename issuer to entityIssuer --- connector/saml/saml.go | 21 ++++++++++++--------- connector/saml/saml_test.go | 14 +++++++------- 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/connector/saml/saml.go b/connector/saml/saml.go index a24df1c8..7f93ba9e 100644 --- a/connector/saml/saml.go +++ b/connector/saml/saml.go @@ -81,8 +81,9 @@ type Config struct { // // https://www.oasis-open.org/committees/download.php/35391/sstc-saml-metadata-errata-2.0-wd-04-diff.pdf - Issuer string `json:"issuer"` - SSOURL string `json:"ssoURL"` + EntityIssuer string `json:"entityIssuer"` + SSOIssuer string `json:"ssoIssuer"` + SSOURL string `json:"ssoURL"` // X509 CA file or raw data to verify XML signatures. CA string `json:"ca"` @@ -154,7 +155,8 @@ func (c *Config) openConnector(logger logrus.FieldLogger) (*provider, error) { } p := &provider{ - issuer: c.Issuer, + entityIssuer: c.EntityIssuer, + ssoIssuer: c.SSOIssuer, ssoURL: c.SSOURL, now: time.Now, usernameAttr: c.UsernameAttr, @@ -217,8 +219,9 @@ func (c *Config) openConnector(logger logrus.FieldLogger) (*provider, error) { } type provider struct { - issuer string - ssoURL string + entityIssuer string + ssoIssuer string + ssoURL string now func() time.Time @@ -251,10 +254,10 @@ func (p *provider) POSTData(s connector.Scopes, id string) (action, value string }, AssertionConsumerServiceURL: p.redirectURI, } - if p.issuer != "" { + if p.entityIssuer != "" { // Issuer for the request is optional. For example, okta always ignores // this value. - r.Issuer = &issuer{Issuer: p.issuer} + r.Issuer = &issuer{Issuer: p.entityIssuer} } data, err := xml.MarshalIndent(r, "", " ") @@ -287,8 +290,8 @@ func (p *provider) HandlePOST(s connector.Scopes, samlResponse, inResponseTo str } if rootElementSigned { - if p.issuer != "" && resp.Issuer != nil && resp.Issuer.Issuer != p.issuer { - return ident, fmt.Errorf("expected Issuer value %s, got %s", p.issuer, resp.Issuer.Issuer) + if p.ssoIssuer != "" && resp.Issuer != nil && resp.Issuer.Issuer != p.ssoIssuer { + return ident, fmt.Errorf("expected Issuer value %s, got %s", p.entityIssuer, resp.Issuer.Issuer) } // Verify InResponseTo value matches the expected ID associated with diff --git a/connector/saml/saml_test.go b/connector/saml/saml_test.go index bb7fa1b2..a84b7071 100644 --- a/connector/saml/saml_test.go +++ b/connector/saml/saml_test.go @@ -278,14 +278,14 @@ func (r responseTest) run(t *testing.T) { } const ( - defaultIssuer = "http://www.okta.com/exk91cb99lKkKSYoy0h7" + defaultSSOIssuer = "http://www.okta.com/exk91cb99lKkKSYoy0h7" defaultRedirectURI = "http://localhost:5556/dex/callback" // Response ID embedded in our testdata. testDataResponseID = "_fd1b3ef9-ec09-44a7-a66b-0d39c250f6a0" ) -// Depricated: Use testing framework established above. +// Deprecated: Use testing framework established above. func runVerify(t *testing.T, ca string, resp string, shouldSucceed bool) { cert, err := loadCert(ca) if err != nil { @@ -311,10 +311,10 @@ func runVerify(t *testing.T, ca string, resp string, shouldSucceed bool) { } } -// Depricated: Use testing framework established above. -func newProvider(issuer string, redirectURI string) *provider { - if issuer == "" { - issuer = defaultIssuer +// Deprecated: Use testing framework established above. +func newProvider(ssoIssuer string, redirectURI string) *provider { + if ssoIssuer == "" { + ssoIssuer = defaultSSOIssuer } if redirectURI == "" { redirectURI = defaultRedirectURI @@ -322,7 +322,7 @@ func newProvider(issuer string, redirectURI string) *provider { now, _ := time.Parse(time.RFC3339, "2017-01-24T20:48:41Z") timeFunc := func() time.Time { return now } return &provider{ - issuer: issuer, + ssoIssuer: ssoIssuer, ssoURL: "http://idp.org/saml/sso", now: timeFunc, usernameAttr: "user", From 8c0eb67ecd7f4de164449975eeacce411c97b336 Mon Sep 17 00:00:00 2001 From: Phu Kieu Date: Thu, 6 Apr 2017 10:54:32 -0700 Subject: [PATCH 2/2] Update documentation --- Documentation/saml-connector.md | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Documentation/saml-connector.md b/Documentation/saml-connector.md index f187e885..dcaea405 100644 --- a/Documentation/saml-connector.md +++ b/Documentation/saml-connector.md @@ -30,17 +30,23 @@ connectors: # CA to use when validating the SAML response. ca: /path/to/ca.pem - # CA's can also be provided inline as a base64'd blob. + # CA's can also be provided inline as a base64'd blob. # # caData: ( RAW base64'd PEM encoded CA ) # To skip signature validation, uncomment the following field. This should # only be used during testing and may be removed in the future. - # - # insucreSkipSignatureValidation: true + # + # insecureSkipSignatureValidation: true + + # Optional: Issuer value for AuthnRequest + entityIssuer: https://dex.example.com/callback + + # Optional: Issuer value for SAML Response + ssoIssuer: https://saml.example.com/sso # Dex's callback URL. Must match the "Destination" attribute of all responses - # exactly. + # exactly. redirectURI: https://dex.example.com/callback # Name of attributes in the returned assertions to map to ID token claims.