Merge pull request #26572 from nextcloud/bugfix/noid/throw-401-when-authentication-is-provided-and-invalid

Throw "401 Unauthenticated" when authentication is provided but invalid

4 files changed, 16 insertions, 6 deletions

diff --git a/build/integration/features/bootstrap/RemoteContext.php b/build/integration/features/bootstrap/RemoteContext.php
index 38f3ab76487..69ebad43208 100644
--- a/build/integration/features/bootstrap/RemoteContext.php
+++ b/build/integration/features/bootstrap/RemoteContext.php
@@ -138,7 +138,13 @@ class RemoteContext implements Context {
 	 * @param string $value
 	 */
 	public function hasCapability($key, $value) {
-		$capabilities = $this->getApiClient()->getCapabilities();
+		try {
+			$capabilities = $this->getApiClient()->getCapabilities();
+		} catch (\Exception $e) {
+			Assert::assertInstanceOf($value, $e);
+			$this->lastException = $e;
+			return;
+		}
 		$current = $capabilities;
 		$parts = explode('.', $key);
 		foreach ($parts as $part) {
diff --git a/build/integration/remoteapi_features/remote.feature b/build/integration/remoteapi_features/remote.feature
index 72daf8226cd..62fd95e0130 100644
--- a/build/integration/remoteapi_features/remote.feature
+++ b/build/integration/remoteapi_features/remote.feature
@@ -34,4 +34,5 @@ Feature: remote
     Given using remote server "REMOTE"
     And user "user0" exists
     And using credentials "user0", "invalid"
-    Then the capability "theming.name" is "Nextcloud"
+    Then the capability "theming.name" is "OC\ForbiddenException"
+    Then the request should throw a "OC\ForbiddenException"
diff --git a/lib/private/User/Session.php b/lib/private/User/Session.php
index c33d79b83cf..5e6501f9045 100644
--- a/lib/private/User/Session.php
+++ b/lib/private/User/Session.php
@@ -599,6 +599,8 @@ class Session implements IUserSession, Emitter {
 
 					return true;
 				}
+				// If credentials were provided, they need to be valid, otherwise we do boom
+				throw new LoginException();
 			} catch (PasswordLoginForbiddenException $ex) {
 				// Nothing to do
 			}
diff --git a/tests/lib/Authentication/TwoFactorAuth/ProviderLoaderTest.php b/tests/lib/Authentication/TwoFactorAuth/ProviderLoaderTest.php
index 1b813b5f36b..d9c565a3f6d 100644
--- a/tests/lib/Authentication/TwoFactorAuth/ProviderLoaderTest.php
+++ b/tests/lib/Authentication/TwoFactorAuth/ProviderLoaderTest.php
@@ -32,15 +32,16 @@ use OC\AppFramework\Bootstrap\ServiceRegistration;
 use OC\Authentication\TwoFactorAuth\ProviderLoader;
 use OCP\App\IAppManager;
 use OCP\Authentication\TwoFactorAuth\IProvider;
+use OCP\IUser;
 use PHPUnit\Framework\MockObject\MockObject;
 use Test\TestCase;
 
 class ProviderLoaderTest extends TestCase {
 
-	/** @var IAppManager|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IAppManager|MockObject */
 	private $appManager;
 
-	/** @var \OCP\IUser|\PHPUnit\Framework\MockObject\MockObject */
+	/** @var IUser|MockObject */
 	private $user;
 
 	/** @var RegistrationContext|MockObject */
@@ -53,7 +54,7 @@ class ProviderLoaderTest extends TestCase {
 		parent::setUp();
 
 		$this->appManager = $this->createMock(IAppManager::class);
-		$this->user = $this->createMock(\OCP\IUser::class);
+		$this->user = $this->createMock(IUser::class);
 
 		$this->registrationContext = $this->createMock(RegistrationContext::class);
 		$coordinator = $this->createMock(Coordinator::class);
@@ -123,7 +124,7 @@ class ProviderLoaderTest extends TestCase {
 			->with($this->user)
 			->willReturn([]);
 
-		$this->registrationContext->method('getTwoFactorProvider')
+		$this->registrationContext->method('getTwoFactorProviders')
 			->willReturn([
 				new ServiceRegistration('twofactor_test', '\\OCA\\TwoFactorTest\\Provider')
 			]);