From 3f3ac94d1acd0d1c1f6f8a104ef6addd93d69ca6 Mon Sep 17 00:00:00 2001 From: Piotr Rogowski Date: Mon, 15 Jun 2020 21:19:01 +0200 Subject: [PATCH 1/2] Implement anonymous access to organization --- config/packages/security.yaml | 4 + src/Controller/OrganizationController.php | 15 ++- src/Entity/Organization.php | 10 ++ .../ChangeAnonymousAccessType.php | 32 +++++ .../Organization/ChangeAnonymousAccess.php | 27 ++++ .../ChangeAnonymousAccessHandler.php | 28 ++++ src/Migrations/Version20200615181216.php | 37 ++++++ src/Query/User/Model/Organization.php | 10 +- .../DbalOrganizationQuery.php | 15 ++- src/Security/AnonymousUserAuthenticator.php | 77 +++++++++++ src/Security/OrganizationProvider.php | 32 ++++- src/Security/TokenAuthenticator.php | 5 + .../OrganizationAnonymousUserVoter.php | 59 +++++++++ .../Organization/OrganizationVoter.php | 22 ++++ templates/base.html.twig | 26 ++-- templates/organization/overview.html.twig | 56 ++++---- templates/organization/packages.html.twig | 120 +++++++++--------- templates/organization/settings.html.twig | 12 ++ .../Controller/OrganizationControllerTest.php | 64 ++++++++++ .../Controller/RepoControllerTest.php | 19 ++- tests/Integration/FixturesManager.php | 8 ++ .../MotherObject/Query/OrganizationMother.php | 6 +- .../Organization/OrganizationVoterTest.php | 8 +- 23 files changed, 583 insertions(+), 109 deletions(-) create mode 100644 src/Form/Type/Organization/ChangeAnonymousAccessType.php create mode 100644 src/Message/Organization/ChangeAnonymousAccess.php create mode 100644 src/MessageHandler/Organization/ChangeAnonymousAccessHandler.php create mode 100644 src/Migrations/Version20200615181216.php create mode 100644 src/Security/AnonymousUserAuthenticator.php create mode 100644 src/Service/Organization/OrganizationAnonymousUserVoter.php diff --git a/config/packages/security.yaml b/config/packages/security.yaml index bba38be8..a1e74674 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -20,6 +20,8 @@ security: guard: authenticators: - Buddy\Repman\Security\TokenAuthenticator + - Buddy\Repman\Security\AnonymousUserAuthenticator + entry_point: Buddy\Repman\Security\AnonymousUserAuthenticator main: anonymous: lazy provider: user_provider @@ -45,6 +47,8 @@ security: - { path: ^/user, roles: ROLE_USER } - { path: ^/organization/new, roles: ROLE_USER } - { path: ^/$, roles: ROLE_USER } + - { path: ^/organization/.+/overview$, roles: ROLE_ORGANIZATION_ANONYMOUS_USER } + - { path: ^/organization/.+/package$, roles: ROLE_ORGANIZATION_ANONYMOUS_USER } - { path: ^/organization/.+(/.+)*, roles: ROLE_ORGANIZATION_MEMBER } - { path: ^/downloads, host: '([a-z0-9_-]+)\.repo\.(.+)', roles: IS_AUTHENTICATED_ANONYMOUSLY} - { path: ^/, host: '([a-z0-9_-]+)\.repo\.(.+)', roles: ROLE_ORGANIZATION } diff --git a/src/Controller/OrganizationController.php b/src/Controller/OrganizationController.php index 6761a186..ad2b5e4f 100644 --- a/src/Controller/OrganizationController.php +++ b/src/Controller/OrganizationController.php @@ -5,10 +5,12 @@ namespace Buddy\Repman\Controller; use Buddy\Repman\Form\Type\Organization\ChangeAliasType; +use Buddy\Repman\Form\Type\Organization\ChangeAnonymousAccessType; use Buddy\Repman\Form\Type\Organization\ChangeNameType; use Buddy\Repman\Form\Type\Organization\CreateType; use Buddy\Repman\Form\Type\Organization\GenerateTokenType; use Buddy\Repman\Message\Organization\ChangeAlias; +use Buddy\Repman\Message\Organization\ChangeAnonymousAccess; use Buddy\Repman\Message\Organization\ChangeName; use Buddy\Repman\Message\Organization\CreateOrganization; use Buddy\Repman\Message\Organization\GenerateToken; @@ -97,7 +99,8 @@ public function overview(Organization $organization): Response public function packages(Organization $organization, Request $request): Response { $count = $this->packageQuery->count($organization->id()); - if ($count === 0 && $organization->isOwner($this->getUser()->id())) { + $user = parent::getUser(); + if ($count === 0 && $user instanceof User && $organization->isOwner($user->id())) { return $this->redirectToRoute('organization_package_new', ['organization' => $organization->alias()]); } @@ -287,10 +290,20 @@ public function settings(Organization $organization, Request $request): Response return $this->redirectToRoute('organization_settings', ['organization' => $aliasForm->get('alias')->getData()]); } + $anonymousAccessForm = $this->createForm(ChangeAnonymousAccessType::class, ['hasAnonymousAccess' => $organization->hasAnonymousAccess()]); + $anonymousAccessForm->handleRequest($request); + if ($anonymousAccessForm->isSubmitted() && $anonymousAccessForm->isValid()) { + $this->dispatchMessage(new ChangeAnonymousAccess($organization->id(), $anonymousAccessForm->get('hasAnonymousAccess')->getData())); + $this->addFlash('success', 'Anonymous access has been successfully changed.'); + + return $this->redirectToRoute('organization_settings', ['organization' => $organization->alias()]); + } + return $this->render('organization/settings.html.twig', [ 'organization' => $organization, 'renameForm' => $renameForm->createView(), 'aliasForm' => $aliasForm->createView(), + 'anonymousAccessForm' => $anonymousAccessForm->createView(), ]); } diff --git a/src/Entity/Organization.php b/src/Entity/Organization.php index d35dd2e0..49d1cd4d 100644 --- a/src/Entity/Organization.php +++ b/src/Entity/Organization.php @@ -65,6 +65,11 @@ class Organization */ private ?Collection $members = null; + /** + * @ORM\Column(type="boolean") + */ + private bool $hasAnonymousAccess = false; + public function __construct(UuidInterface $id, User $owner, string $name, string $alias) { $this->id = $id; @@ -246,6 +251,11 @@ public function members(): Collection return $this->members; } + public function changeAnonymousAccess(bool $hasAnonymousAccess): void + { + $this->hasAnonymousAccess = $hasAnonymousAccess; + } + private function isLastOwner(User $user): bool { $owners = $this->members->filter(fn (Member $member) => $member->isOwner()); diff --git a/src/Form/Type/Organization/ChangeAnonymousAccessType.php b/src/Form/Type/Organization/ChangeAnonymousAccessType.php new file mode 100644 index 00000000..322a0c10 --- /dev/null +++ b/src/Form/Type/Organization/ChangeAnonymousAccessType.php @@ -0,0 +1,32 @@ + $options + */ + public function buildForm(FormBuilderInterface $builder, array $options): void + { + $builder + ->add('hasAnonymousAccess', CheckboxType::class, [ + 'label' => 'Allow anonymous users', + 'required' => false, + ]) + ->add('changeAnonymousAccess', SubmitType::class, ['label' => 'Change']) + ; + } +} diff --git a/src/Message/Organization/ChangeAnonymousAccess.php b/src/Message/Organization/ChangeAnonymousAccess.php new file mode 100644 index 00000000..0ab6e890 --- /dev/null +++ b/src/Message/Organization/ChangeAnonymousAccess.php @@ -0,0 +1,27 @@ +organizationId = $organizationId; + $this->hasAnonymousAccess = $hasAnonymousAccess; + } + + public function organizationId(): string + { + return $this->organizationId; + } + + public function hasAnonymousAccess(): bool + { + return $this->hasAnonymousAccess; + } +} diff --git a/src/MessageHandler/Organization/ChangeAnonymousAccessHandler.php b/src/MessageHandler/Organization/ChangeAnonymousAccessHandler.php new file mode 100644 index 00000000..e0aa1aa6 --- /dev/null +++ b/src/MessageHandler/Organization/ChangeAnonymousAccessHandler.php @@ -0,0 +1,28 @@ +repositories = $repositories; + } + + public function __invoke(ChangeAnonymousAccess $message): void + { + $this->repositories + ->getById(Uuid::fromString($message->organizationId())) + ->changeAnonymousAccess($message->hasAnonymousAccess()) + ; + } +} diff --git a/src/Migrations/Version20200615181216.php b/src/Migrations/Version20200615181216.php new file mode 100644 index 00000000..19c290a5 --- /dev/null +++ b/src/Migrations/Version20200615181216.php @@ -0,0 +1,37 @@ +abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.'); + + $this->addSql('ALTER TABLE organization ADD has_anonymous_access BOOLEAN NOT NULL DEFAULT false'); + $this->addSql('ALTER TABLE "user" ALTER email_scan_result DROP DEFAULT'); + } + + public function down(Schema $schema): void + { + // this down() migration is auto-generated, please modify it to your needs + $this->abortIf($this->connection->getDatabasePlatform()->getName() !== 'postgresql', 'Migration can only be executed safely on \'postgresql\'.'); + + $this->addSql('ALTER TABLE organization DROP has_anonymous_access'); + $this->addSql('ALTER TABLE "user" ALTER email_scan_result SET DEFAULT \'true\''); + } +} diff --git a/src/Query/User/Model/Organization.php b/src/Query/User/Model/Organization.php index 91caa1c0..6afd2638 100644 --- a/src/Query/User/Model/Organization.php +++ b/src/Query/User/Model/Organization.php @@ -12,6 +12,8 @@ final class Organization private string $id; private string $name; private string $alias; + private bool $hasAnonymousAccess; + /** * @var Member[] */ @@ -22,12 +24,13 @@ final class Organization /** * @param Member[] $members */ - public function __construct(string $id, string $name, string $alias, array $members, ?string $token = null) + public function __construct(string $id, string $name, string $alias, array $members, bool $hasAnonymousAccess, ?string $token = null) { $this->id = $id; $this->name = $name; $this->alias = $alias; $this->members = array_map(fn (Member $member) => $member, $members); + $this->hasAnonymousAccess = $hasAnonymousAccess; $this->token = $token; } @@ -93,4 +96,9 @@ public function getMember(string $userId): Option return Option::none(); } + + public function hasAnonymousAccess(): bool + { + return $this->hasAnonymousAccess; + } } diff --git a/src/Query/User/OrganizationQuery/DbalOrganizationQuery.php b/src/Query/User/OrganizationQuery/DbalOrganizationQuery.php index a2882eae..3c9934f4 100644 --- a/src/Query/User/OrganizationQuery/DbalOrganizationQuery.php +++ b/src/Query/User/OrganizationQuery/DbalOrganizationQuery.php @@ -28,7 +28,8 @@ public function __construct(Connection $connection) public function getByAlias(string $alias): Option { $data = $this->connection->fetchAssoc( - 'SELECT id, name, alias FROM "organization" WHERE alias = :alias', [ + 'SELECT id, name, alias, has_anonymous_access + FROM "organization" WHERE alias = :alias', [ ':alias' => $alias, ]); @@ -41,8 +42,9 @@ public function getByAlias(string $alias): Option public function getByInvitation(string $token, string $email): Option { - $data = $this->connection->fetchAssoc('SELECT o.id, o.name, o.alias - FROM "organization" o + $data = $this->connection->fetchAssoc( + 'SELECT o.id, o.name, o.alias, o.has_anonymous_access + FROM "organization" o JOIN organization_invitation i ON o.id = i.organization_id WHERE i.token = :token AND i.email = :email ', [ @@ -70,8 +72,8 @@ public function findAllTokens(string $organizationId, int $limit = 20, int $offs $data['last_used_at'] !== null ? new \DateTimeImmutable($data['last_used_at']) : null ); }, $this->connection->fetchAll(' - SELECT name, value, created_at, last_used_at - FROM organization_token + SELECT name, value, created_at, last_used_at + FROM organization_token WHERE organization_id = :id ORDER BY UPPER(name) ASC LIMIT :limit OFFSET :offset', [ @@ -149,7 +151,7 @@ public function findAllMembers(string $organizationId, int $limit = 20, int $off $row['role'] ); }, $this->connection->fetchAll(' - SELECT u.id, u.email, m.role + SELECT u.id, u.email, m.role FROM organization_member AS m JOIN "user" u ON u.id = m.user_id WHERE m.organization_id = :id @@ -214,6 +216,7 @@ private function hydrateOrganization(array $data): Organization $data['name'], $data['alias'], array_map(fn (array $row) => new Member($row['user_id'], $row['email'], $row['role']), $members), + $data['has_anonymous_access'], $token !== false ? $token : null ); } diff --git a/src/Security/AnonymousUserAuthenticator.php b/src/Security/AnonymousUserAuthenticator.php new file mode 100644 index 00000000..7392594f --- /dev/null +++ b/src/Security/AnonymousUserAuthenticator.php @@ -0,0 +1,77 @@ + 'Authentication Required', + ], Response::HTTP_UNAUTHORIZED); + } + + public function supports(Request $request) + { + return $request->get('_route') !== 'repo_package_downloads' + && !$request->headers->has('PHP_AUTH_USER') + && !$request->headers->has('PHP_AUTH_PW'); + } + + public function getCredentials(Request $request) + { + $organizationAlias = $request->get('organization'); + if ($organizationAlias === null) { + throw new BadCredentialsException(); + } + + return $organizationAlias; + } + + public function getUser($credentials, UserProviderInterface $userProvider) + { + return $userProvider->loadUserByUsername('alias:'.$credentials); + } + + public function checkCredentials($credentials, UserInterface $user) + { + return true; + } + + public function onAuthenticationFailure(Request $request, AuthenticationException $exception): Response + { + return new JsonResponse([ + 'message' => strtr($exception->getMessageKey(), $exception->getMessageData()), + ], Response::HTTP_FORBIDDEN); + } + + public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $providerKey) + { + return null; + } + + /** + * @codeCoverageIgnore + */ + public function supportsRememberMe(): bool + { + return false; + } +} diff --git a/src/Security/OrganizationProvider.php b/src/Security/OrganizationProvider.php index 01aebe25..3bc6f832 100644 --- a/src/Security/OrganizationProvider.php +++ b/src/Security/OrganizationProvider.php @@ -22,6 +22,10 @@ public function __construct(Connection $connection) public function loadUserByUsername(string $username) { + if (strpos($username, 'alias:') === 0) { + return $this->loadUserByAlias(\str_replace('alias:', '', $username)); + } + $data = $this->getUserDataByToken($username); if ($data === false) { @@ -57,20 +61,44 @@ private function updateLastUsed(string $token): void ]); } + private function loadUserByAlias(string $alias): Organization + { + $data = $this->getUserDataByAlias($alias); + if ($data === false) { + throw new BadCredentialsException(); + } + + return $this->hydrateOrganization($data); + } + /** * @return false|mixed[] */ private function getUserDataByToken(string $token) { return $this->connection->fetchAssoc(' - SELECT t.value, o.name, o.alias, o.id FROM organization_token t - JOIN organization o ON o.id = t.organization_id + SELECT t.value, o.name, o.alias, o.id FROM organization_token t + JOIN organization o ON o.id = t.organization_id WHERE t.value = :token', [ ':token' => $token, ]); } + /** + * @return false|mixed[] + */ + private function getUserDataByAlias(string $alias) + { + return $this->connection->fetchAssoc(" + SELECT id, name, alias, 'anonymous' AS value + FROM organization + WHERE alias = :alias AND has_anonymous_access = true", + [ + ':alias' => $alias, + ]); + } + /** * @param mixed[] $data */ diff --git a/src/Security/TokenAuthenticator.php b/src/Security/TokenAuthenticator.php index 54388efb..e9a89dff 100644 --- a/src/Security/TokenAuthenticator.php +++ b/src/Security/TokenAuthenticator.php @@ -15,6 +15,11 @@ final class TokenAuthenticator extends AbstractGuardAuthenticator { + /** + * @codeCoverageIgnore + * + * @return Response + */ public function start(Request $request, AuthenticationException $authException = null) { $data = [ diff --git a/src/Service/Organization/OrganizationAnonymousUserVoter.php b/src/Service/Organization/OrganizationAnonymousUserVoter.php new file mode 100644 index 00000000..58a232ad --- /dev/null +++ b/src/Service/Organization/OrganizationAnonymousUserVoter.php @@ -0,0 +1,59 @@ +organizations = $organizations; + } + + protected function supports(string $attribute, $subject): bool + { + return in_array($attribute, [ + 'ROLE_ORGANIZATION_ANONYMOUS_USER', + ], true); + } + + /** + * @param mixed $subject + * @param mixed[] $attributes + */ + public function vote(TokenInterface $token, $subject, array $attributes): int + { + $user = $token->getUser(); + if ($user instanceof User) { + return self::ACCESS_ABSTAIN; + } + + return parent::vote($token, $subject, $attributes); + } + + /** + * @param mixed|Request $subject + */ + protected function voteOnAttribute(string $attribute, $subject, TokenInterface $token): bool + { + $organization = $subject instanceof Request + ? $this->organizations->getByAlias($subject->get('organization'))->get() + : $subject; + + if ($organization instanceof Organization) { + return $organization->hasAnonymousAccess(); + } + + return false; + } +} diff --git a/src/Service/Organization/OrganizationVoter.php b/src/Service/Organization/OrganizationVoter.php index 901a9495..b594b43f 100644 --- a/src/Service/Organization/OrganizationVoter.php +++ b/src/Service/Organization/OrganizationVoter.php @@ -5,6 +5,7 @@ namespace Buddy\Repman\Service\Organization; use Buddy\Repman\Query\User\Model\Organization; +use Buddy\Repman\Query\User\OrganizationQuery; use Buddy\Repman\Security\Model\User; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; @@ -12,14 +13,35 @@ final class OrganizationVoter extends Voter { + private OrganizationQuery $organizations; + + public function __construct(OrganizationQuery $organizations) + { + $this->organizations = $organizations; + } + protected function supports(string $attribute, $subject): bool { return in_array($attribute, [ 'ROLE_ORGANIZATION_MEMBER', 'ROLE_ORGANIZATION_OWNER', + 'ROLE_ORGANIZATION_ANONYMOUS_USER', ], true); } + /** + * @param mixed $subject + * @param mixed[] $attributes + */ + public function vote(TokenInterface $token, $subject, array $attributes): int + { + if (!$token->getUser() instanceof User) { + return self::ACCESS_ABSTAIN; + } + + return parent::vote($token, $subject, $attributes); + } + /** * @param mixed|Request $subject */ diff --git a/templates/base.html.twig b/templates/base.html.twig index db6fdf30..af9d1c1a 100644 --- a/templates/base.html.twig +++ b/templates/base.html.twig @@ -57,18 +57,20 @@ Packages -
  • - - {% include 'svg/lock.svg' %} - Tokens - -
    • -
  • - - {% include 'svg/users.svg' %} - Members - -
    • + {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} +
  • + + {% include 'svg/lock.svg' %} + Tokens + +
    • +
  • + + {% include 'svg/users.svg' %} + Members + +
    • + {% endif %} {% if is_granted('ROLE_ORGANIZATION_OWNER', organization) %}
  • diff --git a/templates/organization/overview.html.twig b/templates/organization/overview.html.twig index f674ed98..ca0f0b9f 100644 --- a/templates/organization/overview.html.twig +++ b/templates/organization/overview.html.twig @@ -3,9 +3,11 @@ {% block header %}{{ organization.name }} overview: {% endblock %} {% block header_btn %} - - {% include 'svg/bar-chart.svg' %} Package stats - + {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + + {% include 'svg/bar-chart.svg' %} Package stats + + {% endif %} {% endblock %} {% block token %} @@ -17,17 +19,11 @@ {% endif %} {% endblock %} -{% block content %} -
    - {% if organization.token %} -

    Configure global authentication to access this organization's packages:

    -
    - 
    composer config --global --auth http-basic.{{ url('organization_repo_url', {organization: organization.alias}, true) | trim('/') }} token {{ block('token') | spaceless }}
    -
    -

    - Then, add these lines to your composer.json file, or add a new repository URL if you already have one or more: -

    -
    +{% block repositories %} +

    + Add these lines to your composer.json file, or add a new repository URL if you already have one or more: +

    +
     {
     "repositories": [
@@ -35,16 +31,30 @@
     ]
 }
    -
    -

    - You can also use the COMPOSER_AUTH environment variable to authenticate Composer: -

    - 
    COMPOSER_AUTH={"http-basic": {"{{ url('organization_repo_url', {organization: organization.alias}, true) | trim('/') }}": {"username": "token", "password": "{{ block('token') | spaceless }}"}}}
    +
    +{% endblock %} + +{% block content %} +
    + {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + {% if organization.token %} +

    Configure global authentication to access this organization's packages:

    +
    + 
    composer config --global --auth http-basic.{{ url('organization_repo_url', {organization: organization.alias}, true) | trim('/') }} token {{ block('token') | spaceless }}
    +
    + {{ block('repositories') }} +

    + You can also use the COMPOSER_AUTH environment variable to authenticate Composer: +

    + 
    COMPOSER_AUTH={"http-basic": {"{{ url('organization_repo_url', {organization: organization.alias}, true) | trim('/') }}": {"username": "token", "password": "{{ block('token') | spaceless }}"}}}
    + {% else %} + Missing authentication tokens, + + generate new token + to allow users to access your organization packages + {% endif %} {% else %} - Missing authentication tokens, - - generate new token - to allow users to access your organization packages + {{ block('repositories') }} {% endif %}
    {% endblock %} diff --git a/templates/organization/packages.html.twig b/templates/organization/packages.html.twig index 2b996dad..cbf9e4cb 100644 --- a/templates/organization/packages.html.twig +++ b/templates/organization/packages.html.twig @@ -20,8 +20,10 @@ Released Description Webhook - Security - + {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + Security + + {% endif %} @@ -67,76 +69,78 @@ N/A {% endif %} - - {% if package.isSynchronizedSuccessfully %} - - {% if package.isScanResultOk %} - {{ package.scanResultStatus }} - {% elseif package.isScanResultPending or package.isScanResultNotAvailable %} - {{ package.scanResultStatus }} - {% else %} - {{ package.scanResultStatus }} - {% endif %} - - {% endif %} - - - - -
    - {% if package.name %} - - {% include 'svg/bar-chart.svg' %} Statistics - - - {% include 'svg/link.svg' %} Webhook - - {% endif %} + {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + {% if package.isSynchronizedSuccessfully %} - {% include 'svg/shield.svg' %} Scan + href="{{ path('organization_package_scan_results', {organization: organization.alias, package: package.id}) }}" + data-content="{% apply escape %}{% include 'organization/package/scanResultContent.html.twig' with {'result': package.lastScanResultContent, 'simple': true} %}{% endapply %}" + data-title="{{ package.scanResultDate|time_diff }}" + data-toggle="scan-result-popover" + > + {% if package.isScanResultOk %} + {{ package.scanResultStatus }} + {% elseif package.isScanResultPending or package.isScanResultNotAvailable %} + {{ package.scanResultStatus }} + {% else %} + {{ package.scanResultStatus }} + {% endif %} {% endif %} - {% if package.name or package.lastSyncError %} - - {% include 'svg/refresh.svg' %} Update - - {% if is_granted('ROLE_ORGANIZATION_OWNER', organization) %} + + + + +
    + {% if package.name %} + + {% include 'svg/bar-chart.svg' %} Statistics + + + {% include 'svg/link.svg' %} Webhook + + {% endif %} + {% if package.isSynchronizedSuccessfully %} - {% include 'svg/trash.svg' %} Remove + {% include 'svg/shield.svg' %} Scan {% endif %} - {% endif %} -
    -     - + {% if package.name or package.lastSyncError %} + + {% include 'svg/refresh.svg' %} Update + + {% if is_granted('ROLE_ORGANIZATION_OWNER', organization) %} + + {% include 'svg/trash.svg' %} Remove + + {% endif %} + {% endif %} +
    +     + + {% endif %} {% else %} - no packages added + no packages added {% endfor %} diff --git a/templates/organization/settings.html.twig b/templates/organization/settings.html.twig index 7e5924ee..393f1cad 100644 --- a/templates/organization/settings.html.twig +++ b/templates/organization/settings.html.twig @@ -32,6 +32,18 @@ {{ form_end(aliasForm) }}
    +

    Anonymous access

    + {{ form_start(anonymousAccessForm, {attr:{class:'row'}}) }} +
    + {{ form_widget(anonymousAccessForm.hasAnonymousAccess) }} + {{ form_errors(anonymousAccessForm.hasAnonymousAccess) }} +
    +
    + {{ form_widget(anonymousAccessForm.changeAnonymousAccess, {attr:{class:'btn-danger'}}) }} +
    + {{ form_end(anonymousAccessForm) }} +
    +

    Delete this organization

    diff --git a/tests/Functional/Controller/OrganizationControllerTest.php b/tests/Functional/Controller/OrganizationControllerTest.php index ba88828a..beeb8000 100644 --- a/tests/Functional/Controller/OrganizationControllerTest.php +++ b/tests/Functional/Controller/OrganizationControllerTest.php @@ -453,6 +453,20 @@ public function testChangeAlias(): void self::assertStringContainsString('Organization alias has been successfully changed.', $this->lastResponseBody()); } + public function testChangeAnonymousAccess(): void + { + $this->fixtures->createOrganization('buddy', $this->userId); + $this->client->followRedirects(); + $this->client->request('GET', $this->urlTo('organization_settings', ['organization' => 'buddy'])); + $this->client->submitForm('changeAnonymousAccess', [ + 'hasAnonymousAccess' => true, + ]); + + self::assertTrue($this->client->getResponse()->isOk()); + self::assertStringContainsString('repman', $this->lastResponseBody()); + self::assertStringContainsString('Anonymous access has been successfully changed.', $this->lastResponseBody()); + } + public function testRemoveOrganization(): void { $buddyId = $this->fixtures->createOrganization('buddy inc', $this->userId); @@ -664,4 +678,54 @@ public function testPackageScanResultsWithNaStatus(): void self::assertStringContainsString('n/a', $this->lastResponseBody()); self::assertStringContainsString('composer.lock not present', $this->lastResponseBody()); } + + public function testOverviewAllowedForAnonymousUser(): void + { + $otherId = $this->fixtures->createAdmin('cto@buddy.works', 'strong'); + $organizationId = $this->fixtures->createOrganization('public', $otherId); + + $this->fixtures->enableAnonymousUserAccess($organizationId); + + if (static::$booted) { + self::ensureKernelShutdown(); + } + $this->client = static::createClient(); + + $this->client->request('GET', $this->urlTo('organization_overview', ['organization' => 'public'])); + + self::assertTrue($this->client->getResponse()->isOk()); + } + + public function testPackagesAllowedForAnonymousUser(): void + { + $otherId = $this->fixtures->createAdmin('cto@buddy.works', 'strong'); + $organizationId = $this->fixtures->createOrganization('public', $otherId); + + $this->fixtures->enableAnonymousUserAccess($organizationId); + + if (static::$booted) { + self::ensureKernelShutdown(); + } + $this->client = static::createClient(); + + $this->client->request('GET', $this->urlTo('organization_packages', ['organization' => 'public'])); + + self::assertTrue($this->client->getResponse()->isOk()); + } + + public function testTokensNotAllowedForAnonymousUser(): void + { + $otherId = $this->fixtures->createAdmin('cto@buddy.works', 'strong'); + $organizationId = $this->fixtures->createOrganization('public', $otherId); + + $this->fixtures->enableAnonymousUserAccess($organizationId); + + if (static::$booted) { + self::ensureKernelShutdown(); + } + $this->client = static::createClient(); + $this->client->request('GET', $this->urlTo('organization_tokens', ['organization' => 'public'])); + + self::assertTrue($this->client->getResponse()->isRedirect($this->urlTo('app_login'))); + } } diff --git a/tests/Functional/Controller/RepoControllerTest.php b/tests/Functional/Controller/RepoControllerTest.php index 3882d3b2..625a8593 100644 --- a/tests/Functional/Controller/RepoControllerTest.php +++ b/tests/Functional/Controller/RepoControllerTest.php @@ -10,11 +10,18 @@ final class RepoControllerTest extends FunctionalTestCase { + public function testAuthRequired(): void + { + $this->client->request('GET', '/', [], [], ['HTTP_HOST' => 'buddy.repo.repman.wip']); + + self::assertEquals(Response::HTTP_FORBIDDEN, $this->client->getResponse()->getStatusCode()); + } + public function testAuthRequiredForOrganizationRepo(): void { $this->client->request('GET', '/packages.json', [], [], ['HTTP_HOST' => 'buddy.repo.repman.wip']); - self::assertEquals(Response::HTTP_UNAUTHORIZED, $this->client->getResponse()->getStatusCode()); + self::assertEquals(Response::HTTP_FORBIDDEN, $this->client->getResponse()->getStatusCode()); } public function testPackagesActionWithInvalidToken(): void @@ -143,4 +150,14 @@ public function testOrganizationTrackDownloads(): void self::assertEquals(Response::HTTP_BAD_REQUEST, $this->client->getResponse()->getStatusCode()); } + + public function testAnonymousUserAccess(): void + { + $organizationId = $this->fixtures->createOrganization('buddy', $this->fixtures->createUser()); + $this->fixtures->enableAnonymousUserAccess($organizationId); + + $this->client->request('GET', '/packages.json', [], [], ['HTTP_HOST' => 'buddy.repo.repman.wip']); + + self::assertTrue($this->client->getResponse()->isOk()); + } } diff --git a/tests/Integration/FixturesManager.php b/tests/Integration/FixturesManager.php index 7d39cef1..798359a6 100644 --- a/tests/Integration/FixturesManager.php +++ b/tests/Integration/FixturesManager.php @@ -20,6 +20,7 @@ use Buddy\Repman\Message\User\CreateUser; use Buddy\Repman\Message\User\DisableUser; use Buddy\Repman\MessageHandler\Proxy\AddDownloadsHandler; +use Buddy\Repman\Repository\OrganizationRepository; use Buddy\Repman\Repository\PackageRepository; use Buddy\Repman\Repository\ScanResultRepository; use Buddy\Repman\Service\Organization\TokenGenerator; @@ -245,6 +246,13 @@ public function confirmUserEmail(string $token): void $this->container->get(EntityManagerInterface::class)->flush(); } + public function enableAnonymousUserAccess(string $organizationId): void + { + $organization = $this->container->get(OrganizationRepository::class)->getById(Uuid::fromString($organizationId)); + $organization->changeAnonymousAccess(true); + $this->container->get('doctrine.orm.entity_manager')->flush($organization); + } + private function dispatchMessage(object $message): void { $this->container->get(MessageBusInterface::class)->dispatch($message); diff --git a/tests/MotherObject/Query/OrganizationMother.php b/tests/MotherObject/Query/OrganizationMother.php index 71abea0a..f9f222f2 100644 --- a/tests/MotherObject/Query/OrganizationMother.php +++ b/tests/MotherObject/Query/OrganizationMother.php @@ -15,7 +15,8 @@ public static function some(): Organization '448e8c58-dd52-4db6-92bd-ab102d5273de', 'Repman', 'repman', - [new Organization\Member('5c1b8e35-fe7b-4418-b722-ec9cbbf2598a', 'test@repman.io', 'owner')] + [new Organization\Member('5c1b8e35-fe7b-4418-b722-ec9cbbf2598a', 'test@repman.io', 'owner')], + false ); } @@ -25,7 +26,8 @@ public static function withMember(Member $member): Organization '448e8c58-dd52-4db6-92bd-ab102d5273de', 'Repman', 'repman', - [$member] + [$member], + false ); } } diff --git a/tests/Unit/Service/Organization/OrganizationVoterTest.php b/tests/Unit/Service/Organization/OrganizationVoterTest.php index 96af39e6..e60fe4d0 100644 --- a/tests/Unit/Service/Organization/OrganizationVoterTest.php +++ b/tests/Unit/Service/Organization/OrganizationVoterTest.php @@ -5,6 +5,7 @@ namespace Buddy\Repman\Tests\Unit\Service\Organization; use Buddy\Repman\Query\User\Model\Organization\Member; +use Buddy\Repman\Query\User\OrganizationQuery; use Buddy\Repman\Security\Model\User; use Buddy\Repman\Service\Organization\OrganizationVoter; use Buddy\Repman\Tests\MotherObject\Query\OrganizationMother; @@ -28,12 +29,13 @@ protected function setUp(): void new User\Organization('repman', 'name', 'owner'), new User\Organization('buddy', 'name', 'member'), ]), 'password', 'key'); - $this->voter = new OrganizationVoter(); + $queryMock = $this->getMockBuilder(OrganizationQuery::class)->getMock(); + $this->voter = new OrganizationVoter($queryMock); } - public function testSupportOnlyUserInstance(): void + public function testAbstainForAnonymousUser(): void { - self::assertEquals(Voter::ACCESS_DENIED, $this->voter->vote( + self::assertEquals(Voter::ACCESS_ABSTAIN, $this->voter->vote( new AnonymousToken('secret', 'anon', []), 'any', ['ROLE_ORGANIZATION_MEMBER'] From 6b102c9b3c7cc4933c88ff28375a6d5ce0ed42a5 Mon Sep 17 00:00:00 2001 From: Piotr Rogowski Date: Fri, 19 Jun 2020 11:19:36 +0200 Subject: [PATCH 2/2] Code review fixes --- config/packages/security.yaml | 4 ++-- ...nonymousOrganizationUserAuthenticator.php} | 8 +++++-- src/Security/Model/User/Organization.php | 9 +++++++- src/Security/OrganizationProvider.php | 22 ++++++++----------- src/Security/UserProvider.php | 4 ++-- templates/component/js/base.js | 4 ++++ templates/component/user.html.twig | 14 ++++++++++-- templates/organization/packages.html.twig | 16 +++++++------- templates/organization/settings.html.twig | 3 +++ templates/svg/unlock.svg | 1 + .../Organization/OrganizationVoterTest.php | 4 ++-- 11 files changed, 57 insertions(+), 32 deletions(-) rename src/Security/{AnonymousUserAuthenticator.php => AnonymousOrganizationUserAuthenticator.php} (88%) create mode 100644 templates/svg/unlock.svg diff --git a/config/packages/security.yaml b/config/packages/security.yaml index a1e74674..d6a3179d 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -20,8 +20,8 @@ security: guard: authenticators: - Buddy\Repman\Security\TokenAuthenticator - - Buddy\Repman\Security\AnonymousUserAuthenticator - entry_point: Buddy\Repman\Security\AnonymousUserAuthenticator + - Buddy\Repman\Security\AnonymousOrganizationUserAuthenticator + entry_point: Buddy\Repman\Security\AnonymousOrganizationUserAuthenticator main: anonymous: lazy provider: user_provider diff --git a/src/Security/AnonymousUserAuthenticator.php b/src/Security/AnonymousOrganizationUserAuthenticator.php similarity index 88% rename from src/Security/AnonymousUserAuthenticator.php rename to src/Security/AnonymousOrganizationUserAuthenticator.php index 7392594f..735dbeb7 100644 --- a/src/Security/AnonymousUserAuthenticator.php +++ b/src/Security/AnonymousOrganizationUserAuthenticator.php @@ -14,7 +14,7 @@ use Symfony\Component\Security\Core\User\UserProviderInterface; use Symfony\Component\Security\Guard\AbstractGuardAuthenticator; -final class AnonymousUserAuthenticator extends AbstractGuardAuthenticator +final class AnonymousOrganizationUserAuthenticator extends AbstractGuardAuthenticator { /** * @codeCoverageIgnore @@ -47,7 +47,11 @@ public function getCredentials(Request $request) public function getUser($credentials, UserProviderInterface $userProvider) { - return $userProvider->loadUserByUsername('alias:'.$credentials); + if (!$userProvider instanceof OrganizationProvider) { + throw new \InvalidArgumentException(); + } + + return $userProvider->loadUserByAlias($credentials); } public function checkCredentials($credentials, UserInterface $user) diff --git a/src/Security/Model/User/Organization.php b/src/Security/Model/User/Organization.php index f7c80cec..f3b802f2 100644 --- a/src/Security/Model/User/Organization.php +++ b/src/Security/Model/User/Organization.php @@ -9,12 +9,14 @@ final class Organization private string $alias; private string $name; private string $role; + private bool $hasAnonymousAccess; - public function __construct(string $alias, string $name, string $role) + public function __construct(string $alias, string $name, string $role, bool $hasAnonymousAccess) { $this->alias = $alias; $this->name = $name; $this->role = $role; + $this->hasAnonymousAccess = $hasAnonymousAccess; } public function alias(): string @@ -31,4 +33,9 @@ public function role(): string { return $this->role; } + + public function hasAnonymousAccess(): bool + { + return $this->hasAnonymousAccess; + } } diff --git a/src/Security/OrganizationProvider.php b/src/Security/OrganizationProvider.php index 3bc6f832..c5074aa7 100644 --- a/src/Security/OrganizationProvider.php +++ b/src/Security/OrganizationProvider.php @@ -22,10 +22,6 @@ public function __construct(Connection $connection) public function loadUserByUsername(string $username) { - if (strpos($username, 'alias:') === 0) { - return $this->loadUserByAlias(\str_replace('alias:', '', $username)); - } - $data = $this->getUserDataByToken($username); if ($data === false) { @@ -53,15 +49,7 @@ public function supportsClass(string $class) return $class === Organization::class; } - private function updateLastUsed(string $token): void - { - $this->connection->executeQuery('UPDATE organization_token SET last_used_at = :now WHERE value = :value', [ - ':now' => (new \DateTimeImmutable())->format('Y-m-d H:i:s'), - ':value' => $token, - ]); - } - - private function loadUserByAlias(string $alias): Organization + public function loadUserByAlias(string $alias): Organization { $data = $this->getUserDataByAlias($alias); if ($data === false) { @@ -71,6 +59,14 @@ private function loadUserByAlias(string $alias): Organization return $this->hydrateOrganization($data); } + private function updateLastUsed(string $token): void + { + $this->connection->executeQuery('UPDATE organization_token SET last_used_at = :now WHERE value = :value', [ + ':now' => (new \DateTimeImmutable())->format('Y-m-d H:i:s'), + ':value' => $token, + ]); + } + /** * @return false|mixed[] */ diff --git a/src/Security/UserProvider.php b/src/Security/UserProvider.php index 6679cf3b..9677d288 100644 --- a/src/Security/UserProvider.php +++ b/src/Security/UserProvider.php @@ -73,7 +73,7 @@ private function getUserDataByEmail(string $email) private function hydrateUser(array $data): User { $organizations = $this->connection->fetchAll(' - SELECT o.name, o.alias, om.role FROM organization_member om + SELECT o.name, o.alias, om.role, o.has_anonymous_access FROM organization_member om JOIN organization o ON o.id = om.organization_id WHERE om.user_id = :userId ORDER BY o.name ', ['userId' => $data['id']]); @@ -86,7 +86,7 @@ private function hydrateUser(array $data): User $data['email_confirmed_at'] !== null, $data['email_confirm_token'], json_decode($data['roles'], true), - array_map(fn (array $data) => new User\Organization($data['alias'], $data['name'], $data['role']), $organizations), + array_map(fn (array $data) => new User\Organization($data['alias'], $data['name'], $data['role'], $data['has_anonymous_access']), $organizations), $data['email_scan_result'], ); } diff --git a/templates/component/js/base.js b/templates/component/js/base.js index 44855662..c9fdefe5 100644 --- a/templates/component/js/base.js +++ b/templates/component/js/base.js @@ -22,4 +22,8 @@ $(this).parent().find('[data-type="token"]').removeClass('d-none'); $(this).remove(); }); + + $('[data-toggle="tooltip"]').popover({ + trigger: 'hover' + }); })(); diff --git a/templates/component/user.html.twig b/templates/component/user.html.twig index 1a7837a4..89e3dc08 100644 --- a/templates/component/user.html.twig +++ b/templates/component/user.html.twig @@ -1,3 +1,13 @@ +{% macro anonymousIcon(subject) %} + {% if subject.hasAnonymousAccess %} + + {% include 'svg/unlock.svg' %} + + {% endif %} +{% endmacro %} + +{% import _self as icons %} + {% if app.user %} {% if app.user.belongsToAnyOrganization %}
    @@ -5,7 +15,7 @@ class="align-items-center ml-2 btn btn-secondary"> {% if organization is defined %} - {{ organization.name }} + {{ icons.anonymousIcon(organization) }} {{ organization.name }} {% else %} Select organization {% endif %} @@ -17,7 +27,7 @@
    {% for org in app.user.organizations %} - {{ org.name }} + {{ icons.anonymousIcon(org) }} {{ org.name }} {% endfor %}
    diff --git a/templates/organization/packages.html.twig b/templates/organization/packages.html.twig index cbf9e4cb..8e2b2767 100644 --- a/templates/organization/packages.html.twig +++ b/templates/organization/packages.html.twig @@ -19,8 +19,8 @@ Version Released Description - Webhook {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + Webhook Security {% endif %} @@ -62,14 +62,14 @@ {{ package.description }} {% endif %} - - {% if package.webhookCreatedAt is not empty %} - created - {% else %} - N/A - {% endif %} - {% if is_granted('ROLE_ORGANIZATION_MEMBER', organization) %} + + {% if package.webhookCreatedAt is not empty %} + created + {% else %} + N/A + {% endif %} + {% if package.isSynchronizedSuccessfully %}

    Anonymous access

    +

    + Enabling anonymous access allows to browse and download all organization packages. +

    {{ form_start(anonymousAccessForm, {attr:{class:'row'}}) }}
    {{ form_widget(anonymousAccessForm.hasAnonymousAccess) }} diff --git a/templates/svg/unlock.svg b/templates/svg/unlock.svg new file mode 100644 index 00000000..d5e54d23 --- /dev/null +++ b/templates/svg/unlock.svg @@ -0,0 +1 @@ + diff --git a/tests/Unit/Service/Organization/OrganizationVoterTest.php b/tests/Unit/Service/Organization/OrganizationVoterTest.php index e60fe4d0..143c1766 100644 --- a/tests/Unit/Service/Organization/OrganizationVoterTest.php +++ b/tests/Unit/Service/Organization/OrganizationVoterTest.php @@ -26,8 +26,8 @@ protected function setUp(): void { $this->userId = 'some-id'; $this->token = new UsernamePasswordToken(UserMother::withOrganizations($this->userId, [ - new User\Organization('repman', 'name', 'owner'), - new User\Organization('buddy', 'name', 'member'), + new User\Organization('repman', 'name', 'owner', false), + new User\Organization('buddy', 'name', 'member', false), ]), 'password', 'key'); $queryMock = $this->getMockBuilder(OrganizationQuery::class)->getMock(); $this->voter = new OrganizationVoter($queryMock);