@Findus23 opened this Issue on August 12th 2022 Member

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).

@MatomoForumNotifications commented on August 12th 2022

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

@sgiehl commented on August 16th 2022 Member

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.

@jane-twizel commented on August 16th 2022

@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.

@Javi-Ormaechea commented on August 19th 2022

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

@peterhashair commented on August 25th 2022 Contributor

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.

@sgiehl commented on August 25th 2022 Member

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.

@peterhashair commented on August 28th 2022 Contributor

@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.

@bx80 commented on August 29th 2022 Contributor

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

@justinvelluppillai commented on August 29th 2022 Member

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

@peterhashair commented on September 6th 2022 Contributor

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.

@tsteur commented on September 6th 2022 Member

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?

@peterhashair commented on September 6th 2022 Contributor

thanks @tsteur another database field makes more sense.

@peterhashair commented on September 19th 2022 Contributor

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 commented on September 19th 2022 Contributor

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?

@tsteur commented on September 19th 2022 Member

@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?

@peterhashair commented on September 22nd 2022 Contributor

@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

@tsteur commented on September 22nd 2022 Member

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).

@peterhashair commented on September 22nd 2022 Contributor

@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?

@tsteur commented on September 25th 2022 Member

@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)

Powered by GitHub Issue Mirror