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 whitelist configuration on bruteforce_configuration.rst #11347

Merged
merged 1 commit into from
Dec 4, 2023

Conversation

kesselb
Copy link
Contributor

@kesselb kesselb commented Dec 2, 2023

☑️ Resolves

🖼️ Screenshots

image

- Login as admin and go to Administration settings -> Security

Note that any excluded IP address can perform authentication attempts without any throttling.
Its best to exclude as few IP addresses as you can, or even none at all.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are issues with proxies mentioned on this page and that instead of excluding a (reverse) proxy it should be configured as trusted proxy?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, it's mentioned in the troubleshooting section below: https://docs.nextcloud.com/server/latest/admin_manual/configuration_server/bruteforce_configuration.html#troubleshooting

Shall we flip troubleshooting and exclude?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah I would say so

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't know the exact syntax atm, but maybe we make this a warning box as well

Suggested change
Its best to exclude as few IP addresses as you can, or even none at all.
.. warn::
Its best to exclude as few IP addresses as you can, or even none at all.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the good advice.
Reordered the content and updated the screenshot.

Signed-off-by: Quentin Dupont <[email protected]>
Signed-off-by: Daniel Kesselberg <[email protected]>
@nickvergessen nickvergessen merged commit e32a54c into master Dec 4, 2023
9 checks passed
@nickvergessen nickvergessen deleted the patch-2 branch December 4, 2023 18:40
@nickvergessen
Copy link
Member

/backport to stable28

@joshtrichards
Copy link
Member

Thanks @quentinDupont and @kesselb for taking the initiative on this! 😎 Comes up often enough. :)

@nickvergessen
Copy link
Member

Comes up often enough. :)

And should basically never be the solution. In most cases you are doing something else wrong, see the warning box.

@joshtrichards
Copy link
Member

Indeed. I think making the RP/trusted_proxies/forward_for_headers interaction clearer by bumping it above the troubleshooting section and making it more "front and center" - as you suggested/this PR does - should help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants