Merge pull request #103 from owncloud/feature/azure-ad-hack

Azure AD: Use access token payload instead of user info endpoint

lib/Client.php

@@ -99,6 +99,15 @@ public function getWellKnownConfig() {
 		return $this->wellKnownConfig;
 	}
 
+	public function getUserInfo() {
+		$openIdConfig = $this->getOpenIdConfig();
+		if (isset($openIdConfig['use-access-token-payload-for-user-info']) && $openIdConfig['use-access-token-payload-for-user-info']) {
+			return $this->getAccessTokenPayload();
+		}
+
+		return $this->requestUserInfo();
+	}
+
 	/**
 	 * @codeCoverageIgnore
 	 */

lib/Controller/LoginFlowController.php

@@ -126,9 +126,15 @@ public function login(): RedirectResponse {
 			$this->logger->logException($ex);
 			throw new HintException('Error in OpenIdConnect:' . $ex->getMessage());
 		}
-		$this->logger->debug('Access token: ' . $openid->getAccessToken());
-		$this->logger->debug('Refresh token: ' . $openid->getRefreshToken());
-		$userInfo = $openid->requestUserInfo();
+		$debugInfo = \json_encode([
+			'access_token' => $openid->getAccessToken(),
+			'refresh_token' => $openid->getRefreshToken(),
+			'id_token' => $openid->getIdToken(),
+			'access_token_payload' => $openid->getAccessTokenPayload(),
+		], JSON_PRETTY_PRINT);
+		$this->logger->debug('LoginFlowController::login : Token info: ' . $debugInfo);
+
+		$userInfo = $openid->getUserInfo();
 		$this->logger->debug('User info: ' . \json_encode($userInfo));
 		if (!$userInfo) {
 			throw new LoginException('No user information available.');

lib/OpenIdConnectAuthModule.php

@@ -93,6 +93,7 @@ public function auth(IRequest $request): ?IUser {
 	 * @throws LoginException
 	 */
 	public function authToken(string $bearerToken): ?IUser {
+		$this->logger->debug('OpenIdConnectAuthModule::authToken ' . $bearerToken);
 		try {
 			if ($this->client->getOpenIdConfig() === null) {
 				return null;
@@ -104,6 +105,7 @@ public function authToken(string $bearerToken): ?IUser {
 			if ($expiry) {
 				$expiring = $expiry - \time();
 				if ($expiring < 0) {
+					$this->logger->debug("OpenID Connect token expired at $expiry");
 					throw new LoginException('OpenID Connect token expired');
 				}
 			}
@@ -114,6 +116,7 @@ public function authToken(string $bearerToken): ?IUser {
 				$this->updateCache($bearerToken, $user, $expiry);
 				return $user;
 			}
+			$this->logger->debug('OpenIdConnectAuthModule::authToken : no user retrieved from token ' . $bearerToken);
 			return null;
 		} catch (OpenIDConnectClientException $ex) {
 			$this->logger->logException($ex);
@@ -173,33 +176,30 @@ private function verifyJWT($bearerToken) {
 	 * @return ICache
 	 */
 	private function getCache(): ICache {
-		return $this->cacheFactory->create('oca.openid-connect');
+		// TODO: needs cleanup and consolidation with SessionVerifier usage of the cache
+		return $this->cacheFactory->create('oca.openid-connect.2');
 	}
 
 	private function getUserResource($bearerToken) {
 		$cache = $this->getCache();
 		$userInfo = $cache->get($bearerToken);
 		if ($userInfo) {
+			$this->logger->debug('OpenIdConnectAuthModule::getUserResource from cache: ' . \json_encode($userInfo));
 			return $this->manager->get($userInfo['uid']);
 		}
 
 		$this->client->setAccessToken($bearerToken);
 
-		$userInfo = $this->client->requestUserInfo();
-		$this->logger->debug('User info: ' . \json_encode($userInfo));
+		$userInfo = $this->client->getUserInfo();
+		$this->logger->debug('OpenIdConnectAuthModule::getUserResource from cache: ' . \json_encode($userInfo));
 		if ($userInfo === null) {
 			return null;
 		}
 
 		return $this->lookupService->lookupUser($userInfo);
 	}
 
-	/**
-	 * @param string $bearerToken
-	 * @param IUser $user
-	 * @param int $expiry
-	 */
-	private function updateCache($bearerToken, IUser $user, $expiry): void {
+	private function updateCache(string $bearerToken, IUser $user, int $expiry): void {
 		$cache = $this->getCache();
 		$cache->set($bearerToken, [
 			'uid' => $user->getUID(),

lib/Service/UserLookupService.php

@@ -60,6 +60,9 @@ public function lookupUser($userInfo): IUser {
 			$searchByEmail = false;
 		}
 		$attribute = $openIdConfig['search-attribute'] ?? 'email';
+		if (!\property_exists($userInfo, $attribute)) {
+			throw new LoginException("Configured attribute $attribute is not known.");
+		}
 
 		if ($searchByEmail) {
 			$user = $this->userManager->getByEmail($userInfo->$attribute);

tests/unit/ClientTest.php

@@ -48,6 +48,13 @@ class ClientTest extends TestCase {
 	 */
 	private $config;
 
+	public function providesGetUserInfoData(): array {
+		return [
+			'access-token' => [true],
+			'user-info-endpoint' => [false],
+		];
+	}
+
 	protected function setUp(): void {
 		parent::setUp();
 		$this->config = $this->createMock(IConfig::class);
@@ -125,4 +132,43 @@ public function testCtorInsecure(): void {
 		self::assertEquals(false, $this->client->getVerifyHost());
 		self::assertEquals(false, $this->client->getVerifyPeer());
 	}
+
+	/**
+	 * @dataProvider providesGetUserInfoData
+	 * @param $useAccessTokenPayloadForUserInfo
+	 */
+	public function testGetUserInfo($useAccessTokenPayloadForUserInfo): void {
+		$this->config->method('getSystemValue')->willReturnCallback(static function ($key) use ($useAccessTokenPayloadForUserInfo) {
+			if ($key === 'openid-connect') {
+				return [
+					'provider-url' => '$providerUrl',
+					'client-id' => 'client-id',
+					'client-secret' => 'secret',
+					'use-access-token-payload-for-user-info' => $useAccessTokenPayloadForUserInfo
+				];
+			}
+			throw new \InvalidArgumentException("Unexpected key: $key");
+		});
+
+		$this->client = $this->getMockBuilder(Client::class)
+			->setConstructorArgs([$this->config, $this->urlGenerator, $this->session])
+			->onlyMethods(['requestUserInfo', 'getAccessTokenPayload'])
+			->getMock();
+		if ($useAccessTokenPayloadForUserInfo) {
+			$this->client->expects(self::never())->method('requestUserInfo');
+			$this->client->expects(self::once())->method('getAccessTokenPayload')->willReturn([
+				'preferred_username' => '[email protected]'
+			]);
+		} else {
+			$this->client->expects(self::never())->method('getAccessTokenPayload');
+			$this->client->expects(self::once())->method('requestUserInfo')->willReturn([
+				'preferred_username' => '[email protected]'
+			]);
+		}
+
+		$info = $this->client->getUserInfo();
+		self::assertEquals([
+			'preferred_username' => '[email protected]'
+		], $info);
+	}
 }

tests/unit/Controller/LoginFlowControllerLoginTest.php

@@ -117,7 +117,7 @@ public function testLoginNoUserInfo(): void {
 
 	public function testLoginUnknownUser(): void {
 		$this->client->method('getOpenIdConfig')->willReturn([]);
-		$this->client->method('requestUserInfo')->willReturn((object)['email' => '[email protected]']);
+		$this->client->method('getUserInfo')->willReturn((object)['email' => '[email protected]']);
 		$this->userLookup->method('lookupUser')->willThrowException(new LoginException('User foo is not known.'));
 		$this->expectException(LoginException::class);
 		$this->expectExceptionMessage('User foo is not known.');
@@ -127,7 +127,7 @@ public function testLoginUnknownUser(): void {
 
 	public function testLoginCreateSessionFailed(): void {
 		$this->client->method('getOpenIdConfig')->willReturn([]);
-		$this->client->method('requestUserInfo')->willReturn((object)['email' => '[email protected]']);
+		$this->client->method('getUserInfo')->willReturn((object)['email' => '[email protected]']);
 		$user = $this->createMock(IUser::class);
 		$this->userLookup->method('lookupUser')->willReturn($user);
 		$this->userSession->method('createSessionToken')->willReturn(false);
@@ -138,7 +138,7 @@ public function testLoginCreateSessionFailed(): void {
 
 	public function testLoginCreateSuccess(): void {
 		$this->client->method('getOpenIdConfig')->willReturn([]);
-		$this->client->method('requestUserInfo')->willReturn((object)['email' => '[email protected]']);
+		$this->client->method('getUserInfo')->willReturn((object)['email' => '[email protected]']);
 		$this->client->method('getIdToken')->willReturn('id');
 		$this->client->method('getAccessToken')->willReturn('access');
 		$this->client->method('getRefreshToken')->willReturn('refresh');

tests/unit/OpenIdConnectAuthModuleTest.php

@@ -126,7 +126,7 @@ public function testInvalidTokenWithIntrospectionNotActive(): void {
 	public function testValidTokenWithIntrospection(): void {
 		$this->client->method('getOpenIdConfig')->willReturn(['use-token-introspection-endpoint' => true]);
 		$this->client->method('introspectToken')->willReturn((object)['active' => true, 'exp' => \time() + 3600]);
-		$this->client->method('requestUserInfo')->willReturn((object)['email' => '[email protected]']);
+		$this->client->method('getUserInfo')->willReturn((object)['email' => '[email protected]']);
 		$this->cacheFactory->method('create')->willReturn(new ArrayCache());
 		$user = $this->createMock(IUser::class);
 		$this->lookupService->expects(self::once())->method('lookupUser')->willReturn($user);
@@ -141,7 +141,7 @@ public function testValidTokenWithJWT(): void {
 		$this->client->method('getOpenIdConfig')->willReturn(['use-token-introspection-endpoint' => false]);
 		$this->client->method('verifyJWTsignature')->willReturn(true);
 		$this->client->method('getAccessTokenPayload')->willReturn((object)['exp' => \time() + 3600]);
-		$this->client->method('requestUserInfo')->willReturn((object)['email' => '[email protected]']);
+		$this->client->method('getUserInfo')->willReturn((object)['email' => '[email protected]']);
 		$this->cacheFactory->method('create')->willReturn(new ArrayCache());
 		$user = $this->createMock(IUser::class);
 		$this->lookupService->expects(self::once())->method('lookupUser')->willReturn($user);