Watcher icon indicating copy to clipboard operation
Watcher copied to clipboard

[Improvements] Suggesting some improvements

Open danieleperera opened this issue 3 years ago • 4 comments

First of all thanks for creating and sharing such a great project. 💯

I have some suggestions regarding the code and the documentation. For the documentation it would be useful if you can add docker and docker-compose version which are compatible and used by your project.

Regarding the code: Django apps contains core.py maybe it's better to use a different filename as 'core' in Django also refers to builtin core modules of Django. I'd suggest services.py or utils.py it would make the code more clear and readable.

Regarding the threats_watcher app why are you saving only tokenized words in TrendyWord and not the complete post content? IMHO it's more useful to have the complete content of a post.

While I was checking your code I found that RSS-Bridge is used for Twitter. If the docker is used only for Twitter maybe you can use some tool like https://github.com/InQuest/ThreatIngestor which can extract and aggregate threat intelligence from RSS feeds, Twitter, Pastebin and other sources. This would smoother more your project as a docker image like RSS-Bridge is overkill if only used for twitter.

In the site_monitoring app's model rtir – Identification number of RTIR should be a non required field as some people might not have https://github.com/bestpractical/rt RTIR platform configured o in place. Also the Expiry Date field in some cases is not the best option as you might want to monitor a certain domain until it's takendown and you don't know the exact date when this will happen at first.

Screenshots: In site_monitoring and in dns_finder it's an awesome feature if you can take screenshots of domain and also check for changes of the image. You could use https://hub.docker.com/r/scrapinghub/splash to take screenshots and https://pypi.org/project/ImageHash/ to check how the image was changed.

Why are you using MySQL as the DB? There are some other solutions like PostgreSQL which has builtin ArrayFields and JSONFields which could ease some model implementations.

Hope this helps.

danieleperera avatar Dec 30 '20 09:12 danieleperera

Thank you for your support! 💯

  • The project is compatible with latest Docker & docker-compose version (We will add this to the documentation).

  • We will consider to change core.py by services.py.

  • We are not only saving TrendyWord. We save source post urls related to each TrendyWord. In fact, when you click on a new TrendyWord you have direct access to the source posts where the TrendyWord was found by Watcher.

  • Rss-Bridge is, by default, configured with Twitter only, but users can use it for all other sources like: Facebook, DuckDuckGo, GoogleSearch...

    • To do such you need to add the new bridge needed in the Watcher/Rss-bridge/whitelist.txt file (We will add more documentation on that).
  • We have planned to add a default RTIR number.

    • Expiry Date is not mandatory when you add a new DNS to monitoring.
  • We have planned to add screenshot support in the future. Thanks for the link to Splash. 👍

    • We are already monitoring website content with TLSH which is a fuzzy hash algorithm.

Thanks for your contribution mate. This helps!

Felix83000 avatar Jan 04 '21 17:01 Felix83000

In the https://github.com/Felix83000/Watcher/blob/master/Watcher/Watcher/site_monitoring/core.py

Instead of using a lot of If cases you could use something like.

def create_alert(id, site, new_ip, new_ip_second, score):
    ...
    ALERT = {     
            1:message_ip,    
            2:message_ip,    
            3:message_ips,    
            4:message_web,    
            5:message_web_ip,    
            6:message_web_ip,    
            7:message_web_ip,    
            8:message_mail    
            9:message_mail_ip,    
            10:message_mail_ip,    
            11:message_mail_ip,    
            12:message_mail_web,    
            13:message_mail_ip_web,    
            14:message_mail_ip_web,    
            15:message_mail_ip_web,     
        } 
if site.monitored:
    type_ = ALERT[id]
    try:
         new_alert = Alert.objects.create(site=site,
                                             type=type_,
                                             new_ip=new_ip or None,
                                             new_ip_second=new_ip_second or None,
                                             old_ip=site.ip or None,
                                             old_ip_second=site.ip_second or None)
        ...
    except Exception:
        ...

what do you think?

danieleperera avatar Jan 04 '21 21:01 danieleperera

Great idea! May you pull request your work? Thanks 👍

Felix83000 avatar Jan 05 '21 16:01 Felix83000

I could but the Exception handling is not very clear maybe it's better if you push this improvement.

danieleperera avatar Jan 05 '21 18:01 danieleperera