TCK: Test PolicyConfigurationFactory are not specification compliant

[jhanders34]

Presently the PolicyConfigurationFactory test implementations return a new decorated PolicyConfiguration when calling the getPolicyConfiguration(String, boolean) method, but they don't return the same one each time that method is called.

Also the getPolicyConfiguration() and getPolicyConfiguration(String) methods are not returning the decorated PolicyConfiguration that is returned from the getPolicyConfiguration(String, boolean) or null if the getPolicyConfiguration(String, boolean) method was not called yet. Alternatively, those methods, could get and store a decorated PolicyConfiguration object if the Factory that is wrapped returns one from its implementation or null if the wrapped factory returns null.

[arjantijms]

Presently the PolicyConfigurationFactory test implementations

We have at least two of those in the TCK:

• https://github.com/jakartaee/authorization/blob/master/tck/app-custom-trace-policyconfiguration/src/main/java/ee/jakarta/tck/authorization/test/TSPolicyConfigurationFactoryImpl.java
• https://github.com/jakartaee/authorization/blob/master/tck/app-custom-trace-policyconfigurationfactory/src/main/java/ee/jakarta/tck/authorization/test/TSPolicyConfigurationFactoryImpl.java

return a new decorated PolicyConfiguration when calling the getPolicyConfiguration(String, boolean) method, but they don't return the same one each time that method is called.

For the first factory we have a new instance of the wrapper for every call:

public PolicyConfiguration getPolicyConfiguration(String contextId, boolean remove) throws PolicyContextException {
        if (logger.isLoggable(FINER)) {
            logger.entering("PolicyConfigurationFactoryImpl", "getPolicyConfiguration");
        }

        PolicyConfiguration policyConfiguration = new TSPolicyConfigurationImpl(contextId, remove, logger);

        logger.log(INFO, "PolicyConfigurationFactory.getPolicyConfiguration() invoked");
        logger.log(FINER, "Getting PolicyConfiguration object with id = " + contextId);

        return policyConfiguration;
    }

https://github.com/jakartaee/authorization/blob/master/tck/app-custom-trace-policyconfiguration/src/main/java/ee/jakarta/tck/authorization/test/TSPolicyConfigurationFactoryImpl.java#L93

The second factory doesn't wrap at all:

public PolicyConfiguration getPolicyConfiguration(String contextId, boolean remove) throws PolicyContextException {
        if (logger.isLoggable(FINER)) {
            logger.entering("PolicyConfigurationFactoryImpl", "getPolicyConfiguration");
        }

        PolicyConfiguration policyConfiguration = policyConfigurationFactory.getPolicyConfiguration(contextId, remove);

        logger.log(INFO, "PolicyConfigurationFactory.getPolicyConfiguration() invoked");
        logger.log(FINER, "Getting PolicyConfiguration object with id = " + contextId);

        return policyConfiguration;
    }

https://github.com/jakartaee/authorization/blob/master/tck/app-custom-trace-policyconfigurationfactory/src/main/java/ee/jakarta/tck/authorization/test/TSPolicyConfigurationFactoryImpl.java#L111

[arjantijms]

The claim "Test PolicyConfigurationFactory are not specification compliant" refers I guess to the following statemement of jakarta.security.jacc.PolicyConfigurationFactory.getPolicyConfiguration(String, boolean)

For a given value of policy context identifier, this method must always return the same instance of
* PolicyConfiguration and there must be at most one actual instance of a PolicyConfiguration with a given policy
* context identifier (during a process context).

@jhanders34 the claim seems to be valid, but I do wonder; this test class is extremely old it seems. Depending on how to interpret "08/01/02" it's from 2008, 2001, or 2002. In any interpretation, that's ancient. How come this is a problem now?

If it's really a problem for you, can you do a PR for an improved version? I guess simply keeping the created PolicyConfiguration instances in a concurrent map and check that first would fix it?

[jhanders34]

I am not blocked by this issue. I am just saying that it is something we should consider when updating the TCK in a future release. I can make a PR to show what I am thinking, but the authorization repo doesn't have a future branch yet. It only has main which is still associated with 3.0 from what I can tell. This issue is NOT a challenge.

[arjantijms]

This issue is NOT a challenge.

Okay, I removed the accepted label again ;)

You're right, there's no branch yet. We did have the open issue of merging the security specs, but for now let's just create that branch first. Thanks for the reminder.