Add db value store

CHANGELOG.md

@@ -10,6 +10,8 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/).
 
 - Provision accounts based on auto-provisioning claim - [#149](https://github.com/owncloud/openidconnect/issues/149)
 
+- Add app db table as additional, optional config storage - [#67](https://github.com/owncloud/openidconnect/pull/167)
+
 ## [2.0.0] - 2021-01-10
 
 ### Added

README.md

@@ -10,13 +10,32 @@ A distributed memcache setup is required to properly operate this app - like Red
 For development purpose APCu is reasonable as well.
 Please follow the [documentation on how to set up caching](https://doc.owncloud.org/server/admin_manual/configuration/server/caching_configuration.html#supported-caching-backends).
 
-### Setup config.php
-The OpenId integration is established by entering the parameters below to the
-ownCloud configuration file.
+### Setup
+The OpenId integration is established by either entering the parameters below to the
+ownCloud configuration file or saving them to the app config database table.
+
 _provider-url_, _client-id_ and _client-secret- are to be taken from the OpenId
 Provider setup.
 _loginButtonName_ can be chosen freely depending on the installation.
 
+### Settings in database
+Store your settings as JSON formatted string in the app database table `oc_appconfig` with the config key `openid-connect`. The key->value pairs are the same as storing them to config.php. This method is preferred for stateless, clustered setups.
+
+```
+INSERT INTO oc_appconfig (
+  appid,
+  configkey,
+  configvalue
+) VALUES (
+  'openidconnect',
+  'openid-connect',
+  '{"provider-url":"https:\/\/idp.example.net","client-id":"fc9b5c78-ec73-47bf-befc-59d4fe780f6f","client-secret":"e3e5b04a-3c3c-4f4d-b16c-2a6e9fdd3cd1","loginButtonName":"Login via OpenId Connect"}'
+);
+```
+
+The app checks for settings in the database first. If none is found it falls back to the config.php. If a malformed JSON string is found an error is thrown to the logger instance.
+
+### Settings in config.php
 ```php
 <?php
 $CONFIG = [

lib/Application.php

@@ -29,6 +29,8 @@
 use OCP\AppFramework\App;
 
 class Application extends App {
+	public const APPID = 'openidconnect';
+
 	/** @var Logger */
 	private $logger;
 
@@ -39,7 +41,7 @@ class Application extends App {
 	 * @codeCoverageIgnore
 	 */
 	public function __construct(array $urlParams = []) {
-		parent::__construct('openidconnect', $urlParams);
+		parent::__construct(Application::APPID, $urlParams);
 	}
 
 	/**

lib/Client.php

@@ -26,6 +26,7 @@
 use OCP\IConfig;
 use OCP\ISession;
 use OCP\IURLGenerator;
+use OCP\ILogger;
 
 class Client extends OpenIDConnectClient {
 
@@ -35,6 +36,9 @@ class Client extends OpenIDConnectClient {
 	private $config;
 	/** @var array */
 	private $wellKnownConfig;
+	/** @var ILogger */
+	private $logger;
+
 	/**
 	 * @var IURLGenerator
 	 */
@@ -46,15 +50,18 @@ class Client extends OpenIDConnectClient {
 	 * @param IConfig $config
 	 * @param IURLGenerator $generator
 	 * @param ISession $session
+	 * @param ILogger $logger
 	 */
 	public function __construct(
 		IConfig $config,
 		IURLGenerator $generator,
-		ISession $session
+		ISession $session,
+		ILogger $logger
 	) {
 		$this->session = $session;
 		$this->config = $config;
 		$this->generator = $generator;
+		$this->logger = $logger;
 
 		$openIdConfig = $this->getOpenIdConfig();
 		if ($openIdConfig === null) {
@@ -87,6 +94,19 @@ public function __construct(
 	 * @return mixed
 	 */
 	public function getOpenIdConfig() {
+		$configRaw = $this->config->getAppValue(Application::APPID, 'openid-connect', null);
+		if ($configRaw) {
+			$config = json_decode($configRaw, true);
+			if (json_last_error() !== JSON_ERROR_NONE) {
+				$this->logger->error(
+					'Loaded config from DB is not valid (malformed JSON); JSON Last Error: ' . json_last_error(),
+					['app' => Application::APPID]
+				);
+				return $this->config->getSystemValue('openid-connect', null);
+			}
+			return $config;
+		}
+
 		return $this->config->getSystemValue('openid-connect', null);
 	}
 

tests/unit/ClientTest.php

@@ -26,6 +26,7 @@
 use OCP\IConfig;
 use OCP\ISession;
 use OCP\IURLGenerator;
+use OCP\ILogger;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
@@ -43,6 +44,10 @@ class ClientTest extends TestCase {
 	 * @var MockObject | IURLGenerator
 	 */
 	private $urlGenerator;
+	/**
+	 * @var MockObject | ILogger
+	 */
+	private $logger;
 	/**
 	 * @var MockObject | IConfig
 	 */
@@ -55,14 +60,22 @@ public function providesGetUserInfoData(): array {
 		];
 	}
 
+	public function appConfigProvider(): \Generator {
+		yield 'invalid json' => ['from system config', '{[s', 'Loaded config from DB is not valid (malformed JSON); JSON Last Error: 4'];
+		yield 'empty app config' => ['from system config', ''];
+		yield 'empty array' => [[], '[]'];
+		yield 'json object' => [['foo' => 'bar'], '{"foo": "bar"}'];
+	}
+
 	protected function setUp(): void {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
 		$this->urlGenerator = $this->createMock(IURLGenerator::class);
 		$this->session = $this->createMock(ISession::class);
+		$this->logger = $this->createMock(ILogger::class);
 
 		$this->client = $this->getMockBuilder(Client::class)
-			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session])
+			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session, $this->logger])
 			->setMethods(['fetchURL'])
 			->getMock();
 	}
@@ -73,6 +86,21 @@ public function testGetConfig(): void {
 		self::assertEquals('foo', $return);
 	}
 
+	/**
+	 * @dataProvider appConfigProvider
+	 */
+	public function testGetAppConfig($expectedData, $dataInConfig, $expectedErrorMessage = null): void {
+		$this->config->method('getSystemValue')->willReturn('from system config');
+		$this->config->expects(self::once())->method('getAppValue')->willReturnCallback(function ($app, $key, $default) use ($dataInConfig) {
+			return $dataInConfig;
+		});
+		if ($expectedErrorMessage) {
+			$this->logger->expects(self::once())->method('error')->with($expectedErrorMessage);
+		}
+		$return = $this->client->getOpenIdConfig();
+		self::assertEquals($expectedData, $return);
+	}
+
 	public function testGetWellKnown(): void {
 		$this->client->setProviderURL('https://example.net');
 		$this->client->expects(self::once())->method('fetchURL')->with('https://example.net/.well-known/openid-configuration')->willReturn('{"foo": "bar"}');
@@ -97,7 +125,7 @@ public function testCtor(): void {
 			throw new \InvalidArgumentException("Unexpected key: $key");
 		});
 		$this->client = $this->getMockBuilder(Client::class)
-			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session])
+			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session, $this->logger])
 			->setMethods(['fetchURL'])
 			->getMock();
 
@@ -124,7 +152,7 @@ public function testCtorInsecure(): void {
 			throw new \InvalidArgumentException("Unexpected key: $key");
 		});
 		$this->client = $this->getMockBuilder(Client::class)
-			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session])
+			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session, $this->logger])
 			->setMethods(['fetchURL'])
 			->getMock();
 
@@ -151,7 +179,7 @@ public function testGetUserInfo($useAccessTokenPayloadForUserInfo): void {
 		});
 
 		$this->client = $this->getMockBuilder(Client::class)
-			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session])
+			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session, $this->logger])
 			->onlyMethods(['requestUserInfo', 'getAccessTokenPayload'])
 			->getMock();
 		if ($useAccessTokenPayloadForUserInfo) {