Make work with posts and cookies again

Requires https://github.com/nextcloud/server/pull/21479 to fully work.
Basically don't save this info in the session (which is lax by default
starting with NC19 but also soon with new chromes and firefox). We now
save it is a cookie that is set to None. This is the best we can do I
think.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
This commit is contained in:
Roeland Jago Douma 2020-06-19 09:34:24 +02:00
parent a15bc52cbb
commit f5304f6757
No known key found for this signature in database
GPG Key ID: F941078878347C0C
2 changed files with 56 additions and 10 deletions

View File

@ -37,6 +37,7 @@ use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Security\ICrypto;
use OneLogin\Saml2\Auth;
use OneLogin\Saml2\Error;
use OneLogin\Saml2\Settings;
@ -61,6 +62,10 @@ class SAMLController extends Controller {
private $logger;
/** @var IL10N */
private $l;
/**
* @var ICrypto
*/
private $crypto;
/**
* @param string $appName
@ -85,7 +90,8 @@ class SAMLController extends Controller {
IURLGenerator $urlGenerator,
IUserManager $userManager,
ILogger $logger,
IL10N $l) {
IL10N $l,
ICrypto $crypto) {
parent::__construct($appName, $request);
$this->session = $session;
$this->userSession = $userSession;
@ -96,6 +102,7 @@ class SAMLController extends Controller {
$this->userManager = $userManager;
$this->logger = $logger;
$this->l = $l;
$this->crypto = $crypto;
}
/**
@ -175,9 +182,22 @@ class SAMLController extends Controller {
case 'saml':
$auth = new Auth($this->SAMLSettings->getOneLoginSettingsArray($idp));
$ssoUrl = $auth->login(null, [], false, false, true);
$this->session->set('user_saml.AuthNRequestID', $auth->getLastRequestID());
$this->session->set('user_saml.OriginalUrl', $this->request->getParam('originalUrl', ''));
$this->session->set('user_saml.Idp', $idp);
$response = new Http\RedirectResponse($ssoUrl);
// Pack data as JSON so we can properly extract it later
$data = json_encode([
'AuthNRequestID' => $auth->getLastRequestID(),
'OriginalUrl' => $this->request->getParam('originalUrl', ''),
'Idp' => $idp
]);
// Encrypt it
$data = $this->crypto->encrypt($data);
// And base64 encode it
$data = base64_encode($data);
$response->addCookie('saml_data', $data, null, 'None');
break;
case 'environment-variable':
$ssoUrl = $this->request->getParam('originalUrl', '');
@ -198,6 +218,7 @@ class SAMLController extends Controller {
}
$ssoUrl = $this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.notProvisioned');
}
$response = new Http\RedirectResponse($ssoUrl);
break;
default:
throw new \Exception(
@ -208,7 +229,7 @@ class SAMLController extends Controller {
);
}
return new Http\RedirectResponse($ssoUrl);
return $response;
}
/**
@ -244,8 +265,26 @@ class SAMLController extends Controller {
* @throws ValidationError
*/
public function assertionConsumerService() {
$AuthNRequestID = $this->session->get('user_saml.AuthNRequestID');
$idp = $this->session->get('user_saml.Idp');
// Fetch and decrypt the cookie
$cookie = $this->request->getCookie('saml_data');
if ($cookie === null) {
return;
}
// Base64 decode
$cookie = base64_decode($cookie);
// Decrypt and deserialize
try {
$cookie = $this->crypto->decrypt($cookie);
} catch (\Exception $e) {
return;
}
$data = json_decode($cookie, true);
$AuthNRequestID = $data['AuthNRequestID'];
$idp = $data['Idp'];
if(is_null($AuthNRequestID) || $AuthNRequestID === '' || is_null($idp)) {
return;
}
@ -299,18 +338,19 @@ class SAMLController extends Controller {
return new Http\RedirectResponse($this->urlGenerator->linkToRouteAbsolute('user_saml.SAML.notProvisioned'));
}
$originalUrl = $this->session->get('user_saml.OriginalUrl');
$originalUrl = $data['OriginalUrl'];
if($originalUrl !== null && $originalUrl !== '') {
$response = new Http\RedirectResponse($originalUrl);
} else {
$response = new Http\RedirectResponse(\OC::$server->getURLGenerator()->getAbsoluteURL('/'));
}
$this->session->remove('user_saml.OriginalUrl');
// The Nextcloud desktop client expects a cookie with the key of "_shibsession"
// to be there.
if($this->request->isUserAgent(['/^.*(mirall|csyncoC)\/.*$/'])) {
$response->addCookie('_shibsession_', 'authenticated');
}
$response->invalidateCookie('saml_data');
return $response;
}

View File

@ -35,6 +35,8 @@ use OCP\IURLGenerator;
use OCP\IUser;
use OCP\IUserManager;
use OCP\IUserSession;
use OCP\Security\ICrypto;
use PHPUnit\Framework\MockObject\MockObject;
use Test\TestCase;
class SAMLControllerTest extends TestCase {
@ -58,6 +60,8 @@ class SAMLControllerTest extends TestCase {
private $logger;
/** @var IL10N|\PHPUnit_Framework_MockObject_MockObject */
private $l;
/** @var ICrypto|MockObject */
private $crypto;
/** @var SAMLController */
private $samlController;
@ -77,6 +81,7 @@ class SAMLControllerTest extends TestCase {
$this->userManager = $this->createMock(IUserManager::class);
$this->logger = $this->createMock(ILogger::class);
$this->l = $this->createMock(IL10N::class);
$this->crypto = $this->createMock(ICrypto::class);
$this->l->expects($this->any())->method('t')->willReturnCallback(
function($param) {
@ -100,7 +105,8 @@ class SAMLControllerTest extends TestCase {
$this->urlGenerator,
$this->userManager,
$this->logger,
$this->l
$this->l,
$this->crypto
);
}