Skip to content

Commit

Permalink
AppAPI: allowed to bypass Two-Factor
Browse files Browse the repository at this point in the history
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
  • Loading branch information
bigcat88 committed Dec 28, 2023
1 parent 17cb5da commit 26d343d
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 7 deletions.
5 changes: 4 additions & 1 deletion core/Middleware/TwoFactorMiddleware.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,10 @@ public function beforeController($controller, $methodName) {
if ($this->userSession->isLoggedIn()) {
$user = $this->userSession->getUser();

if ($this->session->exists('app_password') || $this->twoFactorManager->isTwoFactorAuthenticated($user)) {
if ($this->session->exists('app_password') // authenticated using an app password
|| $this->session->exists('app_api') // authenticated using an AppAPI Auth
|| $this->twoFactorManager->isTwoFactorAuthenticated($user)) {

Check notice

Code scanning / Psalm

PossiblyNullArgument Note

Argument 1 of OC\Authentication\TwoFactorAuth\Manager::isTwoFactorAuthenticated cannot be null, possibly null value provided

$this->checkTwoFactor($controller, $methodName, $user);
} elseif ($controller instanceof TwoFactorChallengeController) {
// Allow access to the two-factor controllers only if two-factor authentication
Expand Down
4 changes: 2 additions & 2 deletions lib/private/Authentication/TwoFactorAuth/Manager.php
Original file line number Diff line number Diff line change
Expand Up @@ -318,8 +318,8 @@ public function needsSecondFactor(IUser $user = null): bool {
return false;
}

// If we are authenticated using an app password skip all this
if ($this->session->exists('app_password')) {
// If we are authenticated using an app password or AppAPI Auth, skip all this
if ($this->session->exists('app_password') || $this->session->get('app_api') === true) {
return false;
}

Expand Down
23 changes: 19 additions & 4 deletions tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -629,13 +629,26 @@ public function testNeedsSecondFactorSessionAuth() {
return false;
} elseif ($var === 'app_password') {
return false;
} elseif ($var === 'app_api') {
return false;
}
return true;
});
$this->session->method('get')
->willReturnCallback(function ($var) {
if ($var === Manager::SESSION_UID_KEY) {
return 'user';
} elseif ($var === 'app_api') {
return true;
}
return null;
});
$this->session->expects($this->once())
->method('get')
->with(Manager::SESSION_UID_DONE)
->willReturn('user');
->willReturnMap([
[Manager::SESSION_UID_DONE, 'user'],
['app_api', true]
]);

$this->assertFalse($this->manager->needsSecondFactor($user));
}
Expand Down Expand Up @@ -695,8 +708,10 @@ public function testNeedsSecondFactorInvalidToken() {
public function testNeedsSecondFactorAppPassword() {
$user = $this->createMock(IUser::class);
$this->session->method('exists')
->with('app_password')
->willReturn(true);
->willReturnMap([
['app_password', true],
['app_api', true]
]);

$this->assertFalse($this->manager->needsSecondFactor($user));
}
Expand Down

0 comments on commit 26d343d

Please sign in to comment.