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 489b816
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 3 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
4 changes: 4 additions & 0 deletions tests/lib/Authentication/TwoFactorAuth/ManagerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,10 @@ public function testNeedsSecondFactorSessionAuth() {
->method('get')
->with(Manager::SESSION_UID_DONE)
->willReturn('user');
$this->session->expects($this->once())
->method('get')
->with('app_api')
->willReturn(false);

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

0 comments on commit 489b816

Please sign in to comment.