Skip to content

Commit f168423

Browse files
committed
Refactor URL existence check to use repository method and add domain validation for login-only URLs - refs BT#22639
1 parent 5f86c5b commit f168423

File tree

4 files changed

+96
-39
lines changed

4 files changed

+96
-39
lines changed

public/main/admin/access_url_edit.php

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@
99
use Chamilo\CoreBundle\Entity\AccessUrl;
1010
use Chamilo\CoreBundle\Enums\ActionIcon;
1111
use Chamilo\CoreBundle\Framework\Container;
12+
use Doctrine\ORM\NonUniqueResultException;
13+
use Doctrine\ORM\NoResultException;
1214
use Symfony\Component\HttpFoundation\Request as HttpRequest;
1315

1416
$cidReset = true;
@@ -113,15 +115,34 @@
113115
$url_to_go = 'access_urls.php';
114116
$message = get_lang('The URL has been edited');
115117
} else {
116-
$num = UrlManager::url_exist($url);
118+
try {
119+
$exists = $urlRepo->exists($url);
120+
} catch (NoResultException|NonUniqueResultException $e) {
121+
$exists = true;
122+
}
123+
117124
$url_to_go = 'access_url_edit.php';
118125
$message = get_lang('This URL already exists, please select another URL');
119-
if (0 === $num) {
126+
if (!$exists) {
120127
// checking url
121128
if ('/' != substr($url, strlen($url) - 1, strlen($url))) {
122129
$url .= '/';
123130
}
124131

132+
if ($isLoginOnly) {
133+
$sameDomain = $urlHelper->isSameBaseDomain(
134+
array_merge($urlRepo->getUrlList(), [$url])
135+
);
136+
137+
if (!$sameDomain) {
138+
Display::addFlash(
139+
Display::return_message(
140+
get_lang('To use the central login page feature, all URLs defined MUST use the same (root) domain name in order to limit security risks linked to sharing access tokens between URLs. URLs using a different domain name will not be taken into account for access sharing.')
141+
)
142+
);
143+
}
144+
}
145+
125146
$accessUrl = $urlRepo->findOneBy(['url' => $url]);
126147

127148
if (!$accessUrl) {

public/main/inc/lib/urlmanager.lib.php

Lines changed: 3 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88
use Chamilo\CoreBundle\Entity\AccessUrlRelUser;
99
use Chamilo\CoreBundle\Entity\AccessUrlRelUserGroup;
1010
use Chamilo\CoreBundle\Framework\Container;
11+
use Doctrine\ORM\NonUniqueResultException;
12+
use Doctrine\ORM\NoResultException;
1113

1214
/**
1315
* Class UrlManager
@@ -29,9 +31,7 @@ public static function add($url, $description, $active, bool $isLoginOnly = fals
2931
{
3032
$repo = Container::getAccessUrlRepository();
3133

32-
$num = self::url_exist($url);
33-
34-
if (0 !== $num) {
34+
if (!$repo->exists($url)) {
3535
return null;
3636
}
3737

@@ -136,40 +136,6 @@ public static function delete($id)
136136
return true;
137137
}
138138

139-
/**
140-
* @param string $url
141-
*
142-
* @return int
143-
*/
144-
public static function url_exist($url)
145-
{
146-
$table = Database::get_main_table(TABLE_MAIN_ACCESS_URL);
147-
$sql = "SELECT id FROM $table
148-
WHERE url = '".Database::escape_string($url)."' ";
149-
$res = Database::query($sql);
150-
151-
return Database::num_rows($res);
152-
}
153-
154-
/**
155-
* @param int $urlId
156-
*
157-
* @return int
158-
*/
159-
public static function url_id_exist($urlId)
160-
{
161-
$urlId = (int) $urlId;
162-
if (empty($urlId)) {
163-
return false;
164-
}
165-
$table = Database::get_main_table(TABLE_MAIN_ACCESS_URL);
166-
$sql = "SELECT id FROM $table WHERE id = ".$urlId;
167-
$res = Database::query($sql);
168-
$num = Database::num_rows($res);
169-
170-
return $num;
171-
}
172-
173139
/**
174140
* This function get the quantity of URLs.
175141
*

src/CoreBundle/Helpers/AccessUrlHelper.php

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
namespace Chamilo\CoreBundle\Helpers;
88

99
use Chamilo\CoreBundle\Entity\AccessUrl;
10+
use Chamilo\CoreBundle\Framework\Container;
1011
use Chamilo\CoreBundle\Repository\Node\AccessUrlRepository;
12+
use Pdp\Rules;
13+
use Pdp\TopLevelDomains;
1114
use Symfony\Component\HttpFoundation\RequestStack;
1215

1316
use const PHP_SAPI;
@@ -66,4 +69,44 @@ public function getCurrent(): ?AccessUrl
6669

6770
return $accessUrl;
6871
}
72+
73+
public function isSameBaseDomain(array $urls): bool
74+
{
75+
if (empty($urls)) {
76+
return false;
77+
}
78+
79+
$projectDir = Container::$container->getParameter('kernel.project_dir');
80+
81+
$rules = Rules::fromPath($projectDir.'/config/public_suffix_list.dat');
82+
83+
$firstHost = parse_url($urls[0], PHP_URL_HOST);
84+
85+
if (!$firstHost) {
86+
return false;
87+
}
88+
89+
$firstDomain = $rules->resolve($firstHost)->registrableDomain()->toString();
90+
91+
if (!$firstDomain) {
92+
return false;
93+
}
94+
95+
// Comparar con el resto
96+
foreach ($urls as $url) {
97+
$host = parse_url($url, PHP_URL_HOST);
98+
99+
if (!$host) {
100+
return false;
101+
}
102+
103+
$domain = $rules->resolve($host)->registrableDomain();
104+
105+
if ($domain->toString() !== $firstDomain) {
106+
return false;
107+
}
108+
}
109+
110+
return true;
111+
}
69112
}

src/CoreBundle/Repository/Node/AccessUrlRepository.php

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,4 +74,31 @@ public function getOnlyLoginAccessUrl(): ?AccessUrl
7474
{
7575
return $this->findOneBy(['isLoginOnly' => true]);
7676
}
77+
78+
/**
79+
* @throws NonUniqueResultException
80+
* @throws NoResultException
81+
*/
82+
public function exists(string $url): bool
83+
{
84+
return $this->createQueryBuilder('a')
85+
->select('COUNT(a.id)')
86+
->where('a.url = :url')
87+
->setParameter('url', $url)
88+
->getQuery()
89+
->getSingleScalarResult() > 0
90+
;
91+
}
92+
93+
/**
94+
* @return array<int, string>
95+
*/
96+
public function getUrlList(): array
97+
{
98+
return $this->createQueryBuilder('a')
99+
->select('a.url')
100+
->getQuery()
101+
->getSingleColumnResult()
102+
;
103+
}
77104
}

0 commit comments

Comments
 (0)