From 5a9b9d9749a63ec3311178e845fad0632e78c1a3 Mon Sep 17 00:00:00 2001 From: "arjan.tijms" Date: Thu, 5 Nov 2015 19:16:27 +0100 Subject: [PATCH 1/3] Added extra comments and failure explanations --- .../BasicAuthenticationProtectedTest.java | 44 +++++++- .../BasicAuthenticationPublicTest.java | 52 ++++----- .../BasicAuthenticationStatelessTest.java | 105 +++++++++++++++--- 3 files changed, 148 insertions(+), 53 deletions(-) diff --git a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationProtectedTest.java b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationProtectedTest.java index aab2ad658..eb09f87f4 100644 --- a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationProtectedTest.java +++ b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationProtectedTest.java @@ -14,8 +14,8 @@ import org.xml.sax.SAXException; /** - * This tests that we can login from a protected resource (a resource for which security constraints have been set) and then - * access it. + * This tests that we can login from a protected resource (a resource for which + * security constraints have been set) and then access it. * * @author Arjan Tijms * @@ -34,7 +34,10 @@ public void testProtectedPageNotLoggedin() throws IOException, SAXException { String response = getFromServerPath("protected/servlet"); // Not logged-in thus should not be accessible. - assertFalse(response.contains("This is a protected servlet")); + assertFalse( + "Not authenticated, so should not have been able to access protected resource", + response.contains("This is a protected servlet") + ); } @Test @@ -43,7 +46,40 @@ public void testProtectedPageLoggedin() throws IOException, SAXException { String response = getFromServerPath("protected/servlet?doLogin=true"); // Now has to be logged-in so page is accessible - assertTrue(response.contains("This is a protected servlet")); + assertTrue( + "Should have been authenticated, but could not access protected resource", + response.contains("This is a protected servlet") + ); + + // Not only does the page needs to be accessible, the caller should have + // the correct + // name and roles as well + + // Being able to access a page protected by a role but then seeing the un-authenticated + // (anonymous) user would normally be impossible, but could happen if the authorization + // system checks roles on the authenticated subject, but does not correctly expose + // or propagate these to the HttpServletRequest + assertFalse( + "Protected resource could be accessed, but the user appears to be the unauthenticated user. " + + "This should not be possible", + response.contains("web username: null") + ); + + // An authenticated user should have the exact name "test" and nothing else. + assertTrue( + "Protected resource could be accessed, but the username is not correct.", + response.contains("web username: test") + ); + + // Being able to access a page protected by role "architect" but failing + // the test for this role would normally be impossible, but could happen if the + // authorization system checks roles on the authenticated subject, but does not + // correctly expose or propagate these to the HttpServletRequest + assertTrue( + "Resource protected by role \"architect\" could be accessed, but user fails test for this role." + + "This should not be possible", + response.contains("web user has role \"architect\": true") + ); } } \ No newline at end of file diff --git a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationPublicTest.java b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationPublicTest.java index 81e7f3842..b6484aab9 100644 --- a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationPublicTest.java +++ b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationPublicTest.java @@ -32,8 +32,16 @@ public void testPublicPageNotLoggedin() throws IOException, SAXException { String response = getFromServerPath("public/servlet"); // Not logged-in - assertTrue(response.contains("web username: null")); - assertTrue(response.contains("web user has role \"architect\": false")); + assertTrue( + "Not authenticated, but a username other than null was encountered. " + + "This is not correct.", + response.contains("web username: null") + ); + assertTrue( + "Not authenticated, but the user seems to have the role \"architect\". " + + "This is not correct.", + response.contains("web user has role \"architect\": false") + ); } @Test @@ -44,36 +52,16 @@ public void testPublicPageLoggedin() throws IOException, SAXException { String response = getFromServerPath("public/servlet?doLogin"); // Now has to be logged-in - assertTrue(response.contains("web username: test")); - assertTrue(response.contains("web user has role \"architect\": true")); - } - - @Test - public void testPublicPageNotRememberLogin() throws IOException, SAXException { - - // -------------------- Request 1 --------------------------- - - String response = getFromServerPath("public/servlet"); - - // Not logged-in - assertTrue(response.contains("web username: null")); - assertTrue(response.contains("web user has role \"architect\": false")); - - // -------------------- Request 2 --------------------------- - - response = getFromServerPath("public/servlet?doLogin"); - - // Now has to be logged-in - assertTrue(response.contains("web username: test")); - assertTrue(response.contains("web user has role \"architect\": true")); - - // -------------------- Request 3 --------------------------- - - response = getFromServerPath("public/servlet"); - - // Not logged-in - assertTrue(response.contains("web username: null")); - assertTrue(response.contains("web user has role \"architect\": false")); + assertTrue( + "User should have been authenticated and given name \"test\", " + + " but does not appear to have this name", + response.contains("web username: test") + ); + assertTrue( + "User should have been authenticated and given role \"architect\", " + + " but does not appear to have this role", + response.contains("web user has role \"architect\": true") + ); } } \ No newline at end of file diff --git a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java index a714080bf..bf46461f2 100644 --- a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java +++ b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java @@ -41,6 +41,7 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException { // Not logged-in thus should not be accessible. assertFalse(response.contains("This is a protected servlet")); + // -------------------- Request 2 --------------------------- // JASPIC is stateless and login (re-authenticate) has to happen for every request @@ -53,10 +54,13 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException { response = getFromServerPath("protected/servlet?doLogin"); // Now has to be logged-in so page is accessible - assertTrue("Could not access protected page, but should be able to. " - + "Did the container remember the previously set 'unauthenticated identity'?", - response.contains("This is a protected servlet")); + assertTrue( + "Could not access protected page, but should be able to. " + + "Did the container remember the previously set 'unauthenticated identity'?", + response.contains("This is a protected servlet") + ); + // -------------------- Request 3 --------------------------- // JASPIC is stateless and login (re-authenticate) has to happen for every request @@ -66,9 +70,11 @@ public void testProtectedAccessIsStateless() throws IOException, SAXException { response = getFromServerPath("protected/servlet"); // Not logged-in thus should not be accessible. - assertFalse("Could access protected page, but should not be able to. " - + "Did the container remember the authenticated identity that was set in previous request?", - response.contains("This is a protected servlet")); + assertFalse( + "Could access protected page, but should not be able to. " + + "Did the container remember the authenticated identity that was set in previous request?", + response.contains("This is a protected servlet") + ); } /** @@ -83,6 +89,7 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException { // Start with doing a login String response = getFromServerPath("protected/servlet?doLogin"); + // -------------------- Request 2 --------------------------- // JASPIC is stateless and login (re-authenticate) has to happen for every request @@ -94,9 +101,66 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException { response = getFromServerPath("protected/servlet"); // Not logged-in thus should not be accessible. - assertFalse("Could access protected page, but should not be able to. " - + "Did the container remember the authenticated identity that was set in previous request?", - response.contains("This is a protected servlet")); + assertFalse( + "Could access protected page, but should not be able to. " + + "Did the container remember the authenticated identity that was set in the previous request?", + response.contains("This is a protected servlet") + ); + } + + /** + * Tests that access to a public page does not depend on the authenticated identity that was established in a previous + * request. + */ + @Test + public void testPublicAccessIsStateless() throws IOException, SAXException { + + // -------------------- Request 1 --------------------------- + + String response = getFromServerPath("public/servlet"); + + // Establish that we're initially not logged-in + assertTrue( + "Not authenticated, but a username other than null was encountered. " + + "This is not correct.", + response.contains("web username: null") + ); + assertTrue( + "Not authenticated, but the user seems to have the role \"architect\". " + + "This is not correct.", + response.contains("web user has role \"architect\": false") + ); + + + // -------------------- Request 2 --------------------------- + + response = getFromServerPath("public/servlet?doLogin"); + + // Now has to be logged-in + assertTrue( + "User should have been authenticated and given name \"test\", " + + " but does not appear to have this name", + response.contains("web username: test") + ); + assertTrue(response.contains("web user has role \"architect\": true")); + + + // -------------------- Request 3 --------------------------- + + // Accessing public page without login + response = getFromServerPath("public/servlet"); + + // No details should linger around + assertTrue( + "Should not be authenticated, but a username other than null was encountered. " + + "Did the container remember the authenticated identity that was set in the previous request?", + response.contains("web username: null") + ); + assertTrue( + "The unauthenticated user has the role 'architect', which should not be the case. " + + "The container seemed to have remembered it from the previous request.", + response.contains("web user has role \"architect\": false") + ); } /** @@ -111,20 +175,27 @@ public void testUserIdentityIsStateless() throws IOException, SAXException { // Accessing protected page with login String response = getFromServerPath("protected/servlet?doLogin"); + // -------------------- Request 2 --------------------------- // Accessing public page without login response = getFromServerPath("public/servlet"); // No details should linger around - assertFalse("User principal was 'test', but it should be null here. " - + "The container seemed to have remembered it from the previous request.", - response.contains("web username: test")); - assertTrue("User principal was not null, but it should be null here. ", - response.contains("web username: null")); - assertTrue("The unauthenticated user has the role 'architect', which should not be the case. " - + "The container seemed to have remembered it from the previous request.", - response.contains("web user has role \"architect\": false")); + assertFalse( + "User principal was 'test', but it should be null here. " + + "The container seemed to have remembered it from the previous request.", + response.contains("web username: test") + ); + assertTrue( + "User principal was not null, but it should be null here. ", + response.contains("web username: null") + ); + assertTrue( + "The unauthenticated user has the role 'architect', which should not be the case. " + + "The container seemed to have remembered it from the previous request.", + response.contains("web user has role \"architect\": false") + ); } } \ No newline at end of file From c617bc24bfa2386f1fe45ef8879c0e0b849f2dfb Mon Sep 17 00:00:00 2001 From: arjan tijms Date: Sat, 7 Nov 2015 23:06:16 +0100 Subject: [PATCH 2/3] Formatting for Liberty profile --- pom.xml | 70 ++++++++++++++++++++++++++++----------------------------- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/pom.xml b/pom.xml index a77e8aded..c57e6336e 100644 --- a/pom.xml +++ b/pom.xml @@ -772,44 +772,44 @@ liberty-managed-arquillian - - org.jboss.arquillian.container - arquillian-wlp-managed-8.5 - 1.0.0.Beta2 - - - org.glassfish.tyrus - tyrus-client - 1.3 - test - - - org.glassfish - javax.json - 1.0.4 - test - - - org.glassfish.tyrus - tyrus-container-grizzly-client - 1.3 - test + + org.jboss.arquillian.container + arquillian-wlp-managed-8.5 + 1.0.0.Beta2 + + + org.glassfish.tyrus + tyrus-client + 1.3 + test + + + org.glassfish + javax.json + 1.0.4 + test + + + org.glassfish.tyrus + tyrus-container-grizzly-client + 1.3 + test - - org.glassfish.jersey.core - jersey-client - 2.4 - test + + org.glassfish.jersey.core + jersey-client + 2.4 + test - - org.glassfish.jersey.media - jersey-media-json-processing - 2.4 - test + + org.glassfish.jersey.media + jersey-media-json-processing + 2.4 + test - - - + + + maven-surefire-plugin From c366a6ef9074cc71337b87588bce2bb9bb20c420 Mon Sep 17 00:00:00 2001 From: arjan tijms Date: Sun, 8 Nov 2015 22:34:06 +0100 Subject: [PATCH 3/3] Naming, formatting and moved one test method --- .../BasicAuthenticationStatelessTest.java | 2 +- .../CustomPrincipalPublicTest.java | 44 +---------- .../CustomPrincipalStatelessTest.java | 74 +++++++++++++++---- 3 files changed, 64 insertions(+), 56 deletions(-) diff --git a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java index bf46461f2..a4433b9da 100644 --- a/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java +++ b/jaspic/basic-authentication/src/test/java/org/javaee7/jaspic/basicauthentication/BasicAuthenticationStatelessTest.java @@ -168,7 +168,7 @@ public void testPublicAccessIsStateless() throws IOException, SAXException { * authenticated identity are remembered */ @Test - public void testUserIdentityIsStateless() throws IOException, SAXException { + public void testProtectedThenPublicAccessIsStateless() throws IOException, SAXException { // -------------------- Request 1 --------------------------- diff --git a/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalPublicTest.java b/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalPublicTest.java index 3b2b086c1..b03b28b6b 100644 --- a/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalPublicTest.java +++ b/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalPublicTest.java @@ -39,53 +39,13 @@ public void testPublicPageLoggedin() throws IOException, SAXException { "Username is not the expected one 'test'", response.contains("web username: test") ); - assertTrue( - "Username is correct, but the expected role 'architect' is not present.", - response.contains("web user has role \"architect\": true")); - - assertTrue( - "Username and roles are correct, but principal type is not the expected custom type.", - response.contains("isCustomPrincipal: true") - ); - } - - @Test - public void testPublicPageNotRememberLogin() throws IOException, SAXException { - - // -------------------- Request 1 --------------------------- - - String response = getFromServerPath("public/servlet"); - - // Not logged-in - assertTrue(response.contains("web username: null")); - assertTrue(response.contains("web user has role \"architect\": false")); - - // -------------------- Request 2 --------------------------- - - response = getFromServerPath("public/servlet?doLogin"); - - // Now has to be logged-in - assertTrue( - "Username is not the expected one 'test'", - response.contains("web username: test") - ); assertTrue( "Username is correct, but the expected role 'architect' is not present.", response.contains("web user has role \"architect\": true") ); - - // -------------------- Request 3 --------------------------- - - response = getFromServerPath("public/servlet"); - - // Not logged-in - assertTrue( - "Should not be authenticated, but username was not null. Did the container remember it from previous request?", - response.contains("web username: null") - ); assertTrue( - "Request was not authenticated (username correctly null), but unauthenticated user incorrectly has role 'architect'", - response.contains("web user has role \"architect\": false") + "Username and roles are correct, but principal type is not the expected custom type.", + response.contains("isCustomPrincipal: true") ); } diff --git a/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalStatelessTest.java b/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalStatelessTest.java index 3bbc77015..21bb3419c 100644 --- a/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalStatelessTest.java +++ b/jaspic/custom-principal/src/test/java/org/javaee7/jaspictest/customprincipal/CustomPrincipalStatelessTest.java @@ -14,7 +14,7 @@ import org.xml.sax.SAXException; /** - * Idential test as in basic-authentication, but not performed against a SAM which sets a custom principal. + * Idential test as in basic-authentication, but now performed against a SAM which sets a custom principal. * Therefore tests that for this kind of usage of the PrincipalCallback JASPIC is stateless just as well. * * @author Arjan Tijms @@ -96,9 +96,51 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException { response = getFromServerPath("protected/servlet"); // Not logged-in thus should not be accessible. - assertFalse("Could access protected page, but should not be able to. " - + "Did the container remember the authenticated identity that was set in previous request?", - response.contains("This is a protected servlet")); + assertFalse( + "Could access protected page, but should not be able to. " + + "Did the container remember the authenticated identity that was set in previous request?", + response.contains("This is a protected servlet") + ); + } + + @Test + public void testPublicAccessIsStateless() throws IOException, SAXException { + + // -------------------- Request 1 --------------------------- + + String response = getFromServerPath("public/servlet"); + + // Not logged-in + assertTrue(response.contains("web username: null")); + assertTrue(response.contains("web user has role \"architect\": false")); + + // -------------------- Request 2 --------------------------- + + response = getFromServerPath("public/servlet?doLogin"); + + // Now has to be logged-in + assertTrue( + "Username is not the expected one 'test'", + response.contains("web username: test") + ); + assertTrue( + "Username is correct, but the expected role 'architect' is not present.", + response.contains("web user has role \"architect\": true") + ); + + // -------------------- Request 3 --------------------------- + + response = getFromServerPath("public/servlet"); + + // Not logged-in + assertTrue( + "Should not be authenticated, but username was not null. Did the container remember it from previous request?", + response.contains("web username: null") + ); + assertTrue( + "Request was not authenticated (username correctly null), but unauthenticated user incorrectly has role 'architect'", + response.contains("web user has role \"architect\": false") + ); } /** @@ -106,7 +148,7 @@ public void testProtectedAccessIsStateless2() throws IOException, SAXException { * authenticated identity are remembered */ @Test - public void testUserIdentityIsStateless() throws IOException, SAXException { + public void testProtectedThenPublicAccessIsStateless() throws IOException, SAXException { // -------------------- Request 1 --------------------------- @@ -119,14 +161,20 @@ public void testUserIdentityIsStateless() throws IOException, SAXException { response = getFromServerPath("public/servlet"); // No details should linger around - assertFalse("User principal was 'test', but it should be null here. " - + "The container seemed to have remembered it from the previous request.", - response.contains("web username: test")); - assertTrue("User principal was not null, but it should be null here. ", - response.contains("web username: null")); - assertTrue("The unauthenticated user has the role 'architect', which should not be the case. " - + "The container seemed to have remembered it from the previous request.", - response.contains("web user has role \"architect\": false")); + assertFalse( + "User principal was 'test', but it should be null here. " + + "The container seemed to have remembered it from the previous request.", + response.contains("web username: test") + ); + assertTrue( + "User principal was not null, but it should be null here. ", + response.contains("web username: null") + ); + assertTrue( + "The unauthenticated user has the role 'architect', which should not be the case. " + + "The container seemed to have remembered it from the previous request.", + response.contains("web user has role \"architect\": false") + ); } } \ No newline at end of file