The authentication provider's testUserForCreation() method is for
checking whether a given user name is available. The current
user being IP-blocked has nothing to do with that username's
availability so stop checking that. (AuthManager will check it
via AuthManager::authorizeCreateAccount() elsewhere. Although
that method doesn't seem to be doing anything useful and could
probably just be replaced with a direct call to
PermissionManager, but that's left for a separate, less risky
patch.)
Special-case autocreation though, which doesn't use
the more appropiate AuthManager::authorizeCreateAccount() for
performance reasons so it does need an IP block check.
(At least I think it is for performance reasons. Maybe it's
just an unintentional omission, and that should be used instead?)
While we are at it, also fix a TODO in AuthManager where partial
blocks were taken into account for $wgBlockDisablesLogin, and
clarify in the config schema that they aren't, improve some
comments to make it more obvious why some things are/aren't
done in CheckBlocksSecondaryAuthenticationProvider, and make
the logic more similar to the one in testUserForCreation().
Functional changes:
* Partial blocks are ignored for authentication, account
creation and autocreation.
* On $wgBlockDisablesLogin wikis IP blocks won't prevent
login anymore.
* On $wgBlockDisablesLogin wikis, blocks will now prevent
account autocreation even if they are not configured to
prevent account creation. The assumption is that on such
wikis account creation is restricted via some means.
This probably isn't necessary as blocks should also prevent
the conditions needed for autocreation (e.g. log the user
out centrally), but can serve as defense in depth.
Along with the special-casing of autocreation, this means
on such wikis any IP block will prevent autocreation, which
is not great but seems not worth even more code complexity
to avoid.
* The action=query&list=users&usprop=cancreate API won't take
blocks into account anymore.
Bug: T306018
Bug: T208895
Change-Id: Ie94d61640301192b287275311f3452e606469d25
Deprecate AuthManager::checkAccountCreatePermissions in favour of
::authorizeCreateAccount, for full permissions checks and
::probablyCanCreateAccount, for quick permissions checks.
Update SpecialCreateAccount to use the appropriate checks, and
add SpecialCreateAccountTest.
Bug: T283265
Change-Id: I385f2b56ef61f64a4c61e9c97683bef564892dce
Allows AuthenticationResponse to store, when the status is FAIL,
an array of strings that describe the reasons for the failure.
These are stored in $failReasons and are not intended for the
client. On any other status $failReasons is null. These are
optionally provided when calling AuthenticationResponse::newFail
in the parameter $failReasons.
This is implemented to allow the CentralAuth extension to store
whether the password was correct if the account is locked inside
the AuthenticationResponse. The extension CheckUser which hooks
into authentication requests then can read the failure reasons
from the AuthenticationResponse, and can then note in the CU
entry that the login attempt had the correct password.
If whether the correct password was used is stored in the I18n
message, the client would then know if the password they tried
on the locked account was correct. For comprimised accounts this
could be used by mailicious actors to verify that the password
was correct and then try it elsewhere if the account has the same
password as on other sites. This means, unless I have missed
another method, a new array is needed to store these failure reasons.
This, along with some other patches to CheckUser and CentralAuth,
will then allow Checkusers to see if a login attempt for a locked
account had the correct password. Checkusers can then use this,
with the knowledge that the account isn't comprimised, to say that
the login attempt was made by the owner of the account so in cases
of socking the creation of a new account can be more conclusively
said to be by the person who created the now locked sock account.
Bug: T303192
Change-Id: I7b2d9579a518a6c02f05281b1016e31e0d086fe7
Add a hook "TempUserCreatedRedirect" which allows an extension to modify
the post-save redirect after a temporary user is created. CentralAuth
will handle this hook in order to redirect to loginwiki. Loginwiki will
redirect back to the page being saved.
In LoginHelper::showReturnToPage add an "anchor" parameter to specify
the URL fragment to redirect to. This is intended for section editing
with a redirect via loginwiki.
In AuthManager::autoCreateUser(), just allow any valid username, don't
require it to be creatable. Previously a non-creatable username was
allowed only with $source=AUTOCREATE_SOURCE_TEMP, but CentralAuth
depends on user autocreation from the session during refreshCookies. The
session is already validated at this point, so it's not like a user can
exploit this to create arbitrary names.
Change-Id: I623330dbf5d0573c93ae22f962618d6ddbd126d9
This edition brought to you by:
grep -ERIn $(grep -o "'[A-Za-z0-9_]*'" includes/MainConfigNames.php | tr
"\n" '|' | sed 's/|$/\n/') includes/
I only corrected a fraction of the results provided by that command. I'm
submitting the partial patch now so it doesn't bitrot.
Bug: T305805
Change-Id: If1918c0b3d88cdf90403921e4310740e206d6962
Add services and utilities for automatic creation of temporary user
accounts on page save, in order to avoid exposing the user's IP
address.
* Add $wgAutoCreateTempUser, for configuring the system
* Add TempUserConfig service, which interprets the config.
* Add TempUserCreator service, which creates users during page save as
requested by EditPage. With proxy methods to TempUserConfig for
convenience.
* Add table user_autocreate_serial. Table creation is necessary before
the feature is enabled but is not necessary before deployment of this
commit.
Bug: T300263
Change-Id: Ib14a352490fc42039106523118e8d021844e3dfb
Part 1, proof of concept. Hundreds of files left to go. These changes
brought to you in large part by vim macros.
Bug: T305805
Change-Id: I44789091e9f6394c800a11b29f22528c8dcacf71
Make phan stricter about conditional variable declaration
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T259172
Change-Id: I1f200ac37df7448453688bf464a8250c97313e5d
Make phan stricter about array keys
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T304887
Depends-On: I3105a5fd4826f8667b5232834defc5ec93be32a1
Depends-On: Ie9610a6e83731468311edb3ed17f80fc509de385
Change-Id: I701f12ab94478c3b8e7fd82110ade74a8e6b04ef
Conventionally, public constants are accessed via their declaring
class, except for self:: which is an acceptable shortcut.
Change-Id: If05eab72140267e6ef54736710d751d7f24a7860
Make phan stricter about null types by setting null_casts_as_any_type to
false (the default in mediawiki-phan-config)
Remaining false positive issues are suppressed.
The suppression and the setting change can only be done together
Bug: T242536
Bug: T301991
Change-Id: I0f295382b96fb3be8037a01c10487d9d591e7e01
Make phan stricter about scalar types by setting scalar_implicit_cast to
false (the default in mediawiki-phan-config)
Bug: T242536
Bug: T301991
Change-Id: Ia2fe30b17804186571722e728578121c8b75d455
This helps phan to detect unreachable code and also impossible types
after the functions.
It helps phan to avoid false positives for array keys
when the keys are checked before
Bug: T240141
Change-Id: I895f70e82b3053a46cd44135b15437e6f82a07b2
Use the already provided content language to get the language converter
for the content language
Bug: T252095
Change-Id: I2bc085d5ca8ce2570e6da21128ea785799c958e5
AuthManager has been converted to a service in 1.35.
AuthManager::$instance is only used in AuthManager::resetCache(),
where it is set to null.
This patch also removes AuthManager::resetCache() because with
removing AuthManager::$instance AuthManager::resetCache() has no
longer any effect.
Change-Id: I8b27f9a2d0d3f7acec6b93f64f9c256084990026
AuthenticationRequest::getUsernameFromRequests() can return a null
$username, but AuthManager::normalizeUsername() requires a non-null
$username, so let’s check for null before calling the latter method.
(Throttler::increase() is fine to call with a null $username, so it’s
enough to skip the normalize call and put null in the array.)
Also, now that we’re handling null, use that instead of the empty string
as the default in the catch block – I assume it wouldn’t make much sense
to pass '' into the normalize method.
Bug: T284170
Change-Id: Id969a15cb9b4ead89b7caa0025d6b364318f2799
The documentation of SessionProvider::init says that the method can be used in implementation tests.
Now all tests must use SessionProviderTestTrait::initProvider in order to initialize a SessionProvider.
So the entire description after @internal was rewritten. Also the
similar note was added to the documentation of
AbstractAuthenticationProvider::init.
Bug: T283931
Change-Id: I1cde6fd12666ef3a7c9b1e33a718592c11cf9e29
This reverts commit ecf826a2ee.
Reason for revert: need to edit the patch and then it will be GTG in order to finish hard deprecating of User ::getCanonicalName, ::isUsableName, ::isCreatableName
Change-Id: I2f57f56728fcbeada96dc2228f07dc8bcaa5d4f6
Add a helper method for the common use case of temporarily silencing
transaction profiler warnings.
Change-Id: I40de4daf8756da693de969e5526b471b624b2cee
init() method was added to AuthenticationProvider. It helps to inject
dependecies. Overridable postInitSetup() method was added to
AbstractAuthenticationProvider. A provider can override this to do
any necessary setup.
AbstractAuthenticationProvider ::setLogger(), ::setManager(), ::setConfig(),
::setHookContainer() were soft deprecated. Now developers must use
AbstractAuthenticationProvider::init().
Bug: T275030
Change-Id: I6ca63eddac1b177eeadbdcce992e71c44a480160
