Service provider fixes to work as expected.

RichardKnop · RichardKnop · commit f36778566122 · 2017-07-07T16:13:21.000+01:00
- Fixed script hash to remove JS console errors when redirecting
- Fixed samlsp.New method to not overwrite m.ServiceProvider.IDPMetadata
diff --git a/samlsp/middleware.go b/samlsp/middleware.go
@@ -160,11 +160,10 @@ func (m *Middleware) RequireAccount(handler http.Handler) http.Handler {
 			return
 		}
 		if binding == saml.HTTPPostBinding {
-			w.Header().Set("Content-Security-Policy", ""+
+			w.Header().Add("Content-Security-Policy", ""+
 				"default-src; "+
-				"script-src 'sha256-D8xB+y+rJ90RmLdP72xBqEEc0NUatn7yuCND0orkrgk='; "+
-				"reflected-xss block; "+
-				"referrer no-referrer;")
+				"script-src 'sha256-AjPdJSbZmeWHnEc5ykvJFay8FTWeTeRbs9dutfZ0HqE='; "+
+				"reflected-xss block; referrer no-referrer;")
 			w.Header().Add("Content-type", "text/html")
 			w.Write([]byte(`<!DOCTYPE html><html><body>`))
 			w.Write(req.Post(relayState))
@@ -243,7 +242,7 @@ func (m *Middleware) Authorize(w http.ResponseWriter, r *http.Request, assertion
 
 		// delete the cookie
 		stateCookie.Value = ""
-		stateCookie.Expires = time.Unix(1,0) // past time as close to epoch as possible, but not zero time.Time{}
+		stateCookie.Expires = time.Unix(1, 0) // past time as close to epoch as possible, but not zero time.Time{}
 		http.SetCookie(w, stateCookie)
 	}
 
diff --git a/samlsp/middleware_test.go b/samlsp/middleware_test.go
@@ -176,13 +176,13 @@ func (test *MiddlewareTest) TestRequireAccountNoCredsPostBinding(c *C) {
 		"<input type=\"hidden\" name=\"RelayState\" value=\"KCosLjAyNDY4Ojw-QEJERkhKTE5QUlRWWFpcXmBiZGZoamxucHJ0dnh6\" />"+
 		"<input id=\"SAMLSubmitButton\" type=\"submit\" value=\"Submit\" />"+
 		"</form>"+
-		"<script>document.getElementById('SAMLSubmitButton').style.visibility=\"hidden\";</script>"+
-		"<script>document.getElementById('SAMLRequestForm').submit();</script>"+
+		"<script>document.getElementById('SAMLSubmitButton').style.visibility=\"hidden\";"+
+		"document.getElementById('SAMLRequestForm').submit();</script>"+
 		"</body>"+
 		"</html>")
 
 	// check that the CSP script hash is set correctly
-	scriptContent := "document.getElementById('SAMLRequestForm').submit();"
+	scriptContent := "document.getElementById('SAMLSubmitButton').style.visibility=\"hidden\";document.getElementById('SAMLRequestForm').submit();"
 	scriptSum := sha256.Sum256([]byte(scriptContent))
 	scriptHash := base64.StdEncoding.EncodeToString(scriptSum[:])
 	c.Assert(resp.Header().Get("Content-Security-Policy"), Equals,
diff --git a/samlsp/samlsp.go b/samlsp/samlsp.go
@@ -90,29 +90,32 @@ func New(opts Options) (*Middleware, error) {
 			continue
 		}
 
-		entity := &saml.EntityDescriptor{}
+		entity := new(saml.EntityDescriptor)
 		err = xml.Unmarshal(data, entity)
 
 		// this comparison is ugly, but it is how the error is generated in encoding/xml
 		if err != nil && err.Error() == "expected element type <EntityDescriptor> but have <EntitiesDescriptor>" {
-			entities := &saml.EntitiesDescriptor{}
-			if err := xml.Unmarshal(data, entities); err != nil {
+			entities := new(saml.EntitiesDescriptor)
+			if err = xml.Unmarshal(data, entities); err != nil {
 				return nil, err
 			}
 
 			err = fmt.Errorf("no entity found with IDPSSODescriptor")
-			for _, e := range entities.EntityDescriptors {
+			for j := range entities.EntityDescriptors {
+				e := entities.EntityDescriptors[j]
 				if len(e.IDPSSODescriptors) > 0 {
 					entity = &e
 					err = nil
 				}
 			}
 		}
+
+		m.ServiceProvider.IDPMetadata = entity
+
 		if err != nil {
 			return nil, err
 		}
 
-		m.ServiceProvider.IDPMetadata = entity
 		return m, nil
 	}
 
diff --git a/service_provider.go b/service_provider.go
@@ -304,8 +304,8 @@ func (req *AuthnRequest) Post(relayState string) []byte {
 		`<input type="hidden" name="RelayState" value="{{.RelayState}}" />` +
 		`<input id="SAMLSubmitButton" type="submit" value="Submit" />` +
 		`</form>` +
-		`<script>document.getElementById('SAMLSubmitButton').style.visibility="hidden";</script>` +
-		`<script>document.getElementById('SAMLRequestForm').submit();</script>`))
+		`<script>document.getElementById('SAMLSubmitButton').style.visibility="hidden";` +
+		`document.getElementById('SAMLRequestForm').submit();</script>`))
 	data := struct {
 		URL         string
 		SAMLRequest string
diff --git a/service_provider_test.go b/service_provider_test.go
@@ -188,8 +188,8 @@ func (test *ServiceProviderTest) TestCanProducePostRequest(c *C) {
 		`<input type="hidden" name="SAMLRequest" value="PHNhbWxwOkF1dGhuUmVxdWVzdCB4bWxuczpzYW1sPSJ1cm46b2FzaXM6bmFtZXM6dGM6U0FNTDoyLjA6YXNzZXJ0aW9uIiB4bWxuczpzYW1scD0idXJuOm9hc2lzOm5hbWVzOnRjOlNBTUw6Mi4wOnByb3RvY29sIiBJRD0iaWQtMDAwMjA0MDYwODBhMGMwZTEwMTIxNDE2MTgxYTFjMWUyMDIyMjQyNiIgVmVyc2lvbj0iMi4wIiBJc3N1ZUluc3RhbnQ9IjIwMTUtMTItMDFUMDE6MzE6MjFaIiBEZXN0aW5hdGlvbj0iaHR0cHM6Ly9pZHAudGVzdHNoaWIub3JnL2lkcC9wcm9maWxlL1NBTUwyL1BPU1QvU1NPIiBBc3NlcnRpb25Db25zdW1lclNlcnZpY2VVUkw9Imh0dHBzOi8vMTU2NjE0NDQubmdyb2suaW8vc2FtbDIvYWNzIiBQcm90b2NvbEJpbmRpbmc9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpiaW5kaW5nczpIVFRQLVBPU1QiPjxzYW1sOklzc3VlciBGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpuYW1laWQtZm9ybWF0OmVudGl0eSI&#43;aHR0cHM6Ly8xNTY2MTQ0NC5uZ3Jvay5pby9zYW1sMi9tZXRhZGF0YTwvc2FtbDpJc3N1ZXI&#43;PHNhbWxwOk5hbWVJRFBvbGljeSBGb3JtYXQ9InVybjpvYXNpczpuYW1lczp0YzpTQU1MOjIuMDpuYW1laWQtZm9ybWF0OnRyYW5zaWVudCIgQWxsb3dDcmVhdGU9InRydWUiLz48L3NhbWxwOkF1dGhuUmVxdWVzdD4=" />`+
 		`<input type="hidden" name="RelayState" value="relayState" />`+
 		`<input id="SAMLSubmitButton" type="submit" value="Submit" /></form>`+
-		`<script>document.getElementById('SAMLSubmitButton').style.visibility="hidden";</script>`+
-		`<script>document.getElementById('SAMLRequestForm').submit();</script>`)
+		`<script>document.getElementById('SAMLSubmitButton').style.visibility="hidden";`+
+		`document.getElementById('SAMLRequestForm').submit();</script>`)
 }
 
 func (test *ServiceProviderTest) TestCanHandleOneloginResponse(c *C) {

Original file line number	Diff line number	Diff line change
`@@ -90,29 +90,32 @@ func New(opts Options) (*Middleware, error) {`
`90`	`90`	`continue`
`91`	`91`	`}`
`92`	`92`
`93`		`- entity := &saml.EntityDescriptor{}`
	`93`	`+ entity := new(saml.EntityDescriptor)`
`94`	`94`	`err = xml.Unmarshal(data, entity)`
`95`	`95`
`96`	`96`	`// this comparison is ugly, but it is how the error is generated in encoding/xml`
`97`	`97`	`if err != nil && err.Error() == "expected element type <EntityDescriptor> but have <EntitiesDescriptor>" {`
`98`		`- entities := &saml.EntitiesDescriptor{}`
`99`		`- if err := xml.Unmarshal(data, entities); err != nil {`
	`98`	`+ entities := new(saml.EntitiesDescriptor)`
	`99`	`+ if err = xml.Unmarshal(data, entities); err != nil {`
`100`	`100`	`return nil, err`
`101`	`101`	`}`
`102`	`102`
`103`	`103`	`err = fmt.Errorf("no entity found with IDPSSODescriptor")`
`104`		`- for _, e := range entities.EntityDescriptors {`
	`104`	`+ for j := range entities.EntityDescriptors {`
	`105`	`+ e := entities.EntityDescriptors[j]`
`105`	`106`	`if len(e.IDPSSODescriptors) > 0 {`
`106`	`107`	`entity = &e`
`107`	`108`	`err = nil`
`108`	`109`	`}`
`109`	`110`	`}`
`110`	`111`	`}`
	`112`	`+`
	`113`	`+ m.ServiceProvider.IDPMetadata = entity`
	`114`	`+`
`111`	`115`	`if err != nil {`
`112`	`116`	`return nil, err`
`113`	`117`	`}`
`114`	`118`
`115`		`- m.ServiceProvider.IDPMetadata = entity`
`116`	`119`	`return m, nil`
`117`	`120`	`}`
`118`	`121`