👌 [#4398] Process PR feedback

* raise PermissionDenied instead of ImproperlyConfigured in verify_initial_data_ownership in case of bad configuration
* let PermissionDenied errors bubble up during prefill, causing a 403
* show missing validation errors for other objects API prefill fields

src/openforms/js/components/admin/form_design/variables/VariablesEditor.stories.js

@@ -874,7 +874,13 @@ export const ConfigurePrefillObjectsAPIWithValidationErrors = {
           ],
         },
         errors: {
-          prefillOptions: {authAttributePath: 'This list may not be empty.'},
+          prefillPlugin: 'Computer says no.',
+          prefillOptions: {
+            objectsApiGroup: 'Computer says no.',
+            objecttypeUuid: 'Computer says no.',
+            objecttypeVersion: 'Computer says no.',
+            authAttributePath: 'This list may not be empty.',
+          },
         },
       },
     ],

src/openforms/js/components/admin/form_design/variables/prefill/objects_api/ObjectsAPIFields.js

@@ -129,6 +129,9 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
   if (error) throw error;
   const prefillProperties = loading ? LOADING_OPTION : value;
 
+  const [, objectsApiGroupErrors] = normalizeErrors(errors.options?.objectsApiGroup, intl);
+  const [, objecttypeUuidErrors] = normalizeErrors(errors.options?.objecttypeUuid, intl);
+  const [, objecttypeVersionErrors] = normalizeErrors(errors.options?.objecttypeVersion, intl);
   const [, authAttributePathErrors] = normalizeErrors(errors.options?.authAttributePath, intl);
 
   return (
@@ -142,6 +145,7 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
       <Fieldset>
         <ObjectsAPIGroup
           apiGroupChoices={apiGroups}
+          errors={objectsApiGroupErrors}
           onChangeCheck={async () => {
             if (!objecttypeUuid) return true;
             const confirmSwitch = await openApiGroupConfirmationModal();
@@ -176,6 +180,7 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
             name="options.objecttypeUuid"
             apiGroupFieldName="options.objectsApiGroup"
             versionFieldName="options.objecttypeVersion"
+            errors={objecttypeUuidErrors}
             label={
               <FormattedMessage
                 description="Objects API prefill options 'Objecttype' label"
@@ -225,6 +230,7 @@ const ObjectsAPIFields = ({errors, showCopyButton, setShowCopyButton}) => {
                 defaultMessage="Version"
               />
             }
+            errors={objecttypeVersionErrors}
             apiGroupFieldName="options.objectsApiGroup"
             objectTypeFieldName="options.objecttypeUuid"
           />

src/openforms/js/components/admin/forms/objects_api/ObjectTypeSelect.js

@@ -26,6 +26,7 @@ const ObjectTypeSelect = ({
   label,
   helpText,
   versionFieldName = 'objecttypeVersion',
+  errors = [],
 }) => {
   const [fieldProps, , fieldHelpers] = useField(name);
   const {
@@ -68,7 +69,14 @@ const ObjectTypeSelect = ({
 
   return (
     <FormRow>
-      <Field name={name} required label={label} helpText={helpText} noManageChildProps>
+      <Field
+        name={name}
+        required
+        label={label}
+        helpText={helpText}
+        errors={errors}
+        noManageChildProps
+      >
         <ReactSelect
           name={name}
           options={options}
@@ -113,6 +121,10 @@ ObjectTypeSelect.propTypes = {
    * changes, the version will be reset/unset.
    */
   versionFieldName: PropTypes.string,
+  /**
+   * List of errors to be displayed on the field
+   */
+  errors: PropTypes.arrayOf(PropTypes.string),
 };
 
 export default ObjectTypeSelect;

src/openforms/js/components/admin/forms/objects_api/ObjectTypeVersionSelect.js

@@ -30,6 +30,7 @@ const ObjectTypeVersionSelect = ({
   apiGroupFieldName = 'objectsApiGroup',
   objectTypeFieldName = 'objecttype',
   label,
+  errors = [],
 }) => {
   const {getFieldProps} = useFormikContext();
 
@@ -55,7 +56,7 @@ const ObjectTypeVersionSelect = ({
   const options = choices.map(([value, label]) => ({value, label}));
   return (
     <FormRow>
-      <Field name={name} required label={label} noManageChildProps>
+      <Field name={name} required label={label} errors={errors} noManageChildProps>
         <ReactSelect
           name={name}
           required
@@ -87,6 +88,10 @@ ObjectTypeVersionSelect.propTypes = {
    * The label that will be shown before the field
    */
   label: PropTypes.node.isRequired,
+  /**
+   * List of errors to be displayed on the field
+   */
+  errors: PropTypes.arrayOf(PropTypes.string),
 };
 
 export default ObjectTypeVersionSelect;

src/openforms/js/components/admin/forms/objects_api/ObjectsAPIGroup.js

@@ -14,6 +14,7 @@ const ObjectsAPIGroup = ({
   onApiGroupChange,
   isClearable = false,
   required = true,
+  errors = [],
 }) => {
   const [{onChange: onChangeFormik, ...fieldProps}, , {setValue}] = useField(name);
   const {setValues} = useFormikContext();
@@ -40,6 +41,7 @@ const ObjectsAPIGroup = ({
       <Field
         name={name}
         required={required}
+        errors={errors}
         label={
           <FormattedMessage
             description="Objects API group field label"
@@ -119,6 +121,11 @@ ObjectsAPIGroup.propTypes = {
    * Indicate if the field is required or optional.
    */
   required: PropTypes.bool,
+
+  /**
+   * List of errors to be displayed on the field
+   */
+  errors: PropTypes.arrayOf(PropTypes.string),
 };
 
 export default ObjectsAPIGroup;

src/openforms/prefill/contrib/objects_api/plugin.py

@@ -1,6 +1,6 @@
 import logging
 
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import PermissionDenied
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
@@ -56,7 +56,7 @@ def verify_initial_data_ownership(
             logevent.object_ownership_check_improperly_configured(
                 submission, plugin=self
             )
-            raise ImproperlyConfigured(
+            raise PermissionDenied(
                 f"`auth_attribute_path` missing from options {prefill_options}, cannot perform initial data ownership check"
             )
 

src/openforms/prefill/contrib/objects_api/tests/test_initial_data_ownership_validation.py

@@ -153,7 +153,8 @@ def test_verify_initial_data_ownership_raising_errors_causes_prefill_to_fail(sel
             "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership",
             side_effect=PermissionDenied,
         ) as mock_validate_object_ownership:
-            prefill_variables(submission=submission_step.submission)
+            with self.assertRaises(PermissionDenied):
+                prefill_variables(submission=submission_step.submission)
 
             self.assertEqual(mock_validate_object_ownership.call_count, 1)
 
@@ -192,7 +193,8 @@ def test_verify_initial_data_ownership_missing_auth_attribute_path_causes_failin
         with patch(
             "openforms.prefill.contrib.objects_api.plugin.validate_object_ownership",
         ) as mock_validate_object_ownership:
-            prefill_variables(submission=submission_step.submission)
+            with self.assertRaises(PermissionDenied):
+                prefill_variables(submission=submission_step.submission)
 
             self.assertEqual(mock_validate_object_ownership.call_count, 0)
 

src/openforms/prefill/sources.py

@@ -108,7 +108,7 @@ def fetch_prefill_values_from_options(
                 )
             except (PermissionDenied, ImproperlyConfigured) as exc:
                 logevent.prefill_retrieve_failure(submission, plugin, exc)
-                continue
+                raise exc
 
         options_serializer = plugin.options(data=variable.form_variable.prefill_options)
 

src/openforms/prefill/tests/test_prefill_variables.py

@@ -420,7 +420,8 @@ def test_verify_initial_data_ownership(self):
                 "openforms.prefill.contrib.demo.plugin.DemoPrefill.verify_initial_data_ownership",
                 side_effect=PermissionDenied,
             ) as mock_verify_ownership:
-                prefill_variables(submission=submission_step.submission)
+                with self.assertRaises(PermissionDenied):
+                    prefill_variables(submission=submission_step.submission)
 
                 mock_verify_ownership.assert_called_once_with(
                     submission_step.submission, variable.prefill_options

src/openforms/registrations/contrib/objects_api/plugin.py

@@ -4,7 +4,7 @@
 from functools import partial
 from typing import TYPE_CHECKING, Any, override
 
-from django.core.exceptions import ImproperlyConfigured
+from django.core.exceptions import PermissionDenied
 from django.urls import reverse
 from django.utils.translation import gettext_lazy as _
 
@@ -195,7 +195,7 @@ def verify_initial_data_ownership(self, submission: Submission) -> None:
             logevent.object_ownership_check_improperly_configured(
                 submission, plugin=self
             )
-            raise ImproperlyConfigured(
+            raise PermissionDenied(
                 f"{backend} has no `auth_attribute_path` configured, cannot perform initial data ownership check"
             )
 

src/openforms/registrations/contrib/objects_api/tests/test_initial_data_ownership_validation.py

@@ -1,6 +1,6 @@
 from unittest.mock import patch
 
-from django.core.exceptions import ImproperlyConfigured, PermissionDenied
+from django.core.exceptions import PermissionDenied
 from django.test import TestCase, tag
 
 from openforms.contrib.objects_api.tests.factories import ObjectsAPIGroupConfigFactory
@@ -138,7 +138,7 @@ def test_verify_initial_data_ownership_missing_auth_attribute_path_causes_failin
         with patch(
             "openforms.registrations.contrib.objects_api.plugin.validate_object_ownership",
         ) as mock_validate_object_ownership:
-            with self.assertRaises(ImproperlyConfigured):
+            with self.assertRaises(PermissionDenied):
                 pre_registration(submission.id, PostSubmissionEvents.on_completion)
 
             # Not called, due to missing `auth_attribute_path`