matomo icon indicating copy to clipboard operation
matomo copied to clipboard

Show user invite link when inviting user

Open Findus23 opened this issue 2 years ago • 15 comments

At the moment the only way to invite a user is by letting Matomo send an E-Mail to them. But a ton of people have broken E-Mail setups. Either their mail server isn't properly set up with DKIM, etc. and all mails are rejected as SPAM or they don't really have a mail server or they are using some wonky SMTP setup via gmail (which can break for all kinds of Google changes). Before that didn't really matter as unless you need E-Mail reports, you are only missing out on some things (mostly password reset and security notifications). But now with the invite feature Matomo becomes completely unusable unless your Matomo instance is able to successfully and reliably send E-Mails. And I am sure that will trip up and frustrate a lot of newer Matomo uses.

One simple solution would be to display the user invite link that is sent to the new user also to the admin when doing the invite. This also allows the admin to also send the link via other ways than E-Mail and allows people to easily circumvent the invite feature (assuming it works when already signed in).

Findus23 avatar Aug 12 '22 15:08 Findus23

This issue has been mentioned on Matomo forums. There might be relevant details there:

https://forum.matomo.org/t/4-11-0-cant-set-new-user-passwords-but-user-invite-emails-not-sent/46954/2

I would suggest to maybe add a new config flag for this. Having it enabled will show the invite links instead of sending an email at all. Having the possibility to show an invite link on all instances might otherwise make it a bit less secure I guess.

sgiehl avatar Aug 16 '22 10:08 sgiehl

@javi-ormaechea will look at this and come back with a design solution by end of tomorrow. @justinvelluppillai he will be in touch with questions.

jane-twizel avatar Aug 16 '22 23:08 jane-twizel

A solution was created for this issue. Please get in touch with me to get file, thanks! 🙂

Javi-Ormaechea avatar Aug 19 '22 02:08 Javi-Ormaechea

ah, I finished the UI, but I found some tech difficulty. Because once the token is generated (which saved hash string into the database), it can’t be reviewed again (one-way trip), only in the target emails. The only way to get the token again is to regenerate a new one.

discuss with @justinvelluppillai and @bx80, maybe we can merge the send invite and copy link to one button. Or add a new table structure to allow multiple tokens for one user ( which requires quite a few changes). Or save an un hashed token into the database.

peterhashair avatar Aug 25 '22 01:08 peterhashair

Personally I would suggest to give the admin the possibility to either send an email or to show the invite link. Maybe we could also add a config option that allows configuring which options are available. As viewing the invite link is a bit less secure, some people might want to disable it maybe.

Also I think it's totally fine if the token can only be seen once (if the UI explains that clearly). In case the user clicks the "resend" link, we could again give the possibility to either send a mail or view the invite link. But we need to ensure the UI makes it clear, that previously sent token will get invalid.

sgiehl avatar Aug 25 '22 07:08 sgiehl

@sgiehl that makes sense, @justinvelluppillai @bx80 I was wondering about a simple solution. once the admin creates or resends invites, it will show a green notification on the top, I was wondering if just add a copy link button in the notification.

peterhashair avatar Aug 28 '22 23:08 peterhashair

@peterhashair Maybe it would be good to discuss updating the original UI design with @Javi-Ormaechea?

bx80 avatar Aug 29 '22 00:08 bx80

yes good idea - @peterhashair can you please reach out to @Javi-Ormaechea to discuss the changed requirements so he can advise on UI?

justinvelluppillai avatar Aug 29 '22 00:08 justinvelluppillai

Hey @tsteur, just checking security here is that ok to store an invite token into the database without hashing it? Because we need to copy the invite link after the token has been generated.

peterhashair avatar Sep 06 '22 00:09 peterhashair

Good question @peterhashair . Ideally we would hash it and don't store it in plain text.

Is there a chance we could change it to "Generate a link" and mention that this will basically invalidate any previously sent link in an email? Then we wouldn't need to store this in plain text. Or maybe we could have two tokens? One from the email and one generated when clicking on "generate a link" or something?

tsteur avatar Sep 06 '22 00:09 tsteur

thanks @tsteur another database field makes more sense.

peterhashair avatar Sep 06 '22 01:09 peterhashair

As I go, I found the copy link won't always work, see screenshots, it won't work in Safari, etc, and also our UI test environment will show errors as well. I guess the reason is a web page could silently copy rm -rf / or a decompression bomb image to your clipboard.

  • Or we could make the copy link to generate a link, then show the generated link in UI.
  • Adding permission, but I can see some browsers support this, and some are not.
  • Or we could do like the image shows, shows an error notification then, the user could resolve the permission for this.

image

peterhashair avatar Sep 19 '22 01:09 peterhashair

Sorry to bother, @tsteur I got a security question here because we using javascript code navigator.clipboard.writeText to copy the invite js link. Then somehow if the client land on a malicious site after that, the site has code navigator.clipboard.readText which will read your invite link, would that be a security concern?

peterhashair avatar Sep 19 '22 04:09 peterhashair

@peterhashair that could be an issue indeed if another site could read that invite link. Is it possible to use an alternative like document.execCommand('copy'); or is there an issue with it?

tsteur avatar Sep 19 '22 05:09 tsteur

@tsteur I think this function is deprecated, personally, I think copying the password or token link is not a very good practice. Maybe We can either show the link as a string like AWS did or we could remove copy link, but add create a user back next to invite user, invite user will auto disabled when the email is not enabled in config.

image

peterhashair avatar Sep 22 '22 02:09 peterhashair

Interesting, then we might at some point also need to adjust other places where we are using this currently.

Note that I'm just seeing this clipboard API only works on HTTPS meaning we might still need to fallback to execCommand when someone is on HTTP.

I've just reproduced this and the browser does seem to ask you for permission when you want to readText. To lower the chances of an issue, could we maybe clear the value from clipboard after a while? although people might close the tab right after copying the link. Are there any ideas/ thoughts how we could make this more secure? Can we check if browser supports execCommand('copy') and use it as preferred choice and only fallback to clipboard if it's not? (don't know if we can find out if it's supported or not).

tsteur avatar Sep 22 '22 03:09 tsteur

@tsteur execCommand('copy') would be ideal, but I tried different ways, but seems like there is no way we can detect whether the browser support execCommand('copy') unless we make a list and keeps updating it, any suggestions?

peterhashair avatar Sep 22 '22 05:09 peterhashair

@peterhashair Maybe something like https://github.com/sudodoki/copy-to-clipboard/blob/master/index.js#L79-L83 could work (see below)? And it could be also be combined with if (document.queryCommandEnabled) {document.queryCommandEnabled("copy")}? If not available or if unsuccessful we could fallback to writeText?

if (document.queryCommandEnabled) { var isAvailable = document.queryCommandEnabled("copy")}
if (isAvailable && !document.execCommand) { ... }
try {
    var successful = document.execCommand("copy");
    if (!successful) {
      throw new Error("copy command was unsuccessful");
    }
    success = true;
  } catch (err) {
   // fallback
}

If this doesn't work then we may have to use writeText by default on HTTPS (and execCommand on HTTP)

tsteur avatar Sep 25 '22 19:09 tsteur