-
Notifications
You must be signed in to change notification settings - Fork 117
Introduce org setting enable_security_scan #591
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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; | ||
|
|
||
| final class EnableSecurityScanType extends AbstractType | ||
| { | ||
| 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']) | ||
| ; | ||
| } | ||
| } |
| 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; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,8 @@ public function __invoke(AddPackage $message): void | |
| $message->type(), | ||
| $message->url(), | ||
| $message->metadata(), | ||
| $message->keepLastReleases() | ||
| $message->keepLastReleases(), | ||
| $message->hasSecurityScanEnabled() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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())); |
||
| ) | ||
| ) | ||
| ; | ||
|
|
||
| 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 | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
| { | ||||||
| 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()) | ||||||
| ; | ||||||
| } | ||||||
| } | ||||||
| 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'); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,6 +13,7 @@ final class Organization | |
| private string $name; | ||
| private string $alias; | ||
| private bool $hasAnonymousAccess; | ||
| private bool $enableSecurityScan; | ||
|
|
||
| /** | ||
| * @var Member[] | ||
|
|
@@ -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 | ||
|
|
@@ -93,4 +95,9 @@ public function hasAnonymousAccess(): bool | |
| { | ||
| return $this->hasAnonymousAccess; | ||
| } | ||
|
|
||
| public function isSecurityScanEnabled(): bool | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could all methods/properties referring security scan be unified?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mix of |
||
| { | ||
| return $this->enableSecurityScan; | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?