Skip to content

Commit 08bfd2f

Browse files
NinerianOpencodeGitHub Copilot
committed
refactor: address PR review feedback on types, grammar, and aud handling
- DeleteInstanceTrait: declare exitRemoteCall() as never to enforce termination contract; keep deleteInstance() as void with @return never doc - SSOTokenGenerator: accept string|array<string> for aud claim, validate all array elements are non-empty strings before passing to permittedFor() - SSODataTrait: rename $val to $value in getLocale/getTags for consistency - SharedDataTrait: fix grammar 'a editor' -> 'an editor' in docblock - SSOTokenTest: apply operator_linebreak cs-fixer fix Co-Authored-By: Opencode <opencode@noreply.opencode.ai> Co-Authored-By: GitHub Copilot <copilot@noreply.github.com>
1 parent 239e88f commit 08bfd2f

5 files changed

Lines changed: 34 additions & 12 deletions

File tree

src/RemoteCall/DeleteInstanceTrait.php

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,21 @@ trait DeleteInstanceTrait
1111
*
1212
* if a remote call was not handled by the user we die hard here
1313
*/
14-
protected function exitRemoteCall(): void
14+
protected function exitRemoteCall(): never
1515
{
1616
error_log("Warning: The exit procedure for a remote call was not properly handled.");
1717
exit;
1818
}
1919

20-
private function deleteInstance(string $instanceId, RemoteCallInterface $remoteCallHandler): never
20+
/**
21+
* Handle the delete-instance remote call and terminate the request.
22+
*
23+
* @param string $instanceId
24+
* @param RemoteCallInterface $remoteCallHandler
25+
*
26+
* @return never
27+
*/
28+
private function deleteInstance(string $instanceId, RemoteCallInterface $remoteCallHandler): void
2129
{
2230
if ($remoteCallHandler instanceof DeleteInstanceCallHandlerInterface) {
2331
$result = $remoteCallHandler->deleteInstance($instanceId);

src/SSOData/SSODataTrait.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,8 @@ public function getThemeBackgroundColor(): ?string
207207
*/
208208
public function getLocale(): string
209209
{
210-
$val = $this->getClaimSafe(SSODataClaimsInterface::CLAIM_USER_LOCALE);
211-
return is_string($val) ? $val : '';
210+
$value = $this->getClaimSafe(SSODataClaimsInterface::CLAIM_USER_LOCALE);
211+
return is_string($value) ? $value : '';
212212
}
213213

214214
/**
@@ -218,7 +218,7 @@ public function getLocale(): string
218218
*/
219219
public function getTags(): ?array
220220
{
221-
$val = $this->getClaimSafe(SSODataClaimsInterface::CLAIM_USER_TAGS);
222-
return is_array($val) ? $val : null;
221+
$value = $this->getClaimSafe(SSODataClaimsInterface::CLAIM_USER_TAGS);
222+
return is_array($value) ? $value : null;
223223
}
224224
}

src/SSOData/SharedDataTrait.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public function getSubject(): ?string
121121
*
122122
* If this is set to "editor", the requesting user may manage the contents
123123
* of the plugin instance, i.e. she has administration rights.
124-
* The type of the accessing entity can be either a "user" or a "editor".
124+
* The type of the accessing entity can be either a "user" or an "editor".
125125
*
126126
* @return null|string
127127
*/

src/SSOTokenGenerator.php

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,22 @@ private static function buildToken(Configuration $config, array $tokenData): Tok
5454
$builder = $config->builder();
5555
// Validate and coerce required registered claims to the expected types
5656
$audience = $tokenData[SSOData\SharedClaimsInterface::CLAIM_AUDIENCE] ?? '';
57-
if (!is_string($audience) || $audience === '') {
58-
throw new \InvalidArgumentException('aud claim must be a non-empty string for token generation');
57+
if (is_string($audience)) {
58+
if ($audience === '') {
59+
throw new \InvalidArgumentException('aud claim must be a non-empty string or array for token generation');
60+
}
61+
/** @var non-empty-list<non-empty-string> $audiences */
62+
$audiences = [$audience];
63+
} elseif (is_array($audience) && $audience !== []) {
64+
foreach ($audience as $aud) {
65+
if (!is_string($aud) || $aud === '') {
66+
throw new \InvalidArgumentException('aud claim array must contain only non-empty strings for token generation');
67+
}
68+
}
69+
/** @var non-empty-list<non-empty-string> $audiences */
70+
$audiences = array_values($audience);
71+
} else {
72+
throw new \InvalidArgumentException('aud claim must be a non-empty string or array for token generation');
5973
}
6074

6175
$issuedAt = $tokenData[SSOData\SharedClaimsInterface::CLAIM_ISSUED_AT] ?? null;
@@ -74,7 +88,7 @@ private static function buildToken(Configuration $config, array $tokenData): Tok
7488
}
7589

7690
$token = $builder
77-
->permittedFor($audience)
91+
->permittedFor(...$audiences)
7892
->issuedAt($issuedAt)
7993
->canOnlyBeUsedAfter($notBefore)
8094
->expiresAt($expiresAt);

test/SSOTokenTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -208,8 +208,8 @@ public function testAccessorsGiveCorrectValues(): void
208208
$data = $data->getTimestamp();
209209
}
210210

211-
$data = is_array($data) ? print_r($data, true) :
212-
(is_scalar($data) || is_null($data) ? (string) ($data ?? '') : '[complex_type]');
211+
$data = is_array($data) ? print_r($data, true)
212+
: (is_scalar($data) || is_null($data) ? (string) ($data ?? '') : '[complex_type]');
213213

214214
$this->assertEquals(
215215
$tokenData[$key],

0 commit comments

Comments
 (0)