WICKET-7200 align class-vs-package resolution in the role annotations

[papegaaij]

WICKET-7200

The problem

AnnotationsRoleAuthorizationStrategy implemented three different rules for combining a class level annotation with a package level one, and none of them were documented:

Annotation	@Target	Behaviour before this PR
@AuthorizeInstantiation	TYPE, PACKAGE	class replaces package
@AuthorizeInstantiations	TYPE	AND-ed on unconditionally, never overridden
@AuthorizeAction	TYPE, PACKAGE	package annotation never read
@AuthorizeActions	TYPE	class level only
@AuthorizeResource	TYPE, PACKAGE	class AND package

Two consequences that users hit:

1. Because the annotations are @Inherited and the class lookup runs first, an annotation on a superclass in any package counts as the class level annotation and therefore suppresses the package level annotation of the subclass' own package:

// org/mycompany/base/SecuredPage.java
@AuthorizeInstantiation("USER")
public class SecuredPage extends WebPage {
}

// org/mycompany/admin/package-info.java
@AuthorizeInstantiation("ADMIN")
package org.mycompany.admin;

// org/mycompany/admin/ReportPage.java -- requires USER, not ADMIN
public class ReportPage extends SecuredPage {
}

Moving a page into a package guarded by package-info.java has no effect when one of its superclasses is annotated.

2. @AuthorizeAction has declared ElementType.PACKAGE since February 2006, but the action path never looked at the package. A @AuthorizeAction in a package-info.java compiles, looks correct, and protects nothing.

Why override, and not AND

Override is the rule that was designed, and it was affirmed when someone asked for the opposite. The pre-2010 code carried the comment "If roles are defined for the class, that overrides the package", and WICKET-3240 ("…package==false, class==true returns true") asked for conjunctive behaviour and was resolved without changing behaviour — only short-circuiting the lookup, with the comment that is still there: "Check class annotation first because it is more specific than package annotation".

The conjunction on resources, by contrast, was never a design decision. WICKET-5749 introduced it in 7.0.0 as class || package with a missing annotation counting as deny. The follow-up commit "non-annotated resources should be allowed, not denied" corrected the missing-annotation case to allow, and since true || anything is always true, that forced || to become && in the same edit purely to keep the annotation functional. That commit message says nothing about class-vs-package composition, and the annotation's javadoc still claimed it "works analogously to AuthorizeInstantiation" — which it did not.

What this PR does

One resolution path, shared by all three checks: gather the rules declared on the class, and fall back to the package only when the class says nothing.

• @AuthorizeAction / @AuthorizeActions honour package level annotations, resolved per action name: a class level rule for ENABLE overrides only the package rule for ENABLE, and the package rule for RENDER still applies. Resolving wholesale would mean that annotating a class for one action silently unprotects it for the other. @AuthorizeActions gains ElementType.PACKAGE, without which a package cannot express more than one action rule.
• @AuthorizeResource replaces the annotation of its package instead of being AND-ed with it. This also removes an unguarded resourceClass.getPackage() dereference, which the instantiation path already guarded.
• @AuthorizeInstantiations participates in the override like every other rule, and gains ElementType.PACKAGE.

Documentation, which is what prompted the ticket:

• AnnotationsRoleAuthorizationStrategy carries the canonical rules, including the @Inherited pitfall, that package annotations do not cascade to subpackages, that package-info.java has to be compiled and shipped, and that an annotation without roles authorizes everybody and is therefore the way to exempt a class from its package.
• Each annotation points there, and a few long-standing javadoc defects are fixed along the way: @AuthorizeAction claimed it "must be embedded in the AuthorizeActions annotation" (it works standalone, as wicket-examples and the tests show), @AuthorizeActions' example showed the singular annotation, @AuthorizeInstantiations had a self-referential @see, and both roles()/deny() documented their default as "an empty string" rather than an empty array.
• The user guide section on annotations listed two of the five annotations and only class level usage. It now lists all five and has an Annotations on packages subsection with the package-info.java form, the resolution rule, the worked @Inherited trap and the caveats.

Behaviour changes

Both are loosening, and both affect only applications that annotate a class and its package:

• a resource annotated at both levels with different roles previously required both sets of roles, and now requires only the roles of the class;
• a class carrying only @AuthorizeInstantiations inside a package carrying @AuthorizeInstantiation previously required both, and now requires only the class ruleset.

Package level @AuthorizeAction changes from no-op to enforced, which can only tighten authorization, and only for applications that already wrote an annotation that never worked.

The public and protected API surface of AnnotationsRoleAuthorizationStrategy is unchanged, and the @Target additions are additive, so no dependent module is affected at compile time.

Testing

mvn -pl wicket-auth-roles test → 52 tests pass. The 41 pre-existing tests are untouched and unchanged.

New AnnotationsRolePackageTest adds 11 tests over real fixture classes in real annotated packages. Mocks cannot be used here, because a generated mock does not live in the package of the class it mocks — which is also why the package level behaviour had never been covered. Nothing in the repository exercised a package level annotation before this PR.

Reverting only the strategy to its previous version fails exactly 4 of the 11 new tests — the resource override, the @AuthorizeInstantiations override, package level actions, and per-action resolution. The other 7 pass either way, so they pin pre-existing behaviour, including the @Inherited trap.

mvn -pl wicket-auth-roles javadoc:javadoc is clean, and mvn -pl wicket-user-guide package -P guide renders the new section with no new possible invalid reference.

🤖 Generated with Claude Code