pentacent / keila

Open Source Newsletter Tool.
https://keila.io
GNU Affero General Public License v3.0
1.33k stars 69 forks source link

Adding User-Agent filtering #164

Closed panoramix360 closed 1 year ago

panoramix360 commented 1 year ago

Adds User-Agent filtering to open links accordingly to this blog post.

I used the follow User-agent for the filter: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/42.0.2311.135 Safari/537.36 Edge/12.246 Mozilla/5.0

@wmnnd, I'm creating the PR before completing the tests so you can take a look beforehand :smile:

Please let me know what you think.

wmnnd commented 1 year ago

Awesome, this is looking great! I’m just wondering whether we might want to store the User-Agent alongside the event, in order to potentially find further bot opens. But on the other hand, that is a slight privacy compromise. What do you think?

panoramix360 commented 1 year ago

Hmm, understood.

Do you mean saving the event plus the user-agent used for opening the link?

Do you think this is a breach of privacy? I was searching about it and found that WordPress does this too if I'm right hehe

I'm okay with saving it for now. Are you thinking of using it to try to detect more bots in the future, using recurrence or something like that?

panoramix360 commented 1 year ago

@wmnnd I'm finishing the tests and I notice that the documentation for some methods seems a little odd.

The @doc for the open event mentions the link_id as optional, but in the method, there is no link_id or it should be?

def track(:open, %{
        encoded_url: encoded_url,
        recipient_id: recipient_id,
        hmac: hmac,
        user_agent: user_agent
      }) do
    case verify_hmac(hmac, encoded_url, recipient_id) do
      :ok ->
        unless is_user_agent_bot(user_agent) do
          set_recipient_opened_at(recipient_id)
        end

        {:ok, URI.decode_www_form(encoded_url)}

      :error ->
        :error
    end
  end
wmnnd commented 1 year ago

@panoramix360 You’re right, link_id is not used by the open event.

panoramix360 commented 1 year ago

@wmnnd I finished creating the tests for the tracking_controller :tada:

Can you look through my previous message?

Hmm, understood.

Do you mean saving the event plus the user-agent used for opening the link?

Do you think this is a breach of privacy? I was searching about it and found that WordPress does this too if I'm right hehe

I'm okay with saving it for now. Are you thinking of using it to try to detect more bots in the future, using recurrence or something like that?

Do you want me to do that on this PR?

wmnnd commented 1 year ago

Thank you for your contribution, @panoramix360! The PR looks good and it's a great addition to Keila to filter out those requests :+1: