Global search: Resolve and forward the user access context to Pyris - #13380
Global search: Resolve and forward the user access context to Pyris#13380Nayer-kotry wants to merge 10 commits into
Global search: Resolve and forward the user access context to Pyris#13380Conversation
End-to-End Test Results
Test Strategy: Two-phase execution
❌ Failed Tests (Phase 2)
Overall: ❌ E2E: real (non-flaky) test failure 🔗 Workflow Run · 📊 Test Report Phase 1 · 📊 Test Report Phase 2 |
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@Nayer-kotry Static inspection found two independent compilation blockers in the production and test sources, so the access-context behavior cannot currently build or execute. Both calls need to use the current user-course-role APIs instead of the removed group-era interfaces.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughLecture search and asynchronous global-search answers now receive server-resolved Iris access contexts. The context contains role-based course scopes or unrestricted administrator access. Integration tests verify authorization mapping and consistent propagation. ChangesIris access-context propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Principal
participant IrisGlobalSearchResource
participant IrisAccessContextService
participant PyrisConnectorService
participant Pyris
Principal->>IrisGlobalSearchResource: submit search request
IrisGlobalSearchResource->>IrisAccessContextService: resolveAccessContext(user)
IrisAccessContextService-->>IrisGlobalSearchResource: return PyrisAccessContextDTO
IrisGlobalSearchResource->>PyrisConnectorService: forward request and context
PyrisConnectorService->>Pyris: send serialized request
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.java (1)
73-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate the
ask()javadoc to mention access-context resolution.The
@param principaljavadoc on line 78 says the principal is "used to route the WebSocket response." The implementation now also uses it to resolve the caller'sPyrisAccessContextDTO(line 87) and forwards that context toexecuteGlobalSearchIrisAnswer(line 93). Update the javadoc to reflect this added responsibility, since a future reader could miss that this endpoint also enforces role-based course scoping.📝 Proposed javadoc update
* `@param` requestDTO the request containing the query and result limit - * `@param` principal the authenticated user (used to route the WebSocket response) + * `@param` principal the authenticated user; used to route the WebSocket response and to resolve the role-based + * course access forwarded to Pyris * `@return` the {`@link` ResponseEntity} with status {`@code` 202 (Accepted)}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.java` around lines 73 - 95, Update the ask() method’s `@param` principal Javadoc to mention that the authenticated user is also used to resolve the Pyris access context for role-based course scoping, while retaining the existing WebSocket response routing description.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.java`:
- Around line 45-50: Update resolveAccessContext in IrisAccessContextService so
its findAllAccessibleCoursesForUser call matches the CourseRepository API:
either add the required Set<String> groups overload to CourseRepository or pass
a non-null user ID to the existing (Long, boolean) method and preserve separate
role-membership evaluation. Ensure the code compiles and retains the intended
access filtering.
---
Nitpick comments:
In `@src/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.java`:
- Around line 73-95: Update the ask() method’s `@param` principal Javadoc to
mention that the authenticated user is also used to resolve the Pyris access
context for role-based course scoping, while retaining the existing WebSocket
response routing description.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0f5bef00-c891-4098-b956-148a492ef4a0
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/iris/api/IrisLectureSearchApi.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/GlobalSearchLectureRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisAccessContextDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisGlobalSearchAnswerRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisLectureSearchRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.javasrc/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.javasrc/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.java (1)
45-70: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the
findAllAccessibleCoursesForUsercall: argument type mismatch breaks the build.Line 50 calls
courseRepository.findAllAccessibleCoursesForUser(user.getGroups(), false).user.getGroups()returnsSet<String>. CI failures (CodeQL, Build, Bean Instantiation Check, Server Code Style, Server Tests) all report "cannot find symbol" at this exact line, confirming no matching overload accepts aSet<String>as the first argument. This was already flagged on a previous commit, where the reviewer noted the repository method requires aLong userId. The issue is still present in this version.Also related to the same fix: once
coursesresolves correctly, the loop at lines 54-64 callsauthCheckService.isAtLeastEditorInCourseandisAtLeastTeachingAssistantInCourseonce per course. If these checks query course membership per call, this causes one query per accessible course. Loading the user with preloaded course roles (as previously suggested) would let this loop use an in-memory role index instead of per-course queries.🔧 Suggested direction
- var courses = courseRepository.findAllAccessibleCoursesForUser(user.getGroups(), false); + var courses = courseRepository.findAllAccessibleCoursesForUser(user.getId(), false);Verify the exact repository method signature before applying; adjust the argument accordingly if it takes groups directly.
Run the following script to confirm the real signature and existing call sites:
#!/bin/bash # Description: Locate CourseRepository.findAllAccessibleCoursesForUser declaration and all call sites. fd -e java CourseRepository | xargs rg -n -B2 -A6 'findAllAccessibleCoursesForUser' echo "---- call sites ----" rg -n -B3 -A3 'findAllAccessibleCoursesForUser' --type=java🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.java` around lines 45 - 70, Update resolveAccessContext in IrisAccessContextService to call CourseRepository.findAllAccessibleCoursesForUser with the user identifier required by its actual method signature, rather than user.getGroups(). Then reuse preloaded user course-role data for the editor and teaching-assistant checks so the loop avoids per-course membership queries, while preserving the existing access-category assignments.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.java`:
- Line 68: Update the user lookup calls in IrisGlobalSearchResource.search() and
ask() to use an existing UserRepository method that accepts the principal login
and eagerly loads the required groups and authorities; alternatively, add the
missing getUserWithGroupsAndAuthorities method to UserRepository with matching
behavior.
---
Duplicate comments:
In
`@src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.java`:
- Around line 45-70: Update resolveAccessContext in IrisAccessContextService to
call CourseRepository.findAllAccessibleCoursesForUser with the user identifier
required by its actual method signature, rather than user.getGroups(). Then
reuse preloaded user course-role data for the editor and teaching-assistant
checks so the loop avoids per-course membership queries, while preserving the
existing access-category assignments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6597f0a7-1857-4ce9-81cf-6c9a82e4c2b2
📒 Files selected for processing (10)
src/main/java/de/tum/cit/aet/artemis/iris/api/IrisLectureSearchApi.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/GlobalSearchLectureRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisAccessContextDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisGlobalSearchAnswerRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisLectureSearchRequestDTO.javasrc/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.javasrc/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.javasrc/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (8)
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/GlobalSearchLectureRequestDTO.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisAccessContextDTO.java
- src/main/java/de/tum/cit/aet/artemis/iris/api/IrisLectureSearchApi.java
- src/test/java/de/tum/cit/aet/artemis/core/connector/IrisRequestMockProvider.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisGlobalSearchAnswerRequestDTO.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/PyrisConnectorService.java
- src/main/java/de/tum/cit/aet/artemis/iris/service/pyris/dto/search/PyrisLectureSearchRequestDTO.java
- src/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java (1)
202-203: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the editor and teaching-assistant role scopes.
The test creates teaching-assistant, editor, and instructor assignments. It only verifies
courseIds,staffCourseIds, andstudentCourseIds.Add direct assertions for the editor and teaching-assistant course-ID lists. This will detect a removed, swapped, or merged role scope.
Based on PR objectives, the access context contains role-grouped course IDs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java` around lines 202 - 203, Extend the assertions in IrisGlobalSearchIntegrationTest to directly verify the editor and teaching-assistant course-ID lists after the role assignments are created. Assert each role-specific list contains exactly the corresponding course IDs, preserving the existing assertions for courseIds, staffCourseIds, and studentCourseIds.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java`:
- Around line 202-203: Extend the assertions in IrisGlobalSearchIntegrationTest
to directly verify the editor and teaching-assistant course-ID lists after the
role assignments are created. Assert each role-specific list contains exactly
the corresponding course IDs, preserving the existing assertions for courseIds,
staffCourseIds, and studentCourseIds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 963072c3-76ac-42cb-ba25-6255f50ab671
📒 Files selected for processing (3)
src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.javasrc/main/java/de/tum/cit/aet/artemis/iris/web/IrisGlobalSearchResource.javasrc/test/java/de/tum/cit/aet/artemis/iris/IrisGlobalSearchIntegrationTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/de/tum/cit/aet/artemis/iris/service/IrisAccessContextService.java
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@Nayer-kotry The two prior compilation blockers are fixed at this head. However, the prior course-role-loading concern remains unresolved: both endpoints load only authorities, causing access-context resolution to execute up to two role-existence queries per accessible course.
marlonnienaber
left a comment
There was a problem hiding this comment.
Thx for incorporating my CRs
7490ca0 to
7fa43d7
Compare
Pomodorka3
left a comment
There was a problem hiding this comment.
--- TESTED on TS3 ---
All prerequisites have been fulfilled.
✅ As the student, search for the content of the unreleased unit: it must not appear. (user: artemis_test_user_1240)


✅ As the instructor (staff of course X), run the same search: the unreleased unit must appear. (user: artemis_test_user_1241)


✅ As the admin, run the same search: the unreleased unit must appear. (user: ge43fis)



7fa43d7 to
309a674
Compare
d7ee6f7
Introduce PyrisAccessContextDTO (role-grouped course IDs, request timestamp, and an admin unrestricted flag) and add an optional accessContext field to the lecture-search and answer request DTOs sent to Pyris. All new fields are optional with NON_EMPTY serialization, so an absent context leaves the wire identical to today and old Iris is unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add IrisAccessContextService, which resolves the requesting user's accessible courses grouped by role (editor, teaching assistant, student, and the combined staff bypass group) together with the request timestamp, and marks admins as unrestricted. The lecture-search and answer endpoints resolve this context per request and forward it to Pyris via PyrisConnectorService; GlobalSearchLectureRequestDTO carries the optional client course filter. Non-admins are scoped to their courses while admins are sent an unrestricted context, so Pyris can apply course scoping and the staff/admin release-visibility bypass consistently with Artemis.
… roles Add integration tests asserting the access context Artemis resolves and sends to Pyris is consistent with AuthorizationCheckService: a user's per-course role classification (student, teaching assistant, editor, instructor, and a course they cannot access), the unrestricted admin context, and that the lecture-search and answer endpoints forward the same context for one user. Adds a request-capturing mockSearchLectures variant so tests can assert on the sent access context. Absent role lists are omitted on the wire (NON_EMPTY) and treated as empty.
…xt parameter Run Spotless on the access-context DTO, service, API, connector, and the integration test, and add the missing @PARAM accessContext on searchLectures so Checkstyle passes. No behaviour change.
…course API Develop replaced the string-group access model with course-role relationships, removing User.getGroups() and UserRepository.getUserWithGroupsAndAuthorities. Load the requesting user with getUserWithAuthorities() and resolve accessible courses by user id (findAllAccessibleCoursesForUser(userId, isAdmin)); the per-course role classification via AuthorizationCheckService is unchanged. The integration test now enrolls the user per course with CourseRole and reloads the user with course roles for the assertions, instead of assigning string groups.
Both search endpoints loaded the user with authorities only, so each per-course role check in resolveAccessContext fell back to a userCourseRole exists query - up to two per accessible course. Load the user with getUserWithCourseRolesAndAuthorities so the role checks read the in-memory course-role map, resolving the access context in a constant number of queries regardless of the number of accessible courses.
IrisLectureSearchApi forwarded a null access context to Pyris. The access-context contract treats an absent context as the safe-default visibility filter (released content only), so Hyperion's editor-only quiz generation dropped unreleased lecture units the editor is allowed to see. Resolve the requesting user's role-grouped access via IrisAccessContextService and forward it, matching the global-search endpoints. Correct the stale comment that claimed a null context meant no permission filter. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…context lists The record component names already produce the JSON keys through the parameter-names module, so the @JsonProperty annotations were redundant and are removed. A compact constructor now defaults every role-grouped course-ID list to an empty list so the access context never holds null lists.
… filter The type discriminator uses Weaviate word tokenization, so a type = "lecture" filter also matched lecture_unit rows and pulled them into the lecture branch, which has no release-date guard. Add type != "lecture_unit" to that branch so units are gated by the lecture-unit branch instead, and make the client Lectures filter request lecture,lecture_unit so released units still appear (grouped as one chip like communication). Add a Weaviate integration test that reproduces the leak and verifies the guard.
"answer_post" indexes as the tokens ["answer", "post"], so type Equal "post" also matched answer posts in buildPostDisjunct and in the queryPostProperties test helper, where it produced id-dependent false failures. Apply the same NotEqual guard the lecture branch already uses, and cover both colliding pairs in the tokenization test, renamed now that it is not lecture-specific.
d7ee6f7 to
e4e8fb9
Compare
Warning
Must be tested together with the paired Iris PR (ls1intum/edutelligence#712) — the access context is a cross-repo wire contract. Deploy both branches to the same test server before reviewing.
Summary
Artemis resolves the requesting user's role-grouped course access and forwards it to Pyris on both global-search endpoints, so Iris can scope and filter lecture results exactly like the Artemis UI: course scope for everyone, plus the staff/admin bypass that lets authorized users see their own unreleased lecture content. Almost entirely a server change. The client side is limited to the global search modal (4 files, about 38 changed lines): the Lectures filter now also covers
lecture_unit, and removing a grouped filter chip clears every underlying type in that group rather than only the one it is labelled with. No exam-mode or programming-exercise impact.Checklist
General
Server
Motivation and Context
Global search must show each user exactly what they can already reach in the Artemis UI. The role-based access model is enforced for the direct Weaviate search (
GlobalSearchResource), but the Iris pipeline path only had course scope and a role-blind release filter, so admins and staff saw less than the UI. This PR makes Artemis the single source of truth: it resolves the user's role-grouped access and hands it to Pyris, which applies it as an opaque filter.Description
PyrisAccessContextDTO(new): role-grouped course IDs (courseIds,editorCourseIds,taCourseIds,studentCourseIds,staffCourseIds), the request timestampnow, and an adminunrestrictedflag. Added as an optional field on the lecture-search and answer request DTOs sent to Pyris, serializedNON_EMPTYso an absent context leaves the wire identical to today.IrisAccessContextService.resolveAccessContext(new): admins get a present context withunrestricted = true; non-admins get their accessible courses classified into editor / TA / student, withstaff = editor + TA,now = ZonedDateTime.now(), andunrestricted = false.IrisGlobalSearchResourcelecture-search and search-answer) load the user with groups, resolve the context, and forward it throughIrisLectureSearchApi/PyrisConnectorService.GlobalSearchLectureRequestDTOcarries the optional clientcourseIdsfilter.unrestrictedcontext rather than a null one.Steps for Testing
Prerequisites:
Testserver States
You can manage test servers using Helios. Check environment statuses in the environment list. To deploy to a test server, go to the CI/CD page, find your PR or branch, and trigger the deployment.
Review Progress
Code Review
Manual Tests
Test Coverage
Note: Some tests in the Test job did not pass (
failure). Coverage below may be partial.Client
Server
Last updated: 2026-08-25 20:37:41 UTC