akaunting / laravel-firewall

Web Application Firewall (WAF) package for Laravel
https://akaunting.com
MIT License
943 stars 106 forks source link

Allow Multiple Whitelist IPs #58

Closed carlos-reynosa closed 1 year ago

carlos-reynosa commented 2 years ago

Fixes #57 Allow multiple whitelist IPs to be able to be defined within the Whitelist IP environment variable.

otilor commented 2 years ago

Kindly append tests to this PR as that helps to ensure everything works smoothly as possible. Thank you. It also helps us understand better what you're trying to do.

otilor commented 2 years ago

It is expected you pass you IPs in this format 127.0.0.1:192.168.0.1

'whitelist' => [env('FIREWALL_WHITELIST', '')],

The above line then parses it into this

['127.0.0.1:192.168.0.1']

The below is a code from a Symfony helper method. Checkout this method

/**
     * Checks if an IPv4 or IPv6 address is contained in the list of given IPs or subnets.
     *
     * @param string       $requestIp IP to check
     * @param string|array $ips       List of IPs or subnets (can be a string if only a single one)
     *
     * @return bool Whether the IP is valid
  */
public static function checkIp($requestIp, $ips)
    {
        if (!\is_array($ips)) {
            $ips = [$ips];
        }

        $method = substr_count($requestIp, ':') > 1 ? 'checkIp6' : 'checkIp4';

        foreach ($ips as $ip) {
            if (self::$method($requestIp, $ip)) {
                return true;
            }
        }

        return false;
    }

Nothing is broken.

carlos-reynosa commented 2 years ago

Providing the list of IPs delimited by a ":" within the FIREWALL_WHITELIST environment variable does not work. The process does not parse out each IP and does not create an array of IPs for the function checkIp to check individually. Thus if you provide more than one IP it fails. It treats the value "127.0.0.1:192.168.0.1" as one IP when it should be treated as two IPs.

The pull request i provided makes it so that the environment variable can be a list of IPs delimited by some character (",") and results in an array of IPs being passed into the checkIp function.

My expectation was that we should be able to provide multiple IPs through the FIREWALL_WHITELIST environment variable.

carlos-reynosa commented 2 years ago

I added a test case that uses the value you're saying should work. It should actually fail. Maybe that might help illustrate what i'm seeing.