Merge pull request #82 from ThePetrov/allow-authn-request-without-relaystate

Allow authn request without relayState if relayState is empty

Author: pitbulk

toolkit/src/main/java/com/onelogin/saml2/Auth.java

@@ -213,7 +213,8 @@ public void setStrict(Boolean value)
 	 * Initiates the SSO process.
 	 *
 	 * @param returnTo
-	 *				The target URL the user should be returned to after login.
+	 *				The target URL the user should be returned to after login (relayState).
+	 *				Will be a self-routed URL when null, or not be appended at all when an empty string is provided
 	 * @param forceAuthn
 	 *				When true the AuthNRequest will set the ForceAuthn='true'
 	 * @param isPassive
@@ -237,7 +238,10 @@ public void login(String returnTo, Boolean forceAuthn, Boolean isPassive, Boolea
 		} else {
 			relayState = returnTo;
 		}
-		parameters.put("RelayState", relayState);
+
+		if (!relayState.isEmpty()) {
+			parameters.put("RelayState", relayState);
+		}
 
 		if (settings.getAuthnRequestsSigned()) {
 			String sigAlg = settings.getSignatureAlgorithm();
@@ -267,7 +271,8 @@ public void login() throws IOException {
 	 * Initiates the SSO process.
 	 *
 	 * @param returnTo 
-     *				The target URL the user should be returned to after login.
+     *				The target URL the user should be returned to after login (relayState).
+	 *				Will be a self-routed URL when null, or not be appended at all when an empty string is provided.
      *
 	 * @throws IOException
 	 */
@@ -279,7 +284,8 @@ public void login(String returnTo) throws IOException {
 	 * Initiates the SLO process.
 	 *
 	 * @param returnTo 
-     *				The target URL the user should be returned to after logout.
+     *				The target URL the user should be returned to after logout (relayState).
+	 *				Will be a self-routed URL when null, or not be appended at all when an empty string is provided
 	 * @param nameId 
      *				The NameID that will be set in the LogoutRequest.
 	 * @param sessionIndex 
@@ -296,13 +302,15 @@ public void logout(String returnTo, String nameId, String sessionIndex) throws I
 		parameters.put("SAMLRequest", samlLogoutRequest);
 
 		String relayState;
-		if (returnTo == null || returnTo.isEmpty()) {
+		if (returnTo == null) {
 			relayState = ServletUtils.getSelfRoutedURLNoQuery(request);
 		} else {
 			relayState = returnTo;
 		}
 
-		parameters.put("RelayState", relayState);
+		if (!relayState.isEmpty()) {
+			parameters.put("RelayState", relayState);
+		}
 
 		if (settings.getLogoutRequestSigned()) {
 			String sigAlg = settings.getSignatureAlgorithm();
@@ -331,8 +339,9 @@ public void logout() throws IOException, XMLEntityException {
 	/**
 	 * Initiates the SLO process.
 	 *
-	 * @param returnTo 
-     *				The target URL the user should be returned to after logout. 
+	 * @param returnTo
+	 *				The target URL the user should be returned to after logout (relayState).
+	 *				Will be a self-routed URL when null, or not be appended at all when an empty string is provided
 	 *
 	 * @throws IOException
 	 * @throws XMLEntityException
@@ -659,11 +668,11 @@ private String buildSignature(String samlMessage, String relayState, String sign
 		 PrivateKey key = settings.getSPkey();
 
 		 String msg = type + "=" + Util.urlEncoder(samlMessage);
-		 if (relayState != null) {
+		 if (StringUtils.isNotEmpty(relayState)) {
 			 msg += "&RelayState=" + Util.urlEncoder(relayState);
 		 }
 
-		 if (signAlgorithm == null || signAlgorithm.isEmpty()) {
+		 if (StringUtils.isEmpty(signAlgorithm)) {
 			 signAlgorithm = Constants.RSA_SHA1;
 		 }
 

toolkit/src/test/java/com/onelogin/saml2/test/AuthTest.java

@@ -4,6 +4,7 @@
 import static java.util.Collections.singletonMap;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.contains;
 import static org.junit.Assert.assertEquals;
@@ -38,6 +39,7 @@
 import com.onelogin.saml2.settings.SettingsBuilder;
 import com.onelogin.saml2.util.Constants;
 import com.onelogin.saml2.util.Util;
+import org.mockito.ArgumentCaptor;
 
 public class AuthTest {
 
@@ -104,7 +106,7 @@ public void testConstructorWithReqRes() throws IOException, SettingsException, U
 		assertEquals(settings.getIdpEntityId(), auth.getSettings().getIdpEntityId());
 		assertEquals(settings.getSpEntityId(), auth.getSettings().getSpEntityId());
 	}
-	
+
 	/**
 	 * Tests the constructor of Auth
 	 * Case: filename, HttpServletRequest and HttpServletResponse provided
@@ -730,7 +732,7 @@ public void testIsAuthenticated() throws Exception {
 		expectedErrors = new ArrayList<String>();
 		expectedErrors.add("invalid_response");
 		assertEquals(expectedErrors, auth2.getErrors());
-		assertThat(auth2.getLastErrorReason(), containsString("Invalid issuer in the Assertion/Response"));		
+		assertThat(auth2.getLastErrorReason(), containsString("Invalid issuer in the Assertion/Response"));
 
 		samlResponseEncoded = Util.getFileAsString("data/responses/valid_response.xml.base64");
 		when(request.getParameterMap()).thenReturn(singletonMap("SAMLResponse", new String[]{samlResponseEncoded}));
@@ -963,6 +965,36 @@ public void testLoginWithRelayState() throws IOException, SettingsException, URI
 		verify(response).sendRedirect(matches("https:\\/\\/pitbulk.no-ip.org\\/simplesaml\\/saml2\\/idp\\/SSOService.php\\?SAMLRequest=(.)*&RelayState=http%3A%2F%2Flocalhost%3A8080%2Fexpected.jsp"));
 	}
 
+	/**
+	 * Tests the login method of Auth
+	 * Case: Login with empty relayState - no relayState appended
+	 *
+	 * @throws SettingsException
+	 * @throws IOException
+	 * @throws URISyntaxException
+	 *
+	 * @see com.onelogin.saml2.Auth#login
+	 */
+	@Test
+	public void testLoginWithoutRelayState() throws IOException, SettingsException, URISyntaxException {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+		HttpServletResponse response = mock(HttpServletResponse.class);
+		when(request.getScheme()).thenReturn("http");
+		when(request.getServerPort()).thenReturn(8080);
+		when(request.getServerName()).thenReturn("localhost");
+		when(request.getRequestURI()).thenReturn("/initial.jsp");
+
+		Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build();
+		settings.setAuthnRequestsSigned(false);
+
+		Auth auth = new Auth(settings, request, response);
+		auth.login("");
+		final ArgumentCaptor<String> urlCaptor = ArgumentCaptor.forClass(String.class);
+		verify(response).sendRedirect(urlCaptor.capture());
+		assertThat(urlCaptor.getValue(), startsWith("https://pitbulk.no-ip.org/simplesaml/saml2/idp/SSOService.php?SAMLRequest="));
+		assertThat(urlCaptor.getValue(), not(containsString("&RelayState=")));
+	}
+
 	/**
 	 * Tests the login method of Auth
 	 * Case: Signed Login but no sp key
@@ -1080,6 +1112,37 @@ public void testLogoutWithRelayState() throws IOException, SettingsException, XM
 		verify(response).sendRedirect(matches("https:\\/\\/pitbulk.no-ip.org\\/simplesaml\\/saml2\\/idp\\/SingleLogoutService.php\\?SAMLRequest=(.)*&RelayState=http%3A%2F%2Flocalhost%3A8080%2Fexpected.jsp"));
 	}
 
+	/**
+	 * Tests the logout method of Auth
+	 * Case: Logout with empty RelayState - no RelayState appended
+	 *
+	 * @throws IOException
+	 * @throws SettingsException
+	 * @throws XMLEntityException
+	 *
+	 * @see com.onelogin.saml2.Auth#logout
+	 */
+	@Test
+	public void testLogoutWithoutRelayState() throws IOException, SettingsException, XMLEntityException {
+		HttpServletRequest request = mock(HttpServletRequest.class);
+		HttpServletResponse response = mock(HttpServletResponse.class);
+		when(request.getScheme()).thenReturn("http");
+		when(request.getServerPort()).thenReturn(8080);
+		when(request.getServerName()).thenReturn("localhost");
+		when(request.getRequestURI()).thenReturn("/initial.jsp");
+
+		Saml2Settings settings = new SettingsBuilder().fromFile("config/config.my.properties").build();
+		settings.setLogoutRequestSigned(false);
+
+		Auth auth = new Auth(settings, request, response);
+		auth.logout("");
+
+		final ArgumentCaptor<String> urlCaptor = ArgumentCaptor.forClass(String.class);
+		verify(response).sendRedirect(urlCaptor.capture());
+		assertThat(urlCaptor.getValue(), startsWith("https://pitbulk.no-ip.org/simplesaml/saml2/idp/SingleLogoutService.php?SAMLRequest="));
+		assertThat(urlCaptor.getValue(), not(containsString("&RelayState=")));
+	}
+
 	/**
 	 * Tests the logout method of Auth
 	 * Case: Signed Logout but no sp key
@@ -1434,6 +1497,14 @@ public void testBuildSignature() throws URISyntaxException, IOException, Setting
 
 		signature = auth.buildResponseSignature(deflatedEncodedLogoutResponse, null, "");
 		assertEquals(expectedSignature, signature);
+
+		signature = auth.buildRequestSignature(deflatedEncodedAuthNRequest, "", signAlgorithm);
+		expectedSignature = "NS/yZ0WkHHtPU6LBWioxTzFsATJC6k7D8PcmBuM4NcC1klHSX5gmgDJdGs+7ee433RxhsTRLDNXJnXInAFG5iqZQK/Jps1aqx9iCAwfC4GCJs605e/hw3UXWKKo1lKxwE4Zu6eJ0TsMQ2gj/5qLezQL98CgqmFHLhvNgGJZcG6U=";
+		assertEquals(expectedSignature, signature);
+
+		signature = auth.buildRequestSignature(deflatedEncodedLogoutResponse, "", signAlgorithm);
+		expectedSignature = "GiO58DZMcRb8QR+dxUvn9bp5tIp2Eal8+tvOAEbYoAX6+7TMO8tTkpPjRD60pG+SMYjTC+lXQHygX2AXcO5ZQj8snfqx94C3dCOP7gLKOowFcaD0TunmnFCBx6qLv2cOleS9PSx49BSZJiGuffNcfgvTvsyqGwC2EatPP2+AxDM=";
+		assertEquals(expectedSignature, signature);
 	}
 
 }