issue with link and redirect

[ivang76]

Hi there,

I’ve been using your package for a while without any issues, on a Laravel 9 and PHP 8.1 environment.
Now I’ve updated to Laravel 11, PHP 8.3 and the latest version of your package (previously it was the 6), and I’ve noticed that when a user follows a link received in the newsletter, they are no longer redirected to the target URL. Instead, they always fall back to the home page of the web app.

For example, if in the newsletter I have a link like this:
https://mainurl.something/email/n?l=https://www.github.com/jdavidbakr...

when the user clicks on it, they end up at https://mainurl.something instead to reach your github page.

Do you have any advice on what to check and debug? Have you encountered anything similar with this configuration?
thanks!

update:
I'm debbuging the linkClicked function in your MailTrackerController.php and I notice that the tracker->domains_in_context is empty so the flow fail in the return redirect(config('mail-tracker.redirect-missing-links-to') ?: '/')
if I force a return redirect($url); the behaviour is correct.

I read something about the domains_in_context added to the SentEmail in a previous PR #248 by @mirkos93. In my project I needed to anonymize the recipient_email in the SentEmail model, could this create any problems?

class SentEmail extends Model
{

    protected static function booted()
    {
        static::saving(function ($sentEmail) {
           ...
           ...
           $sentEmail->recipient_email = $anonymizeEmail;
        }
    }
}

[mirkos93]

Hi @ivang76
From a quick analysis, it would appear that the problem lies in the regex in Model/SentEmail.php in the getDomainsInContextAttribute() attribute.
Could you provide the part in your HTML pertaining to the href link? The only problem in the regex I am noticing might be that it does not read single quotes. A test I recommend you do to remove this doubt is to use regex101.com to test HTML with regex.

[ivang76]

thanks for the prompt reaction @mirkos93

I'm checking the regex in the function you pointed out, to debug the regex, but I'm noticing a strange thing... the
$this->content when I click on a link of a newsletter for example, is empty... so of course the $matches[2] will be empty.
Any ideas?

(at the moment as I said I'm forcing that return redirect($url); to keep the whole project working as done until now)

[mirkos93]

Ciao @ivang76
have you by any chance already checked whether the whole $tracker variable is null? The problem is probably related to $hash not being found.
If $tracker is null and if you have a chance to do more tests, you should check if and how $hash is being valued and if this is found in the database.

If instead only $this->content is null (and thus is null or empty in the database as well) you probably need to take a closer look at the code where you anonymize the email addresses, I'm noticing that in your code you're not extending the SentEmail model (which in turn implements the IsSentEmailModel trait), maybe that's where the problem lies.

[ivang76]

thanks again @mirkos93

so, the $tracker is not empty and I extended the SentMail wiith something like this:

use jdavidbakr\MailTracker\Model\SentEmail as jdavidSentEmail;

class SentEmail extends jdavidSentEmail
{
  ...

but then I also removed temporary the anonymize function, but still it doesn't work.

I don't know, I'll try to do other debug but I'm a bit stuck now.

[ivang76]

@mirkos93 Indeed I'm noticing that the 'content' is null on all the newsletter sent, even in the older ones in the previous years.
Do you know where and how this field is set normally?
thanks

[ivang76]

I remembered, the content set to null is correct because stems from the log-content attribute in the configuration file, which I set to false years ago.
Because of this setting, your recent change for checking domains doesn't work, and now all the links have become unclickable. Could you please work on a fix for this?

[jdavidbakr]

You might be right, we may have to disable the click link tracker if you don't save the content since that was added for security reasons. If someone wants to submit a PR for that, please feel free to. I might be able to look at it but probably after the first of the year would be the soonest I'll get a chance.

[ivang76]

Hi @jdavidbakr I haven't yet found a proper solution and at the moment I'm still forcing in your MailTrackerController.php to return redirect($url); , in this way everything work as usual, but I'm wondering if you are planning to check this issue and maybe solve in a official patch.
Thanks!

[jdavidbakr]

I think the solution has to be to disable the link rewrite if the email content is not being saved. I'd be happy to accept a PR if you have time to look at it.

[ivang76]

ok @jdavidbakr, done, let me know. Thanks!
Ivan

[jdavidbakr]

Thanks for the PR, however that change defeats the purpose of why we're checking the sent mail in the first place - to prevent your site from becoming a relay for malicious links.

The solution needs to be for the tracker to not rewrite any links if the content is not being saved.

[ivang76]

Ok, I see... It would probably be quicker if you worked on a solution yourself at this point, since you know the project and the best approaches better. But if you don’t have time, I’ll give it another try. Could you point me in the right direction to avoid rewriting the link?

[Khuthaily]

@jdavidbakr thank you for this excellent package.
@ivang76, I ran into this problem, and here is how I worked around it (let's call it temporary fix).

• Override Model > SentEmail.php
• Change the method getDomainsInContextAttribute(); simply, you need to return an array of the domains for the URLs in your email.
• Example,

public function getDomainsInContextAttribute(){
    $domains = ['facebook.com', 'x.com', 'mydoamin.com', 'sub.mydoamin.com', 'example.com', 'en.example.com']; // modify as needed; I added subdomains in mine just in case
    return $domains;
}

Logic
A recent update in the package forces URLs to redirect to the home page (i.e., the value of 'redirect-missing-links-to' => '/' in the file config/mail-tracker.php if the domain of the URL is not whitelisted in getDomainsInContextAttribute(). It is an excellent approach for security; however, regex needs some improvements. I need to do more experiments, and if I succeed, I will submit a PR with improved regex rather than manually adding the domains.

[ivang76]

Hi @Khuthaily thanks for your contribute to this topic. Yes, I know about the domains returned by the getDomainsInContextAttribute, but since I'm using this package in a webapp that lets the editors create custom newsletter with a lot of news inside, it's really difficult to coverall the possible domains.

[Khuthaily]

@jdavidbakr & @ivang76 ... I have just submitted this PR.

[ivang76]

I quickly saw the PR @Khuthaily. It is definitely a useful step forward, so thank you! However, in cases like mine (no logged content and a list of possible domains that is too big and varied to be predefined by the developer), it would not fully fix the problem. Let’s see if @jdavidbakr can expand and improve the solution to cover all cases...