mirror of
https://github.com/nextcloud/server.git
synced 2026-06-11 01:30:50 -04:00
fix(Session): avoid password confirmation on SSO
SSO backends like SAML and OIDC tried a trick to suppress password confirmations as they are not possible by design. At least for SAML it was not reliable when existing user backends where used as user repositories. Now we are setting a special scope with the token, and also make sure that the scope is taken over when tokens are regenerated. Signed-off-by: Arthur Schiwon <blizzz@arthur-schiwon.de>
This commit is contained in:
parent
afeac8f6cb
commit
eea5e1cca2
8 changed files with 144 additions and 46 deletions
|
|
@ -28,6 +28,7 @@
|
|||
namespace OC\Core\Controller;
|
||||
|
||||
use bantu\IniGetWrapper\IniGetWrapper;
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OC\CapabilitiesManager;
|
||||
use OC\Template\JSConfigHelper;
|
||||
use OCP\App\IAppManager;
|
||||
|
|
@ -59,7 +60,10 @@ class OCJSController extends Controller {
|
|||
IniGetWrapper $iniWrapper,
|
||||
IURLGenerator $urlGenerator,
|
||||
CapabilitiesManager $capabilitiesManager,
|
||||
IInitialStateService $initialStateService) {
|
||||
IInitialStateService $initialStateService,
|
||||
IProvider $tokenProvider,
|
||||
) {
|
||||
|
||||
parent::__construct($appName, $request);
|
||||
|
||||
$this->helper = new JSConfigHelper(
|
||||
|
|
@ -73,7 +77,8 @@ class OCJSController extends Controller {
|
|||
$iniWrapper,
|
||||
$urlGenerator,
|
||||
$capabilitiesManager,
|
||||
$initialStateService
|
||||
$initialStateService,
|
||||
$tokenProvider
|
||||
);
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -275,7 +275,8 @@ class DIContainer extends SimpleContainer implements IAppContainer {
|
|||
$c->get(IControllerMethodReflector::class),
|
||||
$c->get(ISession::class),
|
||||
$c->get(IUserSession::class),
|
||||
$c->get(ITimeFactory::class)
|
||||
$c->get(ITimeFactory::class),
|
||||
$c->get(\OC\Authentication\Token\IProvider::class),
|
||||
)
|
||||
);
|
||||
$dispatcher->registerMiddleware(
|
||||
|
|
|
|||
|
|
@ -25,11 +25,16 @@ namespace OC\AppFramework\Middleware\Security;
|
|||
|
||||
use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
|
||||
use OC\AppFramework\Utility\ControllerMethodReflector;
|
||||
use OC\Authentication\Exceptions\ExpiredTokenException;
|
||||
use OC\Authentication\Exceptions\InvalidTokenException;
|
||||
use OC\Authentication\Exceptions\WipeTokenException;
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OCP\AppFramework\Controller;
|
||||
use OCP\AppFramework\Middleware;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OCP\ISession;
|
||||
use OCP\IUserSession;
|
||||
use OCP\Session\Exceptions\SessionNotAvailableException;
|
||||
use OCP\User\Backend\IPasswordConfirmationBackend;
|
||||
|
||||
class PasswordConfirmationMiddleware extends Middleware {
|
||||
|
|
@ -43,6 +48,7 @@ class PasswordConfirmationMiddleware extends Middleware {
|
|||
private $timeFactory;
|
||||
/** @var array */
|
||||
private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
|
||||
private IProvider $tokenProvider;
|
||||
|
||||
/**
|
||||
* PasswordConfirmationMiddleware constructor.
|
||||
|
|
@ -53,13 +59,16 @@ class PasswordConfirmationMiddleware extends Middleware {
|
|||
* @param ITimeFactory $timeFactory
|
||||
*/
|
||||
public function __construct(ControllerMethodReflector $reflector,
|
||||
ISession $session,
|
||||
IUserSession $userSession,
|
||||
ITimeFactory $timeFactory) {
|
||||
ISession $session,
|
||||
IUserSession $userSession,
|
||||
ITimeFactory $timeFactory,
|
||||
IProvider $tokenProvider,
|
||||
) {
|
||||
$this->reflector = $reflector;
|
||||
$this->session = $session;
|
||||
$this->userSession = $userSession;
|
||||
$this->timeFactory = $timeFactory;
|
||||
$this->tokenProvider = $tokenProvider;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -82,8 +91,21 @@ class PasswordConfirmationMiddleware extends Middleware {
|
|||
$backendClassName = $user->getBackendClassName();
|
||||
}
|
||||
|
||||
try {
|
||||
$sessionId = $this->session->getId();
|
||||
$token = $this->tokenProvider->getToken($sessionId);
|
||||
} catch (SessionNotAvailableException|InvalidTokenException|WipeTokenException|ExpiredTokenException) {
|
||||
// States we do not deal with here.
|
||||
return;
|
||||
}
|
||||
$scope = $token->getScopeAsArray();
|
||||
if (isset($scope['sso-based-login']) && $scope['sso-based-login'] === true) {
|
||||
// Users logging in from SSO backends cannot confirm their password by design
|
||||
return;
|
||||
}
|
||||
|
||||
$lastConfirm = (int) $this->session->get('last-password-confirm');
|
||||
// we can't check the password against a SAML backend, so skip password confirmation in this case
|
||||
// TODO: confirm excludedUserBackEnds can go away and remove it
|
||||
if (!isset($this->excludedUserBackEnds[$backendClassName]) && $lastConfirm < ($this->timeFactory->getTime() - (30 * 60 + 15))) { // allow 15 seconds delay
|
||||
throw new NotConfirmedException();
|
||||
}
|
||||
|
|
|
|||
|
|
@ -242,6 +242,7 @@ class PublicKeyTokenProvider implements IProvider {
|
|||
IToken::TEMPORARY_TOKEN,
|
||||
$token->getRemember()
|
||||
);
|
||||
$newToken->setScope($token->getScopeAsArray());
|
||||
|
||||
$this->mapper->delete($token);
|
||||
|
||||
|
|
|
|||
|
|
@ -34,6 +34,10 @@ declare(strict_types=1);
|
|||
namespace OC\Template;
|
||||
|
||||
use bantu\IniGetWrapper\IniGetWrapper;
|
||||
use OC\Authentication\Exceptions\ExpiredTokenException;
|
||||
use OC\Authentication\Exceptions\InvalidTokenException;
|
||||
use OC\Authentication\Exceptions\WipeTokenException;
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OC\CapabilitiesManager;
|
||||
use OC\Share\Share;
|
||||
use OCP\App\AppPathNotFoundException;
|
||||
|
|
@ -49,47 +53,29 @@ use OCP\ISession;
|
|||
use OCP\IURLGenerator;
|
||||
use OCP\ILogger;
|
||||
use OCP\IUser;
|
||||
use OCP\Session\Exceptions\SessionNotAvailableException;
|
||||
use OCP\User\Backend\IPasswordConfirmationBackend;
|
||||
use OCP\Util;
|
||||
|
||||
class JSConfigHelper {
|
||||
protected IL10N $l;
|
||||
protected Defaults $defaults;
|
||||
protected IAppManager $appManager;
|
||||
protected ISession $session;
|
||||
protected ?IUser $currentUser;
|
||||
protected IConfig $config;
|
||||
protected IGroupManager $groupManager;
|
||||
protected IniGetWrapper $iniWrapper;
|
||||
protected IURLGenerator $urlGenerator;
|
||||
protected CapabilitiesManager $capabilitiesManager;
|
||||
protected IInitialStateService $initialStateService;
|
||||
|
||||
/** @var array user back-ends excluded from password verification */
|
||||
private $excludedUserBackEnds = ['user_saml' => true, 'user_globalsiteselector' => true];
|
||||
|
||||
public function __construct(IL10N $l,
|
||||
Defaults $defaults,
|
||||
IAppManager $appManager,
|
||||
ISession $session,
|
||||
?IUser $currentUser,
|
||||
IConfig $config,
|
||||
IGroupManager $groupManager,
|
||||
IniGetWrapper $iniWrapper,
|
||||
IURLGenerator $urlGenerator,
|
||||
CapabilitiesManager $capabilitiesManager,
|
||||
IInitialStateService $initialStateService) {
|
||||
$this->l = $l;
|
||||
$this->defaults = $defaults;
|
||||
$this->appManager = $appManager;
|
||||
$this->session = $session;
|
||||
$this->currentUser = $currentUser;
|
||||
$this->config = $config;
|
||||
$this->groupManager = $groupManager;
|
||||
$this->iniWrapper = $iniWrapper;
|
||||
$this->urlGenerator = $urlGenerator;
|
||||
$this->capabilitiesManager = $capabilitiesManager;
|
||||
$this->initialStateService = $initialStateService;
|
||||
public function __construct(
|
||||
protected IL10N $l,
|
||||
protected Defaults $defaults,
|
||||
protected IAppManager $appManager,
|
||||
protected ISession $session,
|
||||
protected ?IUser $currentUser,
|
||||
protected IConfig $config,
|
||||
protected IGroupManager $groupManager,
|
||||
protected IniGetWrapper $iniWrapper,
|
||||
protected IURLGenerator $urlGenerator,
|
||||
protected CapabilitiesManager $capabilitiesManager,
|
||||
protected IInitialStateService $initialStateService,
|
||||
protected IProvider $tokenProvider,
|
||||
) {
|
||||
}
|
||||
|
||||
public function getConfig(): string {
|
||||
|
|
@ -155,9 +141,13 @@ class JSConfigHelper {
|
|||
}
|
||||
|
||||
if ($this->currentUser instanceof IUser) {
|
||||
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
|
||||
if (!is_int($lastConfirmTimestamp)) {
|
||||
$lastConfirmTimestamp = 0;
|
||||
if ($this->canUserValidatePassword()) {
|
||||
$lastConfirmTimestamp = $this->session->get('last-password-confirm');
|
||||
if (!is_int($lastConfirmTimestamp)) {
|
||||
$lastConfirmTimestamp = 0;
|
||||
}
|
||||
} else {
|
||||
$lastConfirmTimestamp = PHP_INT_MAX;
|
||||
}
|
||||
} else {
|
||||
$lastConfirmTimestamp = 0;
|
||||
|
|
@ -310,4 +300,15 @@ class JSConfigHelper {
|
|||
|
||||
return $result;
|
||||
}
|
||||
|
||||
protected function canUserValidatePassword(): bool {
|
||||
try {
|
||||
$token = $this->tokenProvider->getToken($this->session->getId());
|
||||
} catch (ExpiredTokenException|WipeTokenException|InvalidTokenException|SessionNotAvailableException) {
|
||||
// actually we do not know, so we fall back to this statement
|
||||
return true;
|
||||
}
|
||||
$scope = $token->getScopeAsArray();
|
||||
return !isset($scope['sso-based-login']) || $scope['sso-based-login'] === false;
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@
|
|||
namespace OC;
|
||||
|
||||
use bantu\IniGetWrapper\IniGetWrapper;
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OC\Search\SearchQuery;
|
||||
use OC\Template\CSSResourceLocator;
|
||||
use OC\Template\JSConfigHelper;
|
||||
|
|
@ -235,7 +236,8 @@ class TemplateLayout extends \OC_Template {
|
|||
\OC::$server->get(IniGetWrapper::class),
|
||||
\OC::$server->getURLGenerator(),
|
||||
\OC::$server->getCapabilitiesManager(),
|
||||
\OC::$server->query(IInitialStateService::class)
|
||||
\OCP\Server::get(IInitialStateService::class),
|
||||
\OCP\Server::get(IProvider::class),
|
||||
);
|
||||
$config = $jsConfigHelper->getConfig();
|
||||
if (\OC::$server->getContentSecurityPolicyNonceManager()->browserSupportsCspV3()) {
|
||||
|
|
|
|||
|
|
@ -35,7 +35,7 @@
|
|||
* along with this program. If not, see <http://www.gnu.org/licenses/>
|
||||
*
|
||||
*/
|
||||
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OC\User\LoginException;
|
||||
use OCP\EventDispatcher\IEventDispatcher;
|
||||
use OCP\ILogger;
|
||||
|
|
@ -193,6 +193,14 @@ class OC_User {
|
|||
|
||||
$userSession->createSessionToken($request, $uid, $uid, $password);
|
||||
$userSession->createRememberMeToken($userSession->getUser());
|
||||
|
||||
if (empty($password)) {
|
||||
$tokenProvider = \OC::$server->get(IProvider::class);
|
||||
$token = $tokenProvider->getToken($userSession->getSession()->getId());
|
||||
$token->setScope(['sso-based-login' => true]);
|
||||
$tokenProvider->updateToken($token);
|
||||
}
|
||||
|
||||
// setup the filesystem
|
||||
OC_Util::setupFS($uid);
|
||||
// first call the post_login hooks, the login-process needs to be
|
||||
|
|
|
|||
|
|
@ -27,7 +27,9 @@ use OC\AppFramework\Middleware\Security\Exceptions\NotConfirmedException;
|
|||
use OC\AppFramework\Middleware\Security\PasswordConfirmationMiddleware;
|
||||
use OC\AppFramework\Utility\ControllerMethodReflector;
|
||||
use OCP\AppFramework\Controller;
|
||||
use OC\Authentication\Token\IProvider;
|
||||
use OCP\AppFramework\Utility\ITimeFactory;
|
||||
use OC\Authentication\Token\IToken;
|
||||
use OCP\ISession;
|
||||
use OCP\IUser;
|
||||
use OCP\IUserSession;
|
||||
|
|
@ -48,6 +50,7 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
private $contoller;
|
||||
/** @var ITimeFactory|\PHPUnit\Framework\MockObject\MockObject */
|
||||
private $timeFactory;
|
||||
private IProvider|\PHPUnit\Framework\MockObject\MockObject $tokenProvider;
|
||||
|
||||
protected function setUp(): void {
|
||||
$this->reflector = new ControllerMethodReflector();
|
||||
|
|
@ -56,12 +59,14 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
$this->user = $this->createMock(IUser::class);
|
||||
$this->contoller = $this->createMock(Controller::class);
|
||||
$this->timeFactory = $this->createMock(ITimeFactory::class);
|
||||
$this->tokenProvider = $this->createMock(IProvider::class);
|
||||
|
||||
$this->middleware = new PasswordConfirmationMiddleware(
|
||||
$this->reflector,
|
||||
$this->session,
|
||||
$this->userSession,
|
||||
$this->timeFactory
|
||||
$this->timeFactory,
|
||||
$this->tokenProvider,
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -94,6 +99,13 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
*/
|
||||
public function testAnnotation($backend, $lastConfirm, $currentTime, $exception) {
|
||||
$this->reflector->reflect(__CLASS__, __FUNCTION__);
|
||||
$token = $this->createMock(IToken::class);
|
||||
$token->method('getScopeAsArray')
|
||||
->willReturn([]);
|
||||
$this->tokenProvider->expects($this->once())
|
||||
->method('getToken')
|
||||
->willReturn($token);
|
||||
|
||||
|
||||
$this->user->method('getBackendClassName')
|
||||
->willReturn($backend);
|
||||
|
|
@ -107,6 +119,13 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
$this->timeFactory->method('getTime')
|
||||
->willReturn($currentTime);
|
||||
|
||||
$token = $this->createMock(IToken::class);
|
||||
$token->method('getScopeAsArray')
|
||||
->willReturn([]);
|
||||
$this->tokenProvider->expects($this->once())
|
||||
->method('getToken')
|
||||
->willReturn($token);
|
||||
|
||||
$thrown = false;
|
||||
try {
|
||||
$this->middleware->beforeController($this->contoller, __FUNCTION__);
|
||||
|
|
@ -117,6 +136,8 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
$this->assertSame($exception, $thrown);
|
||||
}
|
||||
|
||||
|
||||
|
||||
public function dataProvider() {
|
||||
return [
|
||||
['foo', 2000, 4000, true],
|
||||
|
|
@ -127,4 +148,41 @@ class PasswordConfirmationMiddlewareTest extends TestCase {
|
|||
['foo', 2000, 3816, true],
|
||||
];
|
||||
}
|
||||
|
||||
public function testSSO() {
|
||||
static $sessionId = 'mySession1d';
|
||||
|
||||
$this->reflector->reflect($this->controller, __FUNCTION__);
|
||||
|
||||
$this->user->method('getBackendClassName')
|
||||
->willReturn('fictional_backend');
|
||||
$this->userSession->method('getUser')
|
||||
->willReturn($this->user);
|
||||
|
||||
$this->session->method('get')
|
||||
->with('last-password-confirm')
|
||||
->willReturn(0);
|
||||
$this->session->method('getId')
|
||||
->willReturn($sessionId);
|
||||
|
||||
$this->timeFactory->method('getTime')
|
||||
->willReturn(9876);
|
||||
|
||||
$token = $this->createMock(IToken::class);
|
||||
$token->method('getScopeAsArray')
|
||||
->willReturn(['sso-based-login' => true]);
|
||||
$this->tokenProvider->expects($this->once())
|
||||
->method('getToken')
|
||||
->with($sessionId)
|
||||
->willReturn($token);
|
||||
|
||||
$thrown = false;
|
||||
try {
|
||||
$this->middleware->beforeController($this->controller, __FUNCTION__);
|
||||
} catch (NotConfirmedException) {
|
||||
$thrown = true;
|
||||
}
|
||||
|
||||
$this->assertSame(false, $thrown);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue