Skip to content

Commit

Permalink
Development: Fix issues with LTI 1.3 for open edx (#7931)
Browse files Browse the repository at this point in the history
  • Loading branch information
basak-akan authored Feb 26, 2024
1 parent 771eda7 commit fdbd2e1
Show file tree
Hide file tree
Showing 21 changed files with 281 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,15 @@ public static Optional<Lti13AgsClaim> from(OidcIdToken idToken) {
agsClaim.setScope(Collections.singletonList(Scopes.AGS_SCORE));
}

JsonElement lineItem = agsClaimJson.get("lineitem");
// For moodle lineItem is stored in lineitem claim, for edX it is in lineitems
JsonElement lineItem;
if (agsClaimJson.get("lineitem") == null) {
lineItem = agsClaimJson.get("lineitems");
}
else {
lineItem = agsClaimJson.get("lineitem");
}

if (lineItem != null) {
agsClaim.setLineItem(lineItem.getAsString());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ public class Lti13LaunchRequest {

private final Lti13AgsClaim agsClaim;

private final String clientRegistrationId;

public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) {
this.iss = ltiIdToken.getClaim("iss");
this.sub = ltiIdToken.getClaim("sub");
Expand All @@ -34,6 +36,7 @@ public Lti13LaunchRequest(OidcIdToken ltiIdToken, String clientRegistrationId) {
this.targetLinkUri = ltiIdToken.getClaim(Claims.TARGET_LINK_URI);

this.agsClaim = Lti13AgsClaim.from(ltiIdToken).orElse(null);
this.clientRegistrationId = clientRegistrationId;

Assert.notNull(iss, "Iss must not be empty in LTI 1.3 launch request");
Assert.notNull(sub, "Sub must not be empty in LTI 1.3 launch request");
Expand Down Expand Up @@ -66,4 +69,8 @@ public String getTargetLinkUri() {
public Lti13AgsClaim getAgsClaim() {
return agsClaim;
}

public String getClientRegistrationId() {
return clientRegistrationId;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@
import javax.persistence.Table;
import javax.validation.constraints.NotNull;

import de.tum.in.www1.artemis.domain.DomainObject;
import de.tum.in.www1.artemis.domain.Exercise;
import de.tum.in.www1.artemis.domain.User;
import de.tum.in.www1.artemis.domain.*;

/**
* Represents an LTI 1.3 Resource Link Launch.
Expand Down Expand Up @@ -38,6 +36,9 @@ public class LtiResourceLaunch extends DomainObject {
@ManyToOne
private Exercise exercise;

@ManyToOne
private LtiPlatformConfiguration ltiPlatformConfiguration;

/**
* Creates an LtiResourceLaunch entity from an LTI1.3 launch request to be saved in the database
*
Expand Down Expand Up @@ -97,4 +98,12 @@ public String getScoreLineItemUrl() {
public void setScoreLineItemUrl(String lineItemUrl) {
this.scoreLineItemUrl = lineItemUrl;
}

public LtiPlatformConfiguration getLtiPlatformConfiguration() {
return ltiPlatformConfiguration;
}

public void setLtiPlatformConfiguration(LtiPlatformConfiguration ltiPlatformConfiguration) {
this.ltiPlatformConfiguration = ltiPlatformConfiguration;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -64,4 +64,11 @@ default LtiPlatformConfiguration findLtiPlatformConfigurationWithEagerLoadedCour
@EntityGraph(type = LOAD, attributePaths = { "onlineCourseConfigurations" })
LtiPlatformConfiguration findWithEagerOnlineCourseConfigurationsById(long platformId);

/**
* Finds an LTI platform configuration by its client ID.
*
* @param clientId The registration ID.
* @return Optional of LtiPlatformConfiguration.
*/
Optional<LtiPlatformConfiguration> findByClientId(String clientId);
}
Original file line number Diff line number Diff line change
Expand Up @@ -172,10 +172,8 @@ private Lti13LaunchRequest launchRequestFrom(OidcIdToken ltiIdToken, String clie
public void onNewResult(StudentParticipation participation) {
Course course = courseRepository.findByIdWithEagerOnlineCourseConfigurationElseThrow(participation.getExercise().getCourseViaExerciseGroupOrCourseMember().getId());

LtiPlatformConfiguration ltiPlatformConfiguration = course.getOnlineCourseConfiguration().getLtiPlatformConfiguration();
ClientRegistration clientRegistration = onlineCourseConfigurationService.getClientRegistration(ltiPlatformConfiguration);
if (clientRegistration == null) {
log.error("Could not transmit score to external LMS for course {}: client registration not found", course.getTitle());
if (!course.isOnlineCourse()) {
log.error("Could not transmit score to external LMS for course {}:", course.getTitle());
return;
}

Expand All @@ -197,7 +195,12 @@ public void onNewResult(StudentParticipation participation) {

String concatenatedFeedbacks = result.get().getFeedbacks().stream().map(Feedback::getDetailText).collect(Collectors.joining(". "));

launches.forEach(launch -> submitScore(launch, clientRegistration, concatenatedFeedbacks, result.get().getScore()));
launches.forEach(launch -> {
LtiPlatformConfiguration returnPlatform = launch.getLtiPlatformConfiguration();
ClientRegistration returnClient = onlineCourseConfigurationService.getClientRegistration(returnPlatform);
submitScore(launch, returnClient, concatenatedFeedbacks, result.get().getScore());

});
});
}

Expand Down Expand Up @@ -303,6 +306,12 @@ private void createOrUpdateResourceLaunch(Lti13LaunchRequest launchRequest, User

launch.setExercise(exercise);
launch.setUser(user);

Optional<LtiPlatformConfiguration> ltiPlatformConfiguration = ltiPlatformConfigurationRepository.findByRegistrationId(launchRequest.getClientRegistrationId());
if (ltiPlatformConfiguration.isPresent()) {
launch.setLtiPlatformConfiguration(ltiPlatformConfiguration.get());
}

launchRepository.save(launch);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package de.tum.in.www1.artemis.web.rest.admin;

import java.util.UUID;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Value;
Expand All @@ -10,6 +12,7 @@

import de.tum.in.www1.artemis.domain.LtiPlatformConfiguration;
import de.tum.in.www1.artemis.repository.LtiPlatformConfigurationRepository;
import de.tum.in.www1.artemis.security.OAuth2JWKSService;
import de.tum.in.www1.artemis.security.annotations.EnforceAdmin;
import de.tum.in.www1.artemis.service.AuthorizationCheckService;
import de.tum.in.www1.artemis.service.connectors.lti.LtiDynamicRegistrationService;
Expand Down Expand Up @@ -37,6 +40,8 @@ public class AdminLtiConfigurationResource {

private final AuthorizationCheckService authCheckService;

private final OAuth2JWKSService oAuth2JWKSService;

/**
* Constructor to initialize the controller with necessary services.
*
Expand All @@ -45,10 +50,11 @@ public class AdminLtiConfigurationResource {
* @param authCheckService Service for authorization checks.
*/
public AdminLtiConfigurationResource(LtiPlatformConfigurationRepository ltiPlatformConfigurationRepository, LtiDynamicRegistrationService ltiDynamicRegistrationService,
AuthorizationCheckService authCheckService) {
AuthorizationCheckService authCheckService, OAuth2JWKSService oAuth2JWKSService) {
this.ltiPlatformConfigurationRepository = ltiPlatformConfigurationRepository;
this.ltiDynamicRegistrationService = ltiDynamicRegistrationService;
this.authCheckService = authCheckService;
this.oAuth2JWKSService = oAuth2JWKSService;
}

/**
Expand Down Expand Up @@ -101,6 +107,27 @@ public ResponseEntity<Void> updateLtiPlatformConfiguration(@RequestBody LtiPlatf
return ResponseEntity.ok().build();
}

/**
* Updates an existing LTI platform configuration.
*
* @param platform the updated LTI platform configuration to be saved.
* @return a {@link ResponseEntity} with status 200 (OK) if the update was successful,
* or with status 400 (Bad Request) if the provided platform configuration is invalid (e.g., missing ID)
*/
@PostMapping("lti-platform")
@EnforceAdmin
public ResponseEntity<Void> addLtiPlatformConfiguration(@RequestBody LtiPlatformConfiguration platform) {
log.debug("REST request to add new lti platform");

String clientRegistrationId = "artemis-" + UUID.randomUUID();
platform.setRegistrationId("artemis-" + UUID.randomUUID());

ltiPlatformConfigurationRepository.save(platform);
oAuth2JWKSService.updateKey(clientRegistrationId);

return ResponseEntity.ok().build();
}

/**
* Handles the dynamic registration process for LTI 1.3 platforms. It uses the provided OpenID
* configuration and an optional registration token.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?xml version="1.0" encoding="UTF-8"?>
<databaseChangeLog xmlns="http://www.liquibase.org/xml/ns/dbchangelog"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://www.liquibase.org/xml/ns/dbchangelog http://www.liquibase.org/xml/ns/dbchangelog/dbchangelog-latest.xsd">
<changeSet author="basak-akan" id="20240115231508">
<!-- Add a lti_platform_configuration_id column to 'lti_resource_launch' table -->
<addColumn tableName="lti_resource_launch">
<column name="lti_platform_configuration_id" type="bigint"/>
</addColumn>
<!-- Add a foreign key constraint to 'lti_resource_launch' table -->
<addForeignKeyConstraint
baseTableName="lti_resource_launch"
baseColumnNames="lti_platform_configuration_id"
constraintName="fk_lti_resource_launch_lti_platform"
referencedTableName="lti_platform_configuration"
referencedColumnNames="id"
onDelete="SET NULL"
/>
</changeSet>
</databaseChangeLog>
1 change: 1 addition & 0 deletions src/main/resources/config/liquibase/master.xml
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@
<include file="classpath:config/liquibase/changelog/20240122000000_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20240125174015_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20231126172632_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20240115231508_changelog.xml" relativeToChangelogFile="false"/>
<include file="classpath:config/liquibase/changelog/20240118175408_changelog.xml" relativeToChangelogFile="false"/>
<!-- NOTE: please use the format "YYYYMMDDhhmmss_changelog.xml", i.e. year month day hour minutes seconds and not something else! -->
<!-- we should also stay in a chronological order! -->
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@
<div class="d-flex">
<div class="flex-grow-1">
<div class="d-flex align-items-center">
<h2 id="jhi-lecture-heading" jhiTranslate="artemisApp.lti.edit.ltiPlatform">Edit LTI Platform Configuration</h2>
<h2 id="jhi-lecture-heading" jhiTranslate="artemisApp.lti.addOrEditLtiPlatform">Add or edit LTI Platform Configuration</h2>
</div>
</div>
</div>
<div>
<div class="form-group" [hidden]="!platform.id">
<div class="form-group" [hidden]="!platform?.id">
<label for="id" jhiTranslate="global.field.id">ID</label>
<input type="text" class="form-control" id="id" name="id" formControlName="id" readonly />
</div>
<div class="form-group mt-2">
<div class="form-group mt-2" [hidden]="!platform?.registrationId">
<label class="form-control-label" jhiTranslate="artemisApp.lti.registrationId" for="field_registrationId">Registration Id</label>
<jhi-help-icon text="artemisApp.lti.edit.registrationIdTooltip" />
<input type="text" class="form-control" name="registrationId" id="field_registrationId" formControlName="registrationId" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,9 @@ import { Component, OnInit } from '@angular/core';
import { ActivatedRoute, Router } from '@angular/router';
import { finalize } from 'rxjs';
import { FormControl, FormGroup } from '@angular/forms';
import { faBan, faSave } from '@fortawesome/free-solid-svg-icons';
import { faBan, faPlus, faSave } from '@fortawesome/free-solid-svg-icons';
import { LtiPlatformConfiguration } from 'app/admin/lti-configuration/lti-configuration.model';
import { LtiConfigurationService } from 'app/admin/lti-configuration/lti-configuration.service';
import { HttpClient } from '@angular/common/http';

@Component({
selector: 'jhi-edit-lti-configuration',
Expand All @@ -21,12 +20,12 @@ export class EditLtiConfigurationComponent implements OnInit {
// Icons
faBan = faBan;
faSave = faSave;
faPlus = faPlus;

constructor(
private route: ActivatedRoute,
private ltiConfigurationService: LtiConfigurationService,
private router: Router,
private http: HttpClient,
private alertService: AlertService,
) {}

Expand All @@ -35,33 +34,37 @@ export class EditLtiConfigurationComponent implements OnInit {
*/
ngOnInit() {
const platformId = this.route.snapshot.paramMap.get('platformId');
this.http.get<LtiPlatformConfiguration>(`api/admin/lti-platform/${platformId}`).subscribe({
next: (data) => {
this.platform = data;

this.platformConfigurationForm = new FormGroup({
id: new FormControl(this.platform?.id),
registrationId: new FormControl(this.platform?.registrationId),
originalUrl: new FormControl(this.platform?.originalUrl),
customName: new FormControl(this.platform?.customName),
clientId: new FormControl(this.platform?.clientId),
authorizationUri: new FormControl(this.platform?.authorizationUri),
tokenUri: new FormControl(this.platform?.tokenUri),
jwkSetUri: new FormControl(this.platform?.jwkSetUri),
});
},
error: (error) => {
this.alertService.error(error);
},
});
if (platformId) {
this.ltiConfigurationService.getLtiPlatformById(Number(platformId)).subscribe({
next: (data) => {
this.platform = data;
this.initializeForm();
},
error: (error) => {
this.alertService.error(error);
},
});
}
this.initializeForm();
}

/**
* Save the changes to the online course configuration
* Create or update lti platform configuration
*/
save() {
this.isSaving = true;
const platformConfiguration = this.platformConfigurationForm.getRawValue();
if (this.platform?.id) {
this.updateLtiConfiguration(platformConfiguration);
} else {
this.addLtiConfiguration(platformConfiguration);
}
}

/**
* Update existing platform configuration
*/
updateLtiConfiguration(platformConfiguration: any) {
this.ltiConfigurationService
.updateLtiPlatformConfiguration(platformConfiguration)
.pipe(
Expand All @@ -77,6 +80,25 @@ export class EditLtiConfigurationComponent implements OnInit {
});
}

/**
* Create new platform configuration
*/
addLtiConfiguration(platformConfiguration: any) {
this.ltiConfigurationService
.addLtiPlatformConfiguration(platformConfiguration)
.pipe(
finalize(() => {
this.isSaving = false;
}),
)
.subscribe({
next: () => this.onSaveSuccess(),
error: (error) => {
this.alertService.error(error);
},
});
}

/**
* Action on successful online course configuration or edit
*/
Expand All @@ -85,6 +107,19 @@ export class EditLtiConfigurationComponent implements OnInit {
this.navigateToLtiConfigurationPage();
}

private initializeForm() {
this.platformConfigurationForm = new FormGroup({
id: new FormControl(this.platform?.id),
registrationId: new FormControl(this.platform?.registrationId),
originalUrl: new FormControl(this.platform?.originalUrl),
customName: new FormControl(this.platform?.customName),
clientId: new FormControl(this.platform?.clientId),
authorizationUri: new FormControl(this.platform?.authorizationUri),
tokenUri: new FormControl(this.platform?.tokenUri),
jwkSetUri: new FormControl(this.platform?.jwkSetUri),
});
}

/**
* Returns to the lti configuration page
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,17 @@ <h4 class="modal-title">{{ 'artemisApp.lti.dynamicRegistrationUrl' | artemisTran
<span>{{ getDynamicRegistrationUrl() }}</span>
<jhi-copy-icon-button [copyText]="getDynamicRegistrationUrl()" />
</dd>
<dt style="display: flex; align-items: center">
<dt style="display: flex; align-items: center; justify-content: space-between; width: 100%">
<div class="modal-header" style="margin-bottom: 20px">
<h4 class="modal-title" style="margin-right: 2px">{{ 'artemisApp.lti.configuredPlatforms' | artemisTranslate }}</h4>
<jhi-help-icon text="artemisApp.lti.configuredPlatformsTooltip" />
</div>
<div class="modal-header" style="margin-bottom: 20px">
<a [routerLink]="['/admin', 'lti-configuration', 'new']" class="btn btn-primary">
<fa-icon [icon]="faPlus" />
<span jhiTranslate="artemisApp.lti13.addNewPlatform">Add New Platform Configuration</span>
</a>
</div>
</dt>
<div class="row">
<div class="col-12 mx-auto">
Expand Down
Loading

0 comments on commit fdbd2e1

Please sign in to comment.