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.

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

OidcAuthenticator generates a new nonce to id_token instead of returning nonce from auth request #154

Open
armatronic opened this issue Jan 17, 2023 · 6 comments · May be fixed by #188
Open

Comments

@armatronic
Copy link

Hi,

I've been using your LTI 1.3 core library in a project (as a platform) for a while with some success. However, I've recently noticed something unexpected in how it handles platform-side OIDC authentication.

IMS Global's documentation states that the nonce is supposed to be passed as is from the auth request into the auth response's id token sent to the LTI tool: however, the \OAT\Library\Lti1p3Core\Security\Oidc\OidcAuthenticator class by default contains a message payload builder which generates a new nonce and injects it into the ID token, thus causing some LTI tools to reject auth responses created by this library.

I've been able to work around this issue, but I believe that since this is a fundamental aspect of OIDC security checking that either:

  • I'm missing something in how OidcAuthenticator instances are created (but I didn't see any other documentation on this, and no other nonce generator implementations exist within this library)
  • the library is mistakenly generating a new nonce for the auth response
  • I and/or the LTI tool provider is mistaken as to the purpose of the nonce

Thank you for your time and advice, your work is appreciated.

@melyouz
Copy link

melyouz commented Mar 2, 2023

We're facing a similar issue.

According to the spec, the nonce claim must be passed through intact in the authentication request. See, for example, https://www.imsglobal.org/spec/security/v1p0/#id-token.

nonce
REQUIRED. String value used to associate a Tool session with an ID Token, and to mitigate replay attacks. The nonce value is a case-sensitive string.

Which in practice means that a Tool (that generated a nonce) must be able to associate that with an ID Token.

But, as the library generates a new one for each authentication request: https://github.com/oat-sa/lib-lti1p3-core/blob/master/src/Message/Payload/Builder/MessagePayloadBuilder.php#L110

That's not possible... and the process fails in the Tool side when validating the nonce.

@armatronic please, can you share the workaround you're applying for this?

@martinclaus
Copy link

Same issue here. Our tool fails to validate the ID Token following the OIDC specifications, since the nonce differs from the value in the initial authentication request.

IMO, any tool properly implementing nonce validation according to the OICD specifications must fail to validate any ID Token issued by this library.

@armatronic
Copy link
Author

can you share the workaround you're applying for this?

When building the auth response I provided a custom implementation of OAT\Library\Lti1p3Core\Security\Nonce\NonceGeneratorInterface which simply returned a \OAT\Library\Lti1p3Core\Security\Nonce\Nonce matching the nonce from the request.

Based on the example code from from https://oat-sa.github.io/doc-lti1p3/libraries/lib-lti1p3-core/doc/message/platform-originating-messages/#oidc-authentication-redirection-automation:

<?php

use OAT\Library\Lti1p3Core\Message\Payload\Builder\MessagePayloadBuilder;
use OAT\Library\Lti1p3Core\Registration\RegistrationRepositoryInterface;
use OAT\Library\Lti1p3Core\Security\Nonce\NonceGeneratorInterface;
use OAT\Library\Lti1p3Core\Security\Oidc\OidcAuthenticator;
use OAT\Library\Lti1p3Core\Security\Oidc\Server\OidcAuthenticationRequestHandler;
use OAT\Library\Lti1p3Core\Security\User\UserAuthenticatorInterface;
use Psr\Http\Message\ServerRequestInterface;

/** @var RegistrationRepositoryInterface $registrationRepository */
$registrationRepository = ...

/** @var UserAuthenticatorInterface $userAuthenticator */
$userAuthenticator = ...

/** @var ServerRequestInterface $request */
$request = ...

// Extract nonce from request somehow, e.g.:
$nonce = $request->getQueryParams()['nonce'];

// Create the OIDC authentication handler
$handler = new OidcAuthenticationRequestHandler(
    new OidcAuthenticator(
        $registrationRepository,
        $userAuthenticator,
        new MessagePayloadBuilder(
            new class ($nonce) implements NonceGeneratorInterface {
                // generate() simply returns $nonce
            };
        ),
    )
);

// Redirect response from OIDC authentication (via form POST)
$response = $handler->handle($request);

@isl-dbouman
Copy link

This workaround worked for me, seems like something that should be part of the library.

@mk-kialo
Copy link

mk-kialo commented Aug 2, 2023

We were also initially confused about the library returning a new nonce and decided that it doesn't make sense. So we implemented the same work around.

For future readers: Beware that you can't simply return the string value of $nonce, but you need to wrap it in an instance of OAT\Library\Lti1p3Core\Security\Nonce\Nonce.

@marcovdb
Copy link

marcovdb commented May 22, 2024

Thanks! I have been tearing my hair out for the past hours trying to figure out why the validation was failing.

I might attempt a PR later to fix this properly. It's a great library otherwise (and the only PHP LTI 1.3 implementation I could find that actually implements everything) and it's a shame they're not getting this rather crucial detail right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants