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

Development: Allow direct login via LTI #9215

Merged
merged 2 commits into from
Aug 31, 2024

Conversation

maximiliansoelch
Copy link
Member

@maximiliansoelch maximiliansoelch commented Aug 14, 2024

Checklist

General

Server

  • Important: I implemented the changes with a very good performance and prevented too many (unnecessary) database calls.
  • I strictly followed the server coding and design guidelines.
  • I added multiple integration tests (Spring) related to the features (with a high test coverage).
  • I added pre-authorization annotations according to the guidelines and checked the course groups for all new REST Calls (security).
  • I documented the Java code using JavaDoc style.

Motivation and Context

Description

  • Add LTI Filter after JWTFilter so authentication info is available at launch again

Steps for Testing

Prerequisites:

  • 1 Instructor
  • 2 Students
  • 1 Programming Exercise with Complaints enabled
  1. Log in to Artemis
  2. Navigate to Course Administration
  3. ...

Testserver States

Note

These badges show the state of the test servers.
Green = Currently available, Red = Currently locked
Click on the badges to get to the test servers.







Review Progress

Performance Review

  • I (as a reviewer) confirm that the client changes (in particular related to REST calls and UI responsiveness) are implemented with a very good performance
  • I (as a reviewer) confirm that the server changes (in particular related to database calls) are implemented with a very good performance

Code Review

  • Code Review 1
  • Code Review 2

Manual Tests

  • Test 1
  • Test 2

Test Coverage

Screenshots

@github-actions github-actions bot added the server Pull requests that update Java code. (Added Automatically!) label Aug 14, 2024
@maximiliansoelch maximiliansoelch temporarily deployed to artemis-test3.artemis.cit.tum.de August 14, 2024 10:55 — with GitHub Actions Inactive
@maximiliansoelch maximiliansoelch temporarily deployed to artemis-test3.artemis.cit.tum.de August 14, 2024 13:59 — with GitHub Actions Inactive
@maximiliansoelch maximiliansoelch temporarily deployed to artemis-test3.artemis.cit.tum.de August 14, 2024 16:08 — with GitHub Actions Inactive
@maximiliansoelch maximiliansoelch temporarily deployed to artemis-test3.artemis.cit.tum.de August 14, 2024 19:42 — with GitHub Actions Inactive
@maximiliansoelch maximiliansoelch temporarily deployed to artemis-test3.artemis.cit.tum.de August 14, 2024 20:53 — with GitHub Actions Inactive
@github-actions github-actions bot added the tests label Aug 16, 2024
@maximiliansoelch maximiliansoelch changed the base branch from develop to bugfix/fix-lti-authentication August 19, 2024 15:30
@maximiliansoelch maximiliansoelch added the stacked-pr PR that depends on another PR label Aug 19, 2024
Copy link

There hasn't been any activity on this pull request recently. Therefore, this pull request has been automatically marked as stale and will be closed if no further activity occurs within seven days. Thank you for your contributions.

@github-actions github-actions bot added the stale label Aug 28, 2024
@krusche krusche marked this pull request as ready for review August 31, 2024 19:51
@krusche krusche requested a review from a team as a code owner August 31, 2024 19:51
@krusche krusche merged commit 072747f into bugfix/fix-lti-authentication Aug 31, 2024
17 of 21 checks passed
@krusche krusche deleted the feature/lti/allow-direct-login branch August 31, 2024 19:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:LTI ready for review server Pull requests that update Java code. (Added Automatically!) stacked-pr PR that depends on another PR stale tests
Projects
Status: Merged
Development

Successfully merging this pull request may close these issues.

2 participants