Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce org setting enable_security_scan #591

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/Controller/Organization/PackageController.php
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ private function packageNewFromUrl(string $label, FormInterface $form, Organizat
$form->get('url')->getData(),
in_array($type, ['git', 'mercurial', 'subversion'], true) ? 'vcs' : $type,
[],
$form->get('keepLastReleases')->getData()
$form->get('keepLastReleases')->getData(),
$organization->isSecurityScanEnabled()
));
$this->messageBus->dispatch(new SynchronizePackage($id));

Expand Down Expand Up @@ -238,7 +239,8 @@ private function packageNewFromGitHub(FormInterface $form, Organization $organiz
"https://github.com/{$repo}",
'github-oauth',
[Metadata::GITHUB_REPO_NAME => $repo],
$form->get('keepLastReleases')->getData()
$form->get('keepLastReleases')->getData(),
$organization->isSecurityScanEnabled()
));
$this->messageBus->dispatch(new SynchronizePackage($id));
$this->messageBus->dispatch(new AddGitHubHook($id));
Expand Down Expand Up @@ -269,7 +271,8 @@ private function packageNewFromGitLab(FormInterface $form, Organization $organiz
$projects->get($projectId)->url(),
'gitlab-oauth',
[Metadata::GITLAB_PROJECT_ID => $projectId],
$form->get('keepLastReleases')->getData()
$form->get('keepLastReleases')->getData(),
$organization->isSecurityScanEnabled()
));
$this->messageBus->dispatch(new SynchronizePackage($id));
$this->messageBus->dispatch(new AddGitLabHook($id));
Expand Down Expand Up @@ -300,7 +303,8 @@ private function packageNewFromBitbucket(FormInterface $form, Organization $orga
$repos->get($repoUuid)->url(),
'bitbucket-oauth',
[Metadata::BITBUCKET_REPO_NAME => $repos->get($repoUuid)->name()],
$form->get('keepLastReleases')->getData()
$form->get('keepLastReleases')->getData(),
$organization->isSecurityScanEnabled()
));
$this->messageBus->dispatch(new SynchronizePackage($id));
$this->messageBus->dispatch(new AddBitbucketHook($id));
Expand Down
12 changes: 12 additions & 0 deletions src/Controller/OrganizationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
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\EnableSecurityScanType;
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\ChangeSecurityScanConfiguration;
use Buddy\Repman\Message\Organization\GenerateToken;
use Buddy\Repman\Message\Organization\Package\AddBitbucketHook;
use Buddy\Repman\Message\Organization\Package\AddGitHubHook;
Expand Down Expand Up @@ -290,11 +292,21 @@ public function settings(Organization $organization, Request $request): Response
return $this->redirectToRoute('organization_settings', ['organization' => $organization->alias()]);
}

$enableSecurityScanForm = $this->createForm(EnableSecurityScanType::class, ['isSecurityScanEnabled' => $organization->isSecurityScanEnabled()]);
$enableSecurityScanForm->handleRequest($request);
if ($enableSecurityScanForm->isSubmitted() && $enableSecurityScanForm->isValid()) {
$this->messageBus->dispatch(new ChangeSecurityScanConfiguration($organization->id(), $enableSecurityScanForm->get('isSecurityScanEnabled')->getData()));
$this->addFlash('success', 'Default package security scans have 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(),
'enableSecurityScanForm' => $enableSecurityScanForm->createView(),
]);
}

Expand Down
10 changes: 10 additions & 0 deletions src/Entity/Organization.php
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,11 @@ class Organization
*/
private bool $hasAnonymousAccess = false;

/**
* @ORM\Column(type="boolean", options={"default": true})
*/
private bool $enable_security_scan = true;
shochdoerfer marked this conversation as resolved.
Show resolved Hide resolved

public function __construct(UuidInterface $id, User $owner, string $name, string $alias)
{
$this->id = $id;
Expand Down Expand Up @@ -256,6 +261,11 @@ public function changeAnonymousAccess(bool $hasAnonymousAccess): void
$this->hasAnonymousAccess = $hasAnonymousAccess;
}

public function enableSecurityScan(bool $enableSecurityScan): void
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
public function enableSecurityScan(bool $enableSecurityScan): void
public function changeSecurityScanAvailability(bool $enabled): void

?

{
$this->enable_security_scan = $enableSecurityScan;
}

private function isLastOwner(User $user): bool
{
$owners = $this->members->filter(fn (Member $member) => $member->isOwner());
Expand Down
36 changes: 36 additions & 0 deletions src/Form/Type/Organization/EnableSecurityScanType.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?php

declare(strict_types=1);

namespace Buddy\Repman\Form\Type\Organization;

use Symfony\Component\Form\AbstractType;
use Symfony\Component\Form\Extension\Core\Type\ChoiceType;
use Symfony\Component\Form\Extension\Core\Type\SubmitType;
use Symfony\Component\Form\FormBuilderInterface;

class EnableSecurityScanType extends AbstractType
shochdoerfer marked this conversation as resolved.
Show resolved Hide resolved
{
public function getBlockPrefix(): string
{
return '';
}

/**
* @param array<mixed> $options
*/
public function buildForm(FormBuilderInterface $builder, array $options): void
{
$builder
->add('isSecurityScanEnabled', ChoiceType::class, [
'choices' => [
'Enable' => true,
'Disable' => false,
],
'label' => 'Enable security scan for new packages',
'required' => true,
])
->add('enableSecurityScan', SubmitType::class, ['label' => 'Change'])
;
}
}
9 changes: 8 additions & 1 deletion src/Message/Organization/AddPackage.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ final class AddPackage
private string $type;
private string $organizationId;
private int $keepLastReleases;
private bool $enableSecurityScan;

/**
* @var mixed[]
Expand All @@ -20,14 +21,15 @@ final class AddPackage
/**
* @param mixed[] $metadata
*/
public function __construct(string $id, string $organizationId, string $url, string $type = 'vcs', array $metadata = [], ?int $keepLastReleases = null)
public function __construct(string $id, string $organizationId, string $url, string $type = 'vcs', array $metadata = [], ?int $keepLastReleases = null, bool $enableSecurityScan = true)
{
$this->id = $id;
$this->organizationId = $organizationId;
$this->url = $url;
$this->type = $type;
$this->metadata = $metadata;
$this->keepLastReleases = $keepLastReleases ?? 0;
$this->enableSecurityScan = $enableSecurityScan;
}

public function id(): string
Expand Down Expand Up @@ -62,4 +64,9 @@ public function keepLastReleases(): int
{
return $this->keepLastReleases;
}

public function hasSecurityScanEnabled(): bool
{
return $this->enableSecurityScan;
}
}
27 changes: 27 additions & 0 deletions src/Message/Organization/ChangeSecurityScanConfiguration.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Buddy\Repman\Message\Organization;

final class ChangeSecurityScanConfiguration
{
private string $organizationId;
private bool $enableSecurityScan;

public function __construct(string $organizationId, bool $enableSecurityScan)
{
$this->organizationId = $organizationId;
$this->enableSecurityScan = $enableSecurityScan;
}

public function organizationId(): string
{
return $this->organizationId;
}

public function hasSecurityScanEnabled(): bool
{
return $this->enableSecurityScan;
}
}
3 changes: 2 additions & 1 deletion src/MessageHandler/Organization/AddPackageHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ public function __invoke(AddPackage $message): void
$message->type(),
$message->url(),
$message->metadata(),
$message->keepLastReleases()
$message->keepLastReleases(),
$message->hasSecurityScanEnabled()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have the organization here in the handler, you do not have to pass this feature through a message object.

$organization = $this->organization->getById(Uuid::fromString($message->organizationId()));
$organization->addPackage(new Package(..., $organization->hasEnabledSecurityScan()));

)
)
;
Expand Down
28 changes: 28 additions & 0 deletions src/MessageHandler/Organization/EnableSecurityScanHandler.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Buddy\Repman\MessageHandler\Organization;

use Buddy\Repman\Message\Organization\ChangeSecurityScanConfiguration;
use Buddy\Repman\Repository\OrganizationRepository;
use Ramsey\Uuid\Uuid;
use Symfony\Component\Messenger\Handler\MessageHandlerInterface;

final class EnableSecurityScanHandler implements MessageHandlerInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
final class EnableSecurityScanHandler implements MessageHandlerInterface
final class ChangeSecurityScanConfigurationHandler implements MessageHandlerInterface

{
private OrganizationRepository $repositories;

public function __construct(OrganizationRepository $repositories)
{
$this->repositories = $repositories;
}

public function __invoke(ChangeSecurityScanConfiguration $message): void
{
$this->repositories
->getById(Uuid::fromString($message->organizationId()))
->enableSecurityScan($message->hasSecurityScanEnabled())
;
}
}
28 changes: 28 additions & 0 deletions src/Migrations/Version20220520191545.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<?php

declare(strict_types=1);

namespace Buddy\Repman\Migrations;

use Doctrine\DBAL\Schema\Schema;
use Doctrine\Migrations\AbstractMigration;

final class Version20220520191545 extends AbstractMigration
{
public function getDescription(): string
{
return '';
}

public function up(Schema $schema): void
{
// this up() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE organization ADD enable_security_scan BOOLEAN DEFAULT \'true\' NOT NULL');
}

public function down(Schema $schema): void
{
// this down() migration is auto-generated, please modify it to your needs
$this->addSql('ALTER TABLE organization DROP enable_security_scan');
}
}
10 changes: 9 additions & 1 deletion src/Query/Api/Model/Organization.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,15 @@ final class Organization implements \JsonSerializable
private string $name;
private string $alias;
private bool $hasAnonymousAccess;
private bool $enableSecurityScan;

public function __construct(string $id, string $name, string $alias, bool $hasAnonymousAccess)
public function __construct(string $id, string $name, string $alias, bool $hasAnonymousAccess, bool $enableSecurityScan)
{
$this->id = $id;
$this->name = $name;
$this->alias = $alias;
$this->hasAnonymousAccess = $hasAnonymousAccess;
$this->enableSecurityScan = $enableSecurityScan;
}

public function getId(): string
Expand All @@ -39,6 +41,11 @@ public function getHasAnonymousAccess(): bool
return $this->hasAnonymousAccess;
}

public function getEnabledSecurityScan(): bool
{
return $this->enableSecurityScan;
}

/**
* @return array<string,mixed>
*/
Expand All @@ -49,6 +56,7 @@ public function jsonSerialize(): array
'name' => $this->getName(),
'alias' => $this->getAlias(),
'hasAnonymousAccess' => $this->getHasAnonymousAccess(),
'enabledSecurityScan' => $this->enableSecurityScan,
];
}
}
5 changes: 3 additions & 2 deletions src/Query/Api/OrganizationQuery/DbalOrganizationQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public function __construct(Connection $connection)
public function getById(string $id): Option
{
$data = $this->connection->fetchAssociative(
'SELECT id, name, alias, has_anonymous_access
'SELECT id, name, alias, has_anonymous_access, enable_security_scan
FROM "organization" WHERE id = :id', [
'id' => $id,
]);
Expand All @@ -41,7 +41,7 @@ public function getUserOrganizations(string $userId, int $limit = 20, int $offse
return array_map(function (array $data): Organization {
return $this->hydrateOrganization($data);
}, $this->connection->fetchAllAssociative(
'SELECT o.id, o.name, o.alias, om.role, o.has_anonymous_access
'SELECT o.id, o.name, o.alias, om.role, o.has_anonymous_access, o.enable_security_scan
FROM organization_member om
JOIN organization o ON o.id = om.organization_id
WHERE om.user_id = :userId
Expand Down Expand Up @@ -143,6 +143,7 @@ private function hydrateOrganization(array $data): Organization
$data['name'],
$data['alias'],
$data['has_anonymous_access'],
$data['enable_security_scan'],
);
}

Expand Down
9 changes: 8 additions & 1 deletion src/Query/User/Model/Organization.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ final class Organization
private string $name;
private string $alias;
private bool $hasAnonymousAccess;
private bool $enableSecurityScan;

/**
* @var Member[]
Expand All @@ -22,13 +23,14 @@ final class Organization
/**
* @param Member[] $members
*/
public function __construct(string $id, string $name, string $alias, array $members, bool $hasAnonymousAccess)
public function __construct(string $id, string $name, string $alias, array $members, bool $hasAnonymousAccess, bool $enableSecurityScan)
{
$this->id = $id;
$this->name = $name;
$this->alias = $alias;
$this->members = array_map(fn (Member $member) => $member, $members);
$this->hasAnonymousAccess = $hasAnonymousAccess;
$this->enableSecurityScan = $enableSecurityScan;
}

public function id(): string
Expand Down Expand Up @@ -93,4 +95,9 @@ public function hasAnonymousAccess(): bool
{
return $this->hasAnonymousAccess;
}

public function isSecurityScanEnabled(): bool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could all methods/properties referring security scan be unified?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't follow. What do you mean by "unified"? Can you give a better example?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mix of is and has

{
return $this->enableSecurityScan;
}
}
5 changes: 3 additions & 2 deletions src/Query/User/OrganizationQuery/DbalOrganizationQuery.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function __construct(Connection $connection)
public function getByAlias(string $alias): Option
{
$data = $this->connection->fetchAssociative(
'SELECT id, name, alias, has_anonymous_access
'SELECT id, name, alias, has_anonymous_access, enable_security_scan
FROM "organization" WHERE alias = :alias', [
'alias' => $alias,
]);
Expand All @@ -44,7 +44,7 @@ public function getByAlias(string $alias): Option
public function getByInvitation(string $token, string $email): Option
{
$data = $this->connection->fetchAssociative(
'SELECT o.id, o.name, o.alias, o.has_anonymous_access
'SELECT o.id, o.name, o.alias, o.has_anonymous_access, o.enable_security_scan
FROM "organization" o
JOIN organization_invitation i ON o.id = i.organization_id
WHERE i.token = :token AND i.email = :email
Expand Down Expand Up @@ -240,6 +240,7 @@ private function hydrateOrganization(array $data): Organization
$data['alias'],
array_map(fn (array $row) => new Member($row['user_id'], $row['email'], $row['role']), $members),
$data['has_anonymous_access'],
$data['enable_security_scan'],
);
}

Expand Down
Loading