Skip to content

Commit f3a29b5

Browse files
CarlSchwanCarlSchwan
committed
refactor: Port more stuff to IAppConfig
Signed-off-by: Carl Schwan <[email protected]>
1 parent 6e09be1 commit f3a29b5

File tree

9 files changed

+121
-137
lines changed

9 files changed

+121
-137
lines changed

‎lib/AppInfo/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use OCP\AppFramework\Bootstrap\IBootstrap;
2727
use OCP\AppFramework\Bootstrap\IRegistrationContext;
2828
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
29+
use OCP\AppFramework\Services\IAppConfig;
2930
use OCP\EventDispatcher\IEventDispatcher;
3031
use OCP\IConfig;
3132
use OCP\IDBConnection;
@@ -55,7 +56,7 @@ public function register(IRegistrationContext $context): void {
5556
$context->registerEventListener(SabrePluginAddEvent::class, SabrePluginEventListener::class);
5657
$context->registerService(DavPlugin::class, fn (ContainerInterface $c) => new DavPlugin(
5758
$c->get(ISession::class),
58-
$c->get(IConfig::class),
59+
$c->get(IAppConfig::class),
5960
$_SERVER,
6061
$c->get(SAMLSettings::class)
6162
));

‎lib/Controller/SAMLController.php‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
use OCA\User_SAML\UserResolver;
2323
use OCP\AppFramework\Controller;
2424
use OCP\AppFramework\Http;
25+
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
26+
use OCP\AppFramework\Http\Attribute\NoCSRFRequired;
27+
use OCP\AppFramework\Http\Attribute\PublicPage;
28+
use OCP\AppFramework\Http\Attribute\UseSession;
29+
use OCP\AppFramework\Services\IAppConfig;
2530
use OCP\IConfig;
2631
use OCP\IL10N;
2732
use OCP\IRequest;
@@ -50,6 +55,7 @@ public function __construct(
5055
private SAMLSettings $samlSettings,
5156
private UserBackend $userBackend,
5257
private IConfig $config,
58+
private IAppConfig $appConfig,
5359
private IURLGenerator $urlGenerator,
5460
private LoggerInterface $logger,
5561
private IL10N $l,
@@ -137,20 +143,19 @@ protected function assertGroupMemberships(): void {
137143
}
138144

139145
/**
140-
* @PublicPage
141-
* @UseSession
142146
* @OnlyUnauthenticatedUsers
143-
* @NoCSRFRequired
144-
*
145147
* @throws Exception
146148
*/
149+
#[PublicPage]
150+
#[UseSession]
151+
#[NoCSRFRequired]
147152
public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse {
148153
$originalUrl = (string)$this->request->getParam('originalUrl', '');
149154
if (!$this->trustedDomainHelper->isTrustedUrl($originalUrl)) {
150155
$originalUrl = '';
151156
}
152157

153-
$type = $this->config->getAppValue($this->appName, 'type');
158+
$type = $this->appConfig->getAppValueString('type');
154159
switch ($type) {
155160
case 'saml':
156161
$settings = $this->samlSettings->getOneLoginSettingsArray($idp);
@@ -273,10 +278,10 @@ public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse
273278
}
274279

275280
/**
276-
* @PublicPage
277-
* @NoCSRFRequired
278281
* @throws Error
279282
*/
283+
#[PublicPage]
284+
#[NoCSRFRequired]
280285
public function getMetadata(int $idp = 1): Http\DataDownloadResponse {
281286
$settings = new Settings($this->samlSettings->getOneLoginSettingsArray($idp));
282287
$metadata = $settings->getSPMetadata();
@@ -292,16 +297,16 @@ public function getMetadata(int $idp = 1): Http\DataDownloadResponse {
292297
}
293298

294299
/**
295-
* @PublicPage
296-
* @NoCSRFRequired
297-
* @UseSession
298300
* @OnlyUnauthenticatedUsers
299301
* @NoSameSiteCookieRequired
300302
*
301303
* @return Http\RedirectResponse
302304
* @throws Error
303305
* @throws ValidationError
304306
*/
307+
#[PublicPage]
308+
#[NoCSRFRequired]
309+
#[UseSession]
305310
public function assertionConsumerService(): Http\RedirectResponse {
306311
// Fetch and decrypt the cookie
307312
$cookie = $this->request->getCookie('saml_data');
@@ -423,12 +428,12 @@ public function assertionConsumerService(): Http\RedirectResponse {
423428
}
424429

425430
/**
426-
* @PublicPage
427-
* @NoAdminRequired
428-
* @NoCSRFRequired
429-
* @UseSession
430431
* @throws Error
431432
*/
433+
#[PublicPage]
434+
#[NoAdminRequired]
435+
#[UseSession]
436+
#[NoCSRFRequired]
432437
public function singleLogoutService(): Http\RedirectResponse {
433438
$isFromGS = ($this->config->getSystemValueBool('gs.enabled', false)
434439
&& $this->config->getSystemValueString('gss.mode', '') === 'master');
@@ -506,7 +511,7 @@ public function singleLogoutService(): Http\RedirectResponse {
506511
}
507512

508513
/**
509-
* @returns [?string, ?Auth]
514+
* @return array{0: ?string, 1: ?Auth}
510515
*/
511516
private function tryProcessSLOResponse(?int $idp): array {
512517
$idps = ($idp !== null) ? [$idp] : array_keys($this->samlSettings->getListOfIdps());
@@ -532,28 +537,28 @@ private function tryProcessSLOResponse(?int $idp): array {
532537
}
533538

534539
/**
535-
* @PublicPage
536-
* @NoCSRFRequired
537540
* @OnlyUnauthenticatedUsers
538541
*/
542+
#[PublicPage]
543+
#[NoCSRFRequired]
539544
public function notProvisioned(): Http\TemplateResponse {
540545
return new Http\TemplateResponse($this->appName, 'notProvisioned', [], 'guest');
541546
}
542547

543548
/**
544-
* @PublicPage
545-
* @NoCSRFRequired
546549
* @OnlyUnauthenticatedUsers
547550
*/
551+
#[PublicPage]
552+
#[NoCSRFRequired]
548553
public function notPermitted(): Http\TemplateResponse {
549554
return new Http\TemplateResponse($this->appName, 'notPermitted', [], 'guest');
550555
}
551556

552557
/**
553-
* @PublicPage
554-
* @NoCSRFRequired
555558
* @OnlyUnauthenticatedUsers
556559
*/
560+
#[PublicPage]
561+
#[NoCSRFRequired]
557562
public function genericError(string $message): Http\TemplateResponse {
558563
if (empty($message)) {
559564
$message = $this->l->t('Unknown error, please check the log file for more details.');
@@ -562,17 +567,17 @@ public function genericError(string $message): Http\TemplateResponse {
562567
}
563568

564569
/**
565-
* @PublicPage
566-
* @NoCSRFRequired
567570
* @OnlyUnauthenticatedUsers
568571
*/
572+
#[PublicPage]
573+
#[NoCSRFRequired]
569574
public function selectUserBackEnd(string $redirectUrl = ''): Http\TemplateResponse {
570575
$attributes = ['loginUrls' => []];
571576

572577
if ($this->samlSettings->allowMultipleUserBackEnds()) {
573578
$displayName = $this->l->t('Direct log in');
574579

575-
$customDisplayName = $this->config->getAppValue('user_saml', 'directLoginName', '');
580+
$customDisplayName = $this->appConfig->getAppValueString('directLoginName');
576581
if ($customDisplayName !== '') {
577582
$displayName = $customDisplayName;
578583
}
@@ -584,10 +589,8 @@ public function selectUserBackEnd(string $redirectUrl = ''): Http\TemplateRespon
584589
}
585590

586591
$attributes['loginUrls']['ssoLogin'] = $this->getIdps($redirectUrl);
587-
588592
$attributes['useCombobox'] = count($attributes['loginUrls']['ssoLogin']) > 4;
589593

590-
591594
return new Http\TemplateResponse($this->appName, 'selectUserBackEnd', $attributes, 'guest');
592595
}
593596

@@ -651,17 +654,14 @@ protected function getSSODisplayName(?string $displayName): string {
651654
* get Nextcloud login URL
652655
*/
653656
private function getDirectLoginUrl(string $redirectUrl): string {
654-
$directUrl = $this->urlGenerator->linkToRouteAbsolute('core.login.tryLogin', [
657+
return $this->urlGenerator->linkToRouteAbsolute('core.login.tryLogin', [
655658
'direct' => '1',
656659
'redirect_url' => $redirectUrl,
657660
]);
658-
return $directUrl;
659661
}
660662

661-
/**
662-
* @PublicPage
663-
* @NoCSRFRequired
664-
*/
663+
#[PublicPage]
664+
#[NoCSRFRequired]
665665
public function base(): Http\TemplateResponse {
666666
$message = $this->l->t('This page should not be visited directly.');
667667
return new Http\TemplateResponse($this->appName, 'error', ['message' => $message], 'guest');

‎lib/DavPlugin.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
namespace OCA\User_SAML;
99

1010
use OCA\DAV\Connector\Sabre\Auth;
11-
use OCP\IConfig;
11+
use OCP\AppFramework\Services\IAppConfig;
1212
use OCP\ISession;
1313
use Sabre\DAV\Server;
1414
use Sabre\DAV\ServerPlugin;
@@ -21,21 +21,21 @@ class DavPlugin extends ServerPlugin {
2121

2222
public function __construct(
2323
private readonly ISession $session,
24-
private readonly IConfig $config,
24+
private readonly IAppConfig $config,
2525
private array $auth,
2626
private readonly SAMLSettings $samlSettings,
2727
) {
2828
}
2929

30-
public function initialize(Server $server) {
30+
public function initialize(Server $server): void {
3131
// before auth
3232
$server->on('beforeMethod:*', $this->beforeMethod(...), 9);
3333
$this->server = $server;
3434
}
3535

36-
public function beforeMethod(RequestInterface $request, ResponseInterface $response) {
36+
public function beforeMethod(RequestInterface $request, ResponseInterface $response): void {
3737
if (
38-
$this->config->getAppValue('user_saml', 'type') === 'environment-variable'
38+
$this->config->getAppValueString('type') === 'environment-variable'
3939
&& !$this->session->exists('user_saml.samlUserData')
4040
) {
4141
$uidMapping = $this->samlSettings->get(1)['general-uid_mapping'];

‎lib/Listener/LoadAdditionalScriptsListener.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
namespace OCA\User_SAML\Listener;
1111

1212
use OCP\AppFramework\Http\Events\BeforeTemplateRenderedEvent;
13+
use OCP\Config\IUserConfig;
1314
use OCP\EventDispatcher\Event;
1415
use OCP\EventDispatcher\IEventListener;
15-
use OCP\IConfig;
1616
use OCP\ISession;
1717
use OCP\IUserSession;
1818
use OCP\Util;
@@ -22,7 +22,7 @@ class LoadAdditionalScriptsListener implements IEventListener {
2222
public function __construct(
2323
private readonly ISession $session,
2424
private readonly IUserSession $userSession,
25-
private readonly IConfig $config,
25+
private readonly IUserConfig $userConfig,
2626
) {
2727
}
2828

@@ -39,7 +39,7 @@ public function handle(Event $event): void {
3939
if ($user === null) {
4040
return; // already checked by $event->isLoggedIn above
4141
}
42-
$timezoneDB = $this->config->getUserValue($user->getUID(), 'core', 'timezone', '');
42+
$timezoneDB = $this->userConfig->getValueString($user->getUID(), 'core', 'timezone');
4343

4444
if ($timezoneDB === '' || !$this->session->exists('timezone')) {
4545
Util::addScript('user_saml', 'vendor/jstz.min');

‎lib/Listener/SabrePluginEventListener.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,16 +11,30 @@
1111

1212
use OCA\DAV\Events\SabrePluginAddEvent;
1313
use OCA\User_SAML\DavPlugin;
14+
use OCA\User_SAML\SAMLSettings;
15+
use OCP\AppFramework\Services\IAppConfig;
1416
use OCP\EventDispatcher\Event;
1517
use OCP\EventDispatcher\IEventListener;
16-
use OCP\Server;
18+
use OCP\ISession;
1719

1820
/** @template-implements IEventListener<SabrePluginAddEvent|Event> */
1921
class SabrePluginEventListener implements IEventListener {
22+
public function __construct(
23+
private readonly ISession $session,
24+
private readonly IAppConfig $appConfig,
25+
private readonly SAMLSettings $settings,
26+
) {
27+
}
28+
2029
public function handle(Event $event): void {
2130
if (!$event instanceof SabrePluginAddEvent) {
2231
return;
2332
}
24-
$event->getServer()->addPlugin(Server::get(DavPlugin::class));
33+
$event->getServer()->addPlugin(new DavPlugin(
34+
$this->session,
35+
$this->appConfig,
36+
$_SERVER,
37+
$this->settings,
38+
));
2539
}
2640
}

‎lib/Middleware/OnlyLoggedInMiddleware.php‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use OCP\AppFramework\Utility\IControllerMethodReflector;
1313
use OCP\IURLGenerator;
1414
use OCP\IUserSession;
15+
use Override;
1516

1617
/**
1718
* Class OnlyLoggedInMiddleware prevents access to a controller method if the user
@@ -33,7 +34,8 @@ public function __construct(
3334
* @param string $methodName
3435
* @throws \Exception
3536
*/
36-
public function beforeController($controller, $methodName) {
37+
#[Override]
38+
public function beforeController($controller, $methodName): void {
3739
if ($this->reflector->hasAnnotation('OnlyUnauthenticatedUsers') && $this->userSession->isLoggedIn()) {
3840
throw new \Exception('User is already logged-in');
3941
}
@@ -43,10 +45,10 @@ public function beforeController($controller, $methodName) {
4345
* @param \OCP\AppFramework\Controller $controller
4446
* @param string $methodName
4547
* @param \Exception $exception
46-
* @return RedirectResponse
4748
* @throws \Exception
4849
*/
49-
public function afterException($controller, $methodName, \Exception $exception) {
50+
#[Override]
51+
public function afterException($controller, $methodName, \Exception $exception): RedirectResponse {
5052
if ($exception->getMessage() === 'User is already logged-in') {
5153
return new RedirectResponse($this->urlGenerator->getAbsoluteURL('/'));
5254
}

‎lib/Settings/Admin.php‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -83,11 +83,6 @@ public function getForm(): TemplateResponse {
8383
'type' => 'line',
8484
'required' => true,
8585
],
86-
'require_provisioned_account' => [
87-
'text' => $this->l10n->t('Only allow authentication if an account exists on some other backend (e.g. LDAP).'),
88-
'type' => 'checkbox',
89-
'global' => true,
90-
],
9186
];
9287
$attributeMappingSettings = [
9388
'displayName_mapping' => [
@@ -188,7 +183,7 @@ public function getForm(): TemplateResponse {
188183
$nameIdFormats[Constants::NAMEID_UNSPECIFIED]['selected'] = true;
189184
}
190185

191-
$type = $this->appConfig->getAppValueString( 'type');
186+
$type = $this->appConfig->getAppValueString('type');
192187

193188
$generalSettings['require_provisioned_account'] = [
194189
'text' => $this->l10n->t('Only allow authentication if an account exists on some other backend (e.g. LDAP).', [$this->defaults->getName()]),
@@ -213,7 +208,7 @@ public function getForm(): TemplateResponse {
213208
'type' => 'checkbox',
214209
'hideForEnv' => true,
215210
'global' => true,
216-
'value' => $this->appConfig->getAppValueInt('general-allow_multiple_user_back_ends' )
211+
'value' => $this->appConfig->getAppValueInt('general-allow_multiple_user_back_ends')
217212
];
218213
}
219214

0 commit comments

Comments
 (0)