Stop validating InResponseTo when AllowIDPInitiated is set

With this change we no longer validate InResponseTo when AllowIDPInitiated is set. Here's why:

The SAML specification does not provide clear guidance for handling InResponseTo for IDP-initiated requests where there is no request to be in response to. The specification says:

   InResponseTo [Optional]
       The ID of a SAML protocol message in response to which an attesting entity can present the
       assertion. For example, this attribute might be used to correlate the assertion to a SAML
       request that resulted in its presentation.

The initial thought was that we should specify a single empty string in possibleRequestIDs for IDP-initiated  requests so that we would ensure that an InResponseTo was *not* provided in those cases where it wasn't expected. Even that turns out to be frustrating for users. And in practice some IDPs (e.g. Rippling)  set a specific non-empty value for InResponseTo in IDP-initiated requests.

Finally, it is unclear that there is significant security value in checking InResponseTo when we allow  IDP initiated assertions.

This issue has been reported quite a few times, including:

Fixes #291 #286 #300 #301 #286

samlidp/session_test.go

@@ -36,7 +36,7 @@ func TestSessionsCrud(t *testing.T) {
 		w.Header().Get("Set-Cookie"))
 	assert.Equal(t,
 		"{\"ID\":\"AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=\",\"CreateTime\":\"2015-12-01T01:57:09Z\",\"ExpireTime\":\"2015-12-01T02:57:09Z\",\"Index\":\"40424446484a4c4e50525456585a5c5e60626466686a6c6e70727476787a7c7e\",\"NameID\":\"\",\"Groups\":null,\"UserName\":\"alice\",\"UserEmail\":\"\",\"UserCommonName\":\"\",\"UserSurname\":\"\",\"UserGivenName\":\"\",\"UserScopedAffiliation\":\"\",\"CustomAttributes\":null}\n",
-	string(w.Body.Bytes()))
+		string(w.Body.Bytes()))
 
 	w = httptest.NewRecorder()
 	r, _ = http.NewRequest("GET", "https://idp.example.com/login", nil)
@@ -53,7 +53,7 @@ func TestSessionsCrud(t *testing.T) {
 	assert.Equal(t, http.StatusOK, w.Code)
 	assert.Equal(t,
 		"{\"ID\":\"AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=\",\"CreateTime\":\"2015-12-01T01:57:09Z\",\"ExpireTime\":\"2015-12-01T02:57:09Z\",\"Index\":\"40424446484a4c4e50525456585a5c5e60626466686a6c6e70727476787a7c7e\",\"NameID\":\"\",\"Groups\":null,\"UserName\":\"alice\",\"UserEmail\":\"\",\"UserCommonName\":\"\",\"UserSurname\":\"\",\"UserGivenName\":\"\",\"UserScopedAffiliation\":\"\",\"CustomAttributes\":null}\n",
-	string(w.Body.Bytes()))
+		string(w.Body.Bytes()))
 
 	w = httptest.NewRecorder()
 	r, _ = http.NewRequest("DELETE", "https://idp.example.com/sessions/AAIEBggKDA4QEhQWGBocHiAiJCYoKiwuMDI0Njg6PD4=", nil)

service_provider.go

@@ -717,14 +717,34 @@ func (sp *ServiceProvider) validateAssertion(assertion *Assertion, possibleReque
 	}
 	for _, subjectConfirmation := range assertion.Subject.SubjectConfirmations {
 		requestIDvalid := false
-		for _, possibleRequestID := range possibleRequestIDs {
-			if subjectConfirmation.SubjectConfirmationData.InResponseTo == possibleRequestID {
-				requestIDvalid = true
-				break
+
+		// We *DO NOT* validate InResponseTo when AllowIDPInitiated is set. Here's why:
+		//
+		// The SAML specification does not provide clear guidance for handling InResponseTo for IDP-initiated
+		// requests where there is no request to be in response to. The specification says:
+		//
+		//   InResponseTo [Optional]
+		//       The ID of a SAML protocol message in response to which an attesting entity can present the
+		//       assertion. For example, this attribute might be used to correlate the assertion to a SAML
+		//       request that resulted in its presentation.
+		//
+		// The initial thought was that we should specify a single empty string in possibleRequestIDs for IDP-initiated
+		// requests so that we would ensure that an InResponseTo was *not* provided in those cases where it wasn't
+		// expected. Even that turns out to be frustrating for users. And in practice some IDPs (e.g. Rippling)
+		// set a specific non-empty value for InResponseTo in IDP-initiated requests.
+		//
+		// Finally, it is unclear that there is significant security value in checking InResponseTo when we allow
+		// IDP initiated assertions.
+		if !sp.AllowIDPInitiated {
+			for _, possibleRequestID := range possibleRequestIDs {
+				if subjectConfirmation.SubjectConfirmationData.InResponseTo == possibleRequestID {
+					requestIDvalid = true
+					break
+				}
+			}
+			if !requestIDvalid {
+				return fmt.Errorf("assertion SubjectConfirmation one of the possible request IDs (%v)", possibleRequestIDs)
 			}
-		}
-		if !requestIDvalid {
-			return fmt.Errorf("assertion SubjectConfirmation one of the possible request IDs (%v)", possibleRequestIDs)
 		}
 		if subjectConfirmation.SubjectConfirmationData.Recipient != sp.AcsURL.String() {
 			return fmt.Errorf("assertion SubjectConfirmation Recipient is not %s", sp.AcsURL.String())