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

[Enhancement]: DNN behind Cloudflare - HTTP_X_FORWARDED_FOR #6145

Open
3 of 4 tasks
fablaser opened this issue Oct 1, 2024 · 16 comments
Open
3 of 4 tasks

[Enhancement]: DNN behind Cloudflare - HTTP_X_FORWARDED_FOR #6145

fablaser opened this issue Oct 1, 2024 · 16 comments

Comments

@fablaser
Copy link

fablaser commented Oct 1, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Description of problem

Hello, I have some websites behind Cloudflare's WAF. I currently use Request.ServerVariables("HTTP_X_FORWARDED_FOR") for my own applicatoin log. So I can log real IP address of clients, not just Cloudflare ip addresses. It would be useful to have an option that allows DNN to log IP using this variable.

Description of solution

implement Request.ServerVariables("HTTP_X_FORWARDED_FOR")

Description of alternatives considered

No response

Anything else?

No response

Do you be plan to contribute code for this enhancement?

  • Yes

Would you be interested in sponsoring this enhancement?

  • Yes

Code of Conduct

  • I agree to follow this project's Code of Conduct
@jeremy-farrance
Copy link
Contributor

Related? to #5073 ?

This sounds like an enhancement that a lot of people would appreciate. Well, at least me!

@valadas
Copy link
Contributor

valadas commented Oct 1, 2024

@fablaser can you try setting it here and see if that works ?

image

@fablaser
Copy link
Author

fablaser commented Oct 2, 2024

@fablaser can you try setting it here and see if that works ?

Hello,
I tried to follow the configuration, but I does not work... After changed it I tried to clear cache and recycle app pool. Anyway any operation (such as failed login) will be logged with Cloudflare ip address... I also tried "x-forwarded-for" variable.

@fablaser
Copy link
Author

fablaser commented Oct 2, 2024

Related? to #5073 ?

This sounds like an enhancement that a lot of people would appreciate. Well, at least me!

Yes, it is related to it! I did not noticed this previous request

@valadas
Copy link
Contributor

valadas commented Oct 2, 2024

Just reading some code, it looks like one could add an entry in the HostSettings table to specify that header

https://github.com/dnnsoftware/Dnn.Platform/blob/9f4833f5dcba24e0dbd194338c2a50a03a43da83/DNN%20Platform/Library/Services/UserRequest/UserRequestIPAddressController.cs#L27C1-L28C1

@fablaser
Copy link
Author

fablaser commented Oct 3, 2024

Hello
thank you for your support.

I tried to add a new record in HostSettings table with these values:

SettingName: UserRequestIPHeader
SettingValue: X-Forwarded-For

Then I cleared cache and recycled app pool, but the result is always the same: every failed login, in logs, has the cloudflare ip address...

@fablaser
Copy link
Author

fablaser commented Oct 3, 2024

Just reading some code, it looks like one could add an entry in the HostSettings table to specify that header

https://github.com/dnnsoftware/Dnn.Platform/blob/9f4833f5dcba24e0dbd194338c2a50a03a43da83/DNN%20Platform/Library/Services/UserRequest/UserRequestIPAddressController.cs#L27C1-L28C1

Hello,
I simply needed to write "X-Forwarded-For" in lower case!
Thanks!!

@jeremy-farrance
Copy link
Contributor

Hello,
I simply needed to write "X-Forwarded-For" in lower case!

Wait, what? Are you saying,

  • in code you need it lower cased... Request.ServerVariables() or

  • the Value it the SQL table HostSettings

needed to be lowercase?

@valadas
Copy link
Contributor

valadas commented Oct 3, 2024

That is surprising, they document it with capitals https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#x-forwarded-for

@fablaser
Copy link
Author

fablaser commented Oct 3, 2024

Wait, what? Are you saying,
* in code you need it lower cased... Request.ServerVariables() or
* the Value it the SQL table HostSettings

needed to be lowercase?

In SQL HostSettings table. When DNN launces this code:

 var userRequestIPHeader = HostController.Instance.GetString("UserRequestIPHeader", "X-Forwarded-For");
 var userIPAddress = string.Empty;
 if (request.Headers.AllKeys.Contains(userRequestIPHeader))

AllKeys.Contains(userRequestIPHeader)) does not matches "X-Forwarded-For", it is seems to be case sensitive.

@fablaser
Copy link
Author

fablaser commented Oct 3, 2024

That is surprising, they document it with capitals https://developers.cloudflare.com/fundamentals/reference/http-request-headers/#x-forwarded-for

Perhaps it should be useful to change the code using Contains(userRequestIPHeader, IEqualityComparer) so that it would be case insensitive...

@valadas
Copy link
Contributor

valadas commented Oct 3, 2024

Oh, I see, so you went from X-FORWARDED-FOR to X-Forwarded-For right ?

@ronnydodd
Copy link

ronnydodd commented Oct 3, 2024 via email

@valadas
Copy link
Contributor

valadas commented Oct 3, 2024

@ronnydodd I did not email you, this is a reply to a github issue, not sure how but you are somehow following this issue. Please go to #6145 and unsubscribe from it if you don't want these emails.

@fablaser
Copy link
Author

fablaser commented Oct 3, 2024

Oh, I see, so you went from X-FORWARDED-FOR to X-Forwarded-For right ?

In my own code I used X-FORWARDED-FOR, but it does not matter if I use request.Headers['X-FORWARDED-FOR'], request.Headers['X-Forwarded-For'] or request.Headers['x-forwarded-for'], they will work fine.

The issue has been caused by "request.Headers.AllKeys.Contains(userRequestIPHeader)" in DNN code you mentioned yesterday. "Contains" method is case sensitive. In my header's collection I see "x-forwarded-for" (lower), so I need to use this string in hostsettings table.

So, the line
if (request.Headers.AllKeys.Contains(userRequestIPHeader))

should be modified in order to use Request.Headers.AllKeys.Contains(userRequestIPHeader, iEqualityComparer) and make the comparison caseinsensitive.

@MarietteNL
Copy link

I use x-forwarded-for in the Hostsettings table and that works fine. All small letters

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

No branches or pull requests

5 participants