Skip to content

Commit 56cdba2

Browse files
paulbalandanpaulbalandan
committed
refactor: improve UX on key:generate 
1 parent df9f137 commit 56cdba2

3 files changed

Lines changed: 72 additions & 49 deletions

File tree

‎system/Commands/Encryption/GenerateKey.php‎

Lines changed: 25 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -104,17 +104,29 @@ protected function execute(array $arguments, array $options): int
104104
$currentKey = env('encryption.key', '');
105105

106106
if ($currentKey !== '' && $options['force'] === false) {
107-
CLI::error('Setting new encryption key aborted.');
107+
if ($this->isInteractive()) {
108+
CLI::write('Setting new encryption key cancelled.', 'yellow');
108109

109-
if (! $this->isInteractive()) {
110-
CLI::error('If you want, use the "--force" option to force overwrite the existing key.');
110+
return EXIT_SUCCESS;
111111
}
112112

113+
CLI::error('Setting new encryption key aborted: pass --force to overwrite the existing key in non-interactive mode.');
114+
115+
return EXIT_ERROR;
116+
}
117+
118+
$envFile = ((new Paths())->envDirectory ?? ROOTPATH) . '.env'; // @phpstan-ignore nullCoalesce.property
119+
$baseEnv = ROOTPATH . 'env';
120+
121+
if (! is_file($envFile) && ! is_file($baseEnv)) {
122+
CLI::write('Both default shipped `env` file and custom `.env` are missing.', 'yellow');
123+
CLI::write(sprintf('Here\'s your new key instead: %s', CLI::color($encodedKey, 'yellow')));
124+
113125
return EXIT_ERROR;
114126
}
115127

116-
if (! $this->writeNewEncryptionKeyToFile($currentKey, $encodedKey)) {
117-
CLI::write('Error in setting new encryption key to .env file.');
128+
if (! $this->writeNewEncryptionKeyToFile($currentKey, $encodedKey, $envFile, $baseEnv)) {
129+
CLI::error(sprintf('Failed to write new encryption key to %s.', clean_path($envFile)));
118130

119131
return EXIT_ERROR;
120132
}
@@ -125,7 +137,7 @@ protected function execute(array $arguments, array $options): int
125137
$dotenv = new DotEnv((new Paths())->envDirectory ?? ROOTPATH); // @phpstan-ignore nullCoalesce.property
126138
$dotenv->load();
127139

128-
CLI::write('Application\'s new encryption key was successfully set.', 'green');
140+
CLI::write(sprintf('New encryption key written to %s.', clean_path($envFile)), 'green');
129141
CLI::newLine();
130142

131143
return EXIT_SUCCESS;
@@ -146,24 +158,19 @@ private function generateRandomKey(string $prefix, int $length): string
146158
}
147159

148160
/**
149-
* Writes the new encryption key to .env file.
161+
* Writes the new encryption key to .env file. The caller is responsible
162+
* for ensuring at least one of `$envFile` or `$baseEnv` exists.
150163
*/
151-
private function writeNewEncryptionKeyToFile(string $oldKey, string $newKey): bool
164+
private function writeNewEncryptionKeyToFile(string $oldKey, string $newKey, string $envFile, string $baseEnv): bool
152165
{
153-
$baseEnv = ROOTPATH . 'env';
154-
$envFile = ((new Paths())->envDirectory ?? ROOTPATH) . '.env'; // @phpstan-ignore nullCoalesce.property
155-
156166
if (! is_file($envFile)) {
157-
if (! is_file($baseEnv)) {
158-
CLI::write('Both default shipped `env` file and custom `.env` are missing.', 'yellow');
159-
CLI::write('Here\'s your new key instead: ' . CLI::color($newKey, 'yellow'));
160-
161-
return false;
162-
}
163-
164167
copy($baseEnv, $envFile);
165168
}
166169

170+
if (! is_writable($envFile)) {
171+
return false;
172+
}
173+
167174
$oldFileContents = (string) file_get_contents($envFile);
168175
$replacementKey = "\nencryption.key = {$newKey}";
169176

‎tests/system/Commands/Encryption/GenerateKeyTest.php‎

Lines changed: 46 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -70,14 +70,6 @@ protected function tearDown(): void
7070
CLI::reset();
7171
}
7272

73-
/**
74-
* Gets buffer contents then releases it.
75-
*/
76-
protected function getBuffer(): string
77-
{
78-
return $this->getStreamFilterBuffer();
79-
}
80-
8173
protected function resetEnvironment(): void
8274
{
8375
putenv('encryption.key');
@@ -88,31 +80,35 @@ protected function resetEnvironment(): void
8880
public function testGenerateKeyShowsEncodedKey(): void
8981
{
9082
command('key:generate --show');
91-
$this->assertStringContainsString('hex2bin:', $this->getBuffer());
83+
$this->assertStringContainsString('hex2bin:', $this->getStreamFilterBuffer());
9284

85+
$this->resetStreamFilterBuffer();
9386
command('key:generate --prefix base64 --show');
94-
$this->assertStringContainsString('base64:', $this->getBuffer());
87+
$this->assertStringContainsString('base64:', $this->getStreamFilterBuffer());
9588

89+
$this->resetStreamFilterBuffer();
9690
command('key:generate --prefix hex2bin --show');
97-
$this->assertStringContainsString('hex2bin:', $this->getBuffer());
91+
$this->assertStringContainsString('hex2bin:', $this->getStreamFilterBuffer());
9892
}
9993

10094
#[PreserveGlobalState(false)]
10195
#[RunInSeparateProcess]
10296
public function testGenerateKeyCreatesNewKey(): void
10397
{
10498
command('key:generate');
105-
$this->assertStringContainsString('successfully set.', $this->getBuffer());
99+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
106100
$this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
107101
$this->assertStringContainsString('hex2bin:', (string) file_get_contents($this->envPath));
108102

103+
$this->resetStreamFilterBuffer();
109104
command('key:generate --prefix base64 --force');
110-
$this->assertStringContainsString('successfully set.', $this->getBuffer());
105+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
111106
$this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
112107
$this->assertStringContainsString('base64:', (string) file_get_contents($this->envPath));
113108

109+
$this->resetStreamFilterBuffer();
114110
command('key:generate --prefix hex2bin --force');
115-
$this->assertStringContainsString('successfully set.', $this->getBuffer());
111+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
116112
$this->assertStringContainsString(env('encryption.key'), (string) file_get_contents($this->envPath));
117113
$this->assertStringContainsString('hex2bin:', (string) file_get_contents($this->envPath));
118114
}
@@ -123,8 +119,9 @@ public function testDefaultShippedEnvIsMissing(): void
123119
command('key:generate');
124120
rename(ROOTPATH . 'lostenv', ROOTPATH . 'env');
125121

126-
$this->assertStringContainsString('Both default shipped', $this->getBuffer());
127-
$this->assertStringContainsString('Error in setting', $this->getBuffer());
122+
$this->assertStringContainsString('Both default shipped', $this->getStreamFilterBuffer());
123+
$this->assertStringContainsString('Here\'s your new key instead:', $this->getStreamFilterBuffer());
124+
$this->assertStringNotContainsString('Failed to write', $this->getStreamFilterBuffer());
128125
}
129126

130127
/**
@@ -136,7 +133,7 @@ public function testKeyGenerateWhenKeyIsMissingInDotEnvFile(): void
136133

137134
command('key:generate');
138135

139-
$this->assertStringContainsString('Application\'s new encryption key was successfully set.', $this->getBuffer());
136+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
140137
$this->assertSame("\nencryption.key = " . env('encryption.key'), file_get_contents($this->envPath));
141138
}
142139

@@ -152,9 +149,9 @@ public function testKeyGenerateWhenNewHexKeyIsSubsequentlyCommentedOut(): void
152149
));
153150
$this->assertSame(1, $count, 'Failed commenting out the previously set application key.');
154151

155-
CITestStreamFilter::$buffer = '';
152+
$this->resetStreamFilterBuffer();
156153
command('key:generate --force');
157-
$this->assertStringContainsString('was successfully set.', $this->getBuffer());
154+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
158155
$this->assertNotSame($key, env('encryption.key', $key), 'Failed replacing the commented out key.');
159156
}
160157

@@ -170,9 +167,9 @@ public function testKeyGenerateWhenNewBase64KeyIsSubsequentlyCommentedOut(): voi
170167
));
171168
$this->assertSame(1, $count, 'Failed commenting out the previously set application key.');
172169

173-
CITestStreamFilter::$buffer = '';
170+
$this->resetStreamFilterBuffer();
174171
command('key:generate --force');
175-
$this->assertStringContainsString('was successfully set.', $this->getBuffer());
172+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
176173
$this->assertNotSame($key, env('encryption.key', $key), 'Failed replacing the commented out key.');
177174
}
178175

@@ -190,15 +187,15 @@ public function testKeyGenerateReplacesUnloadedKeyInDotEnvFile(): void
190187
$this->assertSame('', env('encryption.key', ''));
191188

192189
command('key:generate --force');
190+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
193191

194-
$this->assertStringContainsString('was successfully set.', $this->getBuffer());
195-
196-
$contents = (string) file_get_contents($this->envPath);
192+
$contents = @file_get_contents($this->envPath);
193+
$this->assertIsString($contents, 'Failed to read .env file contents.');
197194
$this->assertStringNotContainsString($existingKey, $contents);
198195
$this->assertStringContainsString('encryption.key = ' . env('encryption.key'), $contents);
199196
}
200197

201-
public function testKeyGenerateAbortsWhenOverwritePromptIsDeclined(): void
198+
public function testKeyGenerateCancelsWhenOverwritePromptIsDeclined(): void
202199
{
203200
command('key:generate');
204201
$key = env('encryption.key', '');
@@ -208,12 +205,13 @@ public function testKeyGenerateAbortsWhenOverwritePromptIsDeclined(): void
208205
$io->setInputs(['n']);
209206
CLI::setInputOutput($io);
210207

208+
$this->resetStreamFilterBuffer();
211209
command('key:generate');
212210

213211
$this->assertSame($key, env('encryption.key', ''), 'Existing key should not change.');
214212
$this->assertStringContainsString($key, (string) file_get_contents($this->envPath));
215213
$this->assertStringContainsString('Overwrite existing key?', $io->getOutput());
216-
$this->assertStringContainsString('Setting new encryption key aborted.', $io->getOutput());
214+
$this->assertStringContainsString('Setting new encryption key cancelled.', $io->getOutput());
217215
}
218216

219217
public function testKeyGenerateOverwritesWhenOverwritePromptIsConfirmed(): void
@@ -226,12 +224,13 @@ public function testKeyGenerateOverwritesWhenOverwritePromptIsConfirmed(): void
226224
$io->setInputs(['y']);
227225
CLI::setInputOutput($io);
228226

227+
$this->resetStreamFilterBuffer();
229228
command('key:generate --prefix base64');
230229

231230
$this->assertNotSame($oldKey, env('encryption.key', $oldKey));
232231
$this->assertStringContainsString('base64:', (string) file_get_contents($this->envPath));
233232
$this->assertStringContainsString('Overwrite existing key?', $io->getOutput());
234-
$this->assertStringContainsString('successfully set.', $io->getOutput());
233+
$this->assertStringContainsString(sprintf('New encryption key written to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $io->getOutput());
235234
}
236235

237236
#[PreserveGlobalState(false)]
@@ -243,19 +242,20 @@ public function testKeyGenerateAbortsNonInteractivelyWithExistingKey(): void
243242
$this->assertNotSame('', $key);
244243

245244
$this->resetStreamFilterBuffer();
246-
247245
command('key:generate --no-interaction');
248246

249247
$this->assertSame($key, env('encryption.key', ''), 'Existing key should not change.');
250-
$this->assertStringContainsString('Setting new encryption key aborted.', $this->getBuffer());
251-
$this->assertStringContainsString('--force', $this->getBuffer());
248+
$this->assertStringContainsString(
249+
'Setting new encryption key aborted: pass --force to overwrite the existing key in non-interactive mode.',
250+
$this->getStreamFilterBuffer(),
251+
);
252252
}
253253

254254
public function testKeyGenerateErrorsOnInvalidPrefixNonInteractively(): void
255255
{
256256
command('key:generate --prefix invalid --show --no-interaction');
257257

258-
$this->assertStringContainsString('Invalid prefix "invalid"', $this->getBuffer());
258+
$this->assertStringContainsString('Invalid prefix "invalid"', $this->getStreamFilterBuffer());
259259
}
260260

261261
public function testKeyGeneratePromptsForInvalidPrefix(): void
@@ -269,4 +269,19 @@ public function testKeyGeneratePromptsForInvalidPrefix(): void
269269
$this->assertStringContainsString('Please provide a valid prefix to use.', $io->getOutput());
270270
$this->assertStringContainsString('hex2bin:', $io->getOutput());
271271
}
272+
273+
public function testKeyGenerateErrorsWhenEnvFileIsNotWritable(): void
274+
{
275+
command('key:generate');
276+
chmod($this->envPath, 0o444);
277+
278+
try {
279+
$this->resetStreamFilterBuffer();
280+
command('key:generate --force');
281+
282+
$this->assertStringContainsString(sprintf('Failed to write new encryption key to ROOTPATH%s.env.', DIRECTORY_SEPARATOR), $this->getStreamFilterBuffer());
283+
} finally {
284+
chmod($this->envPath, 0o644);
285+
}
286+
}
272287
}

‎user_guide_src/source/changelogs/v4.8.0.rst‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ Behavior Changes
3030
- **Commands:** The ``filter:check`` command now requires the HTTP method argument to be uppercase (e.g., ``spark filter:check GET /`` instead of ``spark filter:check get /``).
3131
- **Commands:** Several built-in commands have been migrated from ``BaseCommand`` to the modern ``AbstractCommand`` style. Applications that extend a built-in command to override
3232
behaviour may need to re-implement against the modern API (``configure()`` + ``execute()`` and the ``#[Command]`` attribute) once the class it extends is migrated, or, preferably, compose instead of extending. Invocations on the command line are unaffected.
33+
- **Commands:** Declining the ``key:generate`` overwrite prompt interactively now returns ``EXIT_SUCCESS`` instead of ``EXIT_ERROR``. Output messages were also reworded; CI/automation that branches on the exit code or greps the previous wording will need updating.
3334
- **Database:** The Postgre driver's ``$db->error()['code']`` previously always returned ``''``. It now returns the 5-character SQLSTATE string for query and transaction failures (e.g., ``'42P01'``), or ``'08006'`` for connection-level failures. Code that relied on ``$db->error()['code'] === ''`` will need updating.
3435
- **Filters:** HTTP method matching for method-based filters is now case-sensitive. The keys in ``Config\Filters::$methods`` must exactly match the request method
3536
(e.g., ``GET``, ``POST``). Lowercase method names (e.g., ``post``) will no longer match.

0 commit comments

Comments
 (0)