Client Access Lists

[Indemnity83]

This PR adds client (IP address) based access control to the application in a way that aims to cover the 80% case based on #356

There are a few significant outstanding tasks remaining to make this ready for showtime.

• Address validation needs to be expanded to support IPv4, IPv6, CIDR notation and the 'all' keyword
• The backend needs to validate that either an Authorization or Access rule has been provided
• The Nginx config needs to actually reflect the access list that is defined
• Write tests

[jc21]

Docker Image for build 2 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-360

[jc21]

Thanks for doing the hard work here, greatly appreciated. I'm using your code and I get the following error on save:

[jc21]

Docker Image for build 3 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-360

[Indemnity83]

@jc21 happy to do it, I would really benefit from the feature. I intentionally marked this PR as WIP because there are a couple of fairly significant items that need to be finished to make this complete. You've hit on one of them.

I'm not sure what the best way to handle that one is either. It doesn't look like the JSON API spec supports multiple validation formats? I could do it as one big regex, but that would get really ugly really fast because valid data could be an IPv4 address, IPv6 address or a CIDR notation of either.

I didn't see an example in the codebase of doing validation in what I'll call the controller either? I may just have not known where to look. If you can provide an example of doing validation outside of the JSON schema I'll be happy to implement it.

[jc21]

So this line here is where the JSON payload is validated in the API Handler:

https://github.com/Indemnity83/nginx-proxy-manager/blob/ip-access-control/backend/routes/api/nginx/access_lists.js#L59

And this is the json schema validation for that one as you already know:

https://github.com/Indemnity83/nginx-proxy-manager/blob/ip-access-control/backend/schema/endpoints/access-lists.json#L72

for the address field, yes you can have multiple validations like so:

    "address": {
      "oneOf": [
        {
          "type": "string",
          "format": "ipv4"
        },
        {
          "type": "string",
          "pattern": "^regexHere$"
        },
        {
          "type": "string",
          "pattern": "^anotherRegexHere$"
        }
      ]
    },

I wouldn't worry about validating anywhere else, as the incoming data is guaranteed to be valid at this point.

[Indemnity83]

Perfect, the oneOf directive was what I didn't know existed :)

Is there something similar to make sure that either an "item" or "client" is provided? I have them both set as "minItems": 0 right now. The frontend checks that at least one is provided before submitting, but it should be checked in the backend too.

[jc21]

AJV is the validation library I'm using and it has some great simple documentation for your options: https://github.com/epoberezkin/ajv#validation-keywords

Yes you can make sure that either one of them is completed in the json schema, but the only way to do it is really stupid and you have to wrap what you have in a "oneOf" array and duplicate a lot of the json schema as is. So I don't recommend that. Instead I would just have a quick check here:

https://github.com/Indemnity83/nginx-proxy-manager/blob/ip-access-control/backend/internal/access-list.js#L25

to make sure that the length of the items in the data are > 0 in one of those places.

[jc21]

ie:

	create: (access, data) => {
		return access.can('access_lists:create', data)
			.then((/*access_data*/) => {
				if ((typeof data.items === 'undefined' || !data.items.length) && (typeof data.clients === 'undefined' || !data.clients.length)) {
					throw new error.InternalValidationError('At leaste one user/pass or address must be defined');
				}
				
				// ...
			}

[jc21]

Docker Image for build 4 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-360

[jc21]

Docker Image for build 5 is available on DockerHub as jc21/nginx-proxy-manager:github-pr-360

[Indemnity83]

This should be fully functional now.

It could use a pretty though peer review as I'm sure there is some refactoring that could take place. in particular, this expansion feels gross but I don't see a cleaner solution.

https://github.com/jc21/nginx-proxy-manager/blob/f5ee91aeb3756b824aea32db8bfe72f8d4bb8604/backend/internal/access-list.js#L72-L75

[jc21]

Awesome. Just verified it works as expected! I'll be able to fix up any "mess" later. Thanks heaps for the contribution!

[Thijmen]

@Indemnity83 would it also be possible with this feature to allow the IP's in the list and if the IP is not in the list, provide the HTTP Auth?

[Indemnity83]

That’s exactly what the “satisfy any” option does.

[Thijmen]

Totally missed that, thanks! Great addition to the repo, awesome work man!