Split Response and Assertion Issuer retrieval

The SamlResponse.getIssuers() contract is quite controversial. For a
valid response, it will always return just one element. For an invalid
response, depending on the cause it may:
- fail if no Assertion is present: this means, in particular, that if
the status code is not a success one, it's impossible to retrieve the
Response issuer with this method (although it may be a reasonable
requirement, for logging purposes for instance)
- fail if multiple Assertions are present: again, the Response Issuer
cannot be retrieved in this case either
- fail in the unlikely event that multiple Response Issuers were found
- return up to 2 issuers at most, if different issuers were set on the
Response and on the Assertion (which will make isValid() fail), with the
inability to determine which is the Response Issuer and which is the
Assertion Issuer (indeed: the former will be the first one in the
list, the latter will be the second, but this contract is a bit weak)

For these reasons, two different methods were provided to retrieve the
Response and the Assertion Issuers, with the former that will succeed
even when the status code is not a successful one. Also, because of the
above reasons, the getIssuers() method was deprecated in favour of the
two new ones.

Author: mauromol

core/src/main/java/com/onelogin/saml2/authn/SamlResponse.java

@@ -706,38 +706,89 @@ public List<String> getAudiences() throws XPathExpressionException {
 	}
 
 	/**
-	 * Gets the Issuers (from Response and Assertion).
+	 * Gets the Response Issuer.
 	 *
-	 * @return the issuers of the assertion/response
+	 * @return the Response Issuer, or <code>null</code> if not specified
 	 *
 	 * @throws XPathExpressionException
 	 * @throws ValidationError
+	 *               if multiple Response issuers were found
+	 * @see #getAssertionIssuer()
+	 * @see #getIssuers()
 	 */
-	public List<String> getIssuers() throws XPathExpressionException, ValidationError {
-		List<String> issuers = new ArrayList<String>();
-		String value;
+	public String getResponseIssuer() throws XPathExpressionException, ValidationError {
 		NodeList responseIssuer = Util.query(samlResponseDocument, "/samlp:Response/saml:Issuer");
 		if (responseIssuer.getLength() > 0) {
 			if (responseIssuer.getLength() == 1) {
-				value = responseIssuer.item(0).getTextContent();
-				if (!issuers.contains(value)) {
-					issuers.add(value);
-				}
+				return responseIssuer.item(0).getTextContent();
 			} else {
 				throw new ValidationError("Issuer of the Response is multiple.", ValidationError.ISSUER_MULTIPLE_IN_RESPONSE);
 			}
 		}
-
+		return null;
+	}
+	
+	/**
+	 * Gets the Assertion Issuer.
+	 *
+	 * @return the Assertion Issuer
+	 *
+	 * @throws XPathExpressionException
+	 * @throws ValidationError
+	 *               if no Assertion Issuer could be found, or if multiple Assertion
+	 *               issuers were found
+	 * @see #getResponseIssuer()
+	 * @see #getIssuers()
+	 */
+	public String getAssertionIssuer() throws XPathExpressionException, ValidationError {
 		NodeList assertionIssuer = this.queryAssertion("/saml:Issuer");
 		if (assertionIssuer.getLength() == 1) {
-			value = assertionIssuer.item(0).getTextContent();
-			if (!issuers.contains(value)) {
-				issuers.add(value);
-			}
+			return assertionIssuer.item(0).getTextContent();
 		} else {
 			throw new ValidationError("Issuer of the Assertion not found or multiple.", ValidationError.ISSUER_NOT_FOUND_IN_ASSERTION);
 		}
-
+	}
+	
+	/**
+	 * Gets the Issuers (from Response and Assertion). If the same issuer appears
+	 * both in the Response and in the Assertion (as it should), the returned list
+	 * will contain it just once. Hence, the returned list should always return one
+	 * element and in particular:
+	 * <ul>
+	 * <li>it will never contain zero elements (it means an Assertion Issuer could
+	 * not be found, hence a {@link ValidationError} will be thrown instead)
+	 * <li>if it contains more than one element, it means that the response is
+	 * invalid and one of the returned issuers won't pass the check performed by
+	 * {@link #isValid(String)} (which requires both issuers to be equal to the
+	 * Identity Provider entity id)
+	 * </ul>
+	 * <p>
+	 * Warning: as a consequence of the above, if this response status code is not a
+	 * successful one, this method will throw a {@link ValidationError} because it
+	 * won't find any Assertion Issuer. In this case, if you need to retrieve the
+	 * Response Issuer any way, you must use {@link #getResponseIssuer()} instead.
+	 *
+	 * @return the issuers of the assertion/response
+	 *
+	 * @throws XPathExpressionException
+	 * @throws ValidationError
+	 *               if multiple Response Issuers or multiple Assertion Issuers were
+	 *               found, or if no Assertion Issuer could be found
+	 * @see #getResponseIssuer()
+	 * @see #getAssertionIssuer()
+	 * @deprecated use {@link #getResponseIssuer()} and/or
+	 *             {@link #getAssertionIssuer()}; the contract of this method is
+	 *             quite controversial
+	 */
+	@Deprecated
+	public List<String> getIssuers() throws XPathExpressionException, ValidationError {
+		List<String> issuers = new ArrayList<String>();
+		String responseIssuer = getResponseIssuer();
+		if(responseIssuer != null)
+			issuers.add(responseIssuer);
+		String assertionIssuer = getAssertionIssuer();
+		if(!issuers.contains(assertionIssuer))
+			issuers.add(assertionIssuer);
 		return issuers;
 	}
 

core/src/test/java/com/onelogin/saml2/test/authn/AuthnResponseTest.java

@@ -822,7 +822,7 @@ public void testGetAudiences() throws IOException, Error, XPathExpressionExcepti
 	}
 
 	/**
-	 * Tests the getIssuers method of SamlResponse
+	 * Tests the getIssuers methods of SamlResponse
 	 *
 	 * @throws Error
 	 * @throws IOException
@@ -837,46 +837,61 @@ public void testGetAudiences() throws IOException, Error, XPathExpressionExcepti
 	@Test
 	public void testGetIssuers() throws IOException, Error, XPathExpressionException, ParserConfigurationException, SAXException, SettingsException, ValidationError {
 		Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build();
-		String samlResponseEncoded = Util.getFileAsString("data/responses/response1.xml.base64");
+		String samlResponseEncoded = Util.getFileAsString("data/responses/valid_encrypted_assertion.xml.base64");
 		SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		String expectedIssuer = "http://idp.example.com/";
 		List<String> expectedIssuers = new ArrayList<String>();
-		expectedIssuers.add("http://idp.example.com/");
-		samlResponseEncoded = Util.getFileAsString("data/responses/valid_encrypted_assertion.xml.base64");
-		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		expectedIssuers.add(expectedIssuer);
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
 		expectedIssuers.remove(0);
-		expectedIssuers.add("https://pitbulk.no-ip.org/simplesaml/saml2/idp/metadata.php");
+		expectedIssuer = "https://pitbulk.no-ip.org/simplesaml/saml2/idp/metadata.php";
+		expectedIssuers.add(expectedIssuer);
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/signed_message_encrypted_assertion.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/double_signed_encrypted_assertion.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/signed_encrypted_assertion.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/double_signed_response.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/signed_assertion_response.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertEquals(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 
+		expectedIssuer = "https://app.onelogin.com/saml/metadata/13590";
 		expectedIssuers = new ArrayList<String>();
-		expectedIssuers.add("https://app.onelogin.com/saml/metadata/13590");
+		expectedIssuers.add(expectedIssuer);
 		samlResponseEncoded = Util.getFileAsString("data/responses/invalids/no_issuer_response.xml.base64");
 		samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		assertNull(expectedIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 	}
 
 	/**
-	 * Tests the getIssuers method of SamlResponse
+	 * Tests the getIssuers methods of SamlResponse
 	 * <p>
 	 * Case: different issuers for response and assertion
 	 *
@@ -896,13 +911,44 @@ public void testGetIssuersDifferentIssuers() throws IOException, Error, XPathExp
 		String samlResponseEncoded = Util.getFileAsString("data/responses/invalids/different_issuers.xml.base64");
 		SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
 		List<String> expectedIssuers = new ArrayList<String>();
-		expectedIssuers.add("https://response-issuer.com");
-		expectedIssuers.add("https://assertion-issuer.com");
+		String expectedResponseIssuer = "https://response-issuer.com";
+		String expectedAssertionIssuer = "https://assertion-issuer.com";
+		expectedIssuers.add(expectedResponseIssuer);
+		expectedIssuers.add(expectedAssertionIssuer);
+		assertEquals(expectedResponseIssuer, samlResponse.getResponseIssuer());
+		assertEquals(expectedAssertionIssuer, samlResponse.getAssertionIssuer());
 		assertEquals(expectedIssuers, samlResponse.getIssuers());
 	}
 
 	/**
-	 * Tests the getIssuers method of SamlResponse
+	 * Tests the getAssertionIssuer method of SamlResponse
+	 * <p>
+	 * Case: Issuer of the assertion not found
+	 *
+	 * @throws Error
+	 * @throws IOException
+	 * @throws ValidationError
+	 * @throws SettingsException
+	 * @throws SAXException
+	 * @throws ParserConfigurationException
+	 * @throws XPathExpressionException
+	 *
+	 * @see com.onelogin.saml2.authn.SamlResponse#getIssuers
+	 */
+	@Test
+	public void testGetAssertionIssuerNoInAssertion() throws IOException, Error, XPathExpressionException, ParserConfigurationException, SAXException, SettingsException, ValidationError {
+		Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build();
+		String samlResponseEncoded = Util.getFileAsString("data/responses/invalids/no_issuer_assertion.xml.base64");
+		SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
+		
+		expectedEx.expect(ValidationError.class);
+		expectedEx.expectMessage("Issuer of the Assertion not found or multiple.");
+		samlResponse.getAssertionIssuer();
+	}
+
+	/**
+	 * Tests the getIssuers methods of SamlResponse
+	 * <p>
 	 * Case: Issuer of the assertion not found
 	 *
 	 * @throws Error
@@ -921,11 +967,12 @@ public void testGetIssuersNoInAssertion() throws IOException, Error, XPathExpres
 		String samlResponseEncoded = Util.getFileAsString("data/responses/invalids/no_issuer_assertion.xml.base64");
 		SamlResponse samlResponse = new SamlResponse(settings, newHttpRequest(samlResponseEncoded));
 
+		samlResponse.getResponseIssuer(); // this should not fail
 		expectedEx.expect(ValidationError.class);
 		expectedEx.expectMessage("Issuer of the Assertion not found or multiple.");
 		samlResponse.getIssuers();
 	}
-	
+
 	/**
 	 * Tests the getSessionIndex method of SamlResponse
 	 *