KeycloakPolicyEnforcerAuthorizer should permit if authentication is not done by OIDC

[sberyozkin]

Fixes #15988.
Fixes #14619.
The user reported that when both Basic and OIDC (plus keycloak-authorization) mechanisms are enabled, when the user authenticates with the basic credentials the request is denied - because keycloak-authorization is trying to enforce its rules.
So I've updated the code a bit to return PERMIT if no AccessTokenCredential is available (which also avoids a blocking SecurityIdentity check)
I was confused a bit when I got policyEnforcerTest.testPathConfigurationPrecedenceWhenPathCacheNotDefined failing - it eas expecting 401 and with my updates it started getting 404.
I've looked into it and saw that since api2/resource had an enforcing mode, with the main branch code it is 401 because KeycloakPolicyEnforcerAuthorizer itself is checking of the token is available and if not - denies the request.
However api2/resource did not actually exist as the test endpoint resource - and checking the token itself is what quarkus-oidc does - so I've added ProtectedResource2 which requires an authenticated access to /api2/resource which makes sure the test gets its 401
@pedroigor does it look correct ?

[sberyozkin]

I forgot Keycloak tests were also enabled for the extension itself - so fixed them too.
In general, it does not seem correct that right now 401 is returned for a non-existent resource if the mode is enforcing - however the extension itself does not know if the endpoint exists - it only enforces the policy configuration.

So if we have OIDC (with keycloak-authz) alone and a non-existent resource authorization is enforced - it will be 401 - unless the proactive authentication is disabled (this shifts the auth decision to the JAX-RS chain and hence 404 will be reported as it is known at that point if the endpoint exists or not).

And if we have a combination like Basic + OIDC(with keycloak-authz) and it is a Basic auth now then a non-existent resource authorization is skipped - it will be 404 - but I think it is correct.

So I'd not be worried about a minor inconsistency about the status in one of the branches above - ultimately the most important thing is that the request will fail

[sberyozkin]

Hi Pedro @pedroigor thanks - lets keep an eye on this specific case, we can easily tune if needed

[sberyozkin]

Have resolved the conflict, will wait till tomorrow in case @stuartwdouglas has some comments

[sberyozkin]

@pedroigor
I wonder if quarkus.keycloak.policy-enforcer.paths.*.enforcement-mode would still have a role to play once this PR gets merged - since here, the authentication will already be done (incl checking if the resource is public) before keycloak-authorization is invoked.
So may be quarkus.keycloak.policy-enforcer.paths.*.enforcement-mode should be deprecated ?

[pedroigor]

@pedroigor Makes sense. That flag is used within the policy enforcer to only ignore authorization checks. I can check this better to make sure it won't have any negative impact.

[sberyozkin]

@pedroigor thanks, we can keep if for now for sure - let me merge this one a bit later today as this update is sensitive enough on its own :-), and I'll open another issue to discuss this property - as it would require some doc updates, etc

[sberyozkin]

@pedroigor We probably still need it if I understand you correctly - as we may have a case where quarkus-oidc has authenticated but the user does not want the authorization check for this authenticated resource - the only thing which will change then with this PR is that the users won't really have to configure the exclusion for the public resources :-)

[sberyozkin]

@pedroigor Yeah, for example:

@Test
    public void testHttpResponseFromExternalServiceAsClaim() {
        RestAssured.given().auth().oauth2(getAccessToken("alice"))
                .when().get("/api/permission/http-response-claim-protected")
                .then()
                .statusCode(200)
                .and().body(Matchers.containsString("Http Response Claim Protected Resource"));
        RestAssured.given().auth().oauth2(getAccessToken("jdoe"))
                .when().get("/api/permission/http-response-claim-protected")
                .then()
                .statusCode(403);
    }

here the token is used - but without a root (/*) mode being disabled - the test fails (I'm not sure why but it can be useful) - thought for all other public resources removing DISABLED was not a prob

[sberyozkin]

@pedroigor, I've updated keycloak-authorization/deployment/src/test/resources/application.properties and removed most of the DISABLED settings for the public resources just to confirm it works OK. integration-tests/keycloak-authorization/src/main/resources/application.properties remains unchanged (currently, on main both resources are identical) - so access to the public resources is tested with and without DISABLED

[sberyozkin]

Merging now...