This is an automated email from the ASF dual-hosted git repository. dashorst pushed a commit to branch wicket-10.x in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 1fc739ca8ec061472abb3c88fd1a10a67c5e40e1 Author: Emond Papegaaij <[email protected]> AuthorDate: Thu Sep 24 18:59:12 2026 +0200 Check that the component submitting a form may be invoked Form#onRequest() has passed findSubmitter() to onFormSubmitted() since f67bccff1a, so the block that verifies the submitting component is visible and enabled in its hierarchy - guarded by "if (submitter == null)" - stopped running for an ordinary form submission. findSubmitter() matches on the request parameter name alone, and the request handler only checks the form itself, so a request naming a submit button that the application restricted with IAuthorizationStrategy reached that button's onSubmit(), onAfterSubmit() and onError(). Move the check into checkSubmittingComponent() and call it wherever a submitter becomes known: once before markFormsSubmitted(), and again where the multipart path resolves the submitter after the body has been parsed. Checking up front also covers callOnError(), reached when handleMultiPart() fails, which sits outside the block the check used to live in. An IFormSubmitter that is not a Component is still not checked. Such a submitter is not named by the client but supplied by a Behavior, whose invocation is already authorized against the component it is attached to, so AjaxFormSubmitBehavior and FormComponentUpdatingBehavior are unaffected. FormVisitTest#processInnerFormOnly keeps passing: markFormsSubmitted() still receives the resolved submitter, which is what f67bccff1a was for. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- .../metadata/MetaDataRoleFormSubmitterTest.java | 197 ++++++++ .../html/form/FormSubmitterNotAllowedTest.java | 515 +++++++++++++++++++++ .../org/apache/wicket/markup/html/form/Form.java | 84 +++- 3 files changed, 776 insertions(+), 20 deletions(-) diff --git a/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFormSubmitterTest.java b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFormSubmitterTest.java new file mode 100644 index 0000000000..fa0d2d984e --- /dev/null +++ b/wicket-auth-roles/src/test/java/org/apache/wicket/authroles/authorization/strategies/role/metadata/MetaDataRoleFormSubmitterTest.java @@ -0,0 +1,197 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.authroles.authorization.strategies.role.metadata; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.Serializable; + +import org.apache.wicket.Component; +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.authroles.authorization.strategies.role.IRoleCheckingStrategy; +import org.apache.wicket.authroles.authorization.strategies.role.RoleAuthorizationStrategy; +import org.apache.wicket.authroles.authorization.strategies.role.Roles; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.form.Button; +import org.apache.wicket.markup.html.form.Form; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTester; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * A submit button reserved for a role with + * {@link MetaDataRoleAuthorizationStrategy#authorize(Component, org.apache.wicket.authorization.Action, String)} + * must not be invoked for a user who does not hold that role, even when that user names the button + * in the form submission themselves. + */ +class MetaDataRoleFormSubmitterTest +{ + private WicketTester tester; + + @BeforeEach + void setUp() + { + tester = new WicketTester(); + } + + @AfterEach + void tearDown() + { + tester.destroy(); + } + + private void signInAs(String role) + { + tester.getApplication() + .getSecuritySettings() + .setAuthorizationStrategy(new RoleAuthorizationStrategy(new UserRolesAuthorizer(role))); + } + + @Test + void adminMayInvokeTheAdminButton() + { + signInAs("ADMIN"); + TestPage page = new TestPage(); + tester.startPage(page); + + tester.newFormTester("form").submit("adminOnly"); + + assertTrue(page.adminOnlySubmitted); + } + + @Test + void userMayNotInvokeTheAdminButton() + { + signInAs("USER"); + TestPage page = new TestPage(); + tester.startPage(page); + tester.assertRenderedPage(TestPage.class); + + // the button renders disabled, so the client has its input name + assertTrue(tester.getLastResponseAsString().contains("disabled=\"disabled\"")); + + tester.getRequest() + .getPostParameters() + .setParameterValue(page.adminOnly.getInputName(), "x"); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.adminOnlySubmitted); + assertFalse(page.formSubmitted); + } + + @Test + void userMayStillInvokeTheOrdinaryButton() + { + signInAs("USER"); + TestPage page = new TestPage(); + tester.startPage(page); + + tester.newFormTester("form").submit("ordinary"); + + assertTrue(page.ordinarySubmitted); + assertTrue(page.formSubmitted); + assertFalse(page.adminOnlySubmitted); + } + + /** A form everyone may submit, carrying one button reserved for ADMIN. */ + public static class TestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + Form<Void> form; + Button adminOnly; + Button ordinary; + + boolean formSubmitted; + boolean adminOnlySubmitted; + boolean ordinarySubmitted; + + TestPage() + { + add(form = new Form<Void>("form") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit() + { + formSubmitted = true; + } + }); + form.add(adminOnly = new Button("adminOnly") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + adminOnlySubmitted = true; + } + }); + MetaDataRoleAuthorizationStrategy.authorize(adminOnly, Component.ENABLE, "ADMIN"); + + form.add(ordinary = new Button("ordinary") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + ordinarySubmitted = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<form wicket:id=\"form\">" // + + "<button wicket:id=\"adminOnly\"></button>" // + + "<button wicket:id=\"ordinary\"></button>" // + + "</form></body></html>"); + } + } + + private static final class UserRolesAuthorizer implements IRoleCheckingStrategy, Serializable + { + private static final long serialVersionUID = 1L; + + private final Roles roles; + + UserRolesAuthorizer(String roles) + { + this.roles = new Roles(roles); + } + + @Override + public boolean hasAnyRole(Roles roles) + { + return this.roles.hasAnyRole(roles); + } + } +} diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/FormSubmitterNotAllowedTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/FormSubmitterNotAllowedTest.java new file mode 100644 index 0000000000..7b18cce447 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/FormSubmitterNotAllowedTest.java @@ -0,0 +1,515 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.apache.wicket.Component; +import org.apache.wicket.MarkupContainer; +import org.apache.wicket.WicketRuntimeException; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.ajax.markup.html.form.AjaxButton; +import org.apache.wicket.authorization.Action; +import org.apache.wicket.authorization.IAuthorizationStrategy; +import org.apache.wicket.markup.IMarkupResourceStreamProvider; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.form.upload.FileUploadField; +import org.apache.wicket.model.Model; +import org.apache.wicket.util.resource.IResourceStream; +import org.apache.wicket.util.resource.StringResourceStream; +import org.apache.wicket.util.tester.WicketTestCase; +import org.apache.wicket.validation.INullAcceptingValidator; +import org.apache.wicket.validation.IValidatable; +import org.apache.wicket.validation.IValidator; +import org.apache.wicket.validation.ValidationError; +import org.junit.jupiter.api.Test; + +/** + * A form submitting component that is not visible or not enabled in its hierarchy must never be + * invoked, no matter that the client names it in the request. This is the only place where the + * {@link Action#RENDER} and {@link Action#ENABLE} decisions of an + * {@link IAuthorizationStrategy} are enforced for a submit button, so a component the application + * has restricted must not reach {@link IFormSubmitter#onSubmit()}, + * {@link IFormSubmitter#onAfterSubmit()} or {@link IFormSubmitter#onError()}. + */ +class FormSubmitterNotAllowedTest extends WicketTestCase +{ + /** + * Denies {@code action} for the component with the given id, and allows everything else. + */ + private void deny(final Action action, final String componentId) + { + tester.getApplication() + .getSecuritySettings() + .setAuthorizationStrategy(new IAuthorizationStrategy.AllowAllAuthorizationStrategy() + { + @Override + public boolean isActionAuthorized(Component component, Action a) + { + return !(action.equals(a) && componentId.equals(component.getId())); + } + }); + } + + /** + * Names {@code component} as the submitting component of the next request, the way a browser + * does for the button that was clicked. + */ + private void nameAsSubmitter(FormComponent<?> component) + { + tester.getRequest() + .getPostParameters() + .setParameterValue(component.getInputName(), "x"); + } + + /** + * A button the application restricted with {@link Action#ENABLE} renders disabled, so its + * input name is in the markup the client received. Naming it anyway must not invoke it. + */ + @Test + void enableDeniedSubmitterIsNotInvoked() + { + TestPage page = new TestPage(); + deny(Component.ENABLE, "restricted"); + tester.startPage(page); + tester.assertRenderedPage(TestPage.class); + assertTrue(tester.getLastResponseAsString().contains("disabled=\"disabled\"")); + + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + assertFalse(page.formSubmitted); + } + + /** + * A button the application restricted with {@link Action#RENDER} is absent from the markup, so + * its input name has to be guessed. Guessing it correctly must not invoke it. + */ + @Test + void renderDeniedSubmitterIsNotInvoked() + { + TestPage page = new TestPage(); + deny(Component.RENDER, "restricted"); + tester.startPage(page); + tester.assertRenderedPage(TestPage.class); + assertFalse(tester.getLastResponseAsString().contains("wicket:id=\"restricted\"")); + + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not visible"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + assertFalse(page.formSubmitted); + } + + /** + * The same guard protects a plainly invisible submitter, with no authorization strategy + * involved at all. + */ + @Test + void invisibleSubmitterIsNotInvoked() + { + TestPage page = new TestPage(); + tester.startPage(page); + page.restricted.setVisible(false); + + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not visible"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + assertFalse(page.formSubmitted); + } + + /** + * The same guard protects a plainly disabled submitter, with no authorization strategy + * involved at all. + */ + @Test + void disabledSubmitterIsNotInvoked() + { + TestPage page = new TestPage(); + tester.startPage(page); + page.restricted.setEnabled(false); + + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + assertFalse(page.formSubmitted); + } + + /** + * A submitter that does not want the form processed reaches onSubmit() directly, bypassing + * validation, so it needs the guard just as much. + */ + @Test + void disabledSubmitterWithoutDefaultFormProcessingIsNotInvoked() + { + TestPage page = new TestPage(); + tester.startPage(page); + page.restricted.setDefaultFormProcessing(false); + page.restricted.setEnabled(false); + + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + } + + /** + * onError() is a callback on the submitting component too, so a submitter that fails the guard + * must not receive it when validation fails. + */ + @Test + void disabledSubmitterDoesNotReceiveOnError() + { + TestPage page = new TestPage(); + page.text.add(new AlwaysFail()); + tester.startPage(page); + page.restricted.setEnabled(false); + + tester.getRequest().getPostParameters().setParameterValue(page.text.getInputName(), "x"); + nameAsSubmitter(page.restricted); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.restrictedErrored); + assertFalse(page.restrictedSubmitted); + } + + /** + * Control: restricting one button leaves the others alone. + */ + @Test + void allowedSubmitterIsStillInvoked() + { + TestPage page = new TestPage(); + deny(Component.ENABLE, "restricted"); + tester.startPage(page); + + tester.newFormTester("form").submit("allowed"); + + assertTrue(page.allowedSubmitted); + assertTrue(page.formSubmitted); + assertFalse(page.restrictedSubmitted); + } + + /** + * Control: a submission that names no submitting component is unaffected. + */ + @Test + void submissionWithoutSubmitterIsStillProcessed() + { + TestPage page = new TestPage(); + tester.startPage(page); + + tester.newFormTester("form").submit(); + + assertTrue(page.formSubmitted); + assertFalse(page.restrictedSubmitted); + assertFalse(page.allowedSubmitted); + } + + /** + * Control: an Ajax submit is invoked as a behaviour on the button itself, which + * {@code ListenerRequestHandler} has already authorized, and must keep working. + */ + @Test + void ajaxSubmitterIsStillInvoked() + { + TestPage page = new TestPage(); + tester.startPage(page); + + tester.executeAjaxEvent(page.ajax, "click"); + + assertTrue(page.ajaxSubmitted); + assertTrue(page.formSubmitted); + } + + /** + * An {@link AjaxButton} is a submitting component like any other, so a plain form post naming + * a disabled one must be rejected as well. + */ + @Test + void disabledAjaxSubmitterIsNotInvokedByPlainSubmit() + { + TestPage page = new TestPage(); + tester.startPage(page); + page.ajax.setEnabled(false); + + nameAsSubmitter(page.ajax); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.form)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.formSubmitted); + } + + /** + * The submitter of a nested form is resolved from the root form, so the guard has to hold + * there too. + */ + @Test + void disabledSubmitterOfNestedFormIsNotInvoked() + { + NestedTestPage page = new NestedTestPage(); + tester.startPage(page); + page.innerSubmit.setEnabled(false); + + nameAsSubmitter(page.innerSubmit); + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.submitForm(page.outerForm)); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.innerSubmitted); + assertFalse(page.innerFormSubmitted); + assertFalse(page.outerFormSubmitted); + } + + /** + * Whether the submitting component can be resolved before or after the multipart body has been + * parsed is a property of the servlet container, so the guard must not depend on it. + */ + @Test + void disabledSubmitterOfMultiPartFormIsNotInvoked() + { + MultiPartTestPage page = new MultiPartTestPage(); + tester.startPage(page); + assertTrue(page.form.isMultiPart()); + page.restricted.setEnabled(false); + + WicketRuntimeException e = assertThrows(WicketRuntimeException.class, + () -> tester.newFormTester("form").submit("restricted")); + + assertTrue(e.getMessage().contains("is not enabled"), e.getMessage()); + assertFalse(page.restrictedSubmitted); + assertFalse(page.formSubmitted); + } + + /** */ + public static class TestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + Form<Void> form; + TextField<String> text; + Button restricted; + Button allowed; + AjaxButton ajax; + + boolean formSubmitted; + boolean restrictedSubmitted; + boolean restrictedErrored; + boolean allowedSubmitted; + boolean ajaxSubmitted; + + TestPage() + { + add(form = new Form<Void>("form") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit() + { + formSubmitted = true; + } + }); + form.add(text = new TextField<>("text", Model.of(""))); + form.add(restricted = new Button("restricted") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + restrictedSubmitted = true; + } + + @Override + public void onError() + { + restrictedErrored = true; + } + }); + form.add(allowed = new Button("allowed") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + allowedSubmitted = true; + } + }); + form.add(ajax = new AjaxButton("ajax") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit(AjaxRequestTarget target) + { + ajaxSubmitted = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<form wicket:id=\"form\">" // + + "<input type=\"text\" wicket:id=\"text\"/>" // + + "<button wicket:id=\"restricted\"></button>" // + + "<button wicket:id=\"allowed\"></button>" // + + "<button wicket:id=\"ajax\"></button>" // + + "</form></body></html>"); + } + } + + /** */ + public static class NestedTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + Form<Void> outerForm; + Form<Void> innerForm; + Button innerSubmit; + + boolean outerFormSubmitted; + boolean innerFormSubmitted; + boolean innerSubmitted; + + NestedTestPage() + { + add(outerForm = new Form<Void>("outerForm") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit() + { + outerFormSubmitted = true; + } + }); + outerForm.add(innerForm = new Form<Void>("innerForm") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit() + { + innerFormSubmitted = true; + } + }); + innerForm.add(innerSubmit = new Button("innerSubmit") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + innerSubmitted = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<form wicket:id=\"outerForm\">" // + + "<form wicket:id=\"innerForm\">" // + + "<button wicket:id=\"innerSubmit\"></button>" // + + "</form></form></body></html>"); + } + } + + /** */ + public static class MultiPartTestPage extends WebPage implements IMarkupResourceStreamProvider + { + private static final long serialVersionUID = 1L; + + Form<Void> form; + Button restricted; + + boolean formSubmitted; + boolean restrictedSubmitted; + + MultiPartTestPage() + { + add(form = new Form<Void>("form") + { + private static final long serialVersionUID = 1L; + + @Override + protected void onSubmit() + { + formSubmitted = true; + } + }); + form.add(new FileUploadField("upload")); + form.add(restricted = new Button("restricted") + { + private static final long serialVersionUID = 1L; + + @Override + public void onSubmit() + { + restrictedSubmitted = true; + } + }); + } + + @Override + public IResourceStream getMarkupResourceStream(MarkupContainer container, + Class<?> containerClass) + { + return new StringResourceStream("<html><body>" // + + "<form wicket:id=\"form\">" // + + "<input type=\"file\" wicket:id=\"upload\"/>" // + + "<button wicket:id=\"restricted\"></button>" // + + "</form></body></html>"); + } + } + + private static class AlwaysFail implements IValidator<String>, INullAcceptingValidator<String> + { + private static final long serialVersionUID = 1L; + + @Override + public void validate(IValidatable<String> validatable) + { + validatable.error(new ValidationError("always fails")); + } + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java index 2b959018bc..20c2d278fc 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java @@ -435,6 +435,12 @@ public class Form<T> extends WebMarkupContainer * THIS METHOD IS NOT PART OF THE WICKET PUBLIC API. DO NOT USE IT! * <p> * Gets the IFormSubmittingComponent which submitted this form. + * <p> + * The component is resolved from the request parameters alone: no visibility, enablement or + * authorization check is performed here, so the component returned may well be one the client + * should not be able to submit. Callers that act on the result must verify + * {@link Component#isVisibleInHierarchy()} and {@link Component#isEnabledInHierarchy()} first, + * as {@link #onFormSubmitted(IFormSubmitter)} does. * * @return The component which submitted this form, or null if the processing was not triggered * by a registered IFormSubmittingComponent @@ -734,6 +740,9 @@ public class Form<T> extends WebMarkupContainer * THIS METHOD IS NOT PART OF THE WICKET API. DO NOT ATTEMPT TO OVERRIDE OR CALL IT. * <p> * Handles form submissions. + * <p> + * The submitting component is named by the client, so it is + * {@link #onFormSubmitted(IFormSubmitter) onFormSubmitted} that verifies it may be invoked. * * @see #onFormSubmitted(IFormSubmitter) */ @@ -764,6 +773,10 @@ public class Form<T> extends WebMarkupContainer * @param submitter * listener that will receive form processing events, if {@code null} the form will * attempt to locate one + * @throws WicketRuntimeException + * if the submitter is an {@link IFormSubmittingComponent} that is not visible or + * not enabled in its hierarchy. This holds for a submitter passed in as well as + * for one located by {@link #findSubmitter()} * * @see Form#validate() */ @@ -790,6 +803,8 @@ public class Form<T> extends WebMarkupContainer } } + checkSubmittingComponent(submitter); + markFormsSubmitted(submitter); if (handleMultiPart()) @@ -797,30 +812,14 @@ public class Form<T> extends WebMarkupContainer // Tells FormComponents that a new user input has come inputChanged(); - // First, see if the processing was triggered by a IFormSubmittingComponent + // First, see if the processing was triggered by a IFormSubmittingComponent. + // A multipart request only exposes its parameters once the body has been parsed, + // so for such a request this is where the submitter becomes known. if (submitter == null) { submitter = findSubmitter(); - if (submitter instanceof IFormSubmittingComponent) - { - IFormSubmittingComponent submittingComponent = (IFormSubmittingComponent)submitter; - Component component = (Component)submitter; - - if (!component.isVisibleInHierarchy()) - { - throw new WicketRuntimeException("Submit Button " + - submittingComponent.getInputName() + " (path=" + - component.getPageRelativePath() + ") is not visible"); - } - - if (!component.isEnabledInHierarchy()) - { - throw new WicketRuntimeException("Submit Button " + - submittingComponent.getInputName() + " (path=" + - component.getPageRelativePath() + ") is not enabled"); - } - } + checkSubmittingComponent(submitter); } // When processing was triggered by a Wicket IFormSubmittingComponent and that @@ -862,6 +861,51 @@ public class Form<T> extends WebMarkupContainer }); } + /** + * Checks that a component which submitted this form may be invoked at all, that is, that it is + * visible and enabled in its hierarchy. + * <p> + * This is the only place where the {@link Component#RENDER} and {@link Component#ENABLE} + * decisions of the {@link org.apache.wicket.authorization.IAuthorizationStrategy} are enforced + * for the component that submitted a form: {@link #findSubmitter()} resolves that component + * from the request parameters alone, and the request handler has only authorized the form + * itself. A browser never submits a control it did not render, or rendered as disabled, so a + * request naming one is not a submission the application offered. + * <p> + * An {@link IFormSubmitter} that is not a {@link Component} is not checked here. Such a + * submitter is not named by the client but supplied by a {@link Behavior} - see + * {@link org.apache.wicket.ajax.form.AjaxFormSubmitBehavior} and + * {@link FormComponentUpdatingBehavior} - and the invocation of a behavior is already + * authorized against the component it is attached to. + * + * @param submitter + * the submitter to check, may be {@code null} + * @throws WicketRuntimeException + * if the submitter is a component that is not visible or not enabled in hierarchy + */ + private void checkSubmittingComponent(IFormSubmitter submitter) + { + if (submitter instanceof IFormSubmittingComponent && submitter instanceof Component) + { + IFormSubmittingComponent submittingComponent = (IFormSubmittingComponent)submitter; + Component component = (Component)submitter; + + if (!component.isVisibleInHierarchy()) + { + throw new WicketRuntimeException("Submit Button " + + submittingComponent.getInputName() + " (path=" + + component.getPageRelativePath() + ") is not visible"); + } + + if (!component.isEnabledInHierarchy()) + { + throw new WicketRuntimeException("Submit Button " + + submittingComponent.getInputName() + " (path=" + + component.getPageRelativePath() + ") is not enabled"); + } + } + } + /** * This method finds the correct form that should be processed based on the submitting component * (if there is one) and correctly handles nested forms by also looking at
