Skip to content

Commit

Permalink
feat: cannot sign declaration unless op-adm for org has logged in (#364)
Browse files Browse the repository at this point in the history
* feat: cannot sign declaration unless op-adm has logged in

* chore: refactors and unit tests

* chore: removed unused import

* fix: locked down access to query

* fix: local.php.dist missing quote and tdy up
  • Loading branch information
jerotire authored Oct 4, 2024
1 parent 3414573 commit 47e31e4
Show file tree
Hide file tree
Showing 11 changed files with 968 additions and 2,305 deletions.
1,742 changes: 386 additions & 1,356 deletions app/api/composer.lock

Large diffs are not rendered by default.

7 changes: 3 additions & 4 deletions app/api/config/autoload/local.php.dist
Original file line number Diff line number Diff line change
Expand Up @@ -374,10 +374,9 @@ return [
],
],
'data-gov-uk-export' => [
's3_uri' => 's3://devapp-vol-content/olcs.local.nonprod.dvsa.aws/data-gov-uk-export/'
],

's3_uri' => 's3://devapp-vol-content/olcs.local.nonprod.dvsa.aws/data-gov-uk-export/',
],
'data-dva-ni-export' => [
's3_uri' => 's3://devapp-olcs-pri-integration-dva-s3/
's3_uri' => 's3://devapp-olcs-pri-integration-dva-s3/',
],
];
1 change: 1 addition & 0 deletions app/api/module/Api/config/query-map.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -299,6 +299,7 @@
TransferQuery\User\RoleList::class => QueryHandler\User\RoleList::class,
TransferQuery\User\UserListInternal::class => QueryHandler\User\UserListInternal::class,
Query\User\UserListInternalByTrafficArea::class => QueryHandler\User\UserListInternalByTrafficArea::class,
TransferQuery\User\OperatorAdminForOrganisationHasLoggedIn::class => QueryHandler\User\OperatorAdminForOrganisationHasLoggedIn::class,

// User
TransferQuery\Team\Team::class => QueryHandler\Team\Team::class,
Expand Down
1 change: 1 addition & 0 deletions app/api/module/Api/config/validation-map/user.config.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
QueryHandler\User\UserListInternalByTrafficArea::class => IsInternalUser::class,
QueryHandler\User\UserListSelfserve::class => CanManageUser::class,
QueryHandler\User\UserSelfserve::class => CanReadUser::class,
QueryHandler\User\OperatorAdminForOrganisationHasLoggedIn::class => CanAccessUserList::class,

// Commands
CommandHandler\MyAccount\UpdateMyAccountInternal::class => IsInternalUser::class,
Expand Down
22 changes: 6 additions & 16 deletions app/api/module/Api/src/Domain/Query/User/UserListSelfserve.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@

namespace Dvsa\Olcs\Api\Domain\Query\User;

use Dvsa\Olcs\Transfer\FieldType\Traits\LastLoggedInFromOptional;
use Dvsa\Olcs\Transfer\FieldType\Traits\OrganisationOptional;
use Dvsa\Olcs\Transfer\FieldType\Traits\RolesOptional;
use Dvsa\Olcs\Transfer\Query\AbstractQuery;
use Dvsa\Olcs\Transfer\Query\OrderedQueryInterface;
use Dvsa\Olcs\Transfer\Query\OrderedTrait;
Expand All @@ -19,6 +22,9 @@ final class UserListSelfserve extends AbstractQuery implements PagedQueryInterfa
{
use PagedTrait;
use OrderedTrait;
use RolesOptional;
use OrganisationOptional;
use LastLoggedInFromOptional;

/**
* @Transfer\Filter("Laminas\Filter\Digits")
Expand All @@ -36,14 +42,6 @@ final class UserListSelfserve extends AbstractQuery implements PagedQueryInterfa
*/
protected $partnerContactDetails = null;

/**
* @Transfer\Filter("Laminas\Filter\Digits")
* @Transfer\Validator("Laminas\Validator\Digits")
* @Transfer\Validator("Laminas\Validator\GreaterThan", options={"min": 0})
* @Transfer\Optional
*/
protected $organisation = null;

/**
* @return int
*/
Expand All @@ -59,12 +57,4 @@ public function getPartnerContactDetails()
{
return $this->partnerContactDetails;
}

/**
* @return int
*/
public function getOrganisation()
{
return $this->organisation;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
<?php declare(strict_types=1);

namespace Dvsa\Olcs\Api\Domain\QueryHandler\User;

use Dvsa\Olcs\Api\Domain\Exception\BadRequestException;
use Dvsa\Olcs\Api\Domain\Exception\RuntimeException;
use Dvsa\Olcs\Api\Domain\Query\User\UserListSelfserve as ListDto;
use Dvsa\Olcs\Api\Domain\QueryHandler\AbstractQueryHandler;
use Dvsa\Olcs\Api\Domain\Repository;
use Dvsa\Olcs\Api\Entity;
use Dvsa\Olcs\Transfer\Query\QueryInterface;
use Dvsa\Olcs\Transfer\Query\User\OperatorAdminForOrganisationHasLoggedIn as Qry;

class OperatorAdminForOrganisationHasLoggedIn extends AbstractQueryHandler
{
public const DEFAULT_LAST_LOGGED_IN_FROM = '1970-01-01';

protected $repoServiceName = Repository\User::class;

/**
* Handle query
*
* @param QueryInterface $query query
*
* @throws BadRequestException|RuntimeException
*/
public function handleQuery(QueryInterface $query): array
{
if (!$query instanceof Qry) {
throw new BadRequestException('Expected instance of: ' . Qry::class);
}

if (empty($query->getOrganisation())) {
throw new BadRequestException('Organisation ID is required');
}

$repo = $this->getRepo(Repository\User::class);

$params = [
'organisation' => $query->getOrganisation(),
'roles' => [Entity\User\Role::ROLE_OPERATOR_ADMIN],
'page' => 1,
'limit' => 1,
'sort' => 'id',
'order' => 'DESC',
];

$lastLoginDate = static::DEFAULT_LAST_LOGGED_IN_FROM;
if (!empty($query->getLastLoggedInFrom())) {
$lastLoginDate = $query->getLastLoggedInFrom();
}

$params['lastLoggedInFrom'] = $lastLoginDate;

$userListDto = ListDto::create($params);

$result = [
'organisation' => (int) $query->getOrganisation(),
'lastLoggedInFrom' => $lastLoginDate,
'operatorAdminHasLoggedIn' => false,
];

if ($repo->fetchCount($userListDto) > 0) {
$result['operatorAdminHasLoggedIn'] = true;
}

return $result;
}
}
6 changes: 6 additions & 0 deletions app/api/module/Api/src/Domain/Repository/User.php
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,12 @@ protected function applyListFilters(QueryBuilder $qb, QueryInterface $query)
$qb->setParameter('roles', $query->getRoles());
}

// filter by lastLoggedInFrom if it has been specified
if (method_exists($query, 'getLastLoggedInFrom') && !empty($query->getLastLoggedInFrom())) {
$qb->andWhere($qb->expr()->gte($this->alias . '.lastLoginAt', ':lastLoggedInFrom'))
->setParameter('lastLoggedInFrom', $query->getLastLoggedInFrom());
}

// exclude system user from all lists
$qb->andWhere($qb->expr()->neq($this->alias . '.id', ':systemUser'))
->setParameter('systemUser', IdentityProviderInterface::SYSTEM_USER);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
<?php

namespace Dvsa\OlcsTest\Api\Domain\QueryHandler\User;

use DateTimeImmutable;
use Dvsa\Olcs\Api\Domain\Query\User\UserListSelfserve as ListDto;
use Dvsa\Olcs\Api\Domain\QueryHandler\User\OperatorAdminForOrganisationHasLoggedIn as QueryHandler;
use Dvsa\Olcs\Api\Domain\Repository;
use Dvsa\Olcs\Api\Entity;
use Dvsa\Olcs\Transfer\Query\QueryInterface;
use Dvsa\Olcs\Transfer\Query\User\OperatorAdminForOrganisationHasLoggedIn as Query;
use Dvsa\OlcsTest\Api\Domain\QueryHandler\QueryHandlerTestCase;
use LmcRbacMvc\Service\AuthorizationService;
use Mockery as m;

class OperatorAdminForOrganisationHasLoggedInTest extends QueryHandlerTestCase
{
public function setUp(): void
{
$this->sut = new QueryHandler();
$this->mockRepo(Repository\User::class, Repository\User::class);

$this->mockedSmServices = [
AuthorizationService::class => m::mock(AuthorizationService::class)
];

parent::setUp();
}

public function dpHandleQuery_OrganisationHasOperatorAdminsWhoHaveLoggedIn(): array
{
return [
'HasLoggedIn' => [
'fetchCountResult' => 1,
'expectedOperatorAdminHasLoggedIn' => true,
],
'HasNotLoggedIn' => [
'fetchCountResult' => 0,
'expectedOperatorAdminHasLoggedIn' => false,
],
];
}

/**
* @dataProvider dpHandleQuery_OrganisationHasOperatorAdminsWhoHaveLoggedIn
*/
public function testHandleQuery_OrganisationHasOperatorAdminsWhoHaveLoggedIn(int $fetchCountResult, bool $expectedOperatorAdminHasLoggedIn): void
{
$organisationId = 1;

$query = Query::create(['organisation' => $organisationId]);

$repo = $this->repoMap[Repository\User::class];
$repo->shouldReceive('fetchCount')
->with(m::type(ListDto::class))
->andReturn($fetchCountResult);


$result = $this->sut->handleQuery($query);
$this->assertArrayHasKey('operatorAdminHasLoggedIn', $result);
$this->assertEquals($expectedOperatorAdminHasLoggedIn, $result['operatorAdminHasLoggedIn']);
}

public function dpHandleQuery_UsesLastLoggedInFromFromQueryIfProvided(): array
{
return [
'LastLoggedInFrom not specified' => [
'expectedLastLoggedInFrom' => QueryHandler::DEFAULT_LAST_LOGGED_IN_FROM,
'query' => Query::create(['organisation' => 1]),
],
'LastLoggedInFrom specified' => [
'expectedLastLoggedInFrom' => $specifiedDate = '2020-01-01',
'query' => Query::create(['organisation' => 1, 'lastLoggedInFrom' => $specifiedDate]),
],
];
}

/**
* @dataProvider dpHandleQuery_UsesLastLoggedInFromFromQueryIfProvided
*/
public function testHandleQuery_UsesLastLoggedInFromFromQueryIfProvided(string $expectedLastLoggedInFrom, Query $query): void
{
$repo = $this->repoMap[Repository\User::class];
$repo->shouldReceive('fetchCount')
->withArgs(function (ListDto $dto) use ($expectedLastLoggedInFrom) {
return $dto->getLastLoggedInFrom() === $expectedLastLoggedInFrom;
})
->andReturn(0);

$result = $this->sut->handleQuery($query);
$this->assertArrayHasKey('lastLoggedInFrom', $result);
$this->assertEquals($expectedLastLoggedInFrom, $result['lastLoggedInFrom']);
}

public function testHandleQuery_ResultContainsOrganisationIdFromQuery(): void
{
$organisationId = 1;

$query = Query::create(['organisation' => $organisationId]);

$repo = $this->repoMap[Repository\User::class];
$repo->shouldReceive('fetchCount')
->with(m::type(ListDto::class))
->andReturn(0);

$result = $this->sut->handleQuery($query);
$this->assertArrayHasKey('organisation', $result);
$this->assertEquals($organisationId, $result['organisation']);
}

public function testHandleQuery_ThrowsExceptionIfOrganisationIdNotProvided(): void
{
$this->expectExceptionMessage('Organisation ID is required');
$this->expectException(\Dvsa\Olcs\Api\Domain\Exception\BadRequestException::class);

$this->sut->handleQuery(Query::create([]));
}

public function testHandleQuery_ThrowsExceptionIfQueryIsNotInstanceOfOperatorAdminForOrganisationHasLoggedIn(): void
{
$this->expectExceptionMessage('Expected instance of: ' . Query::class);
$this->expectException(\Dvsa\Olcs\Api\Domain\Exception\BadRequestException::class);

$instance = new class() extends \stdClass implements QueryInterface {
public function exchangeArray(array $array)
{
// TODO: Implement exchangeArray() method.
}

public function getArrayCopy()
{
// TODO: Implement getArrayCopy() method.
}

public static function create(array $data)
{
// TODO: Implement create() method.
}
};

$this->sut->handleQuery($instance);
}
}
8 changes: 7 additions & 1 deletion app/api/test/module/Api/src/Domain/Repository/UserTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ public function testApplyListFilters(): void
[
'localAuthority' => 11,
'partnerContactDetails' => 22,
'organisation' => 43
'organisation' => 43,
'lastLoggedInFrom' => '2015-01-01',
]
);

Expand All @@ -91,6 +92,11 @@ public function testApplyListFilters(): void
->once();
$mockQb->shouldReceive('setParameter')->with('organisation', 43)->once();

$mockQb->shouldReceive('andWhere')->with('lastLoggedInFrom')->once()->andReturnSelf();
$mockQb->shouldReceive('expr->gte')->with('u.lastLoginAt', ':lastLoggedInFrom')->once()
->andReturn('lastLoggedInFrom');
$mockQb->shouldReceive('setParameter')->with('lastLoggedInFrom', '2015-01-01')->once();

$mockQb->shouldReceive('expr->neq')->with('u.id', ':systemUser')->once()->andReturn('systemUser');
$mockQb->shouldReceive('andWhere')->with('systemUser')->once()->andReturnSelf();
$mockQb->shouldReceive('setParameter')->with('systemUser', IdentityProviderInterface::SYSTEM_USER)->once();
Expand Down
Loading

0 comments on commit 47e31e4

Please sign in to comment.