[
https://issues.apache.org/jira/browse/WICKET-7200?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18109806#comment-18109806
]
ASF subversion and git services commented on WICKET-7200:
---------------------------------------------------------
Commit c8fa4527ab912662ff0578fb7c5c69ec370990bd in wicket's branch
refs/heads/WICKET-7200 from Emond Papegaaij
[ https://gitbox.apache.org/repos/asf?p=wicket.git;h=c8fa4527ab ]
WICKET-7200 align class-vs-package resolution in the role annotations
The five annotations in wicket-auth-roles implemented three different
rules for combining a class level annotation with a package level one,
and none of them were documented.
Standardise on the rule that was affirmed in WICKET-3240: the rules on a
class replace the rules on its package, and rules at the same level are
combined with AND.
- @AuthorizeAction and @AuthorizeActions now honour package level
annotations, resolved per action name, so a class level rule for
ENABLE leaves the package rule for RENDER in place. @AuthorizeActions
gains ElementType.PACKAGE, without which a package cannot express more
than one action rule.
- @AuthorizeResource now replaces the annotation of its package instead
of being AND-ed with it, which also removes an unguarded
getPackage() dereference.
- @AuthorizeInstantiations now participates in the override, and gains
ElementType.PACKAGE.
Document the resolution rules on AnnotationsRoleAuthorizationStrategy,
on each of the annotations and in the user guide. The pitfall that
prompted this is called out explicitly: because the annotations are
@Inherited, an annotation on a superclass in another package counts as
an annotation on the class and therefore suppresses the annotation of
the subclass' own package.
Add AnnotationsRolePackageTest, which pins each of these rules with real
classes in real annotated packages. Mocks cannot be used for this, since
a generated mock does not live in the package of the class it mocks.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
> 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
> 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)
