Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: #160 redirect to the actual redirect_uri after verifying that it matches the registered tool host #199

Merged
merged 1 commit into from
Jan 28, 2025
Merged

fix: #160 redirect to the actual redirect_uri after verifying that it matches the registered tool host #199

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 25 additions & 1 deletion src/Security/Oidc/OidcAuthenticator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
use OAT\Library\Lti1p3Core\Message\Payload\Builder\MessagePayloadBuilder;
use OAT\Library\Lti1p3Core\Message\Payload\Builder\MessagePayloadBuilderInterface;
use OAT\Library\Lti1p3Core\Message\Payload\LtiMessagePayloadInterface;
use OAT\Library\Lti1p3Core\Registration\RegistrationInterface;
use OAT\Library\Lti1p3Core\Registration\RegistrationRepositoryInterface;
use OAT\Library\Lti1p3Core\Security\Jwt\Parser\Parser;
use OAT\Library\Lti1p3Core\Security\Jwt\Parser\ParserInterface;
Expand Down Expand Up @@ -99,6 +100,8 @@ public function authenticate(ServerRequestInterface $request): LtiMessageInterfa
throw new LtiBadRequestException('Invalid message hint');
}

$redirectUri = $this->createToolRedirectUrl($registration, $oidcRequest);

$authenticationResult = $this->authenticator->authenticate(
$registration,
$oidcRequest->getParameters()->getMandatory('login_hint')
Expand Down Expand Up @@ -128,7 +131,7 @@ public function authenticate(ServerRequestInterface $request): LtiMessageInterfa
$payload = $this->builder->buildMessagePayload($registration->getPlatformKeyChain());

return new LtiMessage(
$originalToken->getClaims()->getMandatory(LtiMessagePayloadInterface::CLAIM_LTI_TARGET_LINK_URI),
$redirectUri,
[
'id_token' => $payload->getToken()->toString(),
'state' => $oidcRequest->getParameters()->getMandatory('state')
Expand All @@ -153,4 +156,25 @@ private function sanitizeClaims(array $claims): array

return $claims;
}

private function createToolRedirectUrl(
RegistrationInterface $registration,
LtiMessageInterface $oidcRequest
): string {
$uri = $oidcRequest->getParameters()->getMandatory('redirect_uri');
$host = parse_url($uri, PHP_URL_HOST);
/**
* redirect_uri must match one of the values associated with the tool's registration
* however, the library does not support multiple redirect_uris per registration,
* and relies on the launch URL to represent the tool's redirect_uri
*
* in order to support multiple redirect_uris, while still keeping the authentication
* relatively safe, the redirect_uri's host is compared against the pre-registered launch URL's host
*/
if (!$host || $host !== parse_url($registration->getTool()->getLaunchUrl() ?? '', PHP_URL_HOST)) {
throw new LtiBadRequestException('Invalid redirect_uri');
}

return $uri;
}
}
120 changes: 117 additions & 3 deletions tests/Unit/Security/Oidc/OidcAuthenticatorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ public function testAuthenticationSuccess(): void
sprintf('http://platform.com/init?%s', http_build_query([
'login_hint' => 'login_hint',
'lti_message_hint' => $messageHint->toString(),
'state' => 'state'
'state' => 'state',
'redirect_uri' => 'http://tool.com/redirect',
]))
);

Expand All @@ -112,6 +113,10 @@ public function testAuthenticationSuccess(): void
'nonce',
$idToken->getClaims()->get(LtiMessagePayloadInterface::CLAIM_NONCE)
);
$this->assertEquals(
'http://tool.com/redirect',
$result->getUrl()
);
}

public function testAuthenticationSuccessWithUserIdentityClaimsSanitization(): void
Expand Down Expand Up @@ -145,7 +150,8 @@ public function testAuthenticationSuccessWithUserIdentityClaimsSanitization(): v
sprintf('http://platform.com/init?%s', http_build_query([
'login_hint' => 'login_hint',
'lti_message_hint' => $messageHint->toString(),
'state' => 'state'
'state' => 'state',
'redirect_uri' => 'http://tool.com/redirect',
]))
);

Expand All @@ -169,6 +175,113 @@ public function testAuthenticationSuccessWithUserIdentityClaimsSanitization(): v
$this->assertEquals('userPicture', $idToken->getClaims()->get(LtiMessagePayloadInterface::CLAIM_USER_PICTURE));
}

public function testAuthenticationEmptyRedirectUri(): void
{
$this->expectException(LtiException::class);
$this->expectExceptionMessage('OIDC authentication failed: Missing mandatory redirect_uri');

$registration = $this->createTestRegistration();

$messageHint = $this->buildJwt(
[
MessagePayloadInterface::HEADER_KID => $registration->getToolKeyChain()->getIdentifier()
],
[
MessagePayloadInterface::CLAIM_REGISTRATION_ID => $registration->getIdentifier()
],
$registration->getToolKeyChain()->getPrivateKey()
);

$this->repositoryMock
->expects($this->once())
->method('find')
->with($registration->getIdentifier())
->willReturn($registration);

$request = $this->createServerRequest(
'GET',
sprintf('http://platform.com/init?%s', http_build_query([
'login_hint' => 'login_hint',
'lti_message_hint' => $messageHint->toString(),
'state' => 'state',
]))
);

$this->subject->authenticate($request);
}

public function testAuthenticationMalformedRedirectUri(): void
{
$this->expectException(LtiBadRequestException::class);
$this->expectExceptionMessage('Invalid redirect_uri');

$registration = $this->createTestRegistration();

$messageHint = $this->buildJwt(
[
MessagePayloadInterface::HEADER_KID => $registration->getToolKeyChain()->getIdentifier()
],
[
MessagePayloadInterface::CLAIM_REGISTRATION_ID => $registration->getIdentifier()
],
$registration->getToolKeyChain()->getPrivateKey()
);

$this->repositoryMock
->expects($this->once())
->method('find')
->with($registration->getIdentifier())
->willReturn($registration);

$request = $this->createServerRequest(
'GET',
sprintf('http://platform.com/init?%s', http_build_query([
'login_hint' => 'login_hint',
'lti_message_hint' => $messageHint->toString(),
'state' => 'state',
'redirect_uri' => 'tool.com/redirect'
]))
);

$this->subject->authenticate($request);
}

public function testAuthenticationMismatchingRedirectUri(): void
{
$this->expectException(LtiBadRequestException::class);
$this->expectExceptionMessage('Invalid redirect_uri');

$registration = $this->createTestRegistration();

$messageHint = $this->buildJwt(
[
MessagePayloadInterface::HEADER_KID => $registration->getToolKeyChain()->getIdentifier()
],
[
MessagePayloadInterface::CLAIM_REGISTRATION_ID => $registration->getIdentifier()
],
$registration->getToolKeyChain()->getPrivateKey()
);

$this->repositoryMock
->expects($this->once())
->method('find')
->with($registration->getIdentifier())
->willReturn($registration);

$request = $this->createServerRequest(
'GET',
sprintf('http://platform.com/init?%s', http_build_query([
'login_hint' => 'login_hint',
'lti_message_hint' => $messageHint->toString(),
'state' => 'state',
'redirect_uri' => 'http://tool.malicious.com/redirect'
]))
);

$this->subject->authenticate($request);
}

public function testAuthenticationFailureOnExpiredMessageHint(): void
{
$this->expectException(LtiBadRequestException::class);
Expand Down Expand Up @@ -274,7 +387,8 @@ public function testAuthenticationFailureOnAuthenticationFailure(): void
'GET',
sprintf('http://platform.com/init?%s', http_build_query([
'lti_message_hint' => $messageHint->toString(),
'login_hint' => 'login_hint'
'login_hint' => 'login_hint',
'redirect_uri' => 'http://tool.com/redirect',
]))
);

Expand Down