Skip to content

Commit

Permalink
Fix OAuth2 redirects when reusing state
Browse files Browse the repository at this point in the history
  • Loading branch information
thekid committed Nov 3, 2024
1 parent 9573ed9 commit e7a82d0
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 10 deletions.
6 changes: 6 additions & 0 deletions ChangeLog.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ Web Authentication change log

## ?.?.? / ????-??-??

## 5.2.1 / 2024-11-03

* Fixed OAuth2 implementation to redirect to the correct target URL when
reusing state from a previous authorization flow.
(@thekid)

## 5.2.0 / 2024-07-17

* Merged PR #31: Make it possible to change the session namespace (CAS)
Expand Down
12 changes: 3 additions & 9 deletions src/main/php/web/auth/oauth/OAuth2Flow.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -107,15 +107,9 @@ public function authenticate($request, $response, $session) {
// Start authorization flow to acquire an access token
$server= $request->param('state');
if (null === $stored || null === $server) {

// Reuse state
if (isset($stored['state'])) {
$state= $stored['state'];
} else {
$state= bin2hex($this->rand->bytes(16));
$session->register($this->namespace, ['state' => $state, 'target' => (string)$uri]);
$session->transmit($response);
}
$state= $stored['state'] ?? bin2hex($this->rand->bytes(16));
$session->register($this->namespace, ['state' => $state, 'target' => (string)$uri]);
$session->transmit($response);

// Redirect the user to the authorization page
$params= [
Expand Down
3 changes: 2 additions & 1 deletion src/test/php/web/auth/unittest/OAuth2FlowTest.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -183,8 +183,9 @@ public function reuses_state_when_previous_redirect_incomplete() {
$session= (new ForTesting())->create();
$session->register('oauth2::flow', ['state' => 'REUSED_STATE', 'target' => self::SERVICE]);

$this->authenticate($fixture, '/', $session);
$this->authenticate($fixture, '/new', $session);
Assert::equals('REUSED_STATE', $session->value(self::SNS)['state']);
Assert::equals('http://localhost/new', $session->value(self::SNS)['target']);
}

#[Test]
Expand Down

0 comments on commit e7a82d0

Please sign in to comment.