diff --git a/app/V1Module/presenters/LoginPresenter.php b/app/V1Module/presenters/LoginPresenter.php index 8559d138d..b243102bf 100644 --- a/app/V1Module/presenters/LoginPresenter.php +++ b/app/V1Module/presenters/LoginPresenter.php @@ -22,6 +22,7 @@ use App\Security\Roles; use App\Security\TokenScope; use Nette\Security\AuthenticationException; +use Nette\Http\IResponse; /** * Endpoints used to log a user in @@ -201,6 +202,14 @@ public function actionRefresh() $token = $this->getAccessToken(); $user = $this->getCurrentUser(); + if (!$user->isAllowed()) { + throw new ForbiddenRequestException( + "Forbidden Request - User account was disabled", + IResponse::S403_Forbidden, + FrontendErrorMappings::E403_002__USER_NOT_ALLOWED + ); + } + $user->updateLastAuthenticationAt(); $this->users->flush(); @@ -247,6 +256,14 @@ public function actionIssueRestrictedToken() $this->validateEffectiveRole($effectiveRole); $user = $this->getCurrentUser(); + if (!$user->isAllowed()) { + throw new ForbiddenRequestException( + "Forbidden Request - User account was disabled", + IResponse::S403_Forbidden, + FrontendErrorMappings::E403_002__USER_NOT_ALLOWED + ); + } + $user->updateLastAuthenticationAt(); $this->users->flush(); @@ -265,7 +282,7 @@ private function validateScopeRoles(?array $scopes, $expiration) { $forbiddenScopes = [ TokenScope::CHANGE_PASSWORD => - "Password change tokens can only be issued through the password reset endpoint", + "Password change tokens can only be issued through the password reset endpoint", TokenScope::EMAIL_VERIFICATION => "E-mail verification tokens must be received via e-mail", ]; diff --git a/app/V1Module/presenters/UsersPresenter.php b/app/V1Module/presenters/UsersPresenter.php index b15565040..5fa7780f1 100644 --- a/app/V1Module/presenters/UsersPresenter.php +++ b/app/V1Module/presenters/UsersPresenter.php @@ -7,11 +7,13 @@ use App\Exceptions\InvalidArgumentException; use App\Exceptions\NotFoundException; use App\Exceptions\WrongCredentialsException; +use App\Model\Entity\ExternalLogin; use App\Model\Entity\Group; use App\Model\Entity\Login; use App\Model\Entity\SecurityEvent; use App\Model\Entity\User; use App\Model\Entity\UserUiData; +use App\Model\Repository\ExternalLogins; use App\Model\Repository\Logins; use App\Model\Repository\SecurityEvents; use App\Exceptions\BadRequestException; @@ -36,6 +38,12 @@ class UsersPresenter extends BasePresenter */ public $logins; + /** + * @var ExternalLogins + * @inject + */ + public $externalLogins; + /** * @var SecurityEvents * @inject @@ -225,7 +233,7 @@ public function checkUpdateProfile(string $id) * description="New password of current user") * @Param(type="post", name="passwordConfirm", required=false, validation="string:1..", * description="Confirmation of new password of current user") - * @Param(type="post", name="gravatarUrlEnabled", validation="bool", + * @Param(type="post", name="gravatarUrlEnabled", validation="bool", required=false, * description="Enable or disable gravatar profile image") * @throws WrongCredentialsException * @throws NotFoundException @@ -254,7 +262,10 @@ public function actionUpdateProfile(string $id) $req->getPost("passwordConfirm") ); - $user->setGravatar(filter_var($req->getPost("gravatarUrlEnabled"), FILTER_VALIDATE_BOOLEAN)); + $gravatarUrlEnabled = $req->getPost("gravatarUrlEnabled"); + if ($gravatarUrlEnabled !== null) { // null or missing value -> no update + $user->setGravatar(filter_var($gravatarUrlEnabled, FILTER_VALIDATE_BOOLEAN)); + } // make changes permanent $this->users->flush(); @@ -291,7 +302,7 @@ private function changeUserEmail(User $user, ?string $email) } if (filter_var($email, FILTER_VALIDATE_EMAIL) === false) { - throw new InvalidArgumentException("Provided email is not in correct format"); + throw new InvalidArgumentException('email', "Provided email is not in correct format"); } $oldEmail = $user->getEmail(); @@ -372,7 +383,7 @@ private function changeUserPassword( if (!$password || !$passwordConfirm) { // old password was provided but the new ones not, illegal state - throw new InvalidArgumentException("New password was not provided"); + throw new InvalidArgumentException('password|passwordConfirm', "New password was not provided"); } // passwords need to be handled differently @@ -761,4 +772,78 @@ public function actionSetAllowed(string $id) $this->users->flush(); $this->sendSuccessResponse($this->userViewFactory->getUser($user)); } + + public function checkUpdateExternalLogin(string $id, string $service) + { + $user = $this->users->findOrThrow($id); + if (!$this->userAcl->canSetExternalIds($user)) { + throw new ForbiddenRequestException(); + } + + // in the future, we might consider cross-checking the service ID + } + + /** + * Add or update existing external ID of given authentication service. + * @POST + * @param string $id identifier of the user + * @param string $service identifier of the authentication service (login type) + * @Param(type="post", name="externalId", validation="string:1..128") + * @throws InvalidArgumentException + */ + public function actionUpdateExternalLogin(string $id, string $service) + { + $user = $this->users->findOrThrow($id); + + // make sure the external ID is not used for another user + $externalId = $this->getRequest()->getPost("externalId"); + $anotherUser = $this->externalLogins->getUser($service, $externalId); + if ($anotherUser) { + if ($anotherUser->getId() !== $id) { + // oopsie, this external ID is alreay used for a different user + throw new InvalidArgumentException('externalId', "This ID is already used by another user."); + } + // otherwise the external ID is already set to this user, so there is nothing to change... + } else { + // create/update external login entry + $login = $this->externalLogins->findByUser($user, $service); + if ($login) { + $login->setExternalId($externalId); + } else { + $login = new ExternalLogin($user, $service, $externalId); + } + + $this->externalLogins->persist($login); + $this->users->refresh($user); + } + + $this->sendSuccessResponse($this->userViewFactory->getUser($user)); + } + + public function checkRemoveExternalLogin(string $id, string $service) + { + $user = $this->users->findOrThrow($id); + if (!$this->userAcl->canSetExternalIds($user)) { + throw new ForbiddenRequestException(); + } + + // in the future, we might consider cross-checking the service ID + } + + /** + * Remove external ID of given authentication service. + * @DELETE + * @param string $id identifier of the user + * @param string $service identifier of the authentication service (login type) + */ + public function actionRemoveExternalLogin(string $id, string $service) + { + $user = $this->users->findOrThrow($id); + $login = $this->externalLogins->findByUser($user, $service); + if ($login) { + $this->externalLogins->remove($login); + $this->users->refresh($user); + } + $this->sendSuccessResponse($this->userViewFactory->getUser($user)); + } } diff --git a/app/V1Module/router/RouterFactory.php b/app/V1Module/router/RouterFactory.php index c6357b495..b67a4b496 100644 --- a/app/V1Module/router/RouterFactory.php +++ b/app/V1Module/router/RouterFactory.php @@ -493,6 +493,8 @@ private static function createUsersRoutes(string $prefix): RouteList $router[] = new PostRoute("$prefix//create-local", "Users:createLocalAccount"); $router[] = new PostRoute("$prefix//role", "Users:setRole"); $router[] = new PostRoute("$prefix//allowed", "Users:setAllowed"); + $router[] = new PostRoute("$prefix//external-login/", "Users:updateExternalLogin"); + $router[] = new DeleteRoute("$prefix//external-login/", "Users:removeExternalLogin"); $router[] = new GetRoute("$prefix//calendar-tokens", "UserCalendars:userCalendars"); $router[] = new PostRoute("$prefix//calendar-tokens", "UserCalendars:createCalendar"); $router[] = new GetRoute("$prefix//pending-reviews", "AssignmentSolutionReviews:pending"); diff --git a/app/V1Module/security/ACL/IUserPermissions.php b/app/V1Module/security/ACL/IUserPermissions.php index 5d6c1dc11..dabef3e86 100644 --- a/app/V1Module/security/ACL/IUserPermissions.php +++ b/app/V1Module/security/ACL/IUserPermissions.php @@ -36,6 +36,8 @@ public function canSetRole(User $user): bool; public function canSetIsAllowed(User $user): bool; + public function canSetExternalIds(User $user): bool; + public function canInvalidateTokens(User $user): bool; public function canForceChangePassword(User $user): bool; diff --git a/app/V1Module/security/AccessManager.php b/app/V1Module/security/AccessManager.php index 0ac504d53..7fcf77ed6 100644 --- a/app/V1Module/security/AccessManager.php +++ b/app/V1Module/security/AccessManager.php @@ -112,7 +112,7 @@ public function getUser(AccessToken $token): User if (!$user) { throw new ForbiddenRequestException( "Forbidden Request - User does not exist", - IResponse::S403_FORBIDDEN, + IResponse::S403_Forbidden, FrontendErrorMappings::E403_001__USER_NOT_EXIST ); } @@ -120,7 +120,7 @@ public function getUser(AccessToken $token): User if (!$user->isAllowed()) { throw new ForbiddenRequestException( "Forbidden Request - User account was disabled", - IResponse::S403_FORBIDDEN, + IResponse::S403_Forbidden, FrontendErrorMappings::E403_002__USER_NOT_ALLOWED ); } @@ -148,7 +148,7 @@ public function issueToken( if (!$user->isAllowed()) { throw new ForbiddenRequestException( "Forbidden Request - User account was disabled", - IResponse::S403_FORBIDDEN, + IResponse::S403_Forbidden, FrontendErrorMappings::E403_002__USER_NOT_ALLOWED ); } diff --git a/app/V1Module/security/CredentialsAuthenticator.php b/app/V1Module/security/CredentialsAuthenticator.php index 66620e4c6..c79939621 100644 --- a/app/V1Module/security/CredentialsAuthenticator.php +++ b/app/V1Module/security/CredentialsAuthenticator.php @@ -4,10 +4,12 @@ use App\Exceptions\FrontendErrorMappings; use App\Exceptions\WrongCredentialsException; +use App\Exceptions\ForbiddenRequestException; use App\Model\Entity\User; use App\Model\Repository\Logins; use Nette; use Nette\Security\Passwords; +use Nette\Http\IResponse; class CredentialsAuthenticator { @@ -40,8 +42,15 @@ public function authenticate(string $username, string $password) "The username or password is incorrect.", FrontendErrorMappings::E400_101__WRONG_CREDENTIALS_LOCAL ); + } elseif (!$user->isAllowed()) { + throw new ForbiddenRequestException( + "Forbidden Request - User account was disabled", + IResponse::S403_Forbidden, + FrontendErrorMappings::E403_002__USER_NOT_ALLOWED + ); } + return $user; } } diff --git a/app/config/config.local.neon.example b/app/config/config.local.neon.example index 3ffc62785..a6228d35e 100644 --- a/app/config/config.local.neon.example +++ b/app/config/config.local.neon.example @@ -51,8 +51,9 @@ parameters: externalAuthenticators: - name: "cas-auth-ext" jwtSecret: "secretStringSharedWithExternAuth" + jwtAlgorithm: HS256 # optional, HS256 is default expiration: 60 # seconds passed since iat - usedAlgorithm: HS256 # optional, HS256 is default + extraIds: [] # additional service types whose IDs may be provided as extra IDs in the auth token emails: footerUrl: "%webapp.address%" diff --git a/app/config/permissions.neon b/app/config/permissions.neon index 5cd506242..41224d8ba 100644 --- a/app/config/permissions.neon +++ b/app/config/permissions.neon @@ -400,6 +400,7 @@ permissions: - updateProfile - updatePersonalData - setIsAllowed + - setExternalIds - createLocalAccount - invalidateTokens diff --git a/app/helpers/ExternalLogin/ExternalServiceAuthenticator.php b/app/helpers/ExternalLogin/ExternalServiceAuthenticator.php index 752571e7b..e332af03f 100644 --- a/app/helpers/ExternalLogin/ExternalServiceAuthenticator.php +++ b/app/helpers/ExternalLogin/ExternalServiceAuthenticator.php @@ -7,6 +7,7 @@ use App\Exceptions\WrongCredentialsException; use App\Exceptions\InvalidExternalTokenException; use App\Exceptions\InvalidArgumentException; +use App\Exceptions\ForbiddenRequestException; use App\Model\Entity\Instance; use App\Model\Entity\User; use App\Model\Repository\ExternalLogins; @@ -14,7 +15,10 @@ use App\Model\Repository\Users; use App\Model\Repository\Instances; use App\Helpers\EmailVerificationHelper; +use App\Helpers\FailureHelper; use Nette\Utils\Arrays; +use Nette\Http\IResponse; +use Tracy\ILogger; use Firebase\JWT\JWT; use Firebase\JWT\Key; use DomainException; @@ -40,6 +44,12 @@ class ExternalServiceAuthenticator /** @var EmailVerificationHelper */ public $emailVerificationHelper; + /** @var FailureHelper */ + public $failureHelper; + + /** @var ILogger|null */ + public $logger = null; + /** * @var array [ name => { jwtSecret, expiration } ] */ @@ -59,13 +69,17 @@ public function __construct( Users $users, Logins $logins, Instances $instances, - EmailVerificationHelper $emailVerificationHelper + EmailVerificationHelper $emailVerificationHelper, + FailureHelper $failureHelper, + ?ILogger $logger = null, ) { $this->externalLogins = $externalLogins; $this->users = $users; $this->logins = $logins; $this->instances = $instances; $this->emailVerificationHelper = $emailVerificationHelper; + $this->failureHelper = $failureHelper; + $this->logger = $logger; foreach ($authenticators as $auth) { if (!empty($auth['name'] && !empty($auth['jwtSecret']))) { @@ -73,13 +87,26 @@ public function __construct( 'jwtSecret' => $auth['jwtSecret'], 'expiration' => Arrays::get($auth, 'expiration', 60), 'defaultRole' => Arrays::get($auth, 'defaultRole', null), - 'usedAlgorithm' => Arrays::get($auth, 'usedAlgorithm', 'HS256'), // if set, users may register even when extrnal authenticator does not provide role + 'usedAlgorithm' => Arrays::get($auth, 'jwtAlgorithm', 'HS256'), + 'extraIds' => Arrays::get($auth, 'extraIds', []), ]; } } } + private function log($level, $msg, ...$args) + { + if (!$this->logger) { + return; + } + + if ($args) { + $msg = sprintf($msg, ...$args); + } + $this->logger->log($msg, $level); + } + /** * Verify that given authenticator exists. * @param string $name of the external authenticator @@ -120,6 +147,15 @@ public function authenticate(string $authName, string $token, string $instanceId // try to match existing local user by email address if ($user === null) { $user = $this->tryConnect($authName, $userData); + if ($user) { + $this->log( + ILogger::INFO, + "User '%s' was paired with external ID='%s' (%s)", + $user->getId(), + $userData->getId(), + $authName + ); + } } // try to register a new user @@ -139,17 +175,33 @@ public function authenticate(string $authName, string $token, string $instanceId // connect the account to the login method $this->externalLogins->connect($authName, $user, $userData->getId()); $this->emailVerificationHelper->process($user, true); // true = just created + $this->log( + ILogger::INFO, + "User '%s' just registered via external auth '%s' (ID='%s')", + $user->getId(), + $authName, + $userData->getId() + ); } } + // failures throw exceptions... if ($user === null) { throw new WrongCredentialsException( "User authenticated through '$authName' has no corresponding account in ReCodEx.", FrontendErrorMappings::E400_104__EXTERNAL_AUTH_FAILED_USER_NOT_FOUND, ["service" => $authName] ); + } elseif (!$user->isAllowed()) { + throw new ForbiddenRequestException( + "Forbidden Request - User account was disabled", + IResponse::S403_Forbidden, + FrontendErrorMappings::E403_002__USER_NOT_ALLOWED + ); } + $this->handleExtraIds($user, $decodedToken, $this->authenticators[$authName]->extraIds); + return $user; } @@ -228,4 +280,75 @@ private function getInstance($decodedToken, string $instanceId = null): ?Instanc return null; } + + /** + * Process possible additional (extra) identifiers present in the token. + * @param User $user being authenticated + * @param object $decodedToken + * @param string[] $allowedServices whose extra IDs may be added from the token + */ + private function handleExtraIds(User $user, $decodedToken, array $allowedServices) + { + if (empty($decodedToken->extId)) { + return; + } + + $this->log(ILogger::DEBUG, "User '%s' got extra IDs: %s", $user->getId(), json_encode($decodedToken->extId)); + + foreach ($decodedToken->extId as $service => $eid) { + if (!in_array($service, $allowedServices)) { + $this->log( + ILogger::DEBUG, + "User '%s' got new [%s] ID='%s', but this auth service is not allowed", + $user->getId(), + $service, + $eid + ); + + continue; // skip services that are not allowed + } + + $extUser = $this->externalLogins->getUser($service, $eid); + if ($extUser) { // a user with given ID exists + if ($extUser->getId() !== $user->getId()) { + // Identity crysis! ID belongs to another user... + $this->failureHelper->report( + FailureHelper::TYPE_API_ERROR, + sprintf( + "User '%s' was provided with extra ID '%s' (%s), " + . "but that is already associated with user '%s'.", + $user->getId(), + $eid, + $service, + $extUser->getId() + ) + ); + } + + continue; // either already exist or we cannot proceed anyway + } + + $login = $this->externalLogins->findByUser($user, $service); + if ($login) { // a connected external login record for the auth service already exist + if ($login->getExternalId() !== $eid) { + // extra ID has changed (strange, but possible) + $this->log( + ILogger::INFO, + "User '%s' got new [%s] ID='%s', but already had different ID='%s'", + $user->getId(), + $service, + $eid, + $login->getExternalId() + ); + $login->setExternalId($eid); + $this->externalLogins->persist($login); + } + + continue; // either already exist or was duly updated + } + + $this->externalLogins->connect($service, $user, $eid); + $this->log(ILogger::INFO, "User '%s' got new extra ID='%s' [%s]", $user->getId(), $eid, $service); + } + } } diff --git a/app/helpers/FailureHelper/FailureHelper.php b/app/helpers/FailureHelper/FailureHelper.php index 8f590f7d3..1fb3327b3 100644 --- a/app/helpers/FailureHelper/FailureHelper.php +++ b/app/helpers/FailureHelper/FailureHelper.php @@ -15,7 +15,6 @@ */ class FailureHelper { - public const TYPE_BACKEND_ERROR = "BACKEND ERROR"; public const TYPE_API_ERROR = "API ERROR"; diff --git a/app/model/entity/ExternalLogin.php b/app/model/entity/ExternalLogin.php index d07af0c71..4d6de1441 100644 --- a/app/model/entity/ExternalLogin.php +++ b/app/model/entity/ExternalLogin.php @@ -6,7 +6,8 @@ /** * @ORM\Entity - * @ORM\Table(uniqueConstraints={@ORM\UniqueConstraint(columns={"auth_service", "external_id"})}) + * @ORM\Table(uniqueConstraints={@ORM\UniqueConstraint(columns={"auth_service", "user_id"}), + * @ORM\UniqueConstraint(columns={"auth_service", "external_id"})}) */ class ExternalLogin { @@ -60,6 +61,11 @@ public function getExternalId(): string return $this->externalId; } + public function setExternalId(string $externalId): void + { + $this->externalId = $externalId; + } + public function getUser(): User { return $this->user; diff --git a/app/model/repository/ExternalLogins.php b/app/model/repository/ExternalLogins.php index b53487a1d..a1b379040 100644 --- a/app/model/repository/ExternalLogins.php +++ b/app/model/repository/ExternalLogins.php @@ -11,7 +11,6 @@ */ class ExternalLogins extends BaseRepository { - public function __construct(EntityManagerInterface $em) { parent::__construct($em, ExternalLogin::class); diff --git a/app/model/view/UserViewFactory.php b/app/model/view/UserViewFactory.php index f25d11c45..ebe1fd483 100644 --- a/app/model/view/UserViewFactory.php +++ b/app/model/view/UserViewFactory.php @@ -45,7 +45,7 @@ public function __construct(IUserPermissions $userAcl, Logins $logins, \Nette\Se */ private function getExternalIds(User $user, bool $canViewAllExternalIds = false) { - if (!$canViewAllExternalIds) { + if (!$canViewAllExternalIds && !$this->userAcl->canSetExternalIds($user)) { if (!$this->loggedInUser) { return []; } diff --git a/migrations/Version20250209140028.php b/migrations/Version20250209140028.php new file mode 100644 index 000000000..44a760220 --- /dev/null +++ b/migrations/Version20250209140028.php @@ -0,0 +1,31 @@ +addSql('CREATE UNIQUE INDEX UNIQ_9845B893AFB9BA21A76ED395 ON external_login (auth_service, user_id)'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->addSql('DROP INDEX UNIQ_9845B893AFB9BA21A76ED395 ON external_login'); + } +} diff --git a/tests/ExternalLogin/ExternalServiceAuthenticator.phpt b/tests/ExternalLogin/ExternalServiceAuthenticator.phpt index 288de9ca1..74a0ed1fa 100644 --- a/tests/ExternalLogin/ExternalServiceAuthenticator.phpt +++ b/tests/ExternalLogin/ExternalServiceAuthenticator.phpt @@ -1,14 +1,9 @@ users = $container->getByType(Users::class); $this->logins = $container->getByType(Logins::class); $this->authenticator = new ExternalServiceAuthenticator( - [ [ + [[ 'name' => self::AUTH_NAME, 'jwtSecret' => self::AUTH_SECRET, 'expiration' => 60, - ] ], + ]], $this->externalLogins, $this->users, $this->logins, $container->getByType(Instances::class), - $container->getByType(EmailVerificationHelper::class) + $container->getByType(EmailVerificationHelper::class), + $container->getByType(App\Helpers\FailureHelper::class) ); } diff --git a/tests/Presenters/AssignmentSolversPresenter.phpt b/tests/Presenters/AssignmentSolversPresenter.phpt index 5f9253aea..adeb71c10 100644 --- a/tests/Presenters/AssignmentSolversPresenter.phpt +++ b/tests/Presenters/AssignmentSolversPresenter.phpt @@ -2,14 +2,10 @@ $container = require_once __DIR__ . "/../bootstrap.php"; -use App\Helpers\BrokerProxy; use App\V1Module\Presenters\AssignmentSolversPresenter; -use App\Model\Entity\AssignmentSolver; use App\Model\Repository\Assignments; -use App\Model\Repository\AssignmentSolvers; use Doctrine\ORM\EntityManagerInterface; use Tester\Assert; -use Tracy\ILogger; $_SERVER['REMOTE_ADDR'] = '127.0.0.1'; diff --git a/tests/Presenters/LoginPresenter.phpt b/tests/Presenters/LoginPresenter.phpt index 89e55d0bc..a622b4a89 100644 --- a/tests/Presenters/LoginPresenter.phpt +++ b/tests/Presenters/LoginPresenter.phpt @@ -52,6 +52,9 @@ class TestLoginPresenter extends Tester\TestCase /** @var \App\Helpers\EmailVerificationHelper */ private $emailVerificationHelper; + /** @var \App\Helpers\FailureHelper */ + private $failureHelper; + public function __construct($container) { $this->container = $container; @@ -62,6 +65,7 @@ class TestLoginPresenter extends Tester\TestCase $this->externalLogins = $container->getByType(\App\Model\Repository\ExternalLogins::class); $this->instances = $container->getByType(\App\Model\Repository\Instances::class); $this->emailVerificationHelper = $container->getByType(\App\Helpers\EmailVerificationHelper::class); + $this->failureHelper = $container->getByType(App\Helpers\FailureHelper::class); } protected function setUp() @@ -86,8 +90,8 @@ class TestLoginPresenter extends Tester\TestCase "POST", ["action" => "default"], [ - "username" => $this->userLogin, - "password" => $this->userPassword + "username" => $this->userLogin, + "password" => $this->userPassword ] ); @@ -114,8 +118,8 @@ class TestLoginPresenter extends Tester\TestCase "POST", ["action" => "default"], [ - "username" => $this->userLogin, - "password" => $this->userPassword . "42" + "username" => $this->userLogin, + "password" => $this->userPassword . "42" ] ); @@ -135,15 +139,16 @@ class TestLoginPresenter extends Tester\TestCase Assert::count(0, $events); $authenticator = new ExternalServiceAuthenticator( - [ [ + [[ 'name' => 'test-cas', 'jwtSecret' => 'tajnyRetezec', - ] ], + ]], $this->externalLogins, $this->users, $this->logins, $this->instances, - $this->emailVerificationHelper + $this->emailVerificationHelper, + $this->failureHelper ); $user = $this->presenter->users->getByEmail($this->userLogin); @@ -159,7 +164,7 @@ class TestLoginPresenter extends Tester\TestCase $this->presenter->externalServiceAuthenticator = $authenticator; - $request = new Request("V1:Login", "POST", ["action" => "external", "authenticatorName" => "test-cas"], [ 'token' => $token ]); + $request = new Request("V1:Login", "POST", ["action" => "external", "authenticatorName" => "test-cas"], ['token' => $token]); $response = $this->presenter->run($request); Assert::type(JsonResponse::class, $response); diff --git a/tests/Presenters/UsersPresenter.phpt b/tests/Presenters/UsersPresenter.phpt index 74a75099f..26f581228 100644 --- a/tests/Presenters/UsersPresenter.phpt +++ b/tests/Presenters/UsersPresenter.phpt @@ -244,7 +244,6 @@ class TestUsersPresenter extends Tester\TestCase 'lastName' => $lastName, 'titlesBeforeName' => $titlesBeforeName, 'titlesAfterName' => $titlesAfterName, - 'gravatarUrlEnabled' => false ] ); $response = $this->presenter->run($request); @@ -276,6 +275,9 @@ class TestUsersPresenter extends Tester\TestCase $emailVerificationHelper->shouldReceive("process")->with($user)->andReturn()->once(); $this->presenter->emailVerificationHelper = $emailVerificationHelper; + $user->setGravatar(true); + $this->presenter->users->persist($user); + $request = new Nette\Application\Request( $this->presenterPath, 'POST', @@ -286,7 +288,7 @@ class TestUsersPresenter extends Tester\TestCase 'titlesBeforeName' => $titlesBeforeName, 'titlesAfterName' => $titlesAfterName, 'email' => $email, - 'gravatarUrlEnabled' => false + 'gravatarUrlEnabled' => false // make sure gravatar gets reset ] ); $response = $this->presenter->run($request); @@ -313,6 +315,9 @@ class TestUsersPresenter extends Tester\TestCase $user = $this->users->getByEmail(PresenterTestHelper::ADMIN_LOGIN); $login = $this->presenter->logins->findByUsernameOrThrow($user->getEmail()); + $user->setGravatar(true); + $this->presenter->users->persist($user); + $firstName = "firstNameUpdated"; $lastName = "lastNameUpdated"; $titlesBeforeName = "titlesBeforeNameUpdated"; @@ -333,7 +338,6 @@ class TestUsersPresenter extends Tester\TestCase 'oldPassword' => $oldPassword, 'password' => $password, 'passwordConfirm' => $passwordConfirm, - 'gravatarUrlEnabled' => false ] ); $response = $this->presenter->run($request); @@ -345,7 +349,7 @@ class TestUsersPresenter extends Tester\TestCase $updatedUser = $result["payload"]["user"]; Assert::equal("$titlesBeforeName $firstName $lastName $titlesAfterName", $updatedUser["fullName"]); Assert::true($login->passwordsMatchOrEmpty($password, $this->presenter->passwordsService)); - Assert::null($updatedUser["avatarUrl"]); + Assert::true($updatedUser["avatarUrl"] !== null); // gravatar was not reset $storedUpdatedUser = $this->users->get($user->getId()); Assert::equal($updatedUser["id"], $storedUpdatedUser->getId()); @@ -459,7 +463,7 @@ class TestUsersPresenter extends Tester\TestCase [ 'titlesBeforeName' => '', 'titlesAfterName' => '', - 'gravatarUrlEnabled' => false, + 'gravatarUrlEnabled' => null, 'password' => $newPassword, 'passwordConfirm' => $newPassword, ] @@ -467,6 +471,7 @@ class TestUsersPresenter extends Tester\TestCase $updatedUser = $payload["user"]; Assert::equal($updatedUser["privateData"]["email"], PresenterTestHelper::GROUP_SUPERVISOR_LOGIN); + Assert::null($updatedUser["avatarUrl"]); $login = $this->logins->findByUsernameOrThrow($user->getEmail()); Assert::true($login->passwordsMatch($newPassword, $this->presenter->passwordsService)); @@ -579,13 +584,13 @@ class TestUsersPresenter extends Tester\TestCase ); Assert::equal($uiData, $payload["privateData"]["uiData"]); - $nested = [ 'pos1' => 0 ]; + $nested = ['pos1' => 0]; $payload = PresenterTestHelper::performPresenterRequest( $this->presenter, $this->presenterPath, 'POST', ['action' => 'updateUiData', 'id' => $user->getId()], - ['uiData' => [ 'stretcherSize' => 54, 'nestedStructure' => $nested ] ] + ['uiData' => ['stretcherSize' => 54, 'nestedStructure' => $nested]] ); $uiData['stretcherSize'] = 54; $uiData['nestedStructure'] = $nested; @@ -596,7 +601,7 @@ class TestUsersPresenter extends Tester\TestCase $this->presenterPath, 'POST', ['action' => 'updateUiData', 'id' => $user->getId()], - [ 'uiData' => $uiData2, 'overwrite' => true ] + ['uiData' => $uiData2, 'overwrite' => true] ); Assert::equal($uiData2, $payload["privateData"]["uiData"]); @@ -605,7 +610,7 @@ class TestUsersPresenter extends Tester\TestCase $this->presenterPath, 'POST', ['action' => 'updateUiData', 'id' => $user->getId()], - [ 'uiData' => null ] + ['uiData' => null] ); Assert::equal($uiData2, $payload["privateData"]["uiData"]); @@ -614,7 +619,7 @@ class TestUsersPresenter extends Tester\TestCase $this->presenterPath, 'POST', ['action' => 'updateUiData', 'id' => $user->getId()], - [ 'uiData' => null, 'overwrite' => true ] + ['uiData' => null, 'overwrite' => true] ); Assert::null($payload["privateData"]["uiData"]); } @@ -832,6 +837,98 @@ class TestUsersPresenter extends Tester\TestCase Assert::false($payload['privateData']['isAllowed']); Assert::false($this->users->getByEmail($victim)->isAllowed()); } + + public function testSetNewExternalId() + { + $victim = "user2@example.com"; + PresenterTestHelper::loginDefaultAdmin($this->container); + $user = $this->users->getByEmail($victim); + Assert::false($user->hasExternalAccounts()); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + $this->presenterPath, + 'POST', + ['action' => 'updateExternalLogin', 'id' => $user->getId(), 'service' => 'test-cas'], + ['externalId' => 'abc'] + ); + + Assert::equal($user->getId(), $payload['id']); + Assert::true($payload['privateData']['isExternal']); + Assert::equal(['test-cas' => 'abc'], $payload['privateData']['externalIds']); + + $els = $this->presenter->externalLogins->findAll(); + Assert::count(1, $els); + Assert::equal($user->getId(), $els[0]->getUser()->getId()); + Assert::equal('abc', $els[0]->getExternalId()); + Assert::equal('test-cas', $els[0]->getAuthService()); + + $this->users->refresh($user); + Assert::true($user->hasExternalAccounts()); + Assert::equal(['test-cas' => 'abc'], $user->getConsolidatedExternalLogins()); + } + + public function testUpdateExternalId() + { + $victim = "user2@example.com"; + PresenterTestHelper::loginDefaultAdmin($this->container); + $user = $this->users->getByEmail($victim); + Assert::false($user->hasExternalAccounts()); + $this->presenter->externalLogins->connect('test-cas', $user, 'old'); + $this->users->refresh($user); + Assert::equal(['test-cas' => 'old'], $user->getConsolidatedExternalLogins()); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + $this->presenterPath, + 'POST', + ['action' => 'updateExternalLogin', 'id' => $user->getId(), 'service' => 'test-cas'], + ['externalId' => 'abc'] + ); + + Assert::equal($user->getId(), $payload['id']); + Assert::true($payload['privateData']['isExternal']); + Assert::equal(['test-cas' => 'abc'], $payload['privateData']['externalIds']); + + $els = $this->presenter->externalLogins->findAll(); + Assert::count(1, $els); + Assert::equal($user->getId(), $els[0]->getUser()->getId()); + Assert::equal('abc', $els[0]->getExternalId()); + Assert::equal('test-cas', $els[0]->getAuthService()); + + $this->users->refresh($user); + Assert::true($user->hasExternalAccounts()); + Assert::equal(['test-cas' => 'abc'], $user->getConsolidatedExternalLogins()); + } + + public function testRemoveExternalId() + { + $victim = "user2@example.com"; + PresenterTestHelper::loginDefaultAdmin($this->container); + $user = $this->users->getByEmail($victim); + Assert::false($user->hasExternalAccounts()); + $this->presenter->externalLogins->connect('test-cas', $user, 'old'); + $this->users->refresh($user); + Assert::equal(['test-cas' => 'old'], $user->getConsolidatedExternalLogins()); + + $payload = PresenterTestHelper::performPresenterRequest( + $this->presenter, + $this->presenterPath, + 'DELETE', + ['action' => 'removeExternalLogin', 'id' => $user->getId(), 'service' => 'test-cas'] + ); + + Assert::equal($user->getId(), $payload['id']); + Assert::false($payload['privateData']['isExternal']); + Assert::equal([], $payload['privateData']['externalIds']); + + $els = $this->presenter->externalLogins->findAll(); + Assert::count(0, $els); + + $this->users->refresh($user); + Assert::false($user->hasExternalAccounts()); + Assert::equal([], $user->getConsolidatedExternalLogins()); + } } (new TestUsersPresenter())->run();