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

Get the ip when the user is using a proxy #504

Merged
merged 8 commits into from
Mar 25, 2021

Conversation

DeveloperMarius
Copy link
Contributor

Hello,

multiple sources also use the HTTP_CLIENT_IP header with the HTTP_X_FORWARDED_FOR to check if the user is using a proxy.
Here is an explaination, what the difference is and here is an example that explains the headers.
But as pointed out in this comment and with some thinking back to our custom/ client headers with the http- prefix, the user is able to edit this header. This could lead to security issues. I know only some people can do this, but it is still an issue.

I added the cloudflare ip header to the non save headers, because a user can set this header when the website isn't using cloudflare and like the other two headers, submit any value he wants.

~ Marius

@skipperbent
Copy link
Owner

Nice find! We need all the headers we can get!

You added the PR just as I made some changes to the files. I'll make sure to add your changes. It's some really great findings that can maximize the support so it works for everyone.

However your request gave me an idea. Instead of checking for multiple headers manually, why not just let the Request class handle it.

I simplified the getIp method, but it's confusing for someone that doesn't know whats going on (it's using the others as default value).

/**
     * Get id address
     * @return string|null
     */
    public function getIp(): ?string
    {
        return $this->getHeader(
            'http-cf-connecting-ip',
            $this->getHeader(
                'http-x-forwarded-for',
                $this->getHeader('remote-addr')
            )
        );
    }

Instead i'm create a new method getHeaderFromArray that will take an array of headers and return the first one it finds.

$request->getHeaderFromArray([
   'http-x-forwarded-for',
   'remote-addr', //etc
]);

@DeveloperMarius
Copy link
Contributor Author

DeveloperMarius commented Mar 22, 2021

Sounds great, but don't forget to add the validation and the $safe parameter before the ip is returned^^

@skipperbent
Copy link
Owner

Sounds great, but don't forget to add the validation and the $safe parameter before the ip is returned^^

Yeah, i'm guessing all the http- variants of the IP can be easily spoofed since they don't originate from the server.

Thanks for the PR this is definitely some useful info!

@DeveloperMarius
Copy link
Contributor Author

Should I create that function or are you already on it?

@skipperbent
Copy link
Owner

Should I create that function or are you already on it?

It's already made but not yet committed :)

@DeveloperMarius
Copy link
Contributor Author

Okay, perfect!

@DeveloperMarius
Copy link
Contributor Author

Okay, I updated it and the function now uses the new getHeader syntax, Can you pease commit the getHeaderFromArray() fucnction or shoud I create one?

@DeveloperMarius
Copy link
Contributor Author

Do you think we still could add this pull to 4.3.0.0 or is it better for you if we add it later?

~ Marius

@DeveloperMarius
Copy link
Contributor Author

Ready to merge^^

@skipperbent
Copy link
Owner

I've checked out your branch but i cant remember how I push to this pull request.. do you by any change remember?

@skipperbent skipperbent merged commit 11313a3 into skipperbent:v4-development Mar 25, 2021
@DeveloperMarius
Copy link
Contributor Author

I have never worked with pull requests before and never as a project owner...
Did you found a way?

@skipperbent
Copy link
Owner

Think i figured it out. I have to add your repository to my git config and then push it to your branch/pull request.

But back in the day github had a nice little instruction in the bottom on how to push changes with copy-paste examples.

@DeveloperMarius
Copy link
Contributor Author

@DeveloperMarius DeveloperMarius deleted the get-ip-proxy branch November 15, 2022 09:21
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