fix(mail): display full name instead of username on request comments by edivalentinitu · Pull Request #638 · inveniosoftware/invenio-requests

edivalentinitu · 2026-09-11T17:06:06Z

if available, choose the full name from the user profile to render the comment event generated email, then fallback to username

linked with inveniosoftware/invenio-rdm-records#2453, inveniosoftware/invenio-communities#1417

fenekku · 2026-09-24T15:28:17Z

 {# created_by is either a resolved user or an email (for guests) #}
-{% set event_creator_name = invenio_request_event.created_by.username or invenio_request_event.created_by %}
+{% set event_creator_name = invenio_request_event.created_by.profile.full_name
+    or invenio_request_event.created_by.username


nit: the @ used in the text is really meant mostly for usernames. For fullnames it can be strange (we were asked to remove it at my institution I believe)

Suggested change

or invenio_request_event.created_by.username

or (invenio_request_event.created_by.username and "@" ~ invenio_request_event.created_by.username)

removed @ for full names in every related PR

fenekku · 2026-09-24T15:30:09Z

-{% set event_creator_name = invenio_request_event.created_by.username or invenio_request_event.created_by %}
+{% set event_creator_name = invenio_request_event.created_by.profile.full_name
+    or invenio_request_event.created_by.username
+    or invenio_request_event.created_by %}


This is a problem that predates this PR, but we may as well solve it now: created_by is a dict, so even for last resort we don't want to use it.

My preference would be to not even rely on an "anonymous user" placeholder if no name. The 'system' might be the creator (like it can be creator) and it isn't an "anonymous user". This can result in weird problems (e.g., https://github.com/inveniosoftware/invenio-app-rdm/pull/3566/changes) What this would mean is revising the email text below to always provide a subject-less alternative. So something like:

{% if event_creator_name %} <td>{{ _("'{event_creator_name}' commented on '{request_title}':").format(creator_name=event_creator_name, request_title=request_title) }}</td> {% else %} <td>{{ _("A new comment was posted on '{request_title}':").format(request_title=request_title) }}</td> {% endif %}

and so on. There might be ways to alleviate that repetitiveness (leave it up to you).

fixed this in every related PR

fenekku · 2026-09-24T15:56:49Z


 {# created_by is either a resolved user or an email (for guests) #}
-{% set event_creator_name = invenio_request_event.created_by.username or invenio_request_event.created_by %}
+{% set event_creator_name = invenio_request_event.created_by.profile.full_name


About the possibility of link injection surfaced by zenodo/zenodo-rdm#1444 :

On the selfish side of things: our instance uses institutional SSO, so we can trust the fullnames and we prevent users from changing them.

Having validation of the full name on original creation/modification to prevent links would be more graceful, so that we wouldn't have to worry about it everywhere it is used.

one could even have a job that scans users for fullnames that are links and block their account

Obfuscating the fullname (so that autodetection doesn't really work) by inserting no-width space (e.g., ) is fragile and cumbersome too

The cases where this can happen are contrained: attacker relies on legitimacy of instance to make other email recipients assume they can click on a link positioned nonsensically in email... The legitimacy halo is the only advantage over blanket email spam. Not saying it's not legitimate concern, just personally not 'severely' concerned.

All-in-all I'd be curious to hear what others think. I am fine with the change (given the above).

invenio_request_event.created_by may not have a profile at all (so .fullname will raise in template rendering). Logic to determine the event_creator_name will have to be more involved. (Perhaps a custom filter could help but I don't know where would it be registered appropriately.)

The pr you mention zenodo/zenodo-rdm#1444 was made due to the possibility to send emails through the user creation form to existing users using email aliases, so the attacker would enter arbitrary data on the full name, like a url.

As far as I'm aware this path (form api to here) does not allow that, neither allows arbitrary comment data to be displayed.

So I think there would be no problem here.

A comment about :

one could even have a job that scans users for fullnames that are links and block their account

I think is better to user serializers on input everywhere instead of this, since we would have to scan every possible field where a user inputs data that is shown to others.

Edit: @fenekku forgot that users could make an account just for this and set a bad fullname so yes escape it before it gets here, and to make sure to add a regression test for now.

@fenekku - i added a helper global function in this PR which then i use in the other packages too. of course considering this, this PR now will be needed to be merged first

mesemus · 2026-10-06T16:10:12Z

We have the global function defined here:

https://github.com/oarepo/oarepo-requests/blob/main/oarepo_requests/notifications/jinja.py

(no https:// filtering as we use AAI to fill in the fullname - maybe removing :// would be enough)

and then use add_template_global on the flask app during init_app:

https://github.com/oarepo/oarepo-requests/blob/main/oarepo_requests/ext.py#L143

In templates it is:

{% set receiver_name = get_request_user_name(receiver) %}

I'm not sure why we decided to use the global function instead of filter, for filter there is add_template_filter

* if available, choose the full name from the user profile to render the comment event generated email, then fallback to username

* event.created_by is a dict so there is no sense in using it as a last fallback if the username and full name are missing. * if the event_creator_name is undefined, render a subject- less message

* move the logic that determines the user name shown in mail notifications in a python helper file and register it in the extension so we can use it in the jinja template

fenekku

Thanks for all the adjustments on this! That works for me. @mesemus , @ciurana-life or @utnapischtim you want to give it another approval so we can get this in?

fenekku

After discussion with @ciurana-life , we will put this on hold until CERN can investigate / resolve some security implications. Will reconvene at next Telecon to see status of this.

fenekku

Let's doublecheck with @slint about the config. Otherwise everything is good (should be released after invenio-accounts is).

fenekku · 2026-10-23T15:32:29Z

+    if full_name := user_obj.get("profile", {}).get("full_name"):
+        return full_name
+
+    if username := user_obj.get("username"):
+        return f"@{username}"
+


With fix(profiles): sanitize full name invenio-accounts#577 , using the fullname will be secure . So we will just have to wait for it to be merged and released, before releasing this one.

I believe I brainfarted during the Tuesday Oct 21 discussion with respect to the config aspect that @slint was talking about 🤪. I think what he was getting at was a configuration to let the instance select which of the fullname or the username is favored, so that the operator doesn't have to change every single email to select which they would like (30+ email templates).

I think that's fair if Zenodo (or some vocal enough instance that helps maintain the InvenioRDM project) would like to keep username (it is secure afterall, and the pre-existing default approach). If they are fine with switching to full name (no objection from others) we could indeed make the fullname the value without config (just because having a config for every small thing can get exhausting to maintain and navigate). So I am 75% in favor of the new config given the scope of work it would save for folks who would like to keep username.

Let's have @slint double-check though 😁

fenekku reviewed Sep 24, 2026

View reviewed changes

fenekku mentioned this pull request Sep 25, 2026

Backport: display full name instead of username on community actions inveniosoftware/invenio-rdm-records#2461

Open

edivalentinitu force-pushed the mail-template-name branch 4 times, most recently from 9ecb245 to dba6b7d Compare September 29, 2026 11:12

edivalentinitu added 2 commits October 15, 2026 15:08

fix(mail): display full name instead of username on request comments

c57757b

* if available, choose the full name from the user profile to render the comment event generated email, then fallback to username

fix(mail): generic notification instead of created_by dict

afc2925

* event.created_by is a dict so there is no sense in using it as a last fallback if the username and full name are missing. * if the event_creator_name is undefined, render a subject- less message

edivalentinitu force-pushed the mail-template-name branch 2 times, most recently from f148f94 to 58136ff Compare October 15, 2026 15:43

fix(mail): jinja user name helper method

d8a40a8

* move the logic that determines the user name shown in mail notifications in a python helper file and register it in the extension so we can use it in the jinja template

edivalentinitu force-pushed the mail-template-name branch from 58136ff to d8a40a8 Compare October 15, 2026 16:00

fenekku approved these changes Oct 16, 2026

View reviewed changes

fenekku reviewed Oct 16, 2026

View reviewed changes

fenekku self-requested a review October 16, 2026 14:54

fenekku reviewed Oct 23, 2026

View reviewed changes

fenekku self-requested a review October 23, 2026 15:34

	or invenio_request_event.created_by.username
	or (invenio_request_event.created_by.username and "@" ~ invenio_request_event.created_by.username)

Conversation

edivalentinitu commented Sep 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ciurana-life Oct 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mesemus commented Oct 6, 2026

Uh oh!

fenekku left a comment

Choose a reason for hiding this comment

Uh oh!

fenekku left a comment

Choose a reason for hiding this comment

Uh oh!

fenekku left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

edivalentinitu commented Sep 11, 2026 •

edited

Loading

ciurana-life Oct 7, 2026 •

edited

Loading