Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow form URL-parameter to safely influence to which email address a submission is sent to #4650

Closed
2 tasks
Tracked by #4920
joeribekker opened this issue Sep 12, 2024 · 2 comments · Fixed by #4971 or #4977
Closed
2 tasks
Tracked by #4920

Comments

@joeribekker
Copy link
Contributor

joeribekker commented Sep 12, 2024

Thema / Theme

Backend

Omschrijving / Description

Reference: Taiga DB-52

  1. A form-link contains a URL-param that holds an ID (example: myform?destEmailID=78d6f1ab331
  2. The form variabele destEmailID is used to retrieve the name and actual email address via service fetch
  3. The form shows only the name belonging to destEmailID
  4. The submission is sent to the email address belonging to destEmailID

2 tasks are identified to make this scenario possible.

  • Allow specific URL-parameters to be set as (form) variabele (ie. mark variabele as settable via URL-param)
  • Allow sending the registration email to be sent to a variabele (being an email address ofcourse)

Added value / Toegevoegde waarde

Allow mailing a predefined list of people via a form without exposing their email address.

Aanvullende opmerkingen / Additional context

No response

@joeribekker joeribekker added epic Large theme and/or meta issue triage Issue needs to be validated. Remove this label if the issue considered valid. enhancement waiting for approval An estimate was made but the stakeholder still needs to approve it. labels Sep 12, 2024
@joeribekker
Copy link
Contributor Author

Estimated on 4 weeks

@joeribekker joeribekker removed the triage Issue needs to be validated. Remove this label if the issue considered valid. label Sep 23, 2024
@joeribekker joeribekker added approved and removed waiting for approval An estimate was made but the stakeholder still needs to approve it. labels Nov 14, 2024
@robinmolen robinmolen self-assigned this Dec 23, 2024
@robinmolen robinmolen moved this from Todo to In Progress in Development Dec 23, 2024
@joeribekker joeribekker removed the epic Large theme and/or meta issue label Dec 23, 2024
robinmolen added a commit that referenced this issue Dec 24, 2024
Either the `to_emails` or the `to_email_from_variable` fields should used when registering the email backend plugin.

When a variable is selected, the resulting email address will be used for mailings. When the variable doesn't contain an email address, the `to_emails` will be used as fallback.
robinmolen added a commit that referenced this issue Dec 24, 2024
The variable chosen in the email registration (`to_email_from_variable`) will now actually be used for the mailing. `to_emails` is used as fallback, in case that the variable doesn't return a valid email address.

The variable will also be used for the payment status update mailing. If `payment_emails` is defined, these will be used as recipients. Otherwise it will use `to_email_from_variable`, and as a last resort the `to_emails`
robinmolen added a commit that referenced this issue Dec 24, 2024
Either the `to_emails` or the `to_email_from_variable` fields should used when registering the email backend plugin.

When a variable is selected, the resulting email address will be used for mailings. When the variable doesn't contain an email address, the `to_emails` will be used as fallback.
robinmolen added a commit that referenced this issue Dec 24, 2024
The variable chosen in the email registration (`to_email_from_variable`) will now actually be used for the mailing. `to_emails` is used as fallback, in case that the variable doesn't return a valid email address.

The variable will also be used for the payment status update mailing. If `payment_emails` is defined, these will be used as recipients. Otherwise it will use `to_email_from_variable`, and as a last resort the `to_emails`
robinmolen added a commit that referenced this issue Dec 24, 2024
Either the `to_emails` or the `to_email_from_variable` fields should used when registering the email backend plugin.

When a variable is selected, the resulting email address will be used for mailings. When the variable doesn't contain an email address, the `to_emails` will be used as fallback.
robinmolen added a commit that referenced this issue Dec 24, 2024
The variable chosen in the email registration (`to_email_from_variable`) will now actually be used for the mailing. `to_emails` is used as fallback, in case that the variable doesn't return a valid email address.

The variable will also be used for the payment status update mailing. If `payment_emails` is defined, these will be used as recipients. Otherwise it will use `to_email_from_variable`, and as a last resort the `to_emails`
robinmolen added a commit that referenced this issue Dec 24, 2024
@sergei-maertens
Copy link
Member

Allow specific URL-parameters to be set as (form) variabele (ie. mark variabele as settable via URL-param)

How are you going to avoid clashes with reserved URL parameter names, like we have _start and initial_data_reference, time, submission_uuid, product, of-auth-problem, _digid-message, _eherkenning-message, _eidas-message & code? Those are currently in use, but I'm more worried about developers being limited to adding own/internal query parameters for application logic because it may affect forms/break their auto-populate if they happen to shadow a form variable like that, and those will be situations that will be cryptic to debug/reproduce.

I also see a problem that you cannot reliably make it work for all types of form variabels - how do you plan to initialize a repeating group (list of) item(s) from a query parameter? Or a nested object? Those are not simple primitive datastructures that are easily string-encoded in a query parameter.

If we're going to have to write code to implement this feature, can't we instead build on top of existing constructs we already have and buy more time to properly design a prepopulate-via-the-client feature? I mean relying on the initial_data_reference here.

sergei-maertens added a commit that referenced this issue Dec 30, 2024
* Added fieldsets to group related options together
* Use a shorter form field label and describe the behaviour in the
  help text
* Filter down the available variables to compatible data types (only
  strings and arrays can produce valid values)
* Updated stories to provide some available variables in the context
* Mark toEmails configuration field as required, since it always is
* Clean up the formik hook/props usage
* Ensure that the 'empty' value is normalized when calling the onChange
  behaviour to normalize formik data types to what the backend expects
sergei-maertens added a commit that referenced this issue Dec 30, 2024
Simplified tests by removing indirection and only set up the test data
that is actually required by the test. This also uses some more
ergonomic factory utilities rather than hand-wiring everything.

Now new tests should have better examples to follow.
sergei-maertens added a commit that referenced this issue Dec 30, 2024
The test module is aimed at the generic registration mechanism and
just 'happens' to use the email plugin. We do not need to repeat all
the various cases/failure modes that are already covered by the
plugin unit tests.
sergei-maertens added a commit that referenced this issue Dec 30, 2024
* Added fieldsets to group related options together
* Use a shorter form field label and describe the behaviour in the
  help text
* Filter down the available variables to compatible data types (only
  strings and arrays can produce valid values)
* Updated stories to provide some available variables in the context
* Mark toEmails configuration field as required, since it always is
* Clean up the formik hook/props usage
* Ensure that the 'empty' value is normalized when calling the onChange
  behaviour to normalize formik data types to what the backend expects
sergei-maertens added a commit that referenced this issue Dec 30, 2024
With product prefill, the premise is that the product data is personal
and likely contains sensitive/private data that gives only the person
with a particular BSN access. This requires the person to authenticate
and then their BSN is compared with some property in the retrieved
object.

However, there may be 'public' data used to prefill some variables,
either user defined or even form variables. Multiple people can use
those objects, even without authenticating in the first place.

You can now flag a prefill to not require this kind of ownership check.
@sergei-maertens sergei-maertens added this to the Release 3.0 milestone Dec 30, 2024
@sergei-maertens sergei-maertens moved this from In Progress to Implemented in Development Dec 30, 2024
sergei-maertens added a commit that referenced this issue Dec 31, 2024
* Added fieldsets to group related options together
* Use a shorter form field label and describe the behaviour in the
  help text
* Filter down the available variables to compatible data types (only
  strings and arrays can produce valid values)
* Updated stories to provide some available variables in the context
* Mark toEmails configuration field as required, since it always is
* Clean up the formik hook/props usage
* Ensure that the 'empty' value is normalized when calling the onChange
  behaviour to normalize formik data types to what the backend expects
sergei-maertens added a commit that referenced this issue Dec 31, 2024
With product prefill, the premise is that the product data is personal
and likely contains sensitive/private data that gives only the person
with a particular BSN access. This requires the person to authenticate
and then their BSN is compared with some property in the retrieved
object.

However, there may be 'public' data used to prefill some variables,
either user defined or even form variables. Multiple people can use
those objects, even without authenticating in the first place.

You can now flag a prefill to not require this kind of ownership check.
sergei-maertens added a commit that referenced this issue Dec 31, 2024
* Added tests for configuration validation
* Added tests for runtime behaviour
sergei-maertens added a commit that referenced this issue Dec 31, 2024
* Added fieldsets to group related options together
* Use a shorter form field label and describe the behaviour in the
  help text
* Filter down the available variables to compatible data types (only
  strings and arrays can produce valid values)
* Updated stories to provide some available variables in the context
* Mark toEmails configuration field as required, since it always is
* Clean up the formik hook/props usage
* Ensure that the 'empty' value is normalized when calling the onChange
  behaviour to normalize formik data types to what the backend expects
sergei-maertens added a commit that referenced this issue Dec 31, 2024
With product prefill, the premise is that the product data is personal
and likely contains sensitive/private data that gives only the person
with a particular BSN access. This requires the person to authenticate
and then their BSN is compared with some property in the retrieved
object.

However, there may be 'public' data used to prefill some variables,
either user defined or even form variables. Multiple people can use
those objects, even without authenticating in the first place.

You can now flag a prefill to not require this kind of ownership check.
sergei-maertens added a commit that referenced this issue Dec 31, 2024
* Added tests for configuration validation
* Added tests for runtime behaviour
sergei-maertens added a commit that referenced this issue Dec 31, 2024
With product prefill, the premise is that the product data is personal
and likely contains sensitive/private data that gives only the person
with a particular BSN access. This requires the person to authenticate
and then their BSN is compared with some property in the retrieved
object.

However, there may be 'public' data used to prefill some variables,
either user defined or even form variables. Multiple people can use
those objects, even without authenticating in the first place.

You can now flag a prefill to not require this kind of ownership check.
sergei-maertens added a commit that referenced this issue Dec 31, 2024
* Added tests for configuration validation
* Added tests for runtime behaviour
@github-project-automation github-project-automation bot moved this from Implemented to Done in Development Dec 31, 2024
sergei-maertens added a commit that referenced this issue Dec 31, 2024
Some class names and stylesheets were missing for the inline radio
choices, causing the page to look janky.
sergei-maertens added a commit that referenced this issue Dec 31, 2024
When an initial data reference is present, a registration plugin must
implement the ownership check.
sergei-maertens added a commit that referenced this issue Dec 31, 2024
…rence

Prefilling some information/variables from the Objects API in
combination with email registration is a valid use case, as requested
in issue #4650 to keep email addresses private.

The ownership check method must be implemented if a submission with
non-empty initial_data_reference is being processed - there isn't
actually anything to check since registration always sends a new
email and there is no inherent update mechanism, so there's also no
inherent object ownership.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment