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

Cosign improvements - batch 1 #4808

Merged
merged 14 commits into from
Nov 18, 2024
Merged

Conversation

sergei-maertens
Copy link
Member

@sergei-maertens sergei-maertens commented Nov 1, 2024

Closes #4320 (partly)
The frontend aspect is handled in open-formulieren/open-forms-sdk#729

Email/PDF contents will be done in a different PR.

Changes

  • Language: ondertekener -> ondertekenaar
  • Detect if a link is used for cosign or not and return this information to the frontend
  • Support "email with link flow" where the code is included in the link, bypassing the "enter the code" step. If the link is not in the request email, users still need to enter the code.
  • Made the confirmation page title configurable (template), added configuration options for cosign confirmation page title and cosign confirmation page body. When cosign is enabled, the title + body templates for cosign are rendered, otherwise the "regular" ones. The body template supports a template tag for the cosign button to be displayed directly in the SDK/confirmation page.
  • "Toon NIET het nummer indien links worden gebruikt" -> due to the template nature this can be configured by municipalities themselves. Our default templates still include it because it's bad customer support if they have no references at all.

Checklist

Check off the items that are completed or not relevant.

  • Impact on features

    • Checked copying a form
    • Checked import/export of a form
    • Config checks in the configuration overview admin page
    • Problem detection in the admin email digest is handled
  • Release management

    • I have labelled the PR as "needs-backport" accordingly
  • I have updated the translations assets (you do NOT need to provide translations)

    • Ran ./bin/makemessages_js.sh
    • Ran ./bin/compilemessages_js.sh
  • Commit hygiene

    • Commit messages refer to the relevant Github issue
    • Commit messages explain the "why" of change, not the how

Copy link

codecov bot commented Nov 1, 2024

Codecov Report

Attention: Patch coverage is 98.92473% with 1 line in your changes missing coverage. Please review.

Project coverage is 96.56%. Comparing base (c50aa99) to head (7078014).
Report is 47 commits behind head on master.

Files with missing lines Patch % Lines
src/openforms/submissions/views.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4808      +/-   ##
==========================================
+ Coverage   96.55%   96.56%   +0.01%     
==========================================
  Files         748      749       +1     
  Lines       25423    25499      +76     
  Branches     3362     3375      +13     
==========================================
+ Hits        24548    24624      +76     
  Misses        610      610              
  Partials      265      265              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@sergei-maertens sergei-maertens force-pushed the feature/4320-cosign-improvements branch 4 times, most recently from 6ffbad3 to 34608cc Compare November 12, 2024 21:08
Users who have the link in the email are directly taken to the
right place to start the cosign process. This obsoletes the
entrypoint at the form start, and by not returning any login options
for cosign, we prevent this block from appearing in the frontend.

Detection of a link is used or not is done by checking for the
presence of the 'form_url' context variable in the request
email template.
If URLs in emails are enabled for the cosign request email, create a
more useful URL to the form by giving a 'init-cosign' instruction,
passing along the submission reference/code so that we can auto-populate
that information and cut out intermediate screens for the end-user.

If URLs are not enabled, we stick to the old behaviour.
Now also emit whether links are enabled or not, and let the frontend code
decide based on this information whether options should be rendered
or not. We need access to the options in certain views on the frontend,
and we cannot pass a query param to opt in/out from the frontend as
the API call to retrieve the form is done *after* the param processing.
If the code querystring parameter is provided, perform the form validation
in the GET request rather than requiring the user to fill out the form,
and automatically redirect back to the frontend to do the cosigning.

There is a minor risk for CSRF here, *if* the attacker is able to
guess the submission reference + form combination and the victim
has an active, authenticated session. Note that the session idle
time is capped at 15 minutes, so the risk is considered not
problematic.
Added title and confirmation page configuration fields to control
what is displayed on the confirmation page in the SDK.

The runtime code will select the appropriate template depending on
whether cosigning is required for the submission or not (it may be
optional through form logic!)
Depending on whether cosign is enabled or not, different confirmation
page templates are rendered and returned via the status API endpoint
for the frontend to display.
@sergei-maertens sergei-maertens force-pushed the feature/4320-cosign-improvements branch 4 times, most recently from e705a98 to 740f52b Compare November 15, 2024 16:04
@sergei-maertens sergei-maertens changed the title Cosign improvements Cosign improvements - batch 1 Nov 15, 2024
@sergei-maertens sergei-maertens marked this pull request as ready for review November 15, 2024 16:14
Added a custom templatetag that can optionally be included in the
confirmation page content/template to dislay a link/button that allows
users to start the cosign process from the confirmation page.

This is the same link that is included in emails, when the form_url
variable is used in there. The appearance is fixed to primary button,
the button text can be controlled from the template.
Added some introspection in the migration to properly evaluate the translated
default values for the newly added configuration model fields.
Copy link
Contributor

@vaszig vaszig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good to me.
The only thing I don't get is the "Cosign now" button that is shown when the user completes the form. I saw that the task has this as a requirement but why do we have it there since the cosigning step has to do with another user?

@sergei-maertens
Copy link
Member Author

Seems good to me. The only thing I don't get is the "Cosign now" button that is shown when the user completes the form. I saw that the task has this as a requirement but why do we have it there since the cosigning step has to do with another user?

That's for the cases where the cosigner is physically present the original submitter - this makes it possible to finish the full submission more quickly.

@sergei-maertens sergei-maertens merged commit 796bb43 into master Nov 18, 2024
34 checks passed
@sergei-maertens sergei-maertens deleted the feature/4320-cosign-improvements branch November 18, 2024 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Various co-sign improvements
2 participants