diff --git a/.github/workflows/integration.yml b/.github/workflows/integration.yml index bb27dcc8..f6abd994 100644 --- a/.github/workflows/integration.yml +++ b/.github/workflows/integration.yml @@ -1,9 +1,12 @@ +# SPDX-FileCopyrightText: 2021-2024 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: MIT name: Integration tests on: pull_request: push: branches: + - main - master - stable* @@ -25,7 +28,7 @@ jobs: services: postgres: - image: postgres + image: postgres:14 ports: - 4445:5432/tcp env: @@ -34,7 +37,7 @@ jobs: POSTGRES_DB: nextcloud options: --health-cmd pg_isready --health-interval 5s --health-timeout 2s --health-retries 5 mysql: - image: mariadb:10.5 + image: mariadb:10.6 ports: - 4444:3306/tcp env: @@ -43,20 +46,14 @@ jobs: steps: - name: Checkout server - uses: actions/checkout@v2.3.4 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: repository: nextcloud/server ref: ${{ matrix.server-versions }} - - - name: Checkout submodules - shell: bash - run: | - auth_header="$(git config --local --get http.https://github.com/.extraheader)" - git submodule sync --recursive - git -c "http.extraheader=$auth_header" -c protocol.version=2 submodule update --init --force --recursive --depth=1 + submodules: recursive - name: Checkout app - uses: actions/checkout@v2.3.4 + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 with: path: apps/${{ env.APP_NAME }} @@ -67,11 +64,11 @@ jobs: working-directory: apps/${{ env.APP_NAME }}/tests/ - name: Set up php ${{ matrix.php-versions }} - uses: shivammathur/setup-php@v2 + uses: shivammathur/setup-php@44454db4f0199b8b9685a5d763dc37cbf79108e1 # v2.36.0 with: php-version: ${{ matrix.php-versions }} tools: phpunit - extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql, + extensions: zip, gd, mbstring, iconv, fileinfo, intl, sqlite, pdo_sqlite, mysql, pdo_mysql, pgsql, pdo_pgsql coverage: none - name: Set up PHPUnit @@ -87,11 +84,12 @@ jobs: fi mkdir data ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass admin - cat config/config.php ./occ user:list ./occ app:enable --force ${{ env.APP_NAME }} ./occ config:system:set allow_local_remote_servers --value true --type bool - ./occ user_oidc:provider nextcloudci -c nextcloud -s ff75b7c7-20f9-460b-b27c-16bd5d9b4cd0 -d http:/127.0.0.1:8999/auth/realms/nextcloudci/.well-known/openid-configuration + ./occ config:system:set debug --value true --type bool + cat config/config.php + ./occ user_oidc:provider nextcloudci -c nextcloud -s ff75b7c7-20f9-460b-b27c-16bd5d9b4cd0 -d http://127.0.0.1:8999/auth/realms/nextcloudci/.well-known/openid-configuration php -S localhost:8080 & curl -v http://127.0.0.1:8999/auth/realms/nextcloudci/.well-known/openid-configuration diff --git a/.github/workflows/psalm.yml b/.github/workflows/psalm.yml new file mode 100644 index 00000000..e6235808 --- /dev/null +++ b/.github/workflows/psalm.yml @@ -0,0 +1,40 @@ +# This workflow is provided via the organization template repository +# +# https://github.com/nextcloud/.github +# https://docs.github.com/en/actions/learn-github-actions/sharing-workflows-with-your-organization +# +# SPDX-FileCopyrightText: 2022-2024 Nextcloud GmbH and Nextcloud contributors +# SPDX-License-Identifier: MIT + +name: Static analysis + +on: pull_request + +concurrency: + group: psalm-${{ github.head_ref || github.run_id }} + cancel-in-progress: true + +jobs: + static-analysis: + runs-on: ubuntu-latest + + name: static-psalm-analysis + steps: + - name: Checkout + uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1 + + - name: Set up php7.4 + uses: shivammathur/setup-php@44454db4f0199b8b9685a5d763dc37cbf79108e1 # v2.36.0 + with: + php-version: 7.4 + extensions: bz2, ctype, curl, dom, fileinfo, gd, iconv, intl, json, libxml, mbstring, openssl, pcntl, posix, session, simplexml, xmlreader, xmlwriter, zip, zlib, sqlite, pdo_sqlite + coverage: none + ini-file: development + env: + GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Install dependencies + run: composer i + + - name: Run coding standards check + run: composer run psalm diff --git a/composer.json b/composer.json index 498ac65c..fe642d00 100644 --- a/composer.json +++ b/composer.json @@ -9,13 +9,16 @@ "bamarni/composer-bin-plugin": true }, "platform": { - "php": "7.3" + "php": "7.4" } }, "scripts": { "cs:fix": "php-cs-fixer fix", "cs:check": "php-cs-fixer fix --dry-run --diff", "lint": "find . -name \\*.php -not -path './vendor/*' -exec php -l \"{}\" \\;", + "psalm": "psalm.phar --no-cache", + "psalm:update-baseline": "psalm.phar --threads=1 --update-baseline", + "psalm:update-baseline:force": "psalm.phar --threads=1 --update-baseline --set-baseline=tests/psalm-baseline.xml", "test:unit": "phpunit -c tests/phpunit.xml", "post-install-cmd": [ "@composer bin all install --ansi", @@ -36,7 +39,9 @@ "require-dev": { "nextcloud/coding-standard": "^1.0.0", "symfony/event-dispatcher": "^4", - "phpunit/phpunit": "^9.6" + "phpunit/phpunit": "^9.6", + "nextcloud/ocp": "dev-stable20", + "psalm/phar": "5.26" }, "extra": { "mozart": { diff --git a/composer.lock b/composer.lock index 867a5569..a56194e6 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "cac05b88ab1f0a0a0b90881e8d87e0e9", + "content-hash": "19ad5d44101622d8bb0685b615335b50", "packages": [ { "name": "bamarni/composer-bin-plugin", @@ -953,6 +953,45 @@ }, "time": "2021-11-10T08:44:10+00:00" }, + { + "name": "nextcloud/ocp", + "version": "dev-stable20", + "source": { + "type": "git", + "url": "https://github.com/nextcloud-deps/ocp.git", + "reference": "ee973539f8ee3dfa83dc9126390b7155fd5ffe55" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/nextcloud-deps/ocp/zipball/ee973539f8ee3dfa83dc9126390b7155fd5ffe55", + "reference": "ee973539f8ee3dfa83dc9126390b7155fd5ffe55", + "shasum": "" + }, + "require": { + "php": "^7.2" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "20.0.0-dev" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "AGPL-3.0-or-later" + ], + "authors": [ + { + "name": "Christoph Wurst", + "email": "christoph@winzerhof-wurst.at" + } + ], + "description": "Composer package containing Nextcloud's public API (classes, interfaces)", + "support": { + "source": "https://github.com/nextcloud-deps/ocp/tree/stable20" + }, + "time": "2021-11-11T14:03:28+00:00" + }, { "name": "nikic/php-parser", "version": "v4.19.4", @@ -1602,6 +1641,41 @@ ], "time": "2024-12-05T13:48:26+00:00" }, + { + "name": "psalm/phar", + "version": "5.26.0", + "source": { + "type": "git", + "url": "https://github.com/psalm/phar.git", + "reference": "004fa1aa67e1d04198458a2e5b34c75b40257519" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/psalm/phar/zipball/004fa1aa67e1d04198458a2e5b34c75b40257519", + "reference": "004fa1aa67e1d04198458a2e5b34c75b40257519", + "shasum": "" + }, + "require": { + "php": "^7.1 || ^8.0" + }, + "conflict": { + "vimeo/psalm": "*" + }, + "bin": [ + "psalm.phar" + ], + "type": "library", + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Composer-based Psalm Phar", + "support": { + "issues": "https://github.com/psalm/phar/issues", + "source": "https://github.com/psalm/phar/tree/5.26.0" + }, + "time": "2024-09-08T17:24:40+00:00" + }, { "name": "psr/cache", "version": "1.0.1", @@ -4136,13 +4210,15 @@ ], "aliases": [], "minimum-stability": "stable", - "stability-flags": [], + "stability-flags": { + "nextcloud/ocp": 20 + }, "prefer-stable": false, "prefer-lowest": false, - "platform": [], - "platform-dev": [], + "platform": {}, + "platform-dev": {}, "platform-overrides": { - "php": "7.3" + "php": "7.4" }, "plugin-api-version": "2.6.0" } diff --git a/lib/Command/DeleteProvider.php b/lib/Command/DeleteProvider.php index ba9fb164..1f6ecb58 100644 --- a/lib/Command/DeleteProvider.php +++ b/lib/Command/DeleteProvider.php @@ -24,12 +24,12 @@ namespace OCA\UserOIDC\Command; use Exception; -use OCP\AppFramework\Db\DoesNotExistException; -use \Symfony\Component\Console\Command\Command; - use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Service\ProviderService; +use OCP\AppFramework\Db\DoesNotExistException; +use Symfony\Component\Console\Command\Command; + use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Input\InputOption; diff --git a/lib/Command/UpsertProvider.php b/lib/Command/UpsertProvider.php index 625dbea2..cfb92c9d 100644 --- a/lib/Command/UpsertProvider.php +++ b/lib/Command/UpsertProvider.php @@ -23,15 +23,15 @@ namespace OCA\UserOIDC\Command; +use OCA\UserOIDC\Db\ProviderMapper; use OCA\UserOIDC\Service\ProviderService; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\MultipleObjectsReturnedException; -use \Symfony\Component\Console\Command\Command; -use OCA\UserOIDC\Db\ProviderMapper; +use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Helper\Table; use Symfony\Component\Console\Input\InputArgument; -use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Output\OutputInterface; class UpsertProvider extends Command { @@ -137,7 +137,7 @@ protected function execute(InputInterface $input, OutputInterface $output) { // invalidate JWKS cache (even if it was just created) $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE, ''); $this->providerService->setSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP, ''); - } catch (DoesNotExistException | MultipleObjectsReturnedException $e) { + } catch (DoesNotExistException|MultipleObjectsReturnedException $e) { $output->writeln('' . $e->getMessage() . ''); return -1; } diff --git a/lib/Controller/Id4meController.php b/lib/Controller/Id4meController.php index b5cbdfe7..569eb05e 100644 --- a/lib/Controller/Id4meController.php +++ b/lib/Controller/Id4meController.php @@ -31,6 +31,7 @@ use OCA\UserOIDC\Db\UserMapper; use OCA\UserOIDC\Helper\HttpClientHelper; use OCA\UserOIDC\Service\ID4MeService; +use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; use OCP\AppFramework\Controller; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Http; @@ -42,8 +43,12 @@ use OCP\ISession; use OCP\IURLGenerator; use OCP\IUserManager; -use OCP\IUserSession; use OCP\Security\ISecureRandom; +use Psr\Log\LoggerInterface; + +require_once __DIR__ . '/../../vendor/autoload.php'; +use Id4me\RP\Model\OpenIdConfig; +use Id4me\RP\Service; class Id4meController extends Controller { private const STATE = 'oidc.state'; @@ -65,7 +70,7 @@ class Id4meController extends Controller { /** @var UserMapper */ private $userMapper; - /** @var IUserSession */ + /** @var \OC\User\Session */ private $userSession; /** @var IUserManager */ @@ -83,6 +88,9 @@ class Id4meController extends Controller { /** @var IL10N */ private $l10n; + /** @var LoggerInterface */ + private $logger; + public function __construct( IRequest $request, @@ -91,12 +99,13 @@ public function __construct( IClientService $clientService, IURLGenerator $urlGenerator, UserMapper $userMapper, - IUserSession $userSession, + \OC\User\Session $userSession, IUserManager $userManager, HttpClientHelper $clientHelper, Id4MeMapper $id4MeMapper, ID4MeService $id4MeService, - IL10N $l10n + IL10N $l10n, + LoggerInterface $logger, ) { parent::__construct(Application::APP_ID, $request); @@ -111,6 +120,7 @@ public function __construct( $this->id4MeMapper = $id4MeMapper; $this->id4MeService = $id4MeService; $this->l10n = $l10n; + $this->logger = $logger; } private function build403TemplateResponse(): Http\TemplateResponse { @@ -241,7 +251,24 @@ public function code($state = '', $code = '', $scope = '') { $plainHeaders = json_decode(base64_decode($header), true); $plainPayload = json_decode(base64_decode($payload), true); - /** TODO: VALIATE SIGNATURE! */ + // validate the JWT signature + $idTokenRaw = $data['id_token']; + $jwkUri = $openIdConfig->getJwksUri(); + JWT::$leeway = 60; + try { + $jwks = $this->id4MeService->obtainJWK($jwkUri, $data['id_token'], true); + $idTokenPayload = JWT::decode($idTokenRaw, $jwks); + } catch (\Exception|\Throwable $e) { + $this->logger->debug('Failed to decode the JWT token, retrying with fresh JWK'); + try { + $jwks = $this->id4MeService->obtainJWK($jwkUri, $idTokenRaw, false); + $idTokenPayload = JWT::decode($idTokenRaw, $jwks); + } catch (\Exception|\Throwable $e) { + $this->logger->debug('Failed to decode the JWT token with fresh JWK'); + $message = $this->l10n->t('Failed to authenticate'); + return $this->build403TemplateResponse(); + } + } // TODO: validate expiration diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 0b29a00f..cf1e18b6 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -27,14 +27,14 @@ namespace OCA\UserOIDC\Controller; +use OCA\UserOIDC\AppInfo\Application; +use OCA\UserOIDC\Db\ProviderMapper; +use OCA\UserOIDC\Db\UserMapper; use OCA\UserOIDC\Event\AttributeMappedEvent; use OCA\UserOIDC\Event\TokenObtainedEvent; use OCA\UserOIDC\Service\DiscoveryService; use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; -use OCA\UserOIDC\AppInfo\Application; -use OCA\UserOIDC\Db\ProviderMapper; -use OCA\UserOIDC\Db\UserMapper; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; use OCP\AppFramework\Http\JSONResponse; @@ -49,7 +49,6 @@ use OCP\IURLGenerator; use OCP\IUser; use OCP\IUserManager; -use OCP\IUserSession; use OCP\Security\ISecureRandom; class LoginController extends Controller { @@ -73,7 +72,7 @@ class LoginController extends Controller { /** @var UserMapper */ private $userMapper; - /** @var IUserSession */ + /** @var \OC\User\Session */ private $userSession; /** @var IUserManager */ @@ -111,12 +110,12 @@ public function __construct( IClientService $clientService, IURLGenerator $urlGenerator, UserMapper $userMapper, - IUserSession $userSession, + \OC\User\Session $userSession, IUserManager $userManager, ITimeFactory $timeFactory, IEventDispatcher $eventDispatcher, IConfig $config, - ILogger $logger + ILogger $logger, ) { parent::__construct(Application::APP_ID, $request); @@ -141,7 +140,7 @@ public function __construct( * @NoCSRFRequired * @UseSession */ - public function login(int $providerId, string $redirectUrl = null) { + public function login(int $providerId, ?string $redirectUrl = null) { if ($this->userSession->isLoggedIn()) { return new RedirectResponse($redirectUrl); } @@ -208,10 +207,10 @@ public function login(int $providerId, string $redirectUrl = null) { ]; // pass discovery query parameters also on to the authentication $discoveryUrl = parse_url($provider->getDiscoveryEndpoint()); - if (isset($discoveryUrl["query"])) { - $this->logger->debug('Add custom discovery query: ' . $discoveryUrl["query"]); + if (isset($discoveryUrl['query'])) { + $this->logger->debug('Add custom discovery query: ' . $discoveryUrl['query']); $discoveryQuery = []; - parse_str($discoveryUrl["query"], $discoveryQuery); + parse_str($discoveryUrl['query'], $discoveryQuery); $data += $discoveryQuery; } @@ -417,7 +416,6 @@ private function provisionUser(string $userId, int $providerId, object $idTokenP * @UseSession * * @return Http\RedirectResponse - * @throws Error */ public function singleLogoutService() { $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); diff --git a/lib/Controller/SettingsController.php b/lib/Controller/SettingsController.php index 6db2a12c..b514be73 100644 --- a/lib/Controller/SettingsController.php +++ b/lib/Controller/SettingsController.php @@ -49,8 +49,8 @@ public function __construct( IRequest $request, ProviderMapper $providerMapper, ID4MeService $id4meService, - ProviderService $providerService - ) { + ProviderService $providerService, + ) { parent::__construct(Application::APP_ID, $request); $this->providerMapper = $providerMapper; @@ -59,7 +59,7 @@ public function __construct( } public function createProvider(string $identifier, string $clientId, string $clientSecret, string $discoveryEndpoint, - array $settings = [], string $scope = "openid email profile"): JSONResponse { + array $settings = [], string $scope = 'openid email profile'): JSONResponse { if ($this->providerService->getProviderByIdentifier($identifier) !== null) { return new JSONResponse(['message' => 'Provider with the given identifier already exists'], Http::STATUS_CONFLICT); } @@ -77,8 +77,8 @@ public function createProvider(string $identifier, string $clientId, string $cli return new JSONResponse(array_merge($provider->jsonSerialize(), ['settings' => $providerSettings])); } - public function updateProvider(int $providerId, string $identifier, string $clientId, string $discoveryEndpoint, string $clientSecret = null, - array $settings = [], string $scope = "openid email profile"): JSONResponse { + public function updateProvider(int $providerId, string $identifier, string $clientId, string $discoveryEndpoint, ?string $clientSecret = null, + array $settings = [], string $scope = 'openid email profile'): JSONResponse { $provider = $this->providerMapper->getProvider($providerId); if ($this->providerService->getProviderByIdentifier($identifier) === null) { @@ -120,7 +120,7 @@ public function getProviders(): JSONResponse { } public function getID4ME(): JSONResponse { - return new JSONResponse($this->id4meService->getID4ME()); + return new JSONResponse(['value' => $this->id4meService->getID4ME()]); } public function setID4ME(bool $enabled): JSONResponse { diff --git a/lib/Db/Id4MeMapper.php b/lib/Db/Id4MeMapper.php index 25fdd3de..12c65f20 100644 --- a/lib/Db/Id4MeMapper.php +++ b/lib/Db/Id4MeMapper.php @@ -28,6 +28,9 @@ use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; +/** + * @template-extends QBMapper + */ class Id4MeMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'user_oidc_id4me', Id4Me::class); diff --git a/lib/Db/ProviderMapper.php b/lib/Db/ProviderMapper.php index f4b60378..42d69195 100644 --- a/lib/Db/ProviderMapper.php +++ b/lib/Db/ProviderMapper.php @@ -25,11 +25,14 @@ namespace OCA\UserOIDC\Db; +use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; -use OCP\IDBConnection; -use OCP\AppFramework\Db\DoesNotExistException; +use OCP\IDBConnection; +/** + * @template-extends QBMapper + */ class ProviderMapper extends QBMapper { public function __construct(IDBConnection $db) { parent::__construct($db, 'user_oidc_providers', Provider::class); @@ -96,9 +99,9 @@ public function getProviders() { * @throws \OCP\AppFramework\Db\DoesNotExistException * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException */ - public function createOrUpdateProvider(string $identifier, string $clientid = null, - string $clientsecret = null, string $discoveryuri = null, - string $scope = 'openid email profile') { + public function createOrUpdateProvider(string $identifier, ?string $clientid = null, + ?string $clientsecret = null, ?string $discoveryuri = null, + string $scope = 'openid email profile') { try { $provider = $this->findProviderByIdentifier($identifier); } catch (DoesNotExistException $eNotExist) { @@ -108,7 +111,7 @@ public function createOrUpdateProvider(string $identifier, string $clientid = nu if ($provider === null) { $provider = new Provider(); if (($clientid === null) || ($clientsecret === null) || ($discoveryuri === null)) { - throw new DoesNotExistException("Provider must be created. All provider parameters required."); + throw new DoesNotExistException('Provider must be created. All provider parameters required.'); } $provider->setIdentifier($identifier); $provider->setClientId($clientid); diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index 89ad7295..9ec7303b 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -25,16 +25,19 @@ namespace OCA\UserOIDC\Db; +use OC\Cache\CappedMemoryCache; use OCA\UserOIDC\Service\ProviderService; use OCP\AppFramework\Db\IMapperException; use OCP\AppFramework\Db\QBMapper; use OCP\IDBConnection; -use OC\Cache\CappedMemoryCache; +/** + * @template-extends QBMapper + */ class UserMapper extends QBMapper { /** @var ProviderService */ private $providerService; - /** CappedMemoryCache */ + /** @var CappedMemoryCache */ private $userCache; public function __construct(IDBConnection $db, ProviderService $providerService) { @@ -67,6 +70,7 @@ public function getUser(string $uid): User { public function find(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); + /** @psalm-suppress ImplicitToStringCast */ $qb->select('*') ->from($this->getTableName()) ->where($qb->expr()->iLike('user_id', $qb->createPositionalParameter('%' . $this->db->escapeLikeParameter($search) . '%'))) @@ -81,6 +85,7 @@ public function find(string $search, $limit = null, $offset = null): array { public function findDisplayNames(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); + /** @psalm-suppress ImplicitToStringCast */ $qb->select('*') ->from($this->getTableName()) ->where($qb->expr()->iLike('user_id', $qb->createPositionalParameter('%' . $this->db->escapeLikeParameter($search) . '%'))) diff --git a/lib/Event/AttributeMappedEvent.php b/lib/Event/AttributeMappedEvent.php index c258e281..83d8aca8 100644 --- a/lib/Event/AttributeMappedEvent.php +++ b/lib/Event/AttributeMappedEvent.php @@ -1,4 +1,5 @@ * @@ -58,7 +59,7 @@ public function getAttribute(): string { } /** - * @return array the array of claim values associated with the event + * @return object the array of claim values associated with the event */ public function getClaims(): object { return $this->claims; @@ -69,7 +70,7 @@ public function hasValue() : bool { } /** - * @return value for the logged in user attribute + * @return ?string for the logged in user attribute */ public function getValue(): ?string { return $this->value; diff --git a/lib/Event/TokenObtainedEvent.php b/lib/Event/TokenObtainedEvent.php index 06ba71c0..fab63123 100644 --- a/lib/Event/TokenObtainedEvent.php +++ b/lib/Event/TokenObtainedEvent.php @@ -1,4 +1,5 @@ * diff --git a/lib/Event/TokenValidatedEvent.php b/lib/Event/TokenValidatedEvent.php index 99468c96..8b63679e 100644 --- a/lib/Event/TokenValidatedEvent.php +++ b/lib/Event/TokenValidatedEvent.php @@ -1,4 +1,5 @@ * diff --git a/lib/Service/DiscoveryService.php b/lib/Service/DiscoveryService.php index af17a944..ed8a19c5 100644 --- a/lib/Service/DiscoveryService.php +++ b/lib/Service/DiscoveryService.php @@ -1,4 +1,5 @@ * @@ -73,7 +74,7 @@ public function obtainDiscovery(Provider $provider): array { public function obtainJWK(Provider $provider): array { $lastJwksRefresh = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE_TIMESTAMP); - if ($lastJwksRefresh !== '' && (int) $lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) { + if ($lastJwksRefresh !== '' && (int)$lastJwksRefresh > time() - self::INVALIDATE_JWKS_CACHE_AFTER_SECONDS) { $rawJwks = $this->providerService->getSetting($provider->getId(), ProviderService::SETTING_JWKS_CACHE); $rawJwks = json_decode($rawJwks, true); $jwks = JWK::parseKeySet($rawJwks); diff --git a/lib/Service/ID4MeService.php b/lib/Service/ID4MeService.php index 97607dd0..7d5d57c8 100644 --- a/lib/Service/ID4MeService.php +++ b/lib/Service/ID4MeService.php @@ -26,15 +26,36 @@ namespace OCA\UserOIDC\Service; use OCA\UserOIDC\AppInfo\Application; +use OCA\UserOIDC\Vendor\Firebase\JWT\JWK; +use OCP\Http\Client\IClientService; +use OCP\ICacheFactory; use OCP\IConfig; +use Psr\Log\LoggerInterface; class ID4MeService { /** @var IConfig */ private $config; - public function __construct(IConfig $config) { + /** @var \OCP\ICache */ + private $cache; + + /** @var \OCP\Http\Client\IClient */ + private $client; + + /** @var LoggerInterface */ + private $logger; + + public function __construct( + IConfig $config, + IClientService $clientService, + ICacheFactory $cacheFactory, + LoggerInterface $logger, + ) { $this->config = $config; + $this->cache = $cacheFactory->createDistributed('user_oidc'); + $this->client = $clientService->newClient(); + $this->logger = $logger; } public function setID4ME(bool $enabled): void { @@ -44,4 +65,23 @@ public function setID4ME(bool $enabled): void { public function getID4ME(): bool { return $this->config->getAppValue(Application::APP_ID, 'id4me_enabled', '0') === '1'; } + + public function obtainJWK(string $jwkUri, string $tokenToDecode, bool $useCache = true): array { + $cacheKey = 'jwks-' . $jwkUri; + $cachedJwks = $this->cache->get($cacheKey); + if ($cachedJwks !== null && $useCache) { + $rawJwks = json_decode($cachedJwks, true, 512, JSON_THROW_ON_ERROR); + $this->logger->debug('[ID4ME-obtainJWK] jwks cache content', ['jwks_cache' => $rawJwks]); + } else { + $responseBody = (string)$this->client->get($jwkUri)->getBody(); + $rawJwks = json_decode($responseBody, true, 512, JSON_THROW_ON_ERROR); + $this->logger->debug('[ID4ME-obtainJWK] getting fresh jwks', ['jwks' => $rawJwks]); + $this->cache->set($cacheKey, $responseBody, DiscoveryService::INVALIDATE_JWKS_CACHE_AFTER_SECONDS); + } + + $this->logger->debug('[ID4ME-obtainJWK] jwks', ['jwks' => $rawJwks]); + $jwks = JWK::parseKeySet($rawJwks); + $this->logger->debug('Parsed the jwks'); + return $jwks; + } } diff --git a/lib/Service/OIDCService.php b/lib/Service/OIDCService.php index 5e1bbfe2..73597013 100644 --- a/lib/Service/OIDCService.php +++ b/lib/Service/OIDCService.php @@ -1,4 +1,5 @@ * @@ -38,6 +39,9 @@ class OIDCService { /** @var IClientService */ private $clientService; + /** @var DiscoveryService */ + private $discoveryService; + public function __construct(DiscoveryService $discoveryService, LoggerInterface $logger, IClientService $clientService) { $this->discoveryService = $discoveryService; $this->logger = $logger; diff --git a/lib/Service/ProviderService.php b/lib/Service/ProviderService.php index d88265c3..22489abd 100644 --- a/lib/Service/ProviderService.php +++ b/lib/Service/ProviderService.php @@ -1,4 +1,5 @@ * diff --git a/lib/Service/SettingsService.php b/lib/Service/SettingsService.php index efe988d0..21123c79 100644 --- a/lib/Service/SettingsService.php +++ b/lib/Service/SettingsService.php @@ -1,4 +1,5 @@ * diff --git a/lib/Settings/AdminSettings.php b/lib/Settings/AdminSettings.php index f70e4ee7..f80c0f26 100644 --- a/lib/Settings/AdminSettings.php +++ b/lib/Settings/AdminSettings.php @@ -49,9 +49,9 @@ class AdminSettings implements ISettings { private $initialStateService; public function __construct(ProviderService $providerService, - ID4MeService $ID4MEService, - IURLGenerator $urlGenerator, - IInitialStateService $initialStateService) { + ID4MeService $ID4MEService, + IURLGenerator $urlGenerator, + IInitialStateService $initialStateService) { $this->providerService = $providerService; $this->Id4MeService = $ID4MEService; $this->urlGenerator = $urlGenerator; diff --git a/lib/Settings/Section.php b/lib/Settings/Section.php index 3c385ef2..5d03af88 100644 --- a/lib/Settings/Section.php +++ b/lib/Settings/Section.php @@ -43,7 +43,7 @@ class Section implements IIconSection { * @param IURLGenerator $urlGenerator */ public function __construct(IL10N $l, - IURLGenerator $urlGenerator) { + IURLGenerator $urlGenerator) { $this->l = $l; $this->urlGenerator = $urlGenerator; } diff --git a/lib/User/Backend.php b/lib/User/Backend.php index 2d9ef448..0231f7eb 100644 --- a/lib/User/Backend.php +++ b/lib/User/Backend.php @@ -25,18 +25,17 @@ namespace OCA\UserOIDC\User; -use OCA\UserOIDC\Event\TokenValidatedEvent; +use OCA\UserOIDC\AppInfo\Application; use OCA\UserOIDC\Controller\LoginController; +use OCA\UserOIDC\Db\ProviderMapper; +use OCA\UserOIDC\Db\UserMapper; +use OCA\UserOIDC\Event\TokenValidatedEvent; use OCA\UserOIDC\Service\DiscoveryService; use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\User\Validator\SelfEncodedValidator; use OCA\UserOIDC\User\Validator\UserInfoValidator; -use OCA\UserOIDC\AppInfo\Application; -use OCA\UserOIDC\Db\ProviderMapper; -use OCA\UserOIDC\Db\UserMapper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\Authentication\IApacheBackend; -use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; use OCP\IConfig; use OCP\IRequest; @@ -93,16 +92,16 @@ class Backend extends ABackend implements IPasswordConfirmationBackend, IGetDisp private $userManager; public function __construct(IConfig $config, - UserMapper $userMapper, - LoggerInterface $logger, - IRequest $request, - ISession $session, - IURLGenerator $urlGenerator, - IEventDispatcher $eventDispatcher, - DiscoveryService $discoveryService, - ProviderMapper $providerMapper, - ProviderService $providerService, - IUserManager $userManager) { + UserMapper $userMapper, + LoggerInterface $logger, + IRequest $request, + ISession $session, + IURLGenerator $urlGenerator, + IEventDispatcher $eventDispatcher, + DiscoveryService $discoveryService, + ProviderMapper $providerMapper, + ProviderService $providerService, + IUserManager $userManager) { $this->config = $config; $this->userMapper = $userMapper; $this->logger = $logger; @@ -125,7 +124,7 @@ public function deleteUser($uid): bool { $user = $this->userMapper->getUser($uid); $this->userMapper->delete($user); return true; - } catch (Exception $e) { + } catch (\Exception $e) { $this->logger->error('Failed to delete user', [ 'exception' => $e ]); return false; } diff --git a/lib/User/Validator/IBearerTokenValidator.php b/lib/User/Validator/IBearerTokenValidator.php index 0f241495..463881a8 100644 --- a/lib/User/Validator/IBearerTokenValidator.php +++ b/lib/User/Validator/IBearerTokenValidator.php @@ -1,4 +1,5 @@ * diff --git a/lib/User/Validator/SelfEncodedValidator.php b/lib/User/Validator/SelfEncodedValidator.php index 14557814..c6d8a0b0 100644 --- a/lib/User/Validator/SelfEncodedValidator.php +++ b/lib/User/Validator/SelfEncodedValidator.php @@ -1,4 +1,5 @@ * diff --git a/lib/User/Validator/UserInfoValidator.php b/lib/User/Validator/UserInfoValidator.php index 48518e93..4a9c0bac 100644 --- a/lib/User/Validator/UserInfoValidator.php +++ b/lib/User/Validator/UserInfoValidator.php @@ -1,4 +1,5 @@ * @@ -27,8 +28,8 @@ use OCA\UserOIDC\Db\Provider; use OCA\UserOIDC\Service\DiscoveryService; -use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\Service\OIDCService; +use OCA\UserOIDC\Service\ProviderService; use Psr\Log\LoggerInterface; class UserInfoValidator implements IBearerTokenValidator { diff --git a/psalm.xml b/psalm.xml new file mode 100644 index 00000000..3bb0c244 --- /dev/null +++ b/psalm.xml @@ -0,0 +1,81 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/tests/bootstrap.php b/tests/bootstrap.php index 6f46408d..9462def5 100644 --- a/tests/bootstrap.php +++ b/tests/bootstrap.php @@ -1,4 +1,5 @@ * @@ -22,8 +23,8 @@ if (!defined('PHPUNIT_RUN')) { define('PHPUNIT_RUN', 1); } -require_once __DIR__.'/../../../lib/base.php'; -require_once __DIR__.'/../vendor/autoload.php'; +require_once __DIR__ . '/../../../lib/base.php'; +require_once __DIR__ . '/../vendor/autoload.php'; \OC::$loader->addValidRoot(OC::$SERVERROOT . '/tests'); \OC_App::loadApp('user_oidc'); diff --git a/tests/integration/Test.php b/tests/integration/Test.php index a4141c42..2f5d48b4 100644 --- a/tests/integration/Test.php +++ b/tests/integration/Test.php @@ -1,4 +1,5 @@ * @@ -30,11 +31,12 @@ use OCA\UserOIDC\Service\ProviderService; use OCP\IConfig; use OCP\IUserManager; +use Test\TestCase; /** * @group DB */ -class Test extends \Test\TestCase { +class Test extends TestCase { private $oidcIdp = 'http://127.0.0.1:8999'; private $baseUrl = 'http://localhost:8080'; @@ -130,7 +132,7 @@ private function loginToKeycloak($keycloakURL, $username, $password) { $form = $result->item(0); $url = $form->getAttribute('action'); libxml_clear_errors(); - return $this->client->post($url, ['form_params' => ['username' => $username, 'password' => $password, "credentialId" => '']]); + return $this->client->post($url, ['form_params' => ['username' => $username, 'password' => $password, 'credentialId' => '']]); } private function getUserHtmlData($response) { diff --git a/tests/psalm-baseline.xml b/tests/psalm-baseline.xml new file mode 100644 index 00000000..5d91a0bc --- /dev/null +++ b/tests/psalm-baseline.xml @@ -0,0 +1,3 @@ + + + diff --git a/tests/stubs/oc_app.php b/tests/stubs/oc_app.php new file mode 100644 index 00000000..fd3cdd87 --- /dev/null +++ b/tests/stubs/oc_app.php @@ -0,0 +1,11 @@ + * diff --git a/tests/unit/Service/ProviderServiceTest.php b/tests/unit/Service/ProviderServiceTest.php index 6a6eec3d..c0f13fc0 100644 --- a/tests/unit/Service/ProviderServiceTest.php +++ b/tests/unit/Service/ProviderServiceTest.php @@ -1,4 +1,5 @@ *