Skip to content

Commit 8ea745b

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

File tree

2 files changed

+83
-0
lines changed

2 files changed

+83
-0
lines changed

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

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' => '[email protected]',
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' => '[email protected]',
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)