Page MenuHomePhabricator

Pass timestamp and user's access level into LogIPInfoAccessJob
Closed, ResolvedPublic3 Estimated Story Points

Description

Background

When a user accesses IP information returned by the LogHandler or RevisionHandler API endpoints, the fact that they accesses this information is logged. The log includes the time of access and whether the user saw partial or full information.

The log happens via a job, in accordance with the guidance found here:

Instead of a DeferredUpdate, use a job for writing data in the context of a GET request.

Two problems with the current implementation were raised in T295017#7729296 and T295017#7729314:

  • The timestamp of the log entry is the time at which the job runs. If there are delays in the job queue, this could be a lot later than the time when the user accessed the IP information.
  • Whether the user saw basic or full information is determined by their permissions at the time when the job runs, which may be different from their permissions when they accessed the IP information.

We can fix both of these by passing the time and access level to the job as parameters (similar to how the performer, IP and data context are passed):

$this->jobQueueGroup->push(
	new JobSpecification(
		'ipinfoLogIPInfoAccess',
		[
			'performer' => $user->getName(),
			'ip' => $author->getName(),
			'dataContext' => $dataContext
		],
		[],
		null
	)
);

AC

  • Timestamp and access level are passed to LogIPInfoAccessJob
  • The Logger logs the passed-through values

Event Timeline

@Tchanders to clarify -- this task is to address these two problems mentioned in the task desc?

  • The timestamp of the log entry is the time at which the job runs. If there are delays in the job queue, this could be a lot later than the time when the user accessed the IP information.
  • Whether the user saw basic or full information is determined by their permissions at the time when the job runs, which may be different from their permissions when they accessed the IP information.

I think it would be good to do this but don't consider it a testwiki blocker. The audience for testwiki is going to be minimal and adding this blocker may delay the delivery time. It would be good to have this done before production deploy.

Thanks @Niharika - yes it is to address those two problems.

STran set the point value for this task to 3.Mar 14 2022, 5:14 PM

Change 771751 had a related patch set uploaded (by TsepoThoabala; author: TsepoThoabala):

[mediawiki/extensions/IPInfo@master] Pass timestamp and user's access level into LogIPInfoAccessJob

https://gerrit.wikimedia.org/r/771751

Change 771751 merged by jenkins-bot:

[mediawiki/extensions/IPInfo@master] Pass timestamp and user's access level into LogIPInfoAccessJob

https://gerrit.wikimedia.org/r/771751

dom_walden subscribed.

I set $wgJobRunRate = 0 on my local wiki so I could have an arbitrary delay between looking up an IP and the job running which logs this.

The timestamp recorded for the log entry was always the same (as closely as I could tell) as when I looked up the IP. (I left enough of a delay so that if the timestamp was wrong I would see a difference.)

The user access level was also the same as when I looked the IP up, not when I ran the job (even when I changed the group membership of my user between looking the IP up and running the job).

I tried for revisions and logs, including logs which have two IPs associated with them (performer and target) and which produce two log entries.

I tested making lots of lookups in the small period of time, in case there were any race-conditions.

In case of regressions, I also retested logging on testwiki by looking up a large number of IPs in a short period of time. All the appropriate log events were recorded.

Test environments: