Skip to content

Commit

Permalink
[!!!][TASK] Remove lockToDomain feature for BE and FE
Browse files Browse the repository at this point in the history
Both fe_users/be_users and be_groups/fe_groups have a feature called "lockToDomain".

Although it is called the same, it has a different use-case:

* Users: If lockToDomain is set, the user is only allowed to login when a given HTTP_HOST is given.
* Groups: If lockToDomain is set, the group is only added to the logged in user, if the HTTP_HOST matches this domain.

Both features are rarely used, and even in multi-tenant setups not viable or flexible
enough. In addition, the features are not any additional security measures as HTTP_HOST can be faked.

They both add unneeded complexity for the rare use of a similar feature,
a custom extension should be used.

Plus: All of these features can be added via extensions, depending on a
specific use case of an installation, so _if_ people use it, custom extensions
should be used instead for the specific use case they have.

The database fields, TCA definitions, labels, domain model logic in Extbase
and actual validation within the AuthenticationService and BE_USER are removed
without any substitution.

Resolves: #91782
Releases: master
Change-Id: I4a12185b79efaf1e3bded5120675e3c1095dcd42
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/65011
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
  • Loading branch information
bmack authored and georgringer committed Jul 21, 2020
1 parent edce3cc commit 0ce30f0
Show file tree
Hide file tree
Showing 28 changed files with 66 additions and 353 deletions.
49 changes: 3 additions & 46 deletions typo3/sysext/core/Classes/Authentication/AuthenticationService.php
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,7 @@ public function getUser()
}

/**
* Authenticate a user: Check submitted user credentials against stored hashed password,
* check domain lock if configured.
* Authenticate a user: Check submitted user credentials against stored hashed password.
*
* Returns one of the following status codes:
* >= 200: User authenticated successfully. No more checking is needed by other auth services.
Expand All @@ -113,12 +112,9 @@ public function authUser(array $user): int
$submittedUsername = (string)$this->login['uname'];
$submittedPassword = (string)$this->login['uident_text'];
$passwordHashInDatabase = $user['password'];
$queriedDomain = $this->authInfo['HTTP_HOST'];
$configuredDomainLock = $user['lockToDomain'];
$userDatabaseTable = $this->db_user['table'];

$isReHashNeeded = false;
$isDomainLockMet = false;

$saltFactory = GeneralUtility::makeInstance(PasswordHashFactory::class);

Expand Down Expand Up @@ -152,13 +148,6 @@ public function authUser(array $user): int
// instances of the same class.
$isReHashNeeded = true;
}
if (empty($configuredDomainLock)) {
// No domain restriction set for user in db. This is ok.
$isDomainLockMet = true;
} elseif (!strcasecmp($configuredDomainLock, $queriedDomain)) {
// Domain restriction set and it matches given host. Ok.
$isDomainLockMet = true;
}
}

if (!$isValidPassword) {
Expand All @@ -171,16 +160,6 @@ public function authUser(array $user): int
return 0;
}

if (!$isDomainLockMet) {
// Password ok, but configured domain lock not met
$errorMessage = 'Login-attempt from ###IP###, username \'%s\', locked domain \'%s\' did not match \'%s\'!';
$this->writeLogMessage($errorMessage, $user[$this->db_user['username_column']], $configuredDomainLock, $queriedDomain);
$this->writelog(SystemLogType::LOGIN, SystemLogLoginAction::ATTEMPT, SystemLogErrorClassification::SECURITY_NOTICE, 1, $errorMessage, [$user[$this->db_user['username_column']], $configuredDomainLock, $queriedDomain]);
$this->logger->info(sprintf($errorMessage, $user[$this->db_user['username_column']], $configuredDomainLock, $queriedDomain));
// Responsible, authentication ok, but domain lock not ok, do NOT check other services
return 0;
}

if ($isReHashNeeded) {
// Given password validated but a re-hash is needed. Do so.
$this->updatePasswordHashInDatabase(
Expand All @@ -190,7 +169,7 @@ public function authUser(array $user): int
);
}

// Responsible, authentication ok, domain lock ok. Log successful login and return 'auth ok, do NOT check other services'
// Responsible, authentication ok. Log successful login and return 'auth ok, do NOT check other services'
$this->writeLogMessage($this->pObj->loginType . ' Authentication successful for username \'%s\'', $submittedUsername);
return 200;
}
Expand Down Expand Up @@ -236,17 +215,6 @@ public function getGroups($user, $knownGroups)
$queryBuilder->expr()->in(
'uid',
$queryBuilder->createNamedParameter($groups, Connection::PARAM_INT_ARRAY)
),
$queryBuilder->expr()->orX(
$queryBuilder->expr()->eq(
'lockToDomain',
$queryBuilder->createNamedParameter('', \PDO::PARAM_STR)
),
$queryBuilder->expr()->isNull('lockToDomain'),
$queryBuilder->expr()->eq(
'lockToDomain',
$queryBuilder->createNamedParameter($this->authInfo['HTTP_HOST'], \PDO::PARAM_STR)
)
)
)
->execute();
Expand All @@ -273,7 +241,7 @@ public function getGroups($user, $knownGroups)
*/
public function getSubGroups($grList, $idList, &$groups)
{
// Fetching records of the groups in $grList (which are not blocked by lockedToDomain either):
// Fetching records of the groups in $grList:
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable('fe_groups');
if (!empty($this->authInfo['showHiddenRecords'])) {
$queryBuilder->getRestrictions()->removeByType(HiddenRestriction::class);
Expand All @@ -289,17 +257,6 @@ public function getSubGroups($grList, $idList, &$groups)
GeneralUtility::intExplode(',', $grList, true),
Connection::PARAM_INT_ARRAY
)
),
$queryBuilder->expr()->orX(
$queryBuilder->expr()->eq(
'lockToDomain',
$queryBuilder->createNamedParameter('', \PDO::PARAM_STR)
),
$queryBuilder->expr()->isNull('lockToDomain'),
$queryBuilder->expr()->eq(
'lockToDomain',
$queryBuilder->createNamedParameter($this->authInfo['HTTP_HOST'], \PDO::PARAM_STR)
)
)
)
->execute();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1485,7 +1485,7 @@ protected function prepareUserTsConfig(): void
*/
public function fetchGroups($grList, $idList = '')
{
// Fetching records of the groups in $grList (which are not blocked by lockedToDomain either):
// Fetching records of the groups in $grList:
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)->getQueryBuilderForTable($this->usergroup_table);
$expressionBuilder = $queryBuilder->expr();
$constraints = $expressionBuilder->andX(
Expand All @@ -1499,14 +1499,6 @@ public function fetchGroups($grList, $idList = '')
GeneralUtility::intExplode(',', $grList),
Connection::PARAM_INT_ARRAY
)
),
$expressionBuilder->orX(
$expressionBuilder->eq('lockToDomain', $queryBuilder->quote('')),
$expressionBuilder->isNull('lockToDomain'),
$expressionBuilder->eq(
'lockToDomain',
$queryBuilder->createNamedParameter(GeneralUtility::getIndpEnv('HTTP_HOST'), \PDO::PARAM_STR)
)
)
);
// Hook for manipulation of the WHERE sql sentence which controls which BE-groups are included
Expand Down
14 changes: 2 additions & 12 deletions typo3/sysext/core/Configuration/TCA/be_groups.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
'disabled' => 'hidden'
],
'title' => 'LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_groups',
'useColumnsForDefaultValues' => 'lockToDomain, file_permissions',
'useColumnsForDefaultValues' => 'file_permissions',
'versioningWS_alwaysAllowLiveEdit' => true,
'searchFields' => 'title'
],
Expand Down Expand Up @@ -203,16 +203,6 @@
],
]
],
'lockToDomain' => [
'label' => 'LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:lockToDomain',
'config' => [
'type' => 'input',
'size' => 20,
'eval' => 'trim',
'max' => 50,
'softref' => 'substitute'
]
],
'groupMods' => [
'label' => 'LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:userMods',
'config' => [
Expand Down Expand Up @@ -282,7 +272,7 @@
--div--;LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_groups.tabs.mounts_and_workspaces,
workspace_perms, db_mountpoints, file_mountpoints, file_permissions, category_perms,
--div--;LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_groups.tabs.options,
lockToDomain, TSconfig,
TSconfig,
--div--;LLL:EXT:core/Resources/Private/Language/Form/locallang_tabs.xlf:access,
hidden,
--div--;LLL:EXT:core/Resources/Private/Language/Form/locallang_tabs.xlf:notes,
Expand Down
14 changes: 2 additions & 12 deletions typo3/sysext/core/Configuration/TCA/be_users.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
'1' => 'status-user-admin',
'default' => 'status-user-backend'
],
'useColumnsForDefaultValues' => 'usergroup,lockToDomain,options,db_mountpoints,file_mountpoints,file_permissions,userMods',
'useColumnsForDefaultValues' => 'usergroup,options,db_mountpoints,file_mountpoints,file_permissions,userMods',
'versioningWS_alwaysAllowLiveEdit' => true,
'searchFields' => 'username,email,realName'
],
Expand Down Expand Up @@ -97,16 +97,6 @@
$GLOBALS['TYPO3_CONF_VARS']['GFX']['imagefile_ext']
)
],
'lockToDomain' => [
'label' => 'LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:lockToDomain',
'config' => [
'type' => 'input',
'size' => 20,
'eval' => 'trim',
'max' => 50,
'softref' => 'substitute'
]
],
'db_mountpoints' => [
'label' => 'LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_users.options_db_mounts',
'config' => [
Expand Down Expand Up @@ -385,7 +375,7 @@
--div--;LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_users.tabs.mounts_and_workspaces,
workspace_perms, db_mountpoints, options, file_mountpoints, file_permissions, category_perms,
--div--;LLL:EXT:core/Resources/Private/Language/locallang_tca.xlf:be_users.tabs.options,
lockToDomain, disableIPlock, TSconfig,
disableIPlock, TSconfig,
--div--;LLL:EXT:core/Resources/Private/Language/Form/locallang_tabs.xlf:access,
--palette--;;timeRestriction,
--div--;LLL:EXT:core/Resources/Private/Language/Form/locallang_tabs.xlf:notes,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
.. include:: ../../Includes.txt

======================================================================================================
Breaking: #91782 - lockToDomain feature for frontend users / groups and backend users / groups removed
======================================================================================================

See :issue:`91782`

Description
===========

TYPO3 Core shipped with a feature called "lockToDomain" for Frontend users and backend users which made the user login only valid if the exact given HTTP_HOST matches the filled domain.

A similar functionality, but with the same name for groups existed, which only added the group to a specific user during a session, if the user was accessing a TYPO3 site under a specific domain.

Both features have been removed.

Impact
======

Frontend users or backend users that have this option set previously, will now be able to login independent of the defined HTTP_HOST header sent with the login page.

Regardless of any setting of the "lockToDomain" setting of a specific group, all groups added
to a user are now applied during login of a user, both for frontend and backend.


Affected Installations
======================

TYPO3 Installations using this feature in their database records. When in doubt, this can be identified by running SQL SELECT statements to identify users actively using this feature.

Frontend Users:
* "SELECT uid, pid, username FROM fe_users WHERE lockToDomain != '' AND lockToDomain IS NOT NULL;"

Backend Users:
* "SELECT uid, pid, username FROM be_users WHERE lockToDomain != '' AND lockToDomain IS NOT NULL;"

Frontend Groups:
* "SELECT uid, pid, username FROM fe_groups WHERE lockToDomain != '' AND lockToDomain IS NOT NULL;"

Backend Groups:
* "SELECT uid, pid, username FROM be_groups WHERE lockToDomain != '' AND lockToDomain IS NOT NULL;"


Migration
=========

Any installations needing this feature should build this in
custom extensions extending TCA and a custom Authentication Service.

In addition, if such a feature is needed for frontend users
or groups, it is recommended to use the storagePid option to limit
frontend user login by Storage Folders.

.. index:: Database, TCA, NotScanned, ext:core
Original file line number Diff line number Diff line change
Expand Up @@ -203,19 +203,6 @@ Examples from "Getting Started" | https://docs.typo3.org/typo3cms/GettingStarted
<trans-unit id="hidden.details" resname="hidden.details">
<source>If you disable a user group all users which are members of the group will in effect not inherit any properties this group may have given them.</source>
</trans-unit>
<trans-unit id="lockToDomain.description" resname="lockToDomain.description">
<source>Enter the host name from which the user is forced to login. NOTICE: this is not a security feature and can be circumvented by faking HTTP_HOST.</source>
</trans-unit>
<trans-unit id="lockToDomain.details" resname="lockToDomain.details" xml:space="preserve">
<source>A TYPO3 system may host multiple websites on multiple domains. Therefore this option secures that users can login only from a certain host name.
Setting this to for example "www.my-domain.com" will require a user to be logged in from that domain if membership of this group should be gained. Otherwise the group will be ignored for the user.</source>
</trans-unit>
<trans-unit id="_lockToDomain.seeAlso" resname="_lockToDomain.seeAlso" xml:space="preserve">
<source>be_users:lockToDomain,
fe_users:lockToDomain,
fe_groups:lockToDomain</source>
<note from="developer">This string contains an internal text, which must not be changed. Just copy the original text into the translation field. For more information have a look at the Tutorial.</note>
</trans-unit>
<trans-unit id="groupMods.description" resname="groupMods.description">
<source>Select available backend modules for the group members.</source>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,6 @@ The first (top) group in the list is the group which will, by default, be the ow
be_groups</source>
<note from="developer">This string contains an internal text, which must not be changed. Just copy the original text into the translation field. For more information have a look at the Tutorial.</note>
</trans-unit>
<trans-unit id="lockToDomain.description" resname="lockToDomain.description">
<source>Enter the host name from which the user is forced to login. NOTICE: this is not a security feature and can be circumvented by faking HTTP_HOST.</source>
</trans-unit>
<trans-unit id="lockToDomain.details" resname="lockToDomain.details">
<source>A TYPO3 system may have multiple domains pointing to it. Therefore this option secures that users can login only from a certain host name.</source>
</trans-unit>
<trans-unit id="_lockToDomain.seeAlso" resname="_lockToDomain.seeAlso" xml:space="preserve">
<source>be_groups:lockToDomain,
fe_users:lockToDomain,
fe_groups:lockToDomain</source>
<note from="developer">This string contains an internal text, which must not be changed. Just copy the original text into the translation field. For more information have a look at the Tutorial.</note>
</trans-unit>
<trans-unit id="disableIPlock.description" resname="disableIPlock.description">
<source>Disable the lock of the backend users session to the remote IP number.</source>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,6 @@
<trans-unit id="TSconfig_title" resname="TSconfig_title">
<source>TSconfig QuickReference</source>
</trans-unit>
<trans-unit id="lockToDomain" resname="lockToDomain">
<source>Lock to domain</source>
</trans-unit>
<trans-unit id="be_users.username" resname="be_users.username">
<source>Username</source>
</trans-unit>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
<uid>1</uid>
<pid>0</pid>
<title>editor group</title>
<lockToDomain></lockToDomain>
<workspace_perms>0</workspace_perms>
<db_mountpoints>1,3,4,5,40</db_mountpoints>
<tstamp>1544454571</tstamp>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ class BackendGroupsVisibleFieldsTest extends FunctionalTestCase
'file_mountpoints',
'file_permissions',
'category_perms',
'lockToDomain',
'TSconfig',
];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ class BackendUsersVisibleFieldsTest extends FunctionalTestCase
'file_mountpoints',
'file_permissions',
'category_perms',
'lockToDomain',
'TSconfig',
'starttime',
'endtime',
Expand All @@ -52,7 +51,6 @@ class BackendUsersVisibleFieldsTest extends FunctionalTestCase
'workspace_perms',
'file_permissions',
'category_perms',
'lockToDomain',
];

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,6 @@ public function authUserThrowsExceptionIfPasswordInDbDoesNotResolveToAValidHash(
);
$dbUser = [
'password' => 'aPlainTextPassword',
'lockToDomain' => ''
];
self::assertEquals(100, $subject->authUser($dbUser));
}
Expand Down Expand Up @@ -183,7 +182,6 @@ public function authUserReturns0IfPasswordDoesNotMatch(): void
$dbUser = [
// a phpass hash of 'myPassword'
'password' => '$P$C/2Vr3ywuuPo5C7cs75YBnVhgBWpMP1',
'lockToDomain' => ''
];
self::assertSame(0, $subject->authUser($dbUser));
}
Expand Down Expand Up @@ -213,43 +211,7 @@ public function authUserReturns200IfPasswordMatch(): void
$dbUser = [
// an argon2i hash of 'myPassword'
'password' => '$argon2i$v=19$m=65536,t=16,p=1$eGpyelFZbkpRdXN3QVhsUA$rd4abz2fcuksGu3b3fipglQZtHbIy+M3XoIS+sNVSl4',
'lockToDomain' => ''
];
self::assertSame(200, $subject->authUser($dbUser));
}

/**
* @test
*/
public function authUserReturns0IfPasswordMatchButDomainLockDoesNotMatch(): void
{
$subject = new AuthenticationService();
$pObjProphecy = $this->prophesize(AbstractUserAuthentication::class);
$pObjProphecy->loginType = 'BE';
$loggerProphecy = $this->prophesize(Logger::class);
$subject->setLogger($loggerProphecy->reveal());
$subject->initAuth(
'authUserBE',
[
'uident_text' => 'myPassword',
'uname' => 'lolli'
],
[
'db_user' => [
'table' => 'be_users',
'username_column' => 'username',
],
'REMOTE_HOST' => '',
'HTTP_HOST' => 'example.com',
],
$pObjProphecy->reveal()
);
$dbUser = [
// an argon2i hash of 'myPassword'
'password' => '$argon2i$v=19$m=65536,t=16,p=2$LnUzc3ZISWJwQWlSbmpkYw$qD1sRsJFzkUmjcEaKzDeg6LtflwdTpo49VbH3tMeMXU',
'username' => 'lolli',
'lockToDomain' => 'not.example.com'
];
self::assertSame(0, $subject->authUser($dbUser));
}
}
Loading

0 comments on commit 0ce30f0

Please sign in to comment.