Closed Bug 409942 Opened 13 years ago Closed 11 years ago

Page history shows everyone with the same IP address

Categories

(support.mozilla.org :: Knowledge Base Software, task, P4)

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: cilias, Assigned: jsocol)

References

()

Details

(Whiteboard: tiki_test)

Attachments

(1 file, 1 obsolete file)

Go to any article.
Click on "History".
The number in the IP column is the same for everyone.
This also has the problem of us not being able to ban users by IP (through tiki-admin_banning.php). morgamic, can you consult IT if we can do something about this? That IP is our forward proxy right?
morgamic: ping?
FWIW, I just got a password reset email that said this:

Someone coming from IP Address 10.2.81.4 requested password reset for your account  (support.mozilla.com).

The IP address listed is that of the internal address on the Netscaler of course.
Duplicate of this bug: 481123
Server-ops, is there a way around this?
Assignee: nobody → server-ops
Component: Knowledge Base Software → Server Operations
Product: support.mozilla.com → mozilla.org
QA Contact: kb-software → mrz
Version: unspecified → other
09:29 <@clouserw> laura: you should be able to use $_SERVER['HTTP_X_FORWARDED_FOR'] for bug 409942
09:32 <laura> clouserw: oh cool
09:32 <laura> could you comment in the bug so I don't forget?
Note that HTTP_X_FORWARDED_FOR is an array! You'll need to pop() until you get the right value. We have some PHP code somewhere that does the right thing...
Assignee: server-ops → nobody
Component: Server Operations → Knowledge Base Software
Product: mozilla.org → support.mozilla.com
QA Contact: mrz → kb-software
Version: other → unspecified
(In reply to comment #7)
> Note that HTTP_X_FORWARDED_FOR is an array! You'll need to pop() until you get
> the right value. We have some PHP code somewhere that does the right thing...

Yeah, that's true.  You can use the code from AMO if you want: http://viewvc.svn.mozilla.org/vc/addons/trunk/site/app/controllers/components/recaptcha.php?revision=17457&view=markup#l94
I've filed bug 506547 to specifically address the "requested password reset" emails. Fixing this bug would fix that bug as well, but if this can't be done any time soon, we need an interim fix to stop account holders from getting false password reset notifications.
(In reply to comment #9)
> I've filed bug 506547 to specifically address the "requested password reset"
> emails. Fixing this bug would fix that bug as well, but if this can't be done
> any time soon, we need an interim fix to stop account holders from getting
> false password reset notifications.

What do you mean by "false" here? Just because the IP address is wrong doesn't mean that the password reset notification is fake. Somebody must have requested that user's password be reset, though it may not be the actual user himself/herself.
Sorry Reed. My mistake. I guess if not all users are getting these emails, than someone must have tried to reset the password on those specific accounts.
Target Milestone: --- → 1.4.1
Priority: -- → P4
Target Milestone: 1.4.1 → 1.4.2
Isn't this a dupe of bug 512841?
I guess not. But if bug 512841 works, a similar fix would apply here. Adding dep.
Depends on: 512841
I can fix the page history issue, but we'll have to look into the banning system to find where it's enforcing the IPs and modify that as well. That seems like it's worthy of its own bug.
Assignee: nobody → james
Attached patch patch, v1 (obsolete) — Splinter Review
This adds a new global (yes, bad me) named $remote_addr which should be set to the correct remote address, even behind NetScaler (or other well-behaved proxies).

Then it uses that global in tiki-editpage.php to replace $_SERVER["REMOTE_ADDR"]. This was the only place I found that was relevant, but it's a bit hard to test this locally without Squid or something running.

/me goes to set up Squid.
Attachment #405950 - Flags: review?(paulc)
I think I have Squid up and running correctly as a reverse proxy accelerator. Here's the relevant line from the Apache access_log:

127.0.0.1 - - [12/Oct/2009:17:18:30 -0700] "POST /tiki-editpage.php?locale=en-US&page=Forum%20link%20test HTTP/1.0" 302 - "https://10.250.3.206/tiki-editpage.php?locale=en-US&page=Forum%20link%20test" "Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.3) Gecko/20090824 Firefox/3.5.3"

And the matching line from the page history:

Mon 12 of Oct, 2009 16:18 PST 	jsocol	10.250.3.77	3/current
Blocks: 512841
No longer depends on: 512841
Comment on attachment 405950 [details] [diff] [review]
patch, v1

WFM, I still get the right IP set in the db. Definitely prefer this less intrusive approach to the old.
To avoid a global, would it be better to define a function instead and calling it in tiki-editpage?
Attachment #405950 - Flags: review?(paulc) → review+
Global function vs global variable? At least the global variable doesn't have the overhead of a function call, I suppose.
A function would be better for security and safety considerations :), but here comes the "we already have 10,000 other globals" counter-argument.
Attached patch super-patch, v2Splinter Review
Paul is half-right in comment 17, adding another global is bad practice and I will recite 10 Our Compilers in penance.

This patch introduces a constant, REMOTE_ADDR, with the correct value, and then attempts to replace all instances of $_SERVER['REMOTE_ADDR'] with the constant. It fixes both this bug and bug 512841, and hopefully a few other strange behaviours. (Certain hashes, for example, were built from UA and IP. They should be more unique now.)

Unfortunately, this touches 16 files and is not small. But it's all for the best.

I also rewrote the internals of wikiplugin_agentinfo.php to clean up some code smell.
Attachment #405950 - Attachment is obsolete: true
Attachment #408132 - Flags: review?(morgamic)
Attachment #408132 - Flags: review?(morgamic) → review+
Comment on attachment 408132 [details] [diff] [review]
super-patch, v2

This looks sane to me.  Some of the address assignment seems to be duped in the adodblib part, but not a big deal.  Replacements for $_SERVER['REMOTE_ADDR'] look correct.
r54274

This also fixes bug 512841, and hopefully any potential future proxy-safe IP address bugs.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Summary: Page history shows everyone as the same IP number → Page history shows everyone with the same IP address
Whiteboard: tiki_triage
r54586 (prod)
Whiteboard: tiki_triage → tiki_test
You need to log in before you can comment on or make changes to this bug.