GetCurrentUser
Fix possible race condition ( user creation ) Change-Id: Icd0e63a14c602b02c4f351b9459ca009103dc316 Signed-off-by: smarcet <smarcet@gmail.com>
This commit is contained in:
parent
286352f795
commit
91ffe46586
@ -42,28 +42,20 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
*/
|
*/
|
||||||
private $member_service;
|
private $member_service;
|
||||||
|
|
||||||
/**
|
|
||||||
* @var IGroupRepository
|
|
||||||
*/
|
|
||||||
private $group_repository;
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* ResourceServerContext constructor.
|
* ResourceServerContext constructor.
|
||||||
* @param IGroupRepository $group_repository
|
|
||||||
* @param IMemberRepository $member_repository
|
* @param IMemberRepository $member_repository
|
||||||
* @param IMemberService $member_service
|
* @param IMemberService $member_service
|
||||||
* @param ITransactionService $tx_service
|
* @param ITransactionService $tx_service
|
||||||
*/
|
*/
|
||||||
public function __construct
|
public function __construct
|
||||||
(
|
(
|
||||||
IGroupRepository $group_repository,
|
|
||||||
IMemberRepository $member_repository,
|
IMemberRepository $member_repository,
|
||||||
IMemberService $member_service,
|
IMemberService $member_service,
|
||||||
ITransactionService $tx_service
|
ITransactionService $tx_service
|
||||||
)
|
)
|
||||||
{
|
{
|
||||||
$this->member_repository = $member_repository;
|
$this->member_repository = $member_repository;
|
||||||
$this->group_repository = $group_repository;
|
|
||||||
$this->member_service = $member_service;
|
$this->member_service = $member_service;
|
||||||
$this->tx_service = $tx_service;
|
$this->tx_service = $tx_service;
|
||||||
}
|
}
|
||||||
@ -154,62 +146,42 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
return $this->getAuthContextVar('application_type');
|
return $this->getAuthContextVar('application_type');
|
||||||
}
|
}
|
||||||
|
|
||||||
private function getAuthContextVar(string $varName){
|
private function getAuthContextVar(string $varName)
|
||||||
|
{
|
||||||
return isset($this->auth_context[$varName]) ? $this->auth_context[$varName] : null;
|
return isset($this->auth_context[$varName]) ? $this->auth_context[$varName] : null;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
public function getCurrentUser(bool $synch_groups = true): ?Member
|
public function getCurrentUser(bool $synch_groups = true): ?Member
|
||||||
{
|
{
|
||||||
return $this->tx_service->transaction(function() use($synch_groups) {
|
|
||||||
$member = null;
|
$member = null;
|
||||||
// legacy test, for new IDP version this value came on null
|
|
||||||
$id = $this->getCurrentUserExternalId();
|
|
||||||
if(!is_null($id) && !empty($id)){
|
|
||||||
$member = $this->member_repository->getByExternalId(intval($id));
|
|
||||||
if(!is_null($member)) {
|
|
||||||
return $synch_groups ? $this->checkGroups($member) : $member;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// is null
|
|
||||||
if(is_null($member)){
|
|
||||||
// try to get by external id
|
// try to get by external id
|
||||||
$id = $this->getCurrentUserId();
|
|
||||||
|
|
||||||
if(is_null($id)) {
|
|
||||||
return null;
|
|
||||||
}
|
|
||||||
$member = $this->member_repository->getByExternalId(intval($id));
|
|
||||||
|
|
||||||
if(!is_null($member)){
|
|
||||||
$user_first_name = $this->getAuthContextVar('user_first_name');
|
|
||||||
$user_last_name = $this->getAuthContextVar('user_last_name');
|
|
||||||
$user_email = $this->getAuthContextVar('user_email');
|
|
||||||
$user_email_verified = boolval($this->getAuthContextVar('user_email_verified'));
|
|
||||||
|
|
||||||
if(!empty($user_email))
|
|
||||||
$member->setEmail($user_email);
|
|
||||||
if(!empty($user_first_name))
|
|
||||||
$member->setFirstName($user_first_name);
|
|
||||||
if(!empty($user_last_name))
|
|
||||||
$member->setLastName($user_last_name);
|
|
||||||
|
|
||||||
$member->setEmailVerified($user_email_verified);
|
|
||||||
return $synch_groups ? $this->checkGroups($member) : $member;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if(is_null($member)) {
|
|
||||||
// we assume that is new idp version and claims already exists on context
|
|
||||||
$user_external_id = $this->getAuthContextVar('user_id');
|
$user_external_id = $this->getAuthContextVar('user_id');
|
||||||
$user_first_name = $this->getAuthContextVar('user_first_name');
|
$user_first_name = $this->getAuthContextVar('user_first_name');
|
||||||
$user_last_name = $this->getAuthContextVar('user_last_name');
|
$user_last_name = $this->getAuthContextVar('user_last_name');
|
||||||
$user_email = $this->getAuthContextVar('user_email');
|
$user_email = $this->getAuthContextVar('user_email');
|
||||||
$user_email_verified = boolval($this->getAuthContextVar('user_email_verified'));
|
$user_email_verified = boolval($this->getAuthContextVar('user_email_verified'));
|
||||||
|
|
||||||
|
if (is_null($user_external_id)) {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// first we check by external id
|
||||||
|
$member = $this->tx_service->transaction(function () use ($user_external_id) {
|
||||||
|
return $this->member_repository->getByExternalIdExclusiveLock(intval($user_external_id));
|
||||||
|
});
|
||||||
|
|
||||||
|
if (is_null($member)) {
|
||||||
|
// then by primary email
|
||||||
|
$member = $this->tx_service->transaction(function () use ($user_email) {
|
||||||
|
// we assume that is new idp version and claims already exists on context
|
||||||
|
$user_email = $this->getAuthContextVar('user_email');
|
||||||
// at last resort try to get by email
|
// at last resort try to get by email
|
||||||
Log::debug(sprintf("ResourceServerContext::getCurrentUser getting user by email %s", $user_email));
|
Log::debug(sprintf("ResourceServerContext::getCurrentUser getting user by email %s", $user_email));
|
||||||
$member = $this->member_repository->getByEmail($user_email);
|
return $this->member_repository->getByEmailExclusiveLock($user_email);
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
if (is_null($member)) {// user exist on IDP but not in our local DB, proceed to create it
|
if (is_null($member)) {// user exist on IDP but not in our local DB, proceed to create it
|
||||||
Log::debug
|
Log::debug
|
||||||
@ -223,7 +195,7 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
$user_last_name
|
$user_last_name
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
try {
|
||||||
$member = $this->member_service->registerExternalUser
|
$member = $this->member_service->registerExternalUser
|
||||||
(
|
(
|
||||||
new ExternalUserDTO
|
new ExternalUserDTO
|
||||||
@ -236,8 +208,31 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
$user_email_verified
|
$user_email_verified
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
} catch (\Exception $ex) {
|
||||||
|
Log::warning($ex);
|
||||||
|
// race condition lost
|
||||||
|
$member = $this->tx_service->transaction(function () use ($user_external_id) {
|
||||||
|
return $this->member_repository->getByExternalIdExclusiveLock(intval($user_external_id));
|
||||||
|
});
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (is_null($member)) {
|
||||||
|
Log::warning(sprintf("ResourceServerContext::getCurrentUser user not found %s (%s).", $user_external_id, $user_email));
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
return $this->tx_service->transaction(function () use
|
||||||
|
(
|
||||||
|
$member,
|
||||||
|
$user_email,
|
||||||
|
$user_first_name,
|
||||||
|
$user_last_name,
|
||||||
|
$user_external_id,
|
||||||
|
$user_email_verified,
|
||||||
|
$synch_groups
|
||||||
|
) {
|
||||||
|
// update member fields
|
||||||
if (!empty($user_email))
|
if (!empty($user_email))
|
||||||
$member->setEmail($user_email);
|
$member->setEmail($user_email);
|
||||||
if (!empty($user_first_name))
|
if (!empty($user_first_name))
|
||||||
@ -245,10 +240,8 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
if (!empty($user_last_name))
|
if (!empty($user_last_name))
|
||||||
$member->setLastName($user_last_name);
|
$member->setLastName($user_last_name);
|
||||||
|
|
||||||
$member->setEmailVerified(boolval($user_email_verified));
|
|
||||||
$member->setUserExternalId($user_external_id);
|
$member->setUserExternalId($user_external_id);
|
||||||
|
$member->setEmailVerified($user_email_verified);
|
||||||
}
|
|
||||||
|
|
||||||
return $synch_groups ? $this->checkGroups($member) : $member;
|
return $synch_groups ? $this->checkGroups($member) : $member;
|
||||||
});
|
});
|
||||||
@ -258,7 +251,8 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
* @param Member $member
|
* @param Member $member
|
||||||
* @return Member
|
* @return Member
|
||||||
*/
|
*/
|
||||||
private function checkGroups(Member $member):Member{
|
private function checkGroups(Member $member): Member
|
||||||
|
{
|
||||||
Log::debug(sprintf("ResourceServerContext::checkGroups member %s %s", $member->getId(), $member->getEmail()));
|
Log::debug(sprintf("ResourceServerContext::checkGroups member %s %s", $member->getId(), $member->getEmail()));
|
||||||
// check groups
|
// check groups
|
||||||
$groups = [];
|
$groups = [];
|
||||||
@ -274,6 +268,7 @@ final class ResourceServerContext implements IResourceServerContext
|
|||||||
}
|
}
|
||||||
return $this->member_service->synchronizeGroups($member, $groups);
|
return $this->member_service->synchronizeGroups($member, $groups);
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return array
|
* @return array
|
||||||
*/
|
*/
|
||||||
|
@ -20,6 +20,7 @@ use App\Services\Model\dto\ExternalUserDTO;
|
|||||||
use DateTime;
|
use DateTime;
|
||||||
use Illuminate\Support\Facades\Event;
|
use Illuminate\Support\Facades\Event;
|
||||||
use Illuminate\Support\Facades\Log;
|
use Illuminate\Support\Facades\Log;
|
||||||
|
use LaravelDoctrine\ORM\Facades\Registry;
|
||||||
use libs\utils\ICacheService;
|
use libs\utils\ICacheService;
|
||||||
use libs\utils\ITransactionService;
|
use libs\utils\ITransactionService;
|
||||||
use models\exceptions\EntityNotFoundException;
|
use models\exceptions\EntityNotFoundException;
|
||||||
@ -276,6 +277,7 @@ final class MemberService
|
|||||||
public function registerExternalUser(ExternalUserDTO $userDTO): Member
|
public function registerExternalUser(ExternalUserDTO $userDTO): Member
|
||||||
{
|
{
|
||||||
return $this->tx_service->transaction(function () use ($userDTO) {
|
return $this->tx_service->transaction(function () use ($userDTO) {
|
||||||
|
|
||||||
Log::debug
|
Log::debug
|
||||||
(
|
(
|
||||||
sprintf
|
sprintf
|
||||||
@ -288,15 +290,19 @@ final class MemberService
|
|||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
|
||||||
|
$member = $this->member_repository->getByExternalIdExclusiveLock($userDTO->getId());
|
||||||
|
|
||||||
|
if(is_null($member)) {
|
||||||
$member = new Member();
|
$member = new Member();
|
||||||
|
$member->setUserExternalId($userDTO->getId());
|
||||||
$member->setActive($userDTO->isActive());
|
$member->setActive($userDTO->isActive());
|
||||||
$member->setEmailVerified($userDTO->isEmailVerified());
|
$member->setEmailVerified($userDTO->isEmailVerified());
|
||||||
$member->setEmail($userDTO->getEmail());
|
$member->setEmail($userDTO->getEmail());
|
||||||
$member->setFirstName($userDTO->getFirstName());
|
$member->setFirstName($userDTO->getFirstName());
|
||||||
$member->setLastName($userDTO->getLastName());
|
$member->setLastName($userDTO->getLastName());
|
||||||
$member->setUserExternalId($userDTO->getId());
|
|
||||||
$this->member_repository->add($member, true);
|
$this->member_repository->add($member, true);
|
||||||
Event::fire(new NewMember($member->getId()));
|
Event::fire(new NewMember($member->getId()));
|
||||||
|
}
|
||||||
return $member;
|
return $member;
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
@ -1260,6 +1260,7 @@ final class SummitOrderService
|
|||||||
public function reSendOrderEmail(int $order_id):SummitOrder {
|
public function reSendOrderEmail(int $order_id):SummitOrder {
|
||||||
|
|
||||||
return $this->tx_service->transaction(function () use ($order_id) {
|
return $this->tx_service->transaction(function () use ($order_id) {
|
||||||
|
|
||||||
$order = $this->order_repository->getByIdExclusiveLock($order_id);
|
$order = $this->order_repository->getByIdExclusiveLock($order_id);
|
||||||
|
|
||||||
if (is_null($order) || !$order instanceof SummitOrder)
|
if (is_null($order) || !$order instanceof SummitOrder)
|
||||||
@ -1324,13 +1325,14 @@ final class SummitOrderService
|
|||||||
$ownerEmail
|
$ownerEmail
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
$external_id = $user['id'];
|
||||||
|
try {
|
||||||
// we have an user on idp
|
// we have an user on idp
|
||||||
$member = $this->member_service->registerExternalUser
|
$member = $this->member_service->registerExternalUser
|
||||||
(
|
(
|
||||||
new ExternalUserDTO
|
new ExternalUserDTO
|
||||||
(
|
(
|
||||||
$user['id'],
|
$external_id,
|
||||||
$user['email'],
|
$user['email'],
|
||||||
$user['first_name'],
|
$user['first_name'],
|
||||||
$user['last_name'],
|
$user['last_name'],
|
||||||
@ -1338,6 +1340,13 @@ final class SummitOrderService
|
|||||||
boolval($user['email_verified'])
|
boolval($user['email_verified'])
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
}
|
||||||
|
catch (\Exception $ex){
|
||||||
|
Log::warning($ex);
|
||||||
|
// race condition lost
|
||||||
|
$member = $this->member_repository->getByExternalIdExclusiveLock(intval($external_id));
|
||||||
|
$order = $this->order_repository->getByIdExclusiveLock($order_id);
|
||||||
|
}
|
||||||
|
|
||||||
// add the order to newly created member
|
// add the order to newly created member
|
||||||
$member->addSummitRegistrationOrder($order);
|
$member->addSummitRegistrationOrder($order);
|
||||||
@ -3367,11 +3376,13 @@ final class SummitOrderService
|
|||||||
);
|
);
|
||||||
|
|
||||||
// we have an user on idp
|
// we have an user on idp
|
||||||
|
$external_id = $user['id'];
|
||||||
|
try {
|
||||||
$member = $this->member_service->registerExternalUser
|
$member = $this->member_service->registerExternalUser
|
||||||
(
|
(
|
||||||
new ExternalUserDTO
|
new ExternalUserDTO
|
||||||
(
|
(
|
||||||
$user['id'],
|
$external_id,
|
||||||
$user['email'],
|
$user['email'],
|
||||||
$user['first_name'],
|
$user['first_name'],
|
||||||
$user['last_name'],
|
$user['last_name'],
|
||||||
@ -3379,6 +3390,13 @@ final class SummitOrderService
|
|||||||
boolval($user['email_verified'])
|
boolval($user['email_verified'])
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
}
|
||||||
|
catch (\Exception $ex){
|
||||||
|
// race condition lost, try to get it
|
||||||
|
Log::warning($ex);
|
||||||
|
$member = $this->member_repository->getByExternalIdExclusiveLock(intval($external_id));
|
||||||
|
$order = $this->order_repository->getByIdExclusiveLock($orderId);
|
||||||
|
}
|
||||||
// add the order to newly created member
|
// add the order to newly created member
|
||||||
$member->addSummitRegistrationOrder($order);
|
$member->addSummitRegistrationOrder($order);
|
||||||
}
|
}
|
||||||
|
@ -238,13 +238,15 @@ final class SummitRegistrationInvitationService
|
|||||||
$email
|
$email
|
||||||
)
|
)
|
||||||
);
|
);
|
||||||
|
$external_id = $user['id'];
|
||||||
|
try {
|
||||||
|
|
||||||
// we have an user on idp
|
// we have an user on idp
|
||||||
$member = $this->member_service->registerExternalUser
|
$member = $this->member_service->registerExternalUser
|
||||||
(
|
(
|
||||||
new ExternalUserDTO
|
new ExternalUserDTO
|
||||||
(
|
(
|
||||||
$user['id'],
|
$external_id,
|
||||||
$user['email'],
|
$user['email'],
|
||||||
$user['first_name'],
|
$user['first_name'],
|
||||||
$user['last_name'],
|
$user['last_name'],
|
||||||
@ -253,6 +255,13 @@ final class SummitRegistrationInvitationService
|
|||||||
)
|
)
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
catch (\Exception $ex){
|
||||||
|
Log::warning($ex);
|
||||||
|
// race condition lost
|
||||||
|
$member = $this->member_repository->getByExternalIdExclusiveLock(intval($external_id));
|
||||||
|
$invitation = $this->invitation_repository->getByIdExclusiveLock($invitation->getId());
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if(!is_null($member))
|
if(!is_null($member))
|
||||||
|
@ -306,6 +306,7 @@ final class SummitSelectionPlanService
|
|||||||
return $this->tx_service->transaction(function() use($summit, $selection_plan_id, $presentation_id, $payload){
|
return $this->tx_service->transaction(function() use($summit, $selection_plan_id, $presentation_id, $payload){
|
||||||
|
|
||||||
$current_member = $this->resource_server_ctx->getCurrentUser();
|
$current_member = $this->resource_server_ctx->getCurrentUser();
|
||||||
|
|
||||||
if(is_null($current_member))
|
if(is_null($current_member))
|
||||||
throw new AuthzException("User not Found");
|
throw new AuthzException("User not Found");
|
||||||
|
|
||||||
|
@ -78,6 +78,7 @@ final class DoctrineTransactionService implements ITransactionService
|
|||||||
// new entity manager
|
// new entity manager
|
||||||
$con = $em->getConnection();
|
$con = $em->getConnection();
|
||||||
}
|
}
|
||||||
|
|
||||||
$con->setTransactionIsolation($isolationLevel);
|
$con->setTransactionIsolation($isolationLevel);
|
||||||
$con->beginTransaction(); // suspend auto-commit
|
$con->beginTransaction(); // suspend auto-commit
|
||||||
$result = $callback($this);
|
$result = $callback($this);
|
||||||
@ -99,9 +100,8 @@ final class DoctrineTransactionService implements ITransactionService
|
|||||||
} catch (Exception $ex) {
|
} catch (Exception $ex) {
|
||||||
Log::warning("rolling back transaction");
|
Log::warning("rolling back transaction");
|
||||||
Log::warning($ex);
|
Log::warning($ex);
|
||||||
$em->close();
|
|
||||||
Log::warning("DoctrineTransactionService::transaction con->rollBack");
|
|
||||||
$con->rollBack();
|
$con->rollBack();
|
||||||
|
$em->close();
|
||||||
throw $ex;
|
throw $ex;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
40
tests/ResourceServerContextTest.php
Normal file
40
tests/ResourceServerContextTest.php
Normal file
@ -0,0 +1,40 @@
|
|||||||
|
<?php namespace Tests;
|
||||||
|
/**
|
||||||
|
* Copyright 2021 OpenStack Foundation
|
||||||
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
|
* you may not use this file except in compliance with the License.
|
||||||
|
* You may obtain a copy of the License at
|
||||||
|
* http://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
* See the License for the specific language governing permissions and
|
||||||
|
* limitations under the License.
|
||||||
|
**/
|
||||||
|
use Illuminate\Support\Facades\App;
|
||||||
|
use models\oauth2\IResourceServerContext;
|
||||||
|
/**
|
||||||
|
* Class ResourceServerContextTest
|
||||||
|
* @package Tests
|
||||||
|
*/
|
||||||
|
class ResourceServerContextTest extends BrowserKitTestCase
|
||||||
|
{
|
||||||
|
public function testSync(){
|
||||||
|
$ctx = App::make(IResourceServerContext::class);
|
||||||
|
if(!$ctx instanceof IResourceServerContext)
|
||||||
|
throw new \Exception();
|
||||||
|
|
||||||
|
$context = [];
|
||||||
|
$context['user_id'] = "1080";
|
||||||
|
$context['external_user_id'] = "1080";
|
||||||
|
$context['user_identifier'] = "test";
|
||||||
|
$context['user_email'] = "test@test.com";
|
||||||
|
$context['user_email_verified'] = true;
|
||||||
|
$context['user_first_name'] = "test";
|
||||||
|
$context['user_last_name'] = "test";
|
||||||
|
$context['user_groups'] = ['raw-users'];
|
||||||
|
$ctx->setAuthorizationContext($context);
|
||||||
|
|
||||||
|
$member = $ctx->getCurrentUser(true);
|
||||||
|
}
|
||||||
|
}
|
Loading…
Reference in New Issue
Block a user