Skip to content

Conversation

@ottenhoff
Copy link
Contributor

@ottenhoff ottenhoff commented Jan 21, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved authorization controls to ensure users only see assessment group releases they have permission to access. Non-administrator users are now restricted to groups they belong to within the assessment release scope.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 21, 2026

Walkthrough

This change implements runtime authorization filtering for group-based release enrollments in the SectionAwareServiceHelperImpl class. When users lack the AUTHZ_ASSESSMENT_ALL_GROUPS permission, the code restricts release group filtering to only the intersection of site groups the user belongs to and those required by the published assessment.

Changes

Cohort / File(s) Summary
Authorization filtering for group-based enrollments
samigo/samigo-services/src/java/org/sakaiproject/tool/assessment/integration/helper/integrated/SectionAwareServiceHelperImpl.java
Added conditional authorization check in getGroupReleaseEnrollments method that limits releaseGroupIdsSet to user's accessible groups when AUTHZ_ASSESSMENT_ALL_GROUPS permission is absent

Possibly related PRs

Suggested reviewers

  • ern
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'SAK-52287 Samigo honor assessment.all.groups for group releases' accurately and specifically describes the main change: adding authorization checks for the assessment.all.groups permission in group release functionality.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
kernel/kernel-impl/src/main/sql/mysql/sakai_realm.sql (1)

727-735: Remove duplicate assessment.all.groups grant (Line 727 vs Line 735).

The same realm/role/function is inserted twice, which will fail on the (REALM_KEY, ROLE_KEY, FUNCTION_KEY) PK during schema load.

🛠️ Proposed fix (remove the earlier duplicate)
-INSERT INTO SAKAI_REALM_RL_FN VALUES((select REALM_KEY from SAKAI_REALM where REALM_ID = '!site.template'), (select ROLE_KEY from SAKAI_REALM_ROLE where ROLE_NAME = 'maintain'), (select FUNCTION_KEY from SAKAI_REALM_FUNCTION where FUNCTION_NAME = 'assessment.all.groups'));
🤖 Fix all issues with AI agents
In `@kernel/kernel-impl/src/main/sql/oracle/sakai_realm.sql`:
- Around line 739-741: The INSERT into SAKAI_REALM_RL_FN that uses
REALM_ID='!site.template', ROLE_NAME='maintain' and
FUNCTION_NAME='assessment.all.groups' is duplicated (same triple appears
elsewhere) and will violate the primary key on load; remove the redundant INSERT
statement (the one inserting the assessment.all.groups mapping) so only a single
SAKAI_REALM_RL_FN row exists for that (REALM_KEY from SAKAI_REALM where
REALM_ID='!site.template', ROLE_KEY from SAKAI_REALM_ROLE where
ROLE_NAME='maintain', FUNCTION_KEY from SAKAI_REALM_FUNCTION where
FUNCTION_NAME='assessment.all.groups').
🧹 Nitpick comments (1)
kernel/kernel-impl/src/main/sql/mysql/sakai_realm.sql (1)

727-728: Confirm upgrade/backfill in sakai-reference conversion scripts.

Please verify the corresponding conversion SQL exists so upgrades get assessment.all.groups grants (not just new installs).

If you want, I can help draft the conversion SQL for sakai-reference. Based on learnings, ensure this lands under docs/conversion in that repo.

@ottenhoff ottenhoff changed the title SAK-51713 Samigo honor assessment.all.groups for group releases SAK-52287 Samigo honor assessment.all.groups for group releases Jan 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants