Skip to content

Conversation

lukas-heinrich
Copy link
Contributor

@lukas-heinrich lukas-heinrich commented Oct 17, 2025

Hi everyone,

This PR proposes changes to the ECS component to integrate OIDC authentication. It implements the feature described in the wiki entry ECS Support for OpenID Connect enabling authentication on external LMS platforms using OpenID Connect.

During development, we encountered several issues that limited ECS functionality and complicated implementation. Below are the problems we addressed in this PR, along with their fixes. For better traceability, each fix is in a separate commit.

Problem Description Report Commit
Missing dependencyjumbojett/openid-connect-phpin trunk See #9560 See #9560
Exceptions during bootstrapping inopenidconnect.php Incomplete container initialization causes this error. The problem is resolved by using theentry_pointmethod instead ofilInitialisation::initILIAS(), similar toshib_login.php. 1abbeab1
Incomplete data inecs_part_settings Theecs_part_settingstable stores information about imported ECS participants. Entries are created only when settings are first saved, not during object import. This causes errors such as missing array keys and course labels. 41215 508a5a65
Redirect to course fails Clicking the remote link redirects participants to the dashboard instead of the course. This occurs because the link uses the formatgoto.php/crs/123, whileLegacyGotoHandlerexpectsgoto.php?target=crs_123. 75d16219
Error message not displayed when saving invalid settings Validation runs only when the ECS server is enabled. When errors occur, the participant overview appears instead of the form with the error message. 43256 253d0012

I look forward to comments and feedback on this PR. The changes were reviewed in an internal review process and approved by @thojou .

Best Regards
@lukas-heinrich

@mjansenDatabay
Copy link
Contributor

mjansenDatabay commented Oct 17, 2025

@lukas-heinrich IMO I already fixed the "entry point" issue some days ago.

Will you include the JF-approved composer dependency jumbojett/openid-connect-php in this PR, or should we (@thojou and/or I) first integrate PR #9560 ?

@mjansenDatabay mjansenDatabay added improvement php Pull requests that update Php code labels Oct 17, 2025
@lukas-heinrich
Copy link
Contributor Author

Hi @mjansenDatabay ,

I would prefer #9560 to be merged first.

@thojou
Copy link
Contributor

thojou commented Oct 17, 2025

Hey @sKarki999 and @jeph864,

I assigned this PR to you, as you are currently listed as Code Authority for Webservices. All the applied changes are within this module.

We would highly appreciate your feedback on our changes.

Best,
@thojou

@thojou thojou assigned Akumatic and unassigned sKarki999 and jeph864 Oct 17, 2025
@thojou
Copy link
Contributor

thojou commented Oct 17, 2025

Hey @sKarki999 and @jeph864,

Sorry for the disturbance. I mixed up Authorities and removed you from the PR.

Best,
@thojou

@thojou thojou requested a review from Akumatic October 17, 2025 11:05
@mjansenDatabay
Copy link
Contributor

@lukas-heinrich jumbojett/openid-connect-php is now part of the trunk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

improvement php Pull requests that update Php code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants