Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(auth): improve slack messages #281 #287

Merged
merged 5 commits into from
Sep 3, 2021

Conversation

caebwallace
Copy link
Contributor

Auth handler improvements.

Description

Add a visual icon to authentication slack messages (for success or fail) and the client IP.
Generate a log file accessible to the server that log auth attempts and allow to ban hosts that try to brute force auth.

Related Issue

#281

Motivation and Context

As I've installed the bot on a VPS with a protection password, I start to have someone that try to login to the bot.
To avoid that, I've made changes to log auth attempts and make a fail2ban jail to ban bad boys.

How Has This Been Tested?

It's actually installed on my VPS (and works fine)

Extras

I've prepared a configuration variable with the name of the bot, that can be used in the code (instead of typing Binance Trading Bot everytime we need it), in the future be used for all the product and why not be editable from config modal directly.

@chrisleekr
Copy link
Owner

Hey @caebwallace

Thanks for your contribution.

I will take a look and merge it in after my PR.

@habibalkhabbaz
Copy link
Contributor

habibalkhabbaz commented Aug 30, 2021

It looks like a great enhancement!
I think we can move the installation part of fail2ban to docker-compose file. It make sense isn't?

@caebwallace
Copy link
Contributor Author

I use a vps for yet with haproxy as a load balancer + SSL certificate, but could be a good idea to integrate it directly in the docker-compose stack 👍

@chrisleekr
Copy link
Owner

chrisleekr commented Aug 31, 2021

Hey @caebwallace

I pushed some refactored code to your branch.

I managed to use dockerised fail2ban to put in docker-compose stack.

It did ban as per screenshots:
image

And it did add rules to iptrables:
image

However, for some reason, I still can access the frontend.

As I am not an expert on the network, I will need some more to dig into why it's not blocking me.

I used this docker-image https://github.com/crazy-max/docker-fail2ban

@habibalkhabbaz
Copy link
Contributor

Hi @chrisleekr
I am not an expert on the network too, but maybe adding the networks attribute with - internal value will solve the issue?

@caebwallace
Copy link
Contributor Author

Yes, both should be on the same network.
I'm at work now, will check tonight :)

@radumalica
Copy link

You created the f2b chain and added rules but the chain you have created must be added to the main input chain with accept rule and the packets will hit the f2b chain, reject what it needs and return to the input chain.

@chrisleekr
Copy link
Owner

chrisleekr commented Sep 1, 2021

I've been thinking about this.

There are many traders running the bot with Windows which does not have fail2ban. In that case, this change may not effective to them.

So alternatively, I think this would be good to prevent the brute force attack - https://github.com/animir/node-rate-limiter-flexible/wiki/Overall-example#minimal-protection-against-password-brute-force

What do you think?

@caebwallace
Copy link
Contributor Author

caebwallace commented Sep 1, 2021

Using a nodejs library instead of a third party app is generally better, because you've a lot of flexibility (you can use already made notification events).
I'm in for going in that direction by using node-rate-limiter in the auth handler.
We can keep my changes on slack messages + clientIp only for that issue, and open a new one for limiting brute force attempts ?

@chrisleekr
Copy link
Owner

@caebwallace

We can keep my changes on slack messages + clientIp only for that issue, and open a new one for limiting brute force attempts ?

Sounds good!

@caebwallace caebwallace changed the title feat(auth): improve slack messages + add fail2ban logfile #281 feat(auth): improve slack messages #281 Sep 1, 2021
@radumalica
Copy link

@chrisleekr as this PR is about improving slack messages, I took the time to study on Apprise as you suggested in @pedrohusky 's pull request, and although it's written in python, it also provides a REST API which can store Keys for more than 60 messaging services, and it deploys in docker container. Then, you can simply remove altogether the slack code from the bot, add this docker container to your compose file and instruct users how to add their keys for what notification services they want to configure.

The trivial steps would be:
/add to API to add your key for service X
/notify using the specific URL for that service appending the key ID you added before and that's it.

here is the complete information: https://github.com/caronc/apprise-api

@chrisleekr
Copy link
Owner

@radumalica

Thanks for the suggestion.

I think that change will be done later with this issue - #106

@chrisleekr chrisleekr merged commit f53033e into chrisleekr:master Sep 3, 2021
@caebwallace
Copy link
Contributor Author

No gf tonight :)
Have you started to code with node-rate-limiter or I can take that development ?

@chrisleekr
Copy link
Owner

chrisleekr commented Sep 3, 2021

Have you started to code with node-rate-limiter or I can take that development ?

Yes, I started. I will ask you to review later.

@caebwallace caebwallace deleted the feat/issue-281 branch September 4, 2021 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants