Skip to content

Commit c8daaa0

Browse files
committed
fix(auth): reject null/empty secret cleanly, configure CI Facebook creds
Root cause of the CI failure on testMatchedUserIsUnlinkedAndReturnsConfirmation: .env.testing (which supplies FACEBOOK_CLIENT_SECRET locally) is gitignored, and none of the three GitHub Actions workflows ever set FACEBOOK_CLIENT_ID/ FACEBOOK_CLIENT_SECRET - a pre-existing gap this PR was the first to depend on. Config::get('services.facebook.client_secret') resolved to null in CI, and FacebookSignedRequestParser::parse()'s non-nullable string $secret parameter turned that into a fatal TypeError (500) instead of a clean rejection. - Widen parse()'s $secret to ?string and reject immediately on null/'' - a webhook endpoint should fail closed with 400 on missing server config, never 500. - Add FACEBOOK_CLIENT_ID/FACEBOOK_CLIENT_SECRET/FACEBOOK_REDIRECT_URI (same dummy test values as .env.testing) to all three CI workflows. - Two new parser tests lock in the defensive null/empty-secret behavior. Reverts the temporary response-body-dump instrumentation from 477109d, which was used to capture the real TypeError from CI's APP_DEBUG=true error page.
1 parent 477109d commit c8daaa0

6 files changed

Lines changed: 27 additions & 6 deletions

File tree

.github/workflows/nightly_unit_tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ jobs:
3232
SSL_ENABLED: false
3333
SESSION_DRIVER: redis
3434
PHP_VERSION: 8.3
35+
FACEBOOK_CLIENT_ID: 214242500242860
36+
FACEBOOK_CLIENT_SECRET: e62fa81aa898699d8cebf14bf5e586aa
37+
FACEBOOK_REDIRECT_URI: /auth/login/facebook/callback
3538
services:
3639
mysql:
3740
image: mysql:8.0

.github/workflows/pull_request_unit_tests.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,9 @@ jobs:
3737
PHP_VERSION: 8.3
3838
OTEL_SDK_DISABLED: true
3939
OTEL_SERVICE_ENABLED: false
40+
FACEBOOK_CLIENT_ID: 214242500242860
41+
FACEBOOK_CLIENT_SECRET: e62fa81aa898699d8cebf14bf5e586aa
42+
FACEBOOK_REDIRECT_URI: /auth/login/facebook/callback
4043
services:
4144
mysql:
4245
image: mysql:8.0

.github/workflows/push.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ jobs:
3333
PHP_VERSION: 8.3
3434
OTEL_SDK_DISABLED: true
3535
OTEL_SERVICE_ENABLED: false
36+
FACEBOOK_CLIENT_ID: 214242500242860
37+
FACEBOOK_CLIENT_SECRET: e62fa81aa898699d8cebf14bf5e586aa
38+
FACEBOOK_REDIRECT_URI: /auth/login/facebook/callback
3639
services:
3740
mysql:
3841
image: mysql:8.0

app/libs/Auth/FacebookSignedRequestParser.php

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ final class FacebookSignedRequestParser
2323
{
2424
/**
2525
* @param string $signed_request
26-
* @param string $secret
26+
* @param string|null $secret
2727
* @return array|null
2828
*/
29-
public static function parse(string $signed_request, string $secret): ?array
29+
public static function parse(string $signed_request, ?string $secret): ?array
3030
{
31+
if ($secret === null || $secret === '') return null;
32+
3133
$parts = explode('.', $signed_request, 2);
3234
if (count($parts) !== 2) return null;
3335

tests/FacebookDataDeletionApiTest.php

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -65,10 +65,6 @@ public function testMatchedUserIsUnlinkedAndReturnsConfirmation(): void
6565

6666
$response = $this->post(self::CallbackUri, ['signed_request' => $sr]);
6767

68-
if ($response->response->getStatusCode() !== 200) {
69-
fwrite(STDERR, "\n===DEBUG RESPONSE BODY===\n" . $response->response->getContent() . "\n===END DEBUG===\n");
70-
}
71-
7268
$this->assertResponseStatus(200);
7369
$json = json_decode($response->response->getContent(), true);
7470
$this->assertNotEmpty($json['confirmation_code']);

tests/unit/FacebookSignedRequestParserTest.php

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,4 +92,18 @@ public function testMissingUserIdReturnsNull(): void
9292

9393
$this->assertNull(FacebookSignedRequestParser::parse($sr, self::Secret));
9494
}
95+
96+
public function testNullSecretReturnsNullInsteadOfThrowing(): void
97+
{
98+
$sr = $this->buildSignedRequest(['user_id' => '218471'], self::Secret);
99+
100+
$this->assertNull(FacebookSignedRequestParser::parse($sr, null));
101+
}
102+
103+
public function testEmptySecretReturnsNullInsteadOfThrowing(): void
104+
{
105+
$sr = $this->buildSignedRequest(['user_id' => '218471'], self::Secret);
106+
107+
$this->assertNull(FacebookSignedRequestParser::parse($sr, ''));
108+
}
95109
}

0 commit comments

Comments
 (0)