symfony / symfony

The Symfony PHP framework
https://symfony.com
MIT License
29.69k stars 9.44k forks source link

[RFC] Move SECRETS in a new componentsπŸ“¦οΈ #44151

Closed casahugo closed 2 years ago

casahugo commented 2 years ago

Description

I really like the principle of symfony secrets, so much that I would like to use it with other frameworks.

In my case, I would like to use it with phalcon and php 7.1. The problem is that I don't need to have all the components of the FrameworkBundle.

I created a repository to extract the implementation of the symfony secrets. I just copied the code but it's not a viable long term alternative: https://github.com/casahugo/secrets

Thank you for your work.

Example

Create new repository symfony/secrets (or symfony/vault) in packagist to run the command

composer require symfony/secrets

Extract the files

β”œβ”€β”€ composer.json
β”œβ”€β”€ LICENSE
β”œβ”€β”€ phpunit.xml.dist
β”œβ”€β”€ README.md
β”œβ”€β”€ src
β”‚Β Β  β”œβ”€β”€ AbstractVault.php
β”‚Β Β  β”œβ”€β”€ Command
β”‚Β Β  β”‚Β Β  β”œβ”€β”€ SecretsDecryptToLocalCommand.php
β”‚Β Β  β”‚Β Β  β”œβ”€β”€ SecretsEncryptFromLocalCommand.php
β”‚Β Β  β”‚Β Β  β”œβ”€β”€ SecretsGenerateKeysCommand.php
β”‚Β Β  β”‚Β Β  β”œβ”€β”€ SecretsListCommand.php
β”‚Β Β  β”‚Β Β  β”œβ”€β”€ SecretsRemoveCommand.php
β”‚Β Β  β”‚Β Β  └── SecretsSetCommand.php
β”‚Β Β  β”œβ”€β”€ DotenvVault.php
β”‚Β Β  └── SodiumVault.php
└── tests
    β”œβ”€β”€ DotenvVaultTest.php
    └── SodiumSecretTest.php

composer.json

{
    "name": "symfony/secrets",
    "license": "MIT",
    "require": {
        "php": ">=7.2.5",
        "paragonie/sodium_compat": "^1.17",
        "symfony/console": "^4|^5"
    },
    "require-dev": {
        "phpunit/phpunit": "^7.5.20",
        "symfony/dotenv": "^4|^5"
    },
    "autoload": {
        "psr-4": {
            "Symfony\\Secrets\\": "src/"
        }
    },
    "autoload-dev": {
        "psr-4": {
            "Symfony\\Secrets\\Tests\\": "tests/"
        }
    }
}

In https://github.com/symfony/framework-bundle edit composer.json to avoid a BCBreak.

...
"require": {
        ...
        "symfony/secrets": "~1.0"
 }
derrabus commented 2 years ago

Thank you for your proposal. This issue is open for quite some time now and I think we should decide on it.

I think it would make sense to extract the secrets-related classes into a dedicated component to make it reusable.

Regarding your use-case however:

In my case, I would like to use it with phalcon and php 7.1

If a dedicated component is released, it would as of now be released as part of Symfony 6.1 which requires PHP 8 already. If a PHP 7.1 backport is what you need, your fork would be the way to go.

AndreasA commented 2 years ago

@derrabus For me, it would be enough to extract them for future Symfony versions and PHP versions. It would really be helpful to use it without the framework-bundle, as a separate component.

Not sure about others but extracting them is definitely a new feature so not something that needs to be part of 5.4 Of course, the @internal phpdoc has to be removed then :smile:

chalasr commented 2 years ago

Makes sense to me as well. For the one who will be working on this, consider having a look at https://github.com/symfony/symfony/pull/39344.

wouterj commented 2 years ago

Btw, I'm not sure if a new component is needed for this. Wouldn't it make more sense to integrate this with the Dotenv component (if we want to separate it from the framework bundle)?

derrabus commented 2 years ago

@wouterj possibly

chalasr commented 2 years ago

I wouldn't make secrets a Dotenv concern. Dotenv's job is to handle dotenv files, while secrets don't necessarily have to be in dotenv files. I think a dedicated component makes sense even if it is small at first, that would open more possibilities regarding how secrets can be loaded and dumped.

AndreasA commented 2 years ago

Hmm. strictly speaking I could use the SodiumVault without ever needing DotEnv at all. This might be different for the DotEnv vault but the sodium would work standalone and DotEnv itself doesn't necessarily require the vaults.

EDIT: Actually, even the DotEnvVault can be used independently from DotEnv.

AndreasA commented 2 years ago

Any news regarding when the component will be created? Will it be ready for 6.2?

wouterj commented 2 years ago

@AndreasA Symfony is open source, there is no plan. If you're motivated to contribute this to Symfony, feel free to start a pull request.

AndreasA commented 2 years ago

@wouterj OK. I might do so but how would I best go about creating a separate component for it? and what should the name be? otherwise it makes no sense πŸ˜„ I guess I would move it into src/Symfony/Component, e.g. src/Symfony/Component/SecretVault and the name could be symfony/secret-vault?

should the current classes still be available at the beginning (for BC) or can I just remove them altogether in this case as they are marked internal anyway?

wouterj commented 2 years ago

Don't worry too much about the PR not being perfect at the start - almost no PR is (especially the ones introducing new components). As long as there is a starting point to the discussion, it's great.

This means the most important thing to demonstrate is: the classes as-is are usable as a standalone component (e.g. they don't depend on things from the bundle or framework).

I would suggest following the description of this issue as start: symfony/secret And then move the classes to this directory in src/Symfony/Component. For internal classes, this is all that is needed. Any non-internal class needs a BC layer - but we can help with that if you open the PR :)

You also need to copy some basic files from another component - e.g. README, LICENSE, phpunit.xml.dist, etc. We have a CI check to see if all metadocuments are copied correctly, so even here you don't have to worry too much - CI will tell you if you're missing something.

nicolas-grekas commented 2 years ago

Closing as explained in https://github.com/symfony/symfony/pull/45571#issuecomment-1199271557, thanks for proposing.