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

Add captcha whitelist made with redis #13

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

comendantmc
Copy link

Feel free to add it to your code if you want. Let me know if you need some adjustments to this PR

@Galster-dev
Copy link

Hello!

  1. What's the point of using redis for this?
  2. Whitelists should be used with both IP and nickname
  3. Isn't it already implemented here ?

@comendantmc
Copy link
Author

  1. I might have quite an unusual use case where an api adds players to the whitelist automatically and redis seemed like a good idea for least overhead realtime checking without needing to pool changes periodically or putting commands to remote console. Maybe it can be useful to other people as well as I read somewhere you're adding redis dependency anyway.
  2. I see the potential security risk here, but it only adds the whitelist for the captcha and not the other checks. It can probably be improved by adding rate limit to connections with the same nickname or/and remembering previous ip addresses of the user.
  3. As I said the already implemented one whitelists users from all the checks, but this one just disables the captcha

@Galster-dev
Copy link

Just to point out: I'm not a contributor of this project.
In my opinion "unusual" use cases should not modify the plugin that much. A good solution would be to make an API to allow extension plugins force skip some checks maybe?

@hevav

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.

None yet

2 participants