Skip to content

Commit c80a0ed

Browse files
birdcarbirdcar
committed
Fix greptile-reported refresh bug and add test to prevent regressions
1 parent ca8e53b commit c80a0ed

3 files changed

Lines changed: 281 additions & 0 deletions

File tree

‎lib/CookieSession.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,10 @@ public function refresh(array $options = [])
8484
// Use the refresh token to get new authentication tokens
8585
try {
8686
$refreshedAuth = $this->userManagement->authenticateWithRefreshToken(
87+
WorkOS::getClientId(),
8788
$authResult->refreshToken,
89+
null,
90+
null,
8891
$organizationId
8992
);
9093

‎lib/Resource/PaginatedResource.php‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
<?php
2+
3+
namespace WorkOS\Resource;
4+
5+
/**
6+
* Class PaginatedResource
7+
*
8+
* A flexible paginated resource that supports multiple access patterns:
9+
* 1. Bare destructuring (backwards compatible): [$before, $after, $data] = $result
10+
* 2. Named destructuring: ["users" => $users, "after" => $after] = $result
11+
* 3. Fluent property access: $result->users, $result->after, $result->before
12+
*
13+
* This class standardizes pagination across all WorkOS resources while maintaining
14+
* backwards compatibility with existing code.
15+
*/
16+
class PaginatedResource implements \ArrayAccess, \IteratorAggregate
17+
{
18+
/**
19+
* @var ?string Before cursor for pagination
20+
*/
21+
private $before;
22+
23+
/**
24+
* @var ?string After cursor for pagination
25+
*/
26+
private $after;
27+
28+
/**
29+
* @var array The paginated data items
30+
*/
31+
private $data;
32+
33+
/**
34+
* @var string The key name for the data array (e.g., 'users', 'directories')
35+
*/
36+
private $dataKey;
37+
38+
/**
39+
* PaginatedResource constructor.
40+
*
41+
* @param ?string $before Before cursor
42+
* @param ?string $after After cursor
43+
* @param array $data Array of resource objects
44+
* @param string $dataKey The key name for accessing the data
45+
*/
46+
public function __construct(?string $before, ?string $after, array $data, string $dataKey)
47+
{
48+
$this->before = $before;
49+
$this->after = $after;
50+
$this->data = $data;
51+
$this->dataKey = $dataKey;
52+
}
53+
54+
/**
55+
* Construct a PaginatedResource from an API response
56+
*
57+
* @param array $response The API response containing 'data', 'list_metadata', etc.
58+
* @param string $resourceClass The fully qualified class name of the resource type
59+
* @param string $dataKey The key name for the data array (e.g., 'users', 'directories')
60+
* @return self
61+
*/
62+
public static function constructFromResponse(array $response, string $resourceClass, string $dataKey): self
63+
{
64+
$data = [];
65+
list($before, $after) = \WorkOS\Util\Request::parsePaginationArgs($response);
66+
67+
foreach ($response["data"] as $responseData) {
68+
\array_push($data, $resourceClass::constructFromResponse($responseData));
69+
}
70+
71+
return new self($before, $after, $data, $dataKey);
72+
}
73+
74+
/**
75+
* Magic getter for fluent property access
76+
*
77+
* @param string $name Property name
78+
* @return mixed
79+
*/
80+
public function __get(string $name)
81+
{
82+
if ($name === 'before') {
83+
return $this->before;
84+
}
85+
86+
if ($name === 'after') {
87+
return $this->after;
88+
}
89+
90+
if ($name === 'data' || $name === $this->dataKey) {
91+
return $this->data;
92+
}
93+
94+
return null;
95+
}
96+
97+
/**
98+
* ArrayAccess: Check if offset exists
99+
*
100+
* @param mixed $offset
101+
* @return bool
102+
*/
103+
#[\ReturnTypeWillChange]
104+
public function offsetExists($offset): bool
105+
{
106+
// Support numeric indices for bare destructuring
107+
if (is_int($offset)) {
108+
return $offset >= 0 && $offset <= 2;
109+
}
110+
111+
// Support named keys for named destructuring
112+
return in_array($offset, ['before', 'after', 'data', $this->dataKey], true);
113+
}
114+
115+
/**
116+
* ArrayAccess: Get value at offset
117+
*
118+
* @param mixed $offset
119+
* @return mixed
120+
*/
121+
#[\ReturnTypeWillChange]
122+
public function offsetGet($offset)
123+
{
124+
// Support numeric indices for bare destructuring: [0 => before, 1 => after, 2 => data]
125+
if ($offset === 0) {
126+
return $this->before;
127+
}
128+
129+
if ($offset === 1) {
130+
return $this->after;
131+
}
132+
133+
if ($offset === 2) {
134+
return $this->data;
135+
}
136+
137+
// Support named keys for named destructuring
138+
if ($offset === 'before') {
139+
return $this->before;
140+
}
141+
142+
if ($offset === 'after') {
143+
return $this->after;
144+
}
145+
146+
if ($offset === 'data' || $offset === $this->dataKey) {
147+
return $this->data;
148+
}
149+
150+
return null;
151+
}
152+
153+
/**
154+
* ArrayAccess: Set value at offset (not supported)
155+
*
156+
* @param mixed $offset
157+
* @param mixed $value
158+
* @return void
159+
*/
160+
#[\ReturnTypeWillChange]
161+
public function offsetSet($offset, $value): void
162+
{
163+
throw new \BadMethodCallException('PaginatedResource is immutable');
164+
}
165+
166+
/**
167+
* ArrayAccess: Unset offset (not supported)
168+
*
169+
* @param mixed $offset
170+
* @return void
171+
*/
172+
#[\ReturnTypeWillChange]
173+
public function offsetUnset($offset): void
174+
{
175+
throw new \BadMethodCallException('PaginatedResource is immutable');
176+
}
177+
178+
/**
179+
* IteratorAggregate: Get iterator for the data array
180+
*
181+
* @return \ArrayIterator
182+
*/
183+
public function getIterator(): \Traversable
184+
{
185+
return new \ArrayIterator($this->data);
186+
}
187+
188+
/**
189+
* Magic isset for property checking
190+
*
191+
* @param string $name
192+
* @return bool
193+
*/
194+
public function __isset(string $name): bool
195+
{
196+
return in_array($name, ['before', 'after', 'data', $this->dataKey], true);
197+
}
198+
}

‎tests/WorkOS/CookieSessionTest.php‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,4 +94,84 @@ public function testLoadSealedSessionReturnsValidCookieSession()
9494

9595
$this->assertInstanceOf(CookieSession::class, $cookieSession);
9696
}
97+
98+
public function testRefreshPassesCorrectParametersToAuthenticateWithRefreshToken()
99+
{
100+
// REGRESSION TEST: Verify that CookieSession.refresh() passes all 5 parameters
101+
// to authenticateWithRefreshToken(), not just 2. The clientId parameter must be
102+
// included from WorkOS::getClientId() (see CookieSession.php:86-92)
103+
104+
$organizationId = "org_01H7X1M4TZJN5N4HG4XXMA1234";
105+
106+
// Create a mock UserManagement to verify method calls
107+
$userManagementMock = $this->getMockBuilder(UserManagement::class)
108+
->onlyMethods(['authenticateWithSessionCookie', 'authenticateWithRefreshToken', 'sealSession'])
109+
->getMock();
110+
111+
// Mock authenticateWithSessionCookie to return a successful authentication
112+
$authResponseData = [
113+
'authenticated' => true,
114+
'access_token' => 'test_access_token',
115+
'refresh_token' => 'test_refresh_token',
116+
'session_id' => 'session_123',
117+
'user' => [
118+
'object' => 'user',
119+
'id' => 'user_123',
120+
'email' => 'test@test.com',
121+
'first_name' => 'Test',
122+
'last_name' => 'User',
123+
'email_verified' => true,
124+
'created_at' => '2021-01-01T00:00:00.000Z',
125+
'updated_at' => '2021-01-01T00:00:00.000Z'
126+
]
127+
];
128+
$authResponse = Resource\SessionAuthenticationSuccessResponse::constructFromResponse($authResponseData);
129+
$userManagementMock->method('authenticateWithSessionCookie')
130+
->willReturn($authResponse);
131+
132+
// CRITICAL ASSERTION: Verify authenticateWithRefreshToken is called with exactly 5 parameters
133+
$refreshResponseData = [
134+
'access_token' => 'new_access_token',
135+
'refresh_token' => 'new_refresh_token',
136+
'user' => [
137+
'object' => 'user',
138+
'id' => 'user_123',
139+
'email' => 'test@test.com',
140+
'first_name' => 'Test',
141+
'last_name' => 'User',
142+
'email_verified' => true,
143+
'created_at' => '2021-01-01T00:00:00.000Z',
144+
'updated_at' => '2021-01-01T00:00:00.000Z'
145+
]
146+
];
147+
$refreshResponse = Resource\AuthenticationResponse::constructFromResponse($refreshResponseData);
148+
149+
$userManagementMock->expects($this->once())
150+
->method('authenticateWithRefreshToken')
151+
->with(
152+
$this->identicalTo(WorkOS::getClientId()), // clientId from config
153+
$this->identicalTo('test_refresh_token'), // refresh token
154+
$this->identicalTo(null), // ipAddress
155+
$this->identicalTo(null), // userAgent
156+
$this->identicalTo($organizationId) // organizationId
157+
)
158+
->willReturn($refreshResponse);
159+
160+
$userManagementMock->method('sealSession')
161+
->willReturn('new_sealed_session');
162+
163+
// Execute refresh with the mocked UserManagement
164+
$cookieSession = new CookieSession(
165+
$userManagementMock,
166+
$this->sealedSession,
167+
$this->cookiePassword
168+
);
169+
170+
[$response, $newSealedSession] = $cookieSession->refresh([
171+
'organizationId' => $organizationId
172+
]);
173+
174+
// If we reach here without the mock throwing an exception, the test passes
175+
$this->assertInstanceOf(Resource\SessionAuthenticationSuccessResponse::class, $response);
176+
}
97177
}

0 commit comments

Comments
 (0)