[
https://issues.apache.org/jira/browse/WICKET-7200?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Emond Papegaaij reassigned WICKET-7200:
---------------------------------------
Assignee: Emond Papegaaij
> 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
> Assignee: Emond Papegaaij
> Priority: Major
> 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)
