Validate OAuth2 redirect uri

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

apps/oauth2/lib/Controller/SettingsController.php

@@ -27,7 +27,9 @@ use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
 use OCP\AppFramework\Controller;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
 
@@ -40,6 +42,8 @@ class SettingsController extends Controller {
 	private $accessTokenMapper;
 	/** @var  DefaultTokenMapper */
 	private $defaultTokenMapper;
+	/** @var IL10N */
+	private $l;
 
 	const validChars = 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789';
 
@@ -56,17 +60,24 @@ class SettingsController extends Controller {
 								ClientMapper $clientMapper,
 								ISecureRandom $secureRandom,
 								AccessTokenMapper $accessTokenMapper,
-								DefaultTokenMapper $defaultTokenMapper
+								DefaultTokenMapper $defaultTokenMapper,
+								IL10N $l
 	) {
 		parent::__construct($appName, $request);
 		$this->secureRandom = $secureRandom;
 		$this->clientMapper = $clientMapper;
 		$this->accessTokenMapper = $accessTokenMapper;
 		$this->defaultTokenMapper = $defaultTokenMapper;
+		$this->l = $l;
 	}
 
 	public function addClient(string $name,
 							  string $redirectUri): JSONResponse {
+
+		if (filter_var($redirectUri, FILTER_VALIDATE_URL, FILTER_FLAG_SCHEME_REQUIRED|FILTER_FLAG_HOST_REQUIRED) === false) {
+			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);
+		}
+
 		$client = new Client();
 		$client->setName($name);
 		$client->setRedirectUri($redirectUri);

apps/oauth2/src/App.vue

@@ -44,6 +44,7 @@
 
 		<br/>
 		<h3>{{ t('oauth2', 'Add client') }}</h3>
+		<span v-if="newClient.error" class="msg error">{{newClient.errorMsg}}</span>
 		<form @submit.prevent="addClient">
 			<input type="text" id="name" name="name" :placeholder="t('oauth2', 'Name')" v-model="newClient.name">
 			<input type="url" id="redirectUri" name="redirectUri" :placeholder="t('oauth2', 'Redirection URI')" v-model="newClient.redirectUri">
@@ -66,7 +67,9 @@ export default {
 			clients: [],
 			newClient: {
 				name: '',
-				redirectUri: ''
+				redirectUri: '',
+				errorMsg: '',
+				error: false
 			}
 		};
 	},
@@ -92,6 +95,7 @@ export default {
 		addClient() {
 			let requestToken = OC.requestToken;
 			let tokenHeaders = { headers: { requesttoken: requestToken } };
+			this.newClient.error = false;
 
 			axios.post(
 				OC.generateUrl('apps/oauth2/clients'),
@@ -99,14 +103,16 @@ export default {
 					name: this.newClient.name,
 					redirectUri: this.newClient.redirectUri
 				},
-				tokenHeaders)
-				.then((response) => {
-					this.clients.push(response.data)
+				tokenHeaders
+			).then(response => {
+				this.clients.push(response.data);
 
-					this.newClient.name = '';
-					this.newClient.redirectUri = '';
-				}
-			);
+				this.newClient.name = '';
+				this.newClient.redirectUri = '';
+			}).catch(reason => {
+				this.newClient.error = true;
+				this.newClient.errorMsg = reason.response.data.message;
+			});
 		}
 	},
 }

apps/oauth2/tests/Controller/SettingsControllerTest.php

@@ -26,7 +26,9 @@ use OCA\OAuth2\Controller\SettingsController;
 use OCA\OAuth2\Db\AccessTokenMapper;
 use OCA\OAuth2\Db\Client;
 use OCA\OAuth2\Db\ClientMapper;
+use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\JSONResponse;
+use OCP\IL10N;
 use OCP\IRequest;
 use OCP\Security\ISecureRandom;
 use Test\TestCase;
@@ -53,6 +55,9 @@ class SettingsControllerTest extends TestCase {
 		$this->secureRandom = $this->createMock(ISecureRandom::class);
 		$this->accessTokenMapper = $this->createMock(AccessTokenMapper::class);
 		$this->defaultTokenMapper = $this->createMock(DefaultTokenMapper::class);
+		$l = $this->createMock(IL10N::class);
+		$l->method('t')
+			->willReturnArgument(0);
 
 		$this->settingsController = new SettingsController(
 			'oauth2',
@@ -60,7 +65,8 @@ class SettingsControllerTest extends TestCase {
 			$this->clientMapper,
 			$this->secureRandom,
 			$this->accessTokenMapper,
-			$this->defaultTokenMapper
+			$this->defaultTokenMapper,
+			$l
 		);
 	}
 
@@ -178,4 +184,11 @@ class SettingsControllerTest extends TestCase {
 			],
 		], $data);
 	}
+
+	public function testInvalidRedirectUri() {
+		$result = $this->settingsController->addClient('test', 'invalidurl');
+
+		$this->assertEquals(Http::STATUS_BAD_REQUEST, $result->getStatus());
+		$this->assertSame(['message' => 'Your redirect url needs to be a full url for example: https://yourdomain.com/path'], $result->getData());
+	}
 }