Skip to content

fix: omit "Access rules" directives if no rules configured#5543

Open
Matthew-Kilpatrick wants to merge 1 commit into
NginxProxyManager:developfrom
Matthew-Kilpatrick:fix-deny-all-on-no-access-rules
Open

fix: omit "Access rules" directives if no rules configured#5543
Matthew-Kilpatrick wants to merge 1 commit into
NginxProxyManager:developfrom
Matthew-Kilpatrick:fix-deny-all-on-no-access-rules

Conversation

@Matthew-Kilpatrick
Copy link
Copy Markdown

Why

When an access list was associated with a template which had users (items) but no rules (clients), a deny all directive was inserted to the config. This resulted in all requests, including those with valid credentials, being rejected due to the lack of any allow directive.

This PR wraps the access rule configuration inside of an if block, so the deny all; directive is only present when at least one rule is configured.

The Nginx documentation confirms that the deny directive is relevent for client address blocking, and not authentication:

Denies access for the specified network or address. If the special value unix: is specified (1.5.1), denies access for all UNIX-domain sockets.
https://nginx.org/en/docs/http/ngx_http_access_module.html

Closes #5287

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Code refactoring
  • API changes
  • Performance improvement
  • Test addition or update

AI Usage

  • AI was used to write this
  • AI was used to review this

When an access list was associated with a template which had users (items) but no rules (clients), a `deny all` directive was inserted to the config. This resulted in all requests, including those with valid credentials, being rejected due to the lack of any `allow` directive.

This commit wraps the access rule configuration inside of an if block, so the `deny all;` directive is only present when at least one rule is configured.
@nginxproxymanagerci
Copy link
Copy Markdown

Docker Image for build 1 is available on DockerHub:

nginxproxymanager/nginx-proxy-manager-dev:pr-5543

Note

Ensure you backup your NPM instance before testing this image! Especially if there are database changes.
This is a different docker image namespace than the official image.

Warning

Changes and additions to DNS Providers require verification by at least 2 members of the community!

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.

Bug Report: deny all; directive persists in proxy host configuration

1 participant