Commit graph

331 commits

Author SHA1 Message Date
Max Semenik
7a7976ba7a Password: replace equals() with verify()
So far, our key derivation code assumed that it has control over
the salt used by the derivation routines, however I want to add Argon2
support and it doesn't work this way: password_hash() generates the
salt itself, and the only way to verify a password is by using
password_verify(). Current way the things are done doesn't support it
because it relies on the result of password hashing with parameters we
provide to be deterministic.

Therefore, I'm deprecating Password::equals(), as well as whole concept
of comparing Password objects - it's used only in tests anyway. It's
getting replaced with verify() that only accepts password strings.
Uses of old function are fixed with exception of a few calls in tests
that will be addressed in my Argon2 patch.

Change-Id: I2b2be9a422ee0f773490eac316ad81505c3f8571
2019-01-24 13:40:40 -08:00
David Barratt
be27181956 Add NamespaceRestriction class so that BlockRestriction can handle namespaces.
This begins work on making namespaces a valid restriction type. The CRUD
operations of BlockRestriction can now handle namespaces. Since
NamespaceRestriction implements Restriction, enforcement should start working
immediately, but testing enforcement will come in a subsequent patch since it's
impossible to create them.

Bug: T204991
Change-Id: I7264b452d9ad788c146d6ea25d01d4d7cb5ac4f6
2019-01-21 14:19:39 +00:00
Thiemo Kreuz
734a969d55 Safe replacement of a lot of !count() with === []
This was originally a global search and replace. I manually checked all
replacements and reverted them if (due to the lack of type hints) either
null (that would be 0 when counted) or a Countable object can end in the
variable or property in question.

Now this patch only touches places where I'm sure nothing can break.

For the sanity of the honorable reviewers this patch is exclusively touching
negated counts. You should not find a single `!== []` in this patch, that
would be a mistake.

Change-Id: I5eafd4d8fccdb53a668be8e6f25a566f9c3a0a95
2019-01-15 17:28:49 +01:00
jenkins-bot
36bbdfb8ad Merge "Add @since tags to new public methods related to partial blocks" 2019-01-08 18:37:25 +00:00
Thalia
d11e342ed6 Add @since tags to new public methods related to partial blocks
Bug: T210369
Change-Id: I20197ceee8e5e43fac20addc494b841519b915c8
2019-01-08 12:44:33 +00:00
jenkins-bot
dae39db4e3 Merge "Add force option to password policy" 2019-01-07 16:59:55 +00:00
Gergő Tisza
f15ecc60cd
Add force option to password policy
Adds a way to set an array of options for a password policy. Currently
there is one option, 'forceChange', which forces the user to change
their password (if it fails the given check) before logging in.

Bug: T118774
Change-Id: I28c31fc4eae08c3ac44eff3a05f5e785ce4b9e01
2019-01-02 12:38:11 -08:00
Max Semenik
abe2167b67 user: Ensure returned user groups are sorted
Without it, Special:UserRights sometimes fails with a bogus conflict error
just because groups are somehow ordered differently.

Bug: T164211
Change-Id: I9c7f51338e0849d9e134dc780eb13c542960c655
2018-12-22 07:38:51 +00:00
Gergő Tisza
86db28715f
Deprecate User::getPasswordValidity()
Unused, the return format does not seem useful.

Also improve the documentation of $wgPasswordPolicy
and PasswordPolicyChecks.

Change-Id: Ic01e80cfefc4cfb0eee1eccc6a66942f692278a0
2018-12-20 20:26:51 -08:00
Gergő Tisza
07a27f19cf Include bot password app ID in audit data
Authentication audit logs should indicate whether a login is via the
normal password or a bot password (and which one). For failed logins
it could be included in the error message, and it usually is, but
for successful ones there is no message, so we'll send the app ID as
a new AuthManagerLoginAuthenticateAudit parameter.

Bug: T194338
Change-Id: I8aab48177b81a8e80dae118e6476a8f6a32089f1
Depends-On: Id482d2e2205960a0facd334e456d3a23bcad0ece
2018-12-13 19:18:17 +00:00
Brad Jorsch
0c1b6605ee Have BotPassword::login() call AuthManagerLoginAuthenticateAudit
To facilitate logging all authentications, even ones not via
AuthManager, BotPassword::login() should call the
AuthManagerLoginAuthenticateAudit hook.

Bug: T194338
Change-Id: I27b75c56e060fcb8f71d84ddc2eb8e1533fef33a
2018-12-11 15:33:39 -05:00
jenkins-bot
ce54089bdf Merge "Add new rules when user is blocked for UTP" 2018-12-11 13:18:14 +00:00
Dayllan Maza
05a5b8e749 Add new rules when user is blocked for UTP
No changes for sitewide blocks when "Prevent user... edit own talk page"
is checked. On partial blocks, this option will be disabled and ignored. All users
will be allowed to edit their own talk page unless a page restriction
for their page is in place.

New rules will be implemented for Namespace restrictions in a different
patch when Namespace blocking is ready.

Bug: T210475
Change-Id: I096edf2887441bccd59f09bf0eceb3988b36db1e
2018-12-11 00:01:29 -05:00
jenkins-bot
bd78869618 Merge "No yoda conditions" 2018-12-09 01:34:23 +00:00
jenkins-bot
3be252f8cb Merge "User: Bypass repeatable-read when creating an actor_id" 2018-12-07 23:37:09 +00:00
Matthewrbowker
4da89d7d74 Remove User::EDIT_TOKEN_SUFFIX, a deprecated constant since 1.27.
Bug: T61113
Change-Id: I4a461003c881b457283dc6118153e24380fccc88
2018-12-07 22:59:20 +00:00
jenkins-bot
954327b642 Merge "Fix unexpected return type of User::idFromName()" 2018-12-03 18:18:54 +00:00
Thiemo Kreuz
a50014d259 Fix unexpected return type of User::idFromName()
The user_id is an unsigned integer in the database. But not all database
abstractions we use are guaranteed to return integer values as PHP
integers. Sometimes it's a string and needs an integer cast first.

Want proof? Search for usages of this method. Almost all add an (int)
cast. This is weird and should not be necessary.

Change-Id: If1d706f73350fca5b3a0f1e0de59e4518162445b
2018-12-03 14:33:48 +01:00
Brad Jorsch
37f48fdb25 User: Bypass repeatable-read when creating an actor_id
When MySQL is using repeatable-read transaction isolation (which is the
default), the following sequence of events can occur:

1. Request A: Begin a transaction.
2. Request A: Try to select the actor ID for a user. Find no rows.
3. Request B: Insert an actor ID for that user.
4. Request A: Try to insert an actor ID for the user. Fails because one
   exists.
5. Request A: Try to select the actor ID that must exist. Fail because of
   the snapshot created at step 2.

In MySQL we can avoid this issue at step #5 by using a locking select
(FOR UPDATE or LOCK IN SHARE MODE), so let's do that.

Bug: T210621
Change-Id: I6c1d255fdd14c6f49d2ea9790e7bd7d101e98ee4
2018-11-29 11:28:05 -05:00
David Barratt
93a894a8b3
Do not ignore the 'Prevent this user from editing his own talk page while
blocked' option on partial blocks.

Partial blocks currently ignore this option as it gets into an edge case. The
option should take precidence if it is true, but should be ignored if it is
false. On sitewide blocks, the option should always be honored.

Bug: T210475
Change-Id: I33177b48a5c261ec3f510ce01998c1b096077b85
2018-11-27 10:30:21 -05:00
Fomafix
3ee1560232 No yoda conditions
Replace
  if ( 42 === $foo )
by
  if ( $foo === 42 )

Change-Id: Ice320ef1ae64a59ed035c20134326b35d454f943
2018-11-21 17:54:39 +01:00
jenkins-bot
1db4c42f46 Merge "Block: Clean up handling of non-User targets" 2018-11-19 23:56:17 +00:00
jenkins-bot
5466734477 Merge "doc: Modernise parameter names and documentation for 'replica' DBs" 2018-11-13 21:39:14 +00:00
Brad Jorsch
74ff87d291 Block: Clean up handling of non-User targets
The fix applied in d67121f6d took care of the immediate issue in
T208398, but after further analysis it was not a correct fix.

* Near line 770, the method shouldn't even be called unless the target
  is TYPE_USER.
* Near line 1598, it isn't dealing with a target at all.
* Near line 1813, you're not going to get a sensible result trying to
  call `$user->getTalkPage()` for a range or auto-block ID. What you
  would really need there to handle range and auto-blocks correctly is
  to pass in the User actually making the edit.

But after some pushback in code review about passing the User into
Block::preventsEdit() to make line 1813 work, we'll instead replace the
method with Block::appliesToTitle() and put the check for user talk
pages back into User::isBlockedFrom().

Bug: T208398
Bug: T208472
Change-Id: I23d3a3a1925e97f0cabe328c1cc74e978cb4d24a
2018-11-02 12:33:57 -04:00
jenkins-bot
5f27c2fc65 Merge "Improve some queries ordering by rev_timestamp with actor migration READ_NEW" 2018-11-01 06:28:26 +00:00
Roan Kattouw
614dceed00 User: Don't fail mysteriously when passing a User object to idFromName()
If $name is a User object, some code magically works because the object
gets converted to a string, but other code blows up because objects
aren't valid array keys. Prevent this from happening by explicitly
forcing $name to be a string.

Bug: T208469
Change-Id: Icc9ebec93d18609605e2633ccd23b90478e05e51
2018-10-31 15:38:07 -07:00
James D. Forrester
903e8b63de doc: Modernise parameter names and documentation for 'replica' DBs
Non-breaking change. Remaining uses are public interfaces (a constant, two
globals, a config sub-parameter, SQL queries, storage function names), one i18n
message key, and a whole lot of maintenance scripts with calls to the deprecated
function wfWaitForSlaves().

Change-Id: I6ee5ca92ccf6a80c08f53d9efe38ebb4b05064d7
2018-10-31 10:36:48 -07:00
jenkins-bot
6edf7d44fa Merge "Use WikiMap methods for wiki ID logic in more places" 2018-10-30 07:20:29 +00:00
jenkins-bot
c6ad2554f4 Merge "Add isCurrentWikiId()/isCurrentWikiDomain()/getCurrentWikiDomain() to WikiMap" 2018-10-29 22:27:15 +00:00
Aaron Schulz
dbccb3a361 Use WikiMap methods for wiki ID logic in more places
Change-Id: I25b53576a8fecb7cfb0e4d684f064bebf6c968fc
2018-10-29 22:02:00 +00:00
Aaron Schulz
dcd0a3d534 Add isCurrentWikiId()/isCurrentWikiDomain()/getCurrentWikiDomain() to WikiMap
Use these in place of various wfWikiID() calls.

Also cleanup UserRightsProxy wiki ID variable names and removed unused
and poorly named getDBname() method.

Change-Id: Ib28889663989382d845511f8d34712b08317f60e
2018-10-29 14:53:37 -07:00
Aaron Schulz
e2088f1170 Make UserEditCountUpdate faster by using auto-commit mode
Bug: T202715
Change-Id: I92c08694cb5e1c367809439cff42e33a56ff9878
2018-10-27 13:52:45 -07:00
jenkins-bot
cb4d4c4d8f Merge "Move user_editcount updates to a mergeable deferred update" 2018-10-26 20:32:24 +00:00
Brad Jorsch
59e856e3e6 Improve some queries ordering by rev_timestamp with actor migration READ_NEW
The revision_actor_temp table has denormalized copies of rev_page and
rev_timestamp to support the actor indexes equivalent to
`actor_timestamp` and `page_actor_timestamp`. But to get these to be
used, we need to have the WHERE and ORDER BY use the denormalized fields
instead of the main revision-table fields.

We can take generally advantage of the fact that "ORDER BY field" uses
aliased field names before the names in the actual table, but WHERE
conditions do need to explicitly vary.

Bug: T204669
Change-Id: I992aa50f02c35d76e91a5a28e05c870f8a32f858
2018-10-26 16:44:16 +00:00
Aaron Schulz
390fce6db1 Move user_editcount updates to a mergeable deferred update
This should reduce excess contention and lock timeouts.
Previously, it used a pre-commit hook which ran just before the
end of the DB transaction round.

Also removed unused User::incEditCountImmediate() method.

Bug: T202715
Depends-on: I6d239a5ea286afb10d9e317b2ee1436de60f7e4f
Depends-on: I0ad3d17107efc7b0e59f1dd54d5733cd1572a2b7
Change-Id: I0d6d7ddd91bbb21995142808248d162e05696d47
2018-10-25 15:32:18 -07:00
jenkins-bot
ecee5cd7c7 Merge "Use PHP 7 '??' operator instead of if-then-else" 2018-10-24 21:58:04 +00:00
Dayllan Maza
d67121f6d3 Enforce partial blocks
Enforce partial blocks and display a slightly different block
notice depending on if the block is sitewide or not

Bug: T197117
Depends-On: I675316dddf272fd0d6172ecad3882160752bf780
Change-Id: I8a3635a4a04a33912eb139b7b13c4bd874183d31
2018-10-24 00:57:48 +00:00
Fomafix
43244db9a2 Use PHP 7 '??' operator instead of if-then-else
Change-Id: If9d4be5d88c8927f63cbb84dfc8181baf62ea3eb
2018-10-21 21:46:46 +02:00
Aaron Schulz
ebbccf1845 Migrate some wfWikiId() callers to getLocalDomainID()
Change-Id: I33fe222b7ca66babd61610febaebcf52d3806a7d
2018-10-15 23:58:49 -07:00
jenkins-bot
0ecfa4b919 Merge "Drop UserGetImplictGroups, deprecated since 1.25" 2018-10-11 19:41:13 +00:00
Brad Jorsch
993baa3493 ActorMigration: Remove possibility of read-both
When this was originally written, the plan was to read both the old and
new fields during the transition period, while stopping writes to them
midway through. It turns out that the WHERE conditions to do read-both
correctly are generally not handled well by the database and working
around that would require a lot of complicated code (see what's being
removed from ApiQueryUserContribs here, for example).

We can simplify things greatly by instead having it write both fields
during the transition period, reading from the old for the first part
and the new for the second part, as is being done for MCR.

Bug: T204669
Change-Id: I4764c1c7883dc1003cb12729455c8107319f70b1
Depends-On: I845f6ae462f2539ebd35cbb5f2ca8b5714e2c1fb
Depends-On: I88b31b977543fdbdf69f8c1158e77e448df94e11
2018-10-11 12:12:00 +11:00
James D. Forrester
e888061ca9 Drop UserGetImplictGroups, deprecated since 1.25
Change-Id: Iffe29131fcc85d3477fe93508902d8ea982289f5
2018-10-10 15:30:00 -07:00
Aryeh Gregor
1496fd4b4e Improve ApiLogin test coverage
Coverage is 100% except for one session-related bit that seems a bit
involved to test right now.  It looks like it will be easier once
SessionManager becomes a service.

I removed the third parameter from the return value of
canonicalizeLoginData, since af37a4c7 made it always return true.

I also removed three lines of dead code from ApiLogin.php.

Change-Id: Ia0073eddd27c82827518e0031e3c313f83cfd7cc
2018-10-10 11:11:43 +03:00
Gergő Tisza
e391244629 Hard-deprecate password handling in User
Add wfDeprecated for User::checkPassword(), User::setPassword(),
User::setInternalPassword() and User::checkTemporaryPassword().
With AuthManager mediating between the caller and a set of
authentication providers in a potentially multi-step process,
a password check or change now cannot always be expressed
with a single-step succed-or-fail process. Callers should use
AuthManager::beginAuthentication() with a PasswordAuthenticationRequest
for password checks, and AuthManager::changeAuthenticationData()
for a password change.

Bug: T91699
Change-Id: Ib0ae8f1ff10ae6c2655d529db8b3a32e0cb489b0
2018-10-01 23:13:03 +00:00
jenkins-bot
ef44b9075c Merge "Create UserGetRightsRemove hook" 2018-09-30 18:12:50 +00:00
Umherirrender
2af5b6b086 Fix caller name in User::addToDatabase
Seeing {closure} in the logs as caller is not helpful

Change-Id: I876877046ae4bd1756c13e04892c1381904566de
2018-09-30 15:22:29 +02:00
Timo Tijhof
adf774f0b1 user: Remove use of Message:text() from User::isUsableName()
None of the current translations use of "&", "{" or "[", which
presumably means they don't require tranformation. I'd also
very much prefer such dynamic constructs not be added into
this system which would make it rather unpredictable given these
reserved usernames shouldn't change from time to time.

The only match for the below command is qqq.json,

> $ git grep -E '"(double-redirect-fixer|usermessage-editor|\
> proxyblocker|sorbs|spambot.username|autochange-username)"\
> ' | grep -E '[{&[]'

This code was originally introduced with r37928 (5ad5cb4f0a),
which used wfMsgForContent() and made no explicit mention of
transformation, it just happened to be the default. Later, when
that function was deprecated, it got batch-replaced by
->inContentLanguage()->text().

Bug: T189966
Change-Id: Ia4ddf215e83f15de552f8311b9e737559c72b49a
2018-09-29 02:09:35 +01:00
MR70
9ea4764b2f Create UserGetRightsRemove hook
This hook is created specifically for I379e17c.

The purpose of this hook is to override UserGetRights hook and ensure
that the removed rights will not be reinserted by the other callbacks.

Change-Id: Id31b1f25f2eda012bd811f4b96aac525bc3251c4
2018-09-24 03:21:24 +00:00
jenkins-bot
6f3d5a5204 Merge "user: Allow "CAS update failed" exceptions to be normalised" 2018-09-20 22:31:11 +00:00
Brian Wolff
9c0ba336ff SECURITY: Do not allow botpassword login if account locked.
Reported by Rxy

Bug: T194605
Change-Id: Ib41005e69ab4db6f849837de12f0d41398b58f9a
2018-09-20 22:26:11 +01:00