on login page ignore security fixes #222

[gbourant]

If this is about to be added i will add some tests and also investigate to remove some code which is used to avoid redirect loop.

Maybe we should also remove the QuarkusCookie?

[FroMage]

Can you explain why this is needed and what triggers it? Perhaps with a failing test? I'm surprised I didn't run into this earlier.

[gbourant]

I'm surprised too because I recall we had a similar issue that was resolved. However, here's the current failing test scenario:

This issue arises because the QuarkusUser JWT is invalid due to changes in the private/public keys. When you visit the login page, the /login request reaches the Quarkus backend. The backend detects the QuarkusUser token and checks its validity. Since the token is invalid, Quarkus throws an AuthenticationFailedException. This exception is caught by the AuthenticationFailedExceptionMapper#authenticationFailed method, which then triggers return renardeSecurity.makeRedirectToLogin("Invalid session (bad signature), you've been logged out");.

public class LoginFailControllerTest {

    @RegisterExtension
    static final QuarkusUnitTest config = new QuarkusUnitTest()
            .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class)
                    .addClasses(MyController.class)
                    .addAsManifestResource(EmptyAsset.INSTANCE, "beans.xml"));

    @Test
    public void testLoginPage() {

        RestAssured
                .given()
                .redirects().follow(false)
                .when()
                .cookie("QuarkusUser", "eyJjdHkiOiJKV1QiLCJhbGciOiJSU0EtT0FFUCIsImVuYyI6IkEyNTZHQ00ifQ.jFpEGdH3KyvuoEMbHpb2Di-DOgM07sCm32z1m-OntuE64LuIcX_zsmxqj1sMbtRbMlr5j3FBzMQ_xXdQ2UgkOBLGVFsfQX5WltvuffDh3I6zZW9jMM_XGNk2LWvaojdFzTNVUBlQXelsFTc4jOLClREQgwy4etRJhkltJQ4gCGCyGnZ15RjR1cAbSbLXU_Y5xSAWgYaO9m0bbwaAJb8lgQQrskzYjsfAaZHkbyi7KOQD-n5L7rQTjWO6TBVeofTZB-M4LlMz4aENUSwnxdZHG4lgU-34KmaYr-tOdaSCwbpnD324JtFKRw2x13ifbmCQoye3xcikufHkqd0MdSfkTQ.TnkdgC3KUqnELS13.ePyvG4TP4ClcQUZsxXVJA-2ErK97G6IObePC2DzjEr7WZfKBuYJKlcmkev4Ajg7AwDaU852atmrM1Z_rZiH8LqjL6dKngF-HqvVwH39H6kHmlkxAaBrsJbgs9qYyg-4VU94pf-E-49HKMvPXO8Mbf1gX5vO87K5E96XQu1kf7FlpneyEgwvhDrBZItyj1wzgf26Sc0cP-XnoXDMnvabeeHIFue2DTVn8gDpw8ZYZ_5VjWJecI8eAK8lfyEiqu_ZH3e8sXGNhsobXvgkesTLaU6TYFYDCL32Nr1u6P2qJ7QNPDfUsQaIaR1iQ-jHGFH5QX0mhj58_YSDhRkwAA_FfNnWWR7z36tVpmYboLxdeYHtXW7PKmm69B2gSnnrjNZU1E7u05UIpeofftwGzVZftpwVEwnHbxtqDSMOqPlb6Y50NcVS4sWr678udyuoskawJFxpe_FtczH3sY9EIDRhvYdmrTkphdgRK4YhbKUu63WGDYw9iqDiXVjoZZMca2iYogHd5XR1CEU1WXRh-EKgb73DwM7rfDxruTktLyAYFupwSe7441KEy1wweWxYLXiho0iVwYcwdma5pAg6ynt8-_6mSxDYB8qH81XSdBaoxt9wARHD-arr1ud4cDeNWcdHLVGmugfxc7tj4O3jhx0olkoKfOiw1VBG65E6AnwPBoJd02UxFU9sCbN57W3vahLsm2NB3KPvqg6b-6MZpQxi_W5MaF_2Q71Yy0Tzv1Li6_sLKQB9H1L3fANVMm-NNWzhCSU4W-LdVBSRCuo1SP_ussGaDEYpvZUvl_Th28vn4Ng.uX8KCEZZdm3IGMifTujMXg")
                .get("/login").then()
                .statusCode(500);

        RestAssured
                .given()
                .redirects().follow(false)
                .when()
                .get("/ok").then()
                .statusCode(200)
                .body(Matchers.is("OK"));

    }

    public static class MyController extends Controller {

        @Path("/ok")
        public String ok() {
            return "OK";
        }

        @LoginPage
        @Path("/login")
        public String login() {
            return "fake login page";
        }

    }

}

[gbourant]

Also my fix is wrong because it makes SecurityIdentity#isAnonymous to misbehave in the login page. It always returns false even when the user is already logged in.

[FroMage]

OK. So this happens when trying to access the login page with an invalid cookie. Good point. I've always tested accessing a regular page with an invalid cookie, which would redirect me to the login page after clearing the invalid cookie.

I suppose in this case, it's fine to redirect from the login page to the login page, once, simply because we're clearing the cookie. So it's not a loop. Perhaps add a check in RenardeSecurity.makeRedirectToLogin to allow for redirecting to the login page if we have a cookie to clear?

[gbourant]

I've added the check to the RenardeSecurity.makeRedirectToLogin method.

[FroMage]

OK that looks good. Could you add your test please? Probably in JWTTest in quarkus-renarde/deployment.

[gbourant]

I just pushed the test. For some weird reason when i put the test in JWTTest class i get the following error, so i added it to the CustomLoginControllerTest class if that's ok.

[ERROR] Errors: 
[ERROR]   JWTTest » Runtime java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
    [error]: Build step io.quarkiverse.renarde.deployment.RenardeProcessor#collectControllers threw an exception: java.lang.IllegalArgumentException: Cannot provide/consume multiple values for class io.quarkiverse.renarde.deployment.LoginPageBuildItem