[FEATURE] LTI: Add AGS consumer services and deep linking content selection by Saaweel · Pull Request #11835 · ILIAS-eLearning/ILIAS

Saaweel · 2026-07-29T12:54:07Z

Summary

This PR completes the LTI 1.3 consumer-side Assignment and Grade Services (AGS) flow and makes Dynamic Registration / Deep Linking content selection usable from an LTI consumer object.

Adds AGS Line Items and Results endpoints, persistence for tool-created line items, score maximum support, launch endpoint claims, and database migrations.
Validates AGS access against the requested token scope and provider client ID; supports RSA and JWK key validation.
Publishes provider-mode scores, resolves LTI user identifiers consistently, validates score payloads, and maps submitted/fully graded activity states to learning-progress status.
Enables AGS automatically when requested during Dynamic Registration.
Adds a Deep Linking selection action for consumer settings, preserves the selected target link URI for subsequent launches, validates redirect URIs, and applies creation permissions to the content-selection flow.
Adds the required LTI language labels across maintained language files.

Validation

git diff --check upstream/release_11...HEAD passes.
php -l passes for all 22 changed PHP files.
PHP-CS-Fixer ran across all 86 PHP files in LTI Consumer and LTI Provider; it applied one spacing correction.
Rebasing onto current release_11 (e33d29e) completed without unresolved conflicts.
PHPUnit was not run: components/ILIAS/LTIConsumer/phpunit.xml references test/ilModulesLTIConsumerSuite.php, which is absent from this checkout.
Browser/E2E validation remains required with a configured LTI 1.3 tool (for example Moodle): Dynamic Registration, Deep Linking selection and launch, AGS line-item CRUD, score publication, results retrieval, and learning-progress updates.

Reviewer Focus

AGS authorization and client isolation for line items, results, and scores.
Dynamic Registration scope detection and the database migration for persisted line items.
Deep Linking redirect validation, selected target-link propagation, and permission checks.

…linking

(cherry picked from commit 9f2e884)

(cherry picked from commit 0252213) # Conflicts: # components/ILIAS/LTIConsumer/resources/ltidlreturn.php

(cherry picked from commit 0be2f32)

(cherry picked from commit 892e28a)

(cherry picked from commit 13d7d22)

AGS get-score results were returning the stored usr_ident (e.g. email) as userId instead of the LTI sub sent during launch, causing tools to fail to match grades to users. Additionally, scores were returned on a 0-1 scale instead of the original scale submitted by the tool.

mjansenDatabay · 2026-07-29T19:20:56Z


        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'iss', ilObjLTIConsumer::getIliasHttpPath()) . "\n";
-        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'target_link_uri', $this->object->getProvider()->getProviderUrl()) . "\n";
+        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'target_link_uri', htmlspecialchars($this->getTargetLinkUri(), ENT_QUOTES)) . "\n";


Can't we use a Refinery transformation instead?

mjansenDatabay · 2026-07-29T19:21:25Z

        $output = '<form id="lti_launch_form" name="lti_launch_form" action="' . $this->object->getProvider()->getInitiateLogin() . '" method="post" target="' . $target . '" encType="application/x-www-form-urlencoded">';
        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'iss', ilObjLTIConsumer::getIliasHttpPath()) . "\n";
-        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'target_link_uri', $this->object->getProvider()->getProviderUrl()) . "\n";
+        $output .= sprintf('<input type="hidden" name="%s" value="%s" />', 'target_link_uri', htmlspecialchars($this->getTargetLinkUri(), ENT_QUOTES)) . "\n";


Can't we use a Refinery transformation instead?

Is it ensured that all the existing values here surrounding the new lines are already trustworthy (checked, escaped)?

mjansenDatabay · 2026-07-29T19:26:35Z

+                $DIC->http()->saveResponse($DIC->http()->response()->withStatus(400));
+                $DIC->http()->sendResponse();
+                $DIC->http()->close();
+                exit;


The exit is not needed here.

mjansenDatabay · 2026-07-29T19:27:02Z

+        );
+        $DIC->http()->sendResponse();
+        $DIC->http()->close();
+        exit;


The exit is not needed here.

mjansenDatabay · 2026-07-29T19:27:27Z

+        $DIC->http()->close();
+    }
+
+    exit;


The exit is not needed here.

mjansenDatabay · 2026-07-30T09:12:04Z

+                return true;
+            }
+
+            if (strpos($filter, '@') === false && strpos($candidate, $filter . '@') === 0) {


We can use str_starts_with here.

mjansenDatabay · 2026-07-30T09:12:33Z

+            . ' AND usr_id = ' . $ilDB->quote($userId, 'integer');
+        $res = $ilDB->query($query);
+        while ($row = $ilDB->fetchAssoc($res)) {
+            $user = new ilObjUser($userId);


Move new ilObjUser($userId) out of the loop, since it does not depend on $row.

mjansenDatabay · 2026-07-30T09:14:19Z

        return $res;
    }

+    protected function startDeepLinkingCmd()


Suggested change

protected function startDeepLinkingCmd()

protected function startDeepLinkingCmd(): never

mjansenDatabay · 2026-07-30T09:15:29Z

+    /**
+     * Build the "Deep linking" UI (button + modal with iframe).
+     *
+     * @param string $target_gui_class GUI class that has startDeepLinkingCmd()


This is a contradiction to the explicitly used PHP type.

mjansenDatabay · 2026-07-30T09:19:17Z

+ilSession::set('lti13_login_data', $data);
+
+if ($isDlMode) {
+    if ($deploymentId === null || $refId <= 0) {


$deploymentId is always an int here, so the check is IMO obsolete.

mjansenDatabay

Dear @Saaweel,

Thank you a lot for this important contribution to the LTI component of ILIAS. This is a huge step forward.

Besides my inline comments (above), I mainly (but not exclusively) focussed on the AGS-related changes in my review remarks. Other members of the Technical Board will (most probably) also add some review feedback here.

At first, completing the consumer-side AGS surface is a significant piece of work, and the result is
noticeably more usable than what release_11 ships today. I looked into the AGS specification (https://www.imsglobal.org/spec/lti-ags/v2p0) myself a while back, and I'm glad that you've really improved things here.
Some of my remarks not only address the changes in this PR but point to the current state given after this PR is integrated.

A few coding style/architecture issues I would like to mention:

You introduced a lot of superglobals. Our guidelines recommend: "The code contains no $_GET / $_POST / $_REQUEST / $_SESSION"
You introduced a lot of static methods, which do not align with our efforts to reduce the amount of global state in ILIAS in the last years.
There are some new $DIC usages, even inline in the middle of the method body.
A few PHP types in class methods are missing or not well-documented (with PHPDocs).
docs/development/php-coding-style.md requires properties and variables in underscore-case. The new code is consistently camelCase ($contextId, $itemId, $clientId, $scoreMaximum, $objIdOrPseudo, $lineItemUrl, …).

Please answer the following questions:

Score POST answers 200 instead of the 204 required by AGS 2.0 §3.4.3.1, with a comment pointing at a Moodle bug. Is that bug still present in currently supported Moodle versions? If so, could the deviation become a per-provider setting rather than a global one, so conformant tools get a conformant platform?
ilObjLTIConsumer::getDeepLinkingUserIdentifier() / getDeepLinkingUserEmail() return the raw e-mail address whenever it passes FILTER_VALIDATE_EMAIL, which bypasses the provider's configured privacy_ident entirely. Why is the configured pseudonymisation not honoured in the deep-linking flow?
ilLTIConsumerGradeSynchronizationGUI.php extends the multi-actor grade report from hasOutcomesAccess() to hasOutcomesAccess() || hasWriteAccess(), and ilLTIConsumerGradeSynchronizationTableGUI.php now resolves ilObjUser::_lookupFullname(). Together this shows real names of all participants to everyone with write permission. What is the data-protection rationale, and how does it interact with a provider configured for pseudonymous identifiers?
From my understanding, ltiauth.php drops $provider->getContentItemUrl() == $redirectUri from the deep-linking condition, so any login initiation carrying a deployment_id in the message hint is routed into the content-selection flow. What prevents a regular resource-link launch from ending up there?

Please consider the following suggestions. You do not need to follow those, but please indicate
shortly why you prefer to do otherwise:

Model activityProgress and gradingProgress as PHP enums. They currently exist as string literals duplicated across isValidActivityProgress(), isValidGradingProgress(), the LP mapping in Scores and the mapping in ilLPStatusLtiOutcome, which is exactly the kind of duplication that lets the Failed mapping drift (see change requests).
Introduce a LineItem value object plus a repository with a constructor-injected ilDBInterface, and let the four resource classes be thin HTTP adapters over it. As it stands, the AGS classes and ilLPStatusLtiOutcome contain 13 global $DIC statements and hand-concatenated SQL inside HTTP handlers and inside an abstract base class, so none of the new logic can be unit-tested. Recent components (e.g. MetaData, Certificate, ...) are good references for the repository/value-object split.
Give the access token a type: checkTool(): ?object and object $token push property access on an untyped stdClass into five call sites, and getClientIdFromToken() is copied verbatim into three classes. A small AccessToken value object with getClientId() and getScopes() would remove all three copies.
Declare the array shapes (@return array{...} / @param).
Drop the runtime tableExists('lti_consumer_lineitems') guards. A database update step guarantees the schema. The guards turn a broken installation into a silent 500 or an empty result instead of an error.
Consider not using exception codes as HTTP status codes ($response->setCode($code >= 400 && $code < 600 ? $code : 500), repeated in four classes). A small set of dedicated exception types, or returning a response object, keeps the mapping explicit and testable.

Please implement the following changes:

Make the Line Item service conformant to AGS 2.0 §3.2:
- From my understanding, POST must answer 400 when label is missing or blank (§3.2.7) and when scoreMaximum is missing or not strictly greater than 0 (§3.2.8). ilLTIConsumerGradeServiceLineItems.php (in line 152-154) silently substitutes 'Line Item <id>' and 1, so a malformed tool request creates a line item the tool did not ask for.
- From my understanding, POST must answer 404 when resourceLinkId does not denote a resource link that exists in the context and is owned by the same tool (§3.2.9). It is currently stored unchecked.
- When limit truncates a container response, Link: <...>; rel="next" MUST be sent (§3.2.4 for line items, §3.3.6 for results). ilLTIConsumerGradeServiceLineItems.php (line 231) and ilLTIConsumerGradeServiceResults.php (line 194) just array_slice().
- DELETE on a line item backed by an ILIAS object must either delete the line item or answer 405. ilLTIConsumerGradeServiceLineItem.php resets scoreMaximum to 1 and answers 200, so the tool is told the deletion succeeded while the line item is still advertised by GET lineitems.
- GET <lineitem>/results must return the results of that line item (§3.3.3, §3.3.5). ilLTIConsumerGradeServiceResults.php (line 93-95) returns [] unconditionally for tool-created line items, even though the Scores endpoint accepts scores for them, which also makes the score_maximum it reads on line 91 dead.
Make the Score service conformant to AGS 2.0 §3.4:
- From my understanding, §3.4.9 requires that a score whose timestamp is older than the newest one on record MUST NOT update the result. ilLTIConsumerGradeServiceScores.php inserts every payload and ilLPStatus::writeStatus() overwrites the learning progress unconditionally, so a retried or delayed tool request can move a learner backwards.
- §3.4.4 requires that an absent or null scoreGiven clears the previously stored value score. ilLTIConsumerGradeServiceScores.php only writes when $result !== null, so the old lti_consumer_results row survives.
- §3.4.8 defines gradingProgress: Failed as "the grading could not complete", i.e. a grading error, not a failed learner. ilLTIConsumerGradeServiceScores.php and ilLPStatusLtiOutcome.php map it to LP_STATUS_FAILED_NUM, which puts a wrong result into the learning progress and, via ilLPStatusLtiOutcome, also into every aggregating container.
- §3.4.4 explicitly requires supporting scoreGiven greater than scoreMaximum. The derived LP percentage on ilLTIConsumerGradeServiceScores.php (line 181) can therefore exceed 100 and must be IMO clamped.
Unify how the line item URL is built. ilLTIConsumerGradeServiceLineItem::getServiceRootUrl() reconstructs the base URL from $_SERVER['HTTP_HOST'] and $_SERVER['SCRIPT_NAME'], while ilObjLTIConsumer::buildLaunchParametersLTI13() uses getIliasHttpPath(). AGS §3.3.4.3 requires the id a tool reads back and the lineitem claim it received at launch to be identical, and §3.2.3 asks for a stable URL.
If possible, restore validation-before-persistence in ltiauth.php. The PR moves ilSession::set('lti13_login_data', $data) ahead of the message-hint parsing, so unvalidated request data is written to the session even for requests that are rejected two lines later.
If possible, validate the redirect target in ltiauth.php. $url = "../../../" . base64_decode($redirect_uri) builds a redirect from tool-supplied data with no validation, which could IMO be an open redirect and allows path traversal out of the intended directory. As far as I understand the code, there is exactly the right check in ilObjLTIConsumerGUI.php.

On how to proceed:
-The spec-conformance remarks on line items and scores and security-related remarks MUST be resolved prior to merging, as they are the ones I consider blocking for a mid-release integration.

The architecture suggestions SHOULD be addressed soon on the trunk, probably not until Friday (currently planned release/integration). It would be good to agree on fixing those before you start reworking.

Could you give us feedback on how long you expect the change requests to take?

Best regards,
@mjansenDatabay

on behalf of the Technical Board

Saaweel added 18 commits July 29, 2026 14:42

[FIX] LTI: Add AGS consumer line item and score services

41d8725

[FIX] LTI: Publish AGS scores in provider mode

e6d13e9

[FIX] LTI: Update language labels

df58abf

Merge branch 'ilias11_LTI_fix_deep_linking' into ilias11_LTI_ags_deep…

7e4da05

…linking

[FIX] LTI: Add content selection language labels

35e90a9

[FIX] LTI: Add lost $logger var

0723440

[FIX] LTI: Fix deep linking user selection

089b001

[FIX] LTI: Restore attended setter on consumer result

bd46d97

[FIX] LTI: Fix deep linking redirect handling

06728aa

(cherry picked from commit 9f2e884)

[FIX] LTI: Preserve deep linking target link URI

b6fdc47

(cherry picked from commit 0252213) # Conflicts: # components/ILIAS/LTIConsumer/resources/ltidlreturn.php

[FIX] LTI: Resolve AGS score user identifiers

ac965ac

(cherry picked from commit 0be2f32)

[FIX] LTI: Accept AGS score denominator

d0ae088

(cherry picked from commit 892e28a)

[FIX] LTI: Share AGS user identifier matching

6cf5502

(cherry picked from commit 13d7d22)

[FIX] LTI: Allow deep linking content selection

59ca25d

[FIX] LTI: Improve AGS gradebook results

1a72920

[FIX] LTI: Improve consumer creation error handling

471e2f8

[FIX] LTI: Complete fully graded submitted activities

a62b2ae

Saaweel self-assigned this Jul 29, 2026

Saaweel added bugfix php Pull requests that update Php code improvement translations Pull requests that propose changes to ILIAS language files. labels Jul 29, 2026

Saaweel requested a review from rfalkenstein July 29, 2026 13:00

Saaweel added 2 commits July 29, 2026 15:41

[FIX] LTI: Format consumer provider form

b826a58

[FIX] LTI: Sort language entries

4584211

Saaweel force-pushed the ilias11_LTI_ags_deeplinking branch from 6747652 to 4584211 Compare July 29, 2026 13:41

mjansenDatabay reviewed Jul 30, 2026

View reviewed changes

mjansenDatabay requested changes Jul 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[FEATURE] LTI: Add AGS consumer services and deep linking content selection - #11835

[FEATURE] LTI: Add AGS consumer services and deep linking content selection#11835
Saaweel wants to merge 20 commits into
ILIAS-eLearning:release_11from
surlabs:ilias11_LTI_ags_deeplinking

Saaweel commented Jul 29, 2026 •

edited

Loading

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 29, 2026

Uh oh!

mjansenDatabay Jul 30, 2026

Uh oh!

mjansenDatabay Jul 30, 2026

Uh oh!

mjansenDatabay Jul 30, 2026

Uh oh!

mjansenDatabay Jul 30, 2026

Uh oh!

mjansenDatabay Jul 30, 2026

Uh oh!

mjansenDatabay left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	protected function startDeepLinkingCmd()
	protected function startDeepLinkingCmd(): never

Uh oh!

Conversation

Saaweel commented Jul 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Validation

Reviewer Focus

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mjansenDatabay left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Saaweel commented Jul 29, 2026 •

edited

Loading