Skip to content

Upstream pr/lti provider fixes #8783

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

Open
contidos wants to merge 4 commits into
base: 1.11.x
Choose a base branch
from
Open

Upstream pr/lti provider fixes #8783

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
acb4513
Plugin: LTI Provider: Fix OIDC state cookie lost in iframe by requiri…
robertosuarezcinfo Jul 28, 2026
b6b9fad
Plugin: LTI Provider: Fix tool type combo not updating when selected …
robertosuarezcinfo Jul 28, 2026
d4b8b31
Plugin: LTI Provider: Add search/autocomplete (select2) to the quiz/l…
robertosuarezcinfo Jul 28, 2026
29d4fc7
Plugin: LTI Provider: Add option to duplicate a registered platform
robertosuarezcinfo Jul 28, 2026
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
2 changes: 1 addition & 1 deletion plugin/lti_provider/db/lti13_cookie.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public function setCookie($name, $value, $exp = 3600, $options = []): self
// SameSite none and secure will be required for tools to work inside iframes
$sameSiteOptions = [
'samesite' => 'None',
'secure' => false,
'secure' => true,

@krista445 krista445 Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Español: Marcar la cookie como Secure es correcto, pero actualmente no restaura la validación de estado. getCookie devuelve directamente el valor state recibido cuando el nombre coincide, y Packback compara ese mismo valor con la solicitud; por tanto, una solicitud sin cookie almacenada también supera la comprobación. Elimina ese atajo de REQUEST y exige la cookie real o un estado de servidor ligado al navegador. Añade pruebas negativas para cookie ausente y estado distinto.

English: Marking the cookie Secure is correct, but it does not currently restore state validation. getCookie returns the received state directly when the name matches, while Packback compares that same value with the request; therefore, a request with no stored cookie also passes. Remove the REQUEST shortcut and require the real cookie or server-side state bound to the browser. Add negative tests for a missing cookie and a mismatched state.

'httponly' => true,
];

Expand Down
47 changes: 47 additions & 0 deletions plugin/lti_provider/duplicate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
<?php
/* For license terms, see /license.txt */

$cidReset = true;

require_once __DIR__.'/../../main/inc/global.inc.php';
use Chamilo\PluginBundle\Entity\LtiProvider\Platform;

require_once __DIR__.'/LtiProviderPlugin.php';

api_protect_admin_script();

if (!isset($_REQUEST['id'])) {

@krista445 krista445 Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Español: Esta acción que escribe en la base de datos acepta GET mediante REQUEST y no verifica ningún token CSRF. Una navegación externa con una sesión de administrador activa puede crear duplicados sin consentimiento. Exige POST, lee el id únicamente desde POST, valida Security::check_token con post y muestra la acción como un formulario con token.

English: This database-writing action accepts GET through REQUEST and verifies no CSRF token. An external navigation with an active administrator session can create duplicates without consent. Require POST, read the id only from POST, validate Security::check_token with post, and render the action as a token-bearing form.

api_not_allowed(true);
}

$platformId = (int) $_REQUEST['id'];

$plugin = LtiProviderPlugin::create();
$em = Database::getManager();

/** @var Platform $platform */
$platform = $em->find('ChamiloPluginBundle:LtiProvider\Platform', $platformId);

if (!$platform) {
api_not_allowed(true);
}

$newPlatform = new Platform();
$newPlatform->setIssuer($platform->getIssuer());
$newPlatform->setClientId($platform->getClientId());
$newPlatform->setAuthLoginUrl($platform->getAuthLoginUrl());
$newPlatform->setAuthTokenUrl($platform->getAuthTokenUrl());
$newPlatform->setKeySetUrl($platform->getKeySetUrl());
$newPlatform->setDeploymentId($platform->getDeploymentId());
$newPlatform->setKid($platform->getKid());
$newPlatform->setToolProvider($platform->getToolProvider());

$em->persist($newPlatform);

@krista445 krista445 Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P2] Español: El duplicado se guarda como registro activo antes de que el administrador confirme la edición. Si se abandona la redirección queda otra fila con el mismo emisor, client ID y deployment ID. El runtime indexa registros por client ID y también usa findOneBy, por lo que las identidades duplicadas se resuelven de forma ambigua. Precarga el formulario sin persistir y guarda solo tras un POST validado; además, aplica la unicidad que requieren esas búsquedas.

English: The clone is persisted as a live registration before the administrator confirms the edit. Abandoning the redirect leaves another row with the same issuer, client ID, and deployment ID. Runtime lookup indexes registrations by client ID and also uses findOneBy, so duplicate identities resolve ambiguously. Prefill the form without persisting and save only after a validated POST; also enforce the uniqueness required by those lookups.

$em->flush();

Display::addFlash(
Display::return_message($plugin->get_lang('PlatformDuplicated'), 'success')
);

header('Location: '.api_get_path(WEB_PLUGIN_PATH).'lti_provider/edit.php?id='.$newPlatform->getId());
exit;
2 changes: 2 additions & 0 deletions plugin/lti_provider/lang/english.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
$strings['PlatformName'] = 'LMS (issuer)';
$strings['PlatformEdited'] = 'Platform details edited';
$strings['PlatformDeleted'] = 'Platform deleted';
$strings['PlatformDuplicated'] = 'Platform duplicated';
$strings['Duplicate'] = 'Duplicate';
$strings['ClientId'] = 'Client ID';
$strings['LaunchUrl'] = 'Launch URL';
$strings['LoginUrl'] = 'Login URL';
Expand Down
2 changes: 2 additions & 0 deletions plugin/lti_provider/lang/french.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
$strings['PlatformName'] = 'LMS (émetteur)';
$strings['PlatformEdited'] = 'Détails de la plateforme modifiés';
$strings['PlatformDeleted'] = 'Plateforme supprimée';
$strings['PlatformDuplicated'] = 'Plateforme dupliquée';
$strings['Duplicate'] = 'Dupliquer';
$strings['ClientId'] = 'Client ID';
$strings['LaunchUrl'] = 'Lancer l\'URL';
$strings['LoginUrl'] = 'URL de connexion';
Expand Down
2 changes: 2 additions & 0 deletions plugin/lti_provider/lang/spanish.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
$strings['PlatformName'] = 'LMS (emisor)';
$strings['PlatformEdited'] = 'Detalles de la plataforma editados';
$strings['PlatformDeleted'] = 'Plataforma eliminada';
$strings['PlatformDuplicated'] = 'Plataforma duplicada';
$strings['Duplicate'] = 'Duplicar';
$strings['ClientId'] = 'ID de cliente';
$strings['LaunchUrl'] = 'URL de lanzamiento';
$strings['LoginUrl'] = 'URL de inicio de sesión';
Expand Down
3 changes: 0 additions & 3 deletions plugin/lti_provider/src/Form/FrmAdd.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ public function build(): void
[
'quiz' => $plugin->get_lang('Quizzes'),
'lp' => $plugin->get_lang('Learnpaths'),
],
[
'onclick' => 'selectToolProvider(this.value)',
]
);

Expand Down
3 changes: 0 additions & 3 deletions plugin/lti_provider/src/Form/FrmEdit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@ public function build()
[
'quiz' => $plugin->get_lang('Quizzes'),
'lp' => $plugin->get_lang('Learnpaths'),
],
[
'onclick' => 'selectToolProvider(this.value)',
]
);

Expand Down
24 changes: 17 additions & 7 deletions plugin/lti_provider/view/add.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,26 @@
</div>
</div>
<script>
$(function() {
if ($("input[name='tool_type']").length > 0) {
var toolType = $("input[name='tool_type']:checked").val();
selectToolProvider(toolType)
}
});
function selectToolProvider(tool) {
$(".sbox-tool").each(function() {
if ($(this).hasClass('select2-hidden-accessible')) {
$(this).select2('destroy');
}
});
$(".sbox-tool").attr('disabled', 'disabled');
$(".select-tool").hide();
$("#select-"+tool).show();
$("#sbox-tool-"+tool).removeAttr('disabled');
var $select = $("#sbox-tool-"+tool);
$select.removeAttr('disabled');
$select.select2({ width: '100%' });
}
$(function() {
if ($("input[name='tool_type']").length > 0) {
var toolType = $("input[name='tool_type']:checked").val();
selectToolProvider(toolType);
$("input[name='tool_type']").on('change', function() {
selectToolProvider($(this).val());
});
}
});
</script>
3 changes: 3 additions & 0 deletions plugin/lti_provider/view/provider_admin.tpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
<a href="{{ _p.web_plugin }}lti_provider/edit.php?{{ url_params }}">
{{ 'edit.png'|img(22, 'Edit'|get_lang) }}
</a>
<a href="{{ _p.web_plugin }}lti_provider/duplicate.php?{{ url_params }}">
{{ 'copy.png'|img(22, 'Duplicate'|get_plugin_lang('LtiProviderPlugin')) }}
</a>
<a href="{{ _p.web_plugin }}lti_provider/delete.php?{{ url_params }}">
{{ 'delete.png'|img(22, 'Delete'|get_lang) }}
</a>
Expand Down
Loading