portier / portier-broker

Portier Broker reference implementation, written in Rust
http://portier.github.io/
Apache License 2.0
556 stars 17 forks source link

Allow customizable email subjects #230

Open dstaley opened 3 years ago

dstaley commented 3 years ago

Currently, Portier emails use the following subject:

Finish logging in to {{ display_origin }}

Generally, this is fine since display_origin typically matches the URL of the service the user is trying to sign into. However, this isn't always the case, especially if the app is using a subdomain for handling the authentication process. For example, instead of seeing a subject like this:

Finish logging in to https://getunitrack.com

my users would see this:

Finish logging in to https://portier.getunitrack.com

As a good first step, it would be nice to be able to provide a naive override for the email subject, one that doesn't contain any dynamic data.

stephank commented 3 years ago

A secure way to do this is definitely nice, but I'm wondering how we'd do that. I'm not sure what kind of bad things a malicious RP could do if we simply let the RP override this in its call to /auth.

The least problematic way would be to do this in configuration on the broker side, but then customising the templates would require a private broker instance. I think this is already possible per-install by simply modifying the files in tmpl/?

Would it be sufficient for your use case if we simply extended this to be per-origin? So that you can have multiple tmpl directories, then somehow connect origins to the right template directories in config.toml.

Or is a private broker not an option? Then we'd have to figure out some protocol the (public) broker can follow to discover strings/templates, and also somehow make sure it's tamper proof.

dstaley commented 3 years ago

The least problematic way would be to do this in configuration on the broker side

Complete agree, and I should have been more clear in my description that this is the solution I'm looking for.

I think this is already possible per-install by simply modifying the files in tmpl/?

These lines would imply that's not the case. At least based on these lines, it looks like the subject will always include the display origin. I think the easiest solution would be to provide an configuration variable that completely overrides the subject. While something that could interpolate values would be nice, I think it might be too much unnecessary complexity (at least to start with).