Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions appinfo/info.xml
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ While theoretically any other authentication provider implementing either one of
<command>OCA\User_SAML\Command\ConfigDelete</command>
<command>OCA\User_SAML\Command\ConfigGet</command>
<command>OCA\User_SAML\Command\ConfigSet</command>
<command>OCA\User_SAML\Command\ConfigValidate</command>
<command>OCA\User_SAML\Command\GetMetadata</command>
<command>OCA\User_SAML\Command\GroupMigrationCopyIncomplete</command>
<command>OCA\User_SAML\Command\GroupMigrationManual</command>
Expand Down
74 changes: 74 additions & 0 deletions lib/Command/ConfigValidate.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\User_SAML\Command;

use OC\Core\Command\Base;
use OCA\User_SAML\SAMLSettings;
use OCP\AppFramework\Services\IAppConfig;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;

class ConfigValidate extends Base {

private const REQUIRED_FIELDS = [
'idp-entityId',
'idp-singleSignOnService.url',
'general-uid_mapping',
];

public function __construct(
private readonly SAMLSettings $samlSettings,
private readonly IAppConfig $appConfig,
) {
parent::__construct();
}

#[\Override]
protected function configure(): void {
$this->setName('saml:config:validate');
$this->setDescription('Check whether user_saml is correctly configured');
parent::configure();
}

#[\Override]
protected function execute(InputInterface $input, OutputInterface $output): int {
$type = $this->appConfig->getAppValueString('type', '');
if ($type === '') {
$output->writeln('<comment>user_saml is not active (type not set). Configure it via the admin panel.</comment>');
return 1;
}
$output->writeln('Type: <info>' . $type . '</info>');

$idps = $this->samlSettings->getListOfIdps();
if (empty($idps)) {
$output->writeln('<error>No IdP providers configured. Add one via the admin panel or occ saml:config:create.</error>');
return 2;
}

$exitCode = 0;
foreach ($idps as $id => $name) {
$cfg = $this->samlSettings->get($id);
$missing = [];
foreach (self::REQUIRED_FIELDS as $field) {
if (empty($cfg[$field])) {
$missing[] = $field;
}
}
$label = "IdP #{$id}" . ($name !== '' ? " ({$name})" : '');
if (empty($missing)) {
$output->writeln(" {$label}: <info>OK</info>");
} else {
$output->writeln(" {$label}: <error>MISSING: " . implode(', ', $missing) . '</error>');
$exitCode = 2;
}
}

return $exitCode;
}
}
12 changes: 11 additions & 1 deletion lib/Controller/SAMLController.php
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,16 @@ public function login(int $idp = 1): Http\RedirectResponse|Http\TemplateResponse
$type = $this->appConfig->getAppValueString('type');
switch ($type) {
case 'saml':
$settings = $this->samlSettings->getOneLoginSettingsArray($idp);
try {
$settings = $this->samlSettings->getOneLoginSettingsArray($idp);
} catch (\InvalidArgumentException $e) {
return new Http\RedirectResponse(
$this->urlGenerator->linkToRouteAbsolute(
'user_saml.SAML.genericError',
['reason' => 'notConfigured']
)
);
}
$auth = new Auth($settings);
$passthroughParamsString = trim($settings['idp']['passthroughParameters'] ?? '') ;
$passthroughParams = array_map(trim(...), explode(',', $passthroughParamsString));
Expand Down Expand Up @@ -555,6 +564,7 @@ public function genericError(string $reason): Http\TemplateResponse {
$allowedMessages = [
'userDisabled' => $this->l->t('This user account is disabled, please contact your administrator.'),
'authFailed' => $this->l->t('Authentication failed.'),
'notConfigured' => $this->l->t('SAML authentication is not configured. Please ask your administrator to complete the SAML setup in the admin panel.'),
];

$message = $allowedMessages[$reason] ?? $this->l->t('Unknown error, please check the log file for more details.');
Expand Down
8 changes: 7 additions & 1 deletion lib/SAMLSettings.php
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ public function usesSloWebServerDecode(int $idp): bool {
public function getOneLoginSettingsArray(int $idp): array {
$this->ensureConfigurationsLoaded($idp);

if (($this->configurations[$idp] ?? []) === []) {
throw new InvalidArgumentException(
"SAML provider #{$idp} does not exist or has not been configured yet."
);
}

$settings = [
'strict' => true,
'debug' => $this->config->getSystemValueBool('debug'),
Expand All @@ -153,7 +159,7 @@ public function getOneLoginSettingsArray(int $idp): array {
// "sloWebServerDecode" is not expected to be passed to the OneLogin class
],
'sp' => [
'entityId' => (array_key_exists('sp-entityId', $this->configurations[$idp]) && trim($this->configurations[$idp]['sp-entityId']) != '')
'entityId' => (trim($this->configurations[$idp]['sp-entityId'] ?? '') !== '')
? $this->configurations[$idp]['sp-entityId']
: $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.getMetadata'),
'assertionConsumerService' => [
Expand Down
139 changes: 139 additions & 0 deletions tests/unit/Command/ConfigValidateTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
<?php

declare(strict_types=1);
/**
* SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

namespace OCA\User_SAML\Tests\Command;

use OCA\User_SAML\Command\ConfigValidate;
use OCA\User_SAML\SAMLSettings;
use OCP\AppFramework\Services\IAppConfig;
use Override;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Output\OutputInterface;
use Test\TestCase;

class ConfigValidateTest extends TestCase {
private SAMLSettings&MockObject $samlSettings;
private IAppConfig&MockObject $appConfig;
private ConfigValidate $command;

#[Override]
protected function setUp(): void {
parent::setUp();

$this->samlSettings = $this->createMock(SAMLSettings::class);
$this->appConfig = $this->createMock(IAppConfig::class);
$this->command = new ConfigValidate($this->samlSettings, $this->appConfig);
}

private function makeInput(): InputInterface {
$input = $this->createMock(InputInterface::class);
$input->method('getOption')->willReturn(null);
$input->method('getArgument')->willReturn(null);
return $input;
}

public function testExitsWithCodeOneWhenTypeNotSet(): void {
$this->appConfig->expects($this->once())
->method('getAppValueString')
->with('type', '')
->willReturn('');

$output = $this->createMock(OutputInterface::class);
$output->expects($this->once())
->method('writeln')
->with($this->stringContains('not active'));

$exitCode = $this->invokePrivate($this->command, 'execute', [$this->makeInput(), $output]);

$this->assertEquals(1, $exitCode);
}

public function testExitsWithCodeTwoWhenNoIdpsConfigured(): void {
$this->appConfig->expects($this->once())
->method('getAppValueString')
->with('type', '')
->willReturn('saml');

$this->samlSettings->expects($this->once())
->method('getListOfIdps')
->willReturn([]);

$output = $this->createMock(OutputInterface::class);
$output->expects($this->exactly(2))
->method('writeln')
->willReturnCallback(function (string $msg): void {
// just capture; specific assertions follow via mock constraints
});

$exitCode = $this->invokePrivate($this->command, 'execute', [$this->makeInput(), $output]);

$this->assertEquals(2, $exitCode);
}

public function testExitsZeroWhenFullyConfigured(): void {
$this->appConfig->method('getAppValueString')
->with('type', '')
->willReturn('saml');

$this->samlSettings->method('getListOfIdps')
->willReturn([1 => 'My IdP']);

$this->samlSettings->method('get')
->with(1)
->willReturn([
'idp-entityId' => 'https://idp.example.com',
'idp-singleSignOnService.url' => 'https://idp.example.com/sso',
'general-uid_mapping' => 'uid',
]);

$output = $this->createMock(OutputInterface::class);
$messages = [];
$output->method('writeln')
->willReturnCallback(function (string $msg) use (&$messages): void {
$messages[] = $msg;
});

$exitCode = $this->invokePrivate($this->command, 'execute', [$this->makeInput(), $output]);

$this->assertEquals(0, $exitCode);
$this->assertTrue(
count(array_filter($messages, fn (string $m) => str_contains($m, 'OK'))) > 0,
'Expected at least one "OK" message'
);
}

public function testExitsWithCodeTwoWhenRequiredFieldsMissing(): void {
$this->appConfig->method('getAppValueString')
->with('type', '')
->willReturn('saml');

$this->samlSettings->method('getListOfIdps')
->willReturn([1 => 'My IdP']);

$this->samlSettings->method('get')
->with(1)
->willReturn([]); // all required fields missing

$output = $this->createMock(OutputInterface::class);
$messages = [];
$output->method('writeln')
->willReturnCallback(function (string $msg) use (&$messages): void {
$messages[] = $msg;
});

$exitCode = $this->invokePrivate($this->command, 'execute', [$this->makeInput(), $output]);

$this->assertEquals(2, $exitCode);

$errorMessages = implode(' ', $messages);
$this->assertStringContainsString('idp-entityId', $errorMessages);
$this->assertStringContainsString('idp-singleSignOnService.url', $errorMessages);
$this->assertStringContainsString('general-uid_mapping', $errorMessages);
}
}
27 changes: 27 additions & 0 deletions tests/unit/Controller/SAMLControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -421,4 +421,31 @@ public function testUserFilterNotApplicable(): void {

$this->invokePrivate($this->samlController, 'assertGroupMemberships');
}

public function testLoginWithUnconfiguredIdpRedirectsToGenericError(): void {
$this->appConfig->expects($this->once())
->method('getAppValueString')
->with('type')
->willReturn('saml');

$this->request->expects($this->any())
->method('getParam')
->willReturn('');

$this->samlSettings->expects($this->once())
->method('getOneLoginSettingsArray')
->with(1)
->willThrowException(new \InvalidArgumentException('SAML provider #1 does not exist'));

$errorUrl = 'https://example.com/error';
$this->urlGenerator->expects($this->once())
->method('linkToRouteAbsolute')
->with('user_saml.SAML.genericError', ['reason' => 'notConfigured'])
->willReturn($errorUrl);

$result = $this->samlController->login(1);

$this->assertInstanceOf(RedirectResponse::class, $result);
$this->assertEquals($errorUrl, $result->getRedirectURL());
}
}
Loading
Loading