Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add Validator infrastructure #27961

Closed
wants to merge 10 commits into from
Closed

Add Validator infrastructure #27961

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7e6b882
Fix IDN domain name not being allowed
szaimen Mar 5, 2022
0031bfb
Add an validator service with various constraints
CarlSchwan Mar 9, 2022
9003fd1
Add missing copyright
CarlSchwan Mar 9, 2022
72294f4
Apply suggestions from review
CarlSchwan Mar 10, 2022
27104b0
add validator to setup check
szaimen Mar 10, 2022
5751e9d
Merge constraint and validator
CarlSchwan Mar 14, 2022
2176988
fixup! Merge constraint and validator
CarlSchwan Mar 14, 2022
0bcae13
fixup! Merge constraint and validator
CarlSchwan Mar 14, 2022
ec67ee4
fixup! Merge constraint and validator
CarlSchwan Mar 14, 2022
df30b14
fixup! Merge constraint and validator
CarlSchwan Mar 14, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 11 additions & 10 deletions apps/oauth2/lib/Controller/SettingsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,15 @@
use OCP\IL10N;
use OCP\IRequest;
use OCP\Security\ISecureRandom;
use OCP\Validator\Constraints\Url;
use OCP\Validator\IValidator;

class SettingsController extends Controller {
/** @var ClientMapper */
private $clientMapper;
/** @var ISecureRandom */
private $secureRandom;
/** @var AccessTokenMapper */
private $accessTokenMapper;
/** @var IL10N */
private $l;
private ClientMapper $clientMapper;
private ISecureRandom $secureRandom;
private AccessTokenMapper $accessTokenMapper;
private IL10N $l;
private IValidator $validator;

public const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';

Expand All @@ -57,18 +56,20 @@ public function __construct(string $appName,
ClientMapper $clientMapper,
ISecureRandom $secureRandom,
AccessTokenMapper $accessTokenMapper,
IL10N $l
IL10N $l,
IValidator $validator
) {
parent::__construct($appName, $request);
$this->secureRandom = $secureRandom;
$this->clientMapper = $clientMapper;
$this->accessTokenMapper = $accessTokenMapper;
$this->l = $l;
$this->validator = $validator;
}

public function addClient(string $name,
string $redirectUri): JSONResponse {
if (filter_var($redirectUri, FILTER_VALIDATE_URL) === false) {
if (count($this->validator->validate($redirectUri, [new Url()])) > 0) {
return new JSONResponse(['message' => $this->l->t('Your redirect URL needs to be a full URL for example: https://yourdomain.com/path')], Http::STATUS_BAD_REQUEST);
}

Expand Down
13 changes: 11 additions & 2 deletions apps/settings/lib/Controller/CheckSetupController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,9 @@
use OCP\Lock\ILockingProvider;
use OCP\Notification\IManager;
use OCP\Security\ISecureRandom;
use OCP\Validator\Constraints\Url;
use OCP\Validator\Constraints\NotBlank;
use OCP\Validator\IValidator;
use Psr\Log\LoggerInterface;
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
use Symfony\Component\EventDispatcher\GenericEvent;
Expand Down Expand Up @@ -125,6 +128,7 @@ class CheckSetupController extends Controller {
private $appManager;
/** @var IServerContainer */
private $serverContainer;
private IValidator $validator;

public function __construct($AppName,
IRequest $request,
Expand All @@ -145,7 +149,8 @@ public function __construct($AppName,
ITempManager $tempManager,
IManager $manager,
IAppManager $appManager,
IServerContainer $serverContainer
IServerContainer $serverContainer,
IValidator $validator
) {
parent::__construct($AppName, $request);
$this->config = $config;
Expand All @@ -166,6 +171,7 @@ public function __construct($AppName,
$this->manager = $manager;
$this->appManager = $appManager;
$this->serverContainer = $serverContainer;
$this->validator = $validator;
}

/**
Expand Down Expand Up @@ -618,7 +624,10 @@ protected function getSuggestedOverwriteCliURL(): string {
$suggestedOverwriteCliUrl = $this->request->getServerProtocol() . '://' . $this->request->getInsecureServerHost() . \OC::$WEBROOT;

// Check correctness by checking if it is a valid URL
if (filter_var($currentOverwriteCliUrl, FILTER_VALIDATE_URL)) {
if ($this->validator->isValid($currentOverwriteCliUrl, [
new NotBlank(),
Copy link
Contributor

@artonge artonge Mar 15, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't Url already check for NotBlank?

new Url(),
])) {
$suggestedOverwriteCliUrl = '';
}

Expand Down
126 changes: 58 additions & 68 deletions apps/theming/lib/Controller/ThemingController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@
use OCP\IRequest;
use OCP\ITempManager;
use OCP\IURLGenerator;
use OCP\Validator\Constraints\CssColor;
use OCP\Validator\Constraints\Length;
use OCP\Validator\Constraints\Url;
use OCP\Validator\IValidator;
use OCP\Validator\Violation;

/**
* Class ThemingController
Expand All @@ -63,39 +68,20 @@
* @package OCA\Theming\Controller
*/
class ThemingController extends Controller {
/** @var ThemingDefaults */
private $themingDefaults;
/** @var IL10N */
private $l10n;
/** @var IConfig */
private $config;
/** @var ITempManager */
private $tempManager;
/** @var IAppData */
private $appData;
/** @var SCSSCacher */
private $scssCacher;
/** @var IURLGenerator */
private $urlGenerator;
/** @var IAppManager */
private $appManager;
/** @var ImageManager */
private $imageManager;
private ThemingDefaults $themingDefaults;
private IL10N $l10n;
private IConfig $config;
private ITempManager $tempManager;
private IAppData $appData;
private SCSSCacher $scssCacher;
private IURLGenerator $urlGenerator;
private IAppManager $appManager;
private ImageManager $imageManager;
private IValidator $validator;

/**
* ThemingController constructor.
*
* @param string $appName
* @param IRequest $request
* @param IConfig $config
* @param ThemingDefaults $themingDefaults
* @param IL10N $l
* @param ITempManager $tempManager
* @param IAppData $appData
* @param SCSSCacher $scssCacher
* @param IURLGenerator $urlGenerator
* @param IAppManager $appManager
* @param ImageManager $imageManager
*/
public function __construct(
$appName,
CarlSchwan marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -108,7 +94,8 @@ public function __construct(
SCSSCacher $scssCacher,
IURLGenerator $urlGenerator,
IAppManager $appManager,
ImageManager $imageManager
ImageManager $imageManager,
IValidator $validator
) {
parent::__construct($appName, $request);

Expand All @@ -121,6 +108,7 @@ public function __construct(
$this->urlGenerator = $urlGenerator;
$this->appManager = $appManager;
$this->imageManager = $imageManager;
$this->validator = $validator;
}

/**
Expand All @@ -132,52 +120,62 @@ public function __construct(
*/
public function updateStylesheet($setting, $value) {
$value = trim($value);
$error = null;
$violations = [];
switch ($setting) {
case 'name':
if (strlen($value) > 250) {
$error = $this->l10n->t('The given name is too long');
}
$violations = $this->validator->validate($value, [
new Length([
'max' => 250,
'maxMessage' => $this->l10n->t('The given name is too long'),
])
]);
break;
case 'url':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given web address is too long');
}
if (!$this->isValidUrl($value)) {
$error = $this->l10n->t('The given web address is not a valid URL');
}
$violations = $this->validator->validate($value, [
new Length([
'max' => 500,
'maxMessage' => $this->l10n->t('The given web address is too long'),
]),
new Url(false, ['http', 'https'], $this->l10n->t('The given web address is not a valid URL')),
]);

break;
case 'imprintUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given legal notice address is too long');
}
if (!$this->isValidUrl($value)) {
$error = $this->l10n->t('The given legal notice address is not a valid URL');
}
$violations = $this->validator->validate($value, [
new Length([
'max' => 500,
'maxMessage' => $this->l10n->t('The given legal notice address is too long'),
]),
new Url(false, ['http', 'https'], $this->l10n->t('The given legal notice address is not a valid URL')),
]);
break;
case 'privacyUrl':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given privacy policy address is too long');
}
if (!$this->isValidUrl($value)) {
$error = $this->l10n->t('The given privacy policy address is not a valid URL');
}
$violations = $this->validator->validate($value, [
new Length([
'max' => 500,
'maxMessage' => $this->l10n->t('The given privacy policy address is too long'),
]),
new Url(false, ['http', 'https'], $this->l10n->t('The given privacy policy address is not a valid URL')),
]);
break;
case 'slogan':
if (strlen($value) > 500) {
$error = $this->l10n->t('The given slogan is too long');
}
$violations = $this->validator->validate($value, [
new Length([
'max' => 500,
'maxMessage' => $this->l10n->t('The given slogan is too long'),
])
]);
break;
case 'color':
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $value)) {
$error = $this->l10n->t('The given color is invalid');
}
$violations = $this->validator->validate($value, [
new CssColor($this->l10n->t('The given color is invalid')),
]);
break;
}
if ($error !== null) {
if (count($violations) > 0) {
return new DataResponse([
'data' => [
'message' => $error,
'message' => implode('. ', array_map(fn (Violation $violation) => $violation->getMessage(), $violations)) . '.',
],
'status' => 'error'
], Http::STATUS_BAD_REQUEST);
Expand All @@ -200,14 +198,6 @@ public function updateStylesheet($setting, $value) {
);
}

/**
* Check that a string is a valid http/https url
*/
private function isValidUrl(string $url): bool {
return ((strpos($url, 'http://') === 0 || strpos($url, 'https://') === 0) &&
filter_var($url, FILTER_VALIDATE_URL) !== false);
}

/**
* @AuthorizedAdminSetting(settings=OCA\Theming\Settings\Admin)
* @return DataResponse
Expand Down
19 changes: 14 additions & 5 deletions apps/theming/lib/ThemingDefaults.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,11 @@
use OCP\IL10N;
use OCP\INavigationManager;
use OCP\IURLGenerator;
use OCP\Util as OCPUtil;
use OCP\Validator\Constraints\CssColor;
use OCP\Validator\Constraints\NotBlank;
use OCP\Validator\Constraints\Url;
use OCP\Validator\IValidator;

class ThemingDefaults extends \OC_Defaults {

Expand Down Expand Up @@ -90,6 +95,7 @@ class ThemingDefaults extends \OC_Defaults {
private $AndroidClientUrl;
/** @var string */
private $FDroidClientUrl;
private IValidator $validator;

/**
* ThemingDefaults constructor.
Expand All @@ -109,7 +115,8 @@ public function __construct(IConfig $config,
Util $util,
ImageManager $imageManager,
IAppManager $appManager,
INavigationManager $navigationManager
INavigationManager $navigationManager,
IValidator $validator
) {
parent::__construct();
$this->config = $config;
Expand All @@ -120,6 +127,7 @@ public function __construct(IConfig $config,
$this->util = $util;
$this->appManager = $appManager;
$this->navigationManager = $navigationManager;
$this->validator = $validator;

$this->name = parent::getName();
$this->title = parent::getTitle();
Expand Down Expand Up @@ -208,9 +216,10 @@ public function getShortFooter() {
$legalLinks = '';
$divider = '';
foreach ($links as $link) {
if ($link['url'] !== ''
&& filter_var($link['url'], FILTER_VALIDATE_URL)
) {
if ($this->validator->isValid($link['url'], [
new NotBlank(),
new Url(),
])) {
$legalLinks .= $divider . '<a href="' . $link['url'] . '" class="legal" target="_blank"' .
' rel="noreferrer noopener">' . $link['text'] . '</a>';
$divider = ' · ';
Expand All @@ -230,7 +239,7 @@ public function getShortFooter() {
*/
public function getColorPrimary() {
$color = $this->config->getAppValue('theming', 'color', $this->color);
if (!preg_match('/^\#([0-9a-f]{3}|[0-9a-f]{6})$/i', $color)) {
if (!$this->validator->isValid($color, [new CssColor()])) {
$color = '#0082c9';
}
return $color;
Expand Down
Loading