Skip to content

Commit 642575c

Browse files
fraggerfoxUnnikrishnanBhargavakurup
fraggerfox
and
UnnikrishnanBhargavakurup
authored
Fix E2E test API responses (#35)
* fix: updated API response format for auth request, configured backend URL for Playwright, and fixed responses for invite API * chore: update MOCKIN URL to use localhost instead of 127.0.0.1 * Uncomment the e2e tests from GHA. * Convert GET request with op and login_hint to POST params before routing in HelloClient handler * Command handler: default sub to empty string when JWT claim is missing; add GET /post-test (REQUEST_METHOD=GET) to simulate POST * fix: unit test, invalid schema for mock redirect url * add redirect to login when request contains iss, login_hint, domain_hint, target_link_uri, or redirect_uri and validate issuer * fix: tighten types and null checks in HelloClient - Updated getAuth() return docblock and added array checks before accessing authCookie - Casted fields (email, name, picture, sub) to string/bool to satisfy static analysis - Ensured json() always receives associative array<string, mixed> - Casted $iss and $op to string before use and fixed error message format * test: fix asset string in testCanGenerateInviteUrl - Corrected invite URL asset string in InviteTest::testCanGenerateInviteUrl - Verified Invite handler generates expected URL * refactor: clean up HelloClient constructor and remove unreachable ternary - Directly use getHelloDomain() for issuer initialization (typed as string) - Removed redundant else branch flagged by PHPStan - Preserved all other logic and type safety * fix(invite): normalize return_uri to remove unreachable ternary - Precompute $returnUri before building request to avoid PHPStan warning - Keeps logic same but eliminates 'else branch is unreachable' issue * fix(invite): replace ternary with if block for returnUri - Removed unreachable else branch flagged by PHPStan - Now sets $returnUri using a simple if statement * Name the artifact uniquely e2e matrix. * Fix lint issues. getHelloDomain() method will never return a null, so no need to do a null check on it. --------- Co-authored-by: Unnikrishnan B <unnikrishnanadoor@gmail.com>
1 parent f880290 commit 642575c

File tree

9 files changed

+258
-91
lines changed

9 files changed

+258
-91
lines changed

‎.github/workflows/continuous-integration.yml‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -187,14 +187,14 @@ jobs:
187187
- name: Install dependencies
188188
run: cd e2e-tests && npm ci && npx playwright install --with-deps
189189

190-
# # Run the e2e tests
191-
# - name: Run Playwright tests
192-
# run: cd e2e-tests && npx playwright test
193-
194-
# # Upload report after each run
195-
# - uses: actions/upload-artifact@v4
196-
# if: ${{ !cancelled() }}
197-
# with:
198-
# name: playwright-report
199-
# path: ./e2e-tests/playwright-report/
200-
# retention-days: 30
190+
# Run the e2e tests
191+
- name: Run Playwright tests
192+
run: cd e2e-tests && npx playwright test
193+
194+
# Upload report after each run
195+
- uses: actions/upload-artifact@v4
196+
if: ${{ !cancelled() }}
197+
with:
198+
name: playwright-report-${{ matrix.php-version }}-${{ matrix.operating-system }}
199+
path: ./e2e-tests/playwright-report/
200+
retention-days: 30

‎e2e-tests/index.php‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,14 @@
55
use HelloCoop\Config\HelloConfigBuilder;
66
use HelloCoop\HelloClient;
77

8-
98
define('API_ROUTE', '/api/hellocoop');
109

1110
$builder = new HelloConfigBuilder();
1211
$config = $builder
13-
->setApiRoute('/api/hellocoop')
14-
->setAuthApiRoute('/api/hellocoop?op=auth')
15-
->setLoginApiRoute('/api/hellocoop?op=login')
16-
->setLogoutApiRoute('/api/hellocoop?op=logout')
12+
->setApiRoute(API_ROUTE)
13+
->setAuthApiRoute(API_ROUTE . '?op=auth')
14+
->setLoginApiRoute(API_ROUTE . '?op=login')
15+
->setLogoutApiRoute(API_ROUTE . '?op=logout')
1716
->setSameSiteStrict(false)
1817
->setClientId('000000-0000-0000-0000-000000000000')
1918
->setRedirectURI('http://localhost:8000/api/hellocoop')
@@ -22,17 +21,42 @@
2221
->setScope(['openid', 'profile', 'email'])
2322
->build();
2423

25-
// Step 3: Create an instance of HelloClient
2624
$helloClient = new HelloClient($config);
2725

28-
$requestUri = $_SERVER['REQUEST_URI'];
29-
$parsedUrl = parse_url($requestUri); // Extract the path from the request URI, ignoring query parameters
30-
$requestPath = $parsedUrl['path'] ?? '';
26+
// Current request path (ignore query string)
27+
$requestUri = $_SERVER['REQUEST_URI'] ?? '/';
28+
$parsedUrl = parse_url($requestUri);
29+
$requestPath = $parsedUrl['path'] ?? '/';
3130

32-
// Step 4: Route Hellō API requests
31+
// 1) Direct Hellō API route → just route it
3332
if ($requestPath === API_ROUTE) {
34-
$helloClient->route(); // Handle the routing of the API request
33+
print $helloClient->route();
34+
exit;
35+
}
36+
37+
// 2) If a GET like /post-test?op=login&login_hint=... arrives,
38+
// convert it into a POST and let HelloClient handle it.
39+
// Handle GET /post-test?... by converting selected query params to POST
40+
if ($requestPath === '/post-test' && $_SERVER['REQUEST_METHOD'] === 'GET') {
41+
$allowed = ['op', 'login_hint', 'domain_hint', 'iss', 'command_token'];
42+
43+
$payload = [];
44+
foreach ($allowed as $key) {
45+
if (isset($_GET[$key])) {
46+
$payload[$key] = $_GET[$key];
47+
}
48+
}
49+
50+
if (!empty($payload)) {
51+
$_POST = $payload;
52+
$_SERVER['REQUEST_METHOD'] = 'POST';
53+
$_SERVER['CONTENT_TYPE'] = $_SERVER['CONTENT_TYPE'] ?? 'application/x-www-form-urlencoded';
54+
55+
print $helloClient->route();
56+
exit;
57+
}
3558
}
3659

60+
// 3) Fallback: return current auth status
3761
header('Content-Type: application/json');
3862
print json_encode($helloClient->getAuth());

‎e2e-tests/php.spec.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
const APP_HOME = 'http://localhost:8000/'
22
const MOCKIN = 'http://localhost:3333/'
3+
34
const APP_API = APP_HOME + 'api/hellocoop'
45

56
import { test, expect } from '@playwright/test'

‎src/Handler/Command.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ public function handleCommand(): string
145145
/** @var string $iss */
146146
$iss = $claims['iss'];
147147
/** @var string $sub */
148-
$sub = $claims['sub'];
148+
$sub = $claims['sub'] ?? '';
149149
/** @var string|null $tenant */
150150
$tenant = isset($claims['tenant']) ? $claims['tenant'] : null;
151151
/** @var array<string>|null $groups */

‎src/Handler/Invite.php‎

Lines changed: 102 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
<?php
22

3+
declare(strict_types=1);
4+
35
namespace HelloCoop\Handler;
46

57
use Exception;
@@ -8,7 +10,7 @@
810
use HelloCoop\Config\ConfigInterface;
911
use HelloCoop\Lib\Auth as AuthLib;
1012

11-
class Invite
13+
final class Invite
1214
{
1315
private HelloResponseInterface $helloResponse;
1416
private HelloRequestInterface $helloRequest;
@@ -20,9 +22,9 @@ public function __construct(
2022
HelloResponseInterface $helloResponse,
2123
ConfigInterface $config
2224
) {
23-
$this->helloRequest = $helloRequest;
25+
$this->helloRequest = $helloRequest;
2426
$this->helloResponse = $helloResponse;
25-
$this->config = $config;
27+
$this->config = $config;
2628
}
2729

2830
private function getAuthLib(): AuthLib
@@ -34,43 +36,123 @@ private function getAuthLib(): AuthLib
3436
);
3537
}
3638

39+
/**
40+
* Build origin like "https://example.com[:port]" from a URL.
41+
* Falls back to empty string if not derivable.
42+
*/
43+
private function buildOrigin(?string $url): string
44+
{
45+
if ($url === null || $url === '') {
46+
return '';
47+
}
48+
$parts = parse_url($url);
49+
if ($parts === false || !isset($parts['scheme'], $parts['host'])) {
50+
return '';
51+
}
52+
$origin = $parts['scheme'] . '://' . $parts['host'];
53+
if (isset($parts['port'])) {
54+
$origin .= ':' . (string)$parts['port'];
55+
}
56+
return $origin;
57+
}
58+
59+
/**
60+
* Safely fetch a string from an array of mixed.
61+
* @param array<string,mixed> $arr
62+
*/
63+
private function strFrom(array $arr, string $key, ?string $default = null): ?string
64+
{
65+
$v = $arr[$key] ?? null;
66+
return is_string($v) ? $v : $default;
67+
}
68+
3769
/**
3870
* @throws Exception
3971
*/
4072
public function generateInviteUrl(): string
4173
{
74+
/** @var array<string,mixed> $params */
4275
$params = $this->helloRequest->fetchMultiple([
4376
'target_uri',
4477
'app_name',
4578
'prompt',
4679
'role',
4780
'tenant',
4881
'state',
49-
'redirect_uri'
82+
'redirect_uri',
5083
]);
5184

52-
$auth = $this->getAuthLib()->getAuthfromCookies();
53-
if (empty($auth->toArray()['authCookie'])) {
54-
throw new Exception("User not logged in");
85+
$auth = $this->getAuthLib()->getAuthfromCookies();
86+
$authArr = $auth->toArray();
87+
88+
/** @var array<string,mixed>|null $cookie */
89+
$cookie = isset($authArr['authCookie']) && is_array($authArr['authCookie'])
90+
? $authArr['authCookie']
91+
: null;
92+
93+
if ($cookie === null) {
94+
throw new Exception('User not logged in');
95+
}
96+
97+
$inviterSub = $this->strFrom($cookie, 'sub', '');
98+
if ($inviterSub === '') {
99+
throw new Exception('User cookie missing');
55100
}
56101

57-
if (empty($auth->toArray()['authCookie']['sub'])) {
58-
throw new Exception("User coookie missing");
102+
// Choose redirect URI: request param overrides config if present
103+
$redirectURIParam = $this->strFrom($params, 'redirect_uri');
104+
$redirectURIConf = $this->config->getRedirectURI(); // ?string
105+
$redirectURI = $redirectURIParam !== null && $redirectURIParam !== ''
106+
? $redirectURIParam
107+
: ($redirectURIConf ?? '');
108+
109+
$origin = $this->buildOrigin($redirectURI);
110+
$defaultTargetURI = ($origin !== '' ? $origin . '/' : '/');
111+
112+
// Safe inviter/app names
113+
$inviterName = $this->strFrom($cookie, 'name')
114+
?? $this->strFrom($authArr, 'name')
115+
?? 'Someone';
116+
117+
$appName = $this->strFrom($params, 'app_name', 'your app');
118+
119+
$defaultPrompt = sprintf(
120+
'%s has invited you to join %s',
121+
$inviterName,
122+
$appName
123+
);
124+
125+
// Safe scalar config values
126+
$clientIdRaw = $this->config->getClientId();
127+
$clientId = is_string($clientIdRaw) ? $clientIdRaw : '';
128+
$helloDomain = $this->config->getHelloDomain();
129+
130+
$targetUri = $this->strFrom($params, 'target_uri');
131+
$returnUri = $defaultTargetURI;
132+
if ($targetUri !== null && $targetUri !== '') {
133+
$returnUri = $targetUri;
59134
}
60135

61136
$request = [
62-
'app_name' => $params['app_name'],
63-
'prompt' => $params['prompt'],
64-
'role' => $params['role'],
65-
'tenant' => $params['tenant'],
66-
'state' => $params['state'],
67-
'inviter' => $auth->toArray()['authCookie']['sub'], //TODO: add a getter function for this value.
68-
'client_id' => $this->config->getClientId(),
69-
'initiate_login_uri' => $this->config->getRedirectURI() ?? '/', //TODO: need to fix this
70-
'return_uri' => $params['target_uri']
137+
'app_name' => $this->strFrom($params, 'app_name'),
138+
'prompt' => $this->strFrom($params, 'prompt', $defaultPrompt),
139+
'role' => $this->strFrom($params, 'role'),
140+
'tenant' => $this->strFrom($params, 'tenant'),
141+
'state' => $this->strFrom($params, 'state'),
142+
'inviter' => $inviterSub,
143+
'client_id' => $clientId,
144+
'initiate_login_uri' => $redirectURI !== '' ? $redirectURI : '/',
145+
'return_uri' => $returnUri,
71146
];
72147

73-
$queryString = http_build_query($request);
74-
return "https://wallet.{$this->config->getHelloDomain()}/invite?" . $queryString;
148+
// Remove nulls so http_build_query only serializes present fields
149+
$request = array_filter(
150+
$request,
151+
static fn($v) => $v !== null
152+
);
153+
154+
$queryString = http_build_query($request, '', '&', PHP_QUERY_RFC3986);
155+
156+
return 'https://wallet.' . $helloDomain . '/invite?' . $queryString;
75157
}
76158
}

‎src/Handler/Login.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ public function generateLoginUrl(): string
126126
$authResponse = $this->getAuthHelper()->createAuthRequest($request);
127127

128128
/** @var string $targetUri */
129-
$targetUri = $params['target_uri'];
129+
$targetUri = $params['target_uri'] ?? '/';
130130

131131
$this->getOIDCManager()->saveOidc(OIDC::fromArray([
132132
'nonce' => $authResponse['nonce'],

0 commit comments

Comments
 (0)