Emond Papegaaij created WICKET-7200:
---------------------------------------
Summary: Align class-vs-package resolution across the
wicket-auth-roles annotations
Key: WICKET-7200
URL: https://issues.apache.org/jira/browse/WICKET-7200
Project: Wicket
Issue Type: Improvement
Components: wicket-auth-roles
Reporter: Emond Papegaaij
Fix For: 11.0.0
h3. Problem
{{AnnotationsRoleAuthorizationStrategy}} implements three different rules for
combining a class level annotation with a package level one, and none of them
are documented:
* {{@AuthorizeInstantiation}}: the class annotation replaces the package
annotation.
* {{@AuthorizeAction}}: the package annotation is never read at all, even
though the
annotation has declared {{ElementType.PACKAGE}} since February 2006. An
{{@AuthorizeAction}} in a {{package-info.java}} compiles, looks correct, and
protects
nothing.
* {{@AuthorizeResource}}: the class annotation and the package annotation are
AND-ed.
* {{@AuthorizeInstantiations}}: the ruleset is AND-ed on unconditionally and
never
participates in the override, and it is class level only.
Two consequences that users hit in practice:
# Because these 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.
Moving a page into a package guarded by {{package-info.java}} has no effect
when one
of its superclasses is annotated:
{code:java}
// 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 {
}
{code}
# Package level {{@AuthorizeAction}} silently does nothing, so a package that
appears
to be render protected is not.
h3. Which rule is the intended one
The override rule used for instantiation is the deliberate one. It carried the
comment
"If roles are defined for the class, that overrides the package" from the
start, and
WICKET-3240 asked for conjunctive behaviour and was resolved by keeping the
override
and only short-circuiting the lookup, with the comment "Check class annotation
first
because it is more specific than package annotation".
The conjunction used for resources is not a design decision. WICKET-5749
introduced it
in Wicket 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", which forced {{||}} to become {{&&}} in
the
same edit purely to keep the annotation functional. That commit message does not
mention class-vs-package composition at all, and the annotation's own javadoc
still
states that it "works analogously to AuthorizeInstantiation", which it does not.
h3. Proposed change
Standardise on the override rule: the rules on a class replace the rules on its
package, and rules at the same level are combined with AND. Document it on
{{AnnotationsRoleAuthorizationStrategy}}, on each annotation, and in the user
guide.
* {{@AuthorizeAction}} and {{@AuthorizeActions}}: honour package level
annotations,
resolved per action name, so a class level rule for {{ENABLE}} overrides only
the
package rule for {{ENABLE}} and the package rule for {{RENDER}} still applies.
{{@AuthorizeActions}} gains {{ElementType.PACKAGE}}, without which a package
cannot
express more than one action rule.
* {{@AuthorizeResource}}: the class level annotation replaces the package level
one
instead of being AND-ed with it. This also removes an unguarded
{{resourceClass.getPackage()}} dereference, unlike the guarded instantiation
path.
* {{@AuthorizeInstantiations}}: participates in the override like every other
rule,
and gains {{ElementType.PACKAGE}}.
h3. Compatibility
Two behaviour changes, both loosening, both affecting 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.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
