Skip to content

SAK-51375 Content fix Edit Folder Permissions in sub-folder #13631

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.

Sign up for GitHub

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

Merged
ottenhoff merged 6 commits into from
May 7, 2025
Merged

SAK-51375 Content fix Edit Folder Permissions in sub-folder #13631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
47dd792
SAK-51375 Content fix Edit Folder Permissions in sub-folder
ottenhoff Apr 21, 2025
ff365d9
Update PermissionsController.java
ottenhoff May 6, 2025
1026b22
changes per core call
ottenhoff May 6, 2025
70ea587
Revert spaces
ottenhoff May 6, 2025
8d1cf5c
test fix
ottenhoff May 6, 2025
6294ea2
Merge branch 'master' into SAK-51375
ottenhoff May 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 6 additions & 32 deletions content/content-tool/tool/src/java/org/sakaiproject/content/tool/ResourcesAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4869,15 +4869,15 @@ public String buildPermissionsPageContext(VelocityPortlet portlet, Context conte
log.debug("{}.buildPermissionsPageContext()", this);

String reference = (String) state.getAttribute("folder_group_reference");
String overrideReference = null;
if (StringUtils.isNotBlank(reference)) {
overrideReference = contentHostingService.getContainingCollectionId(reference);
}

String siteId = toolManager.getCurrentPlacement().getContext();

String siteCollectionId = contentHostingService.getSiteCollection(siteId);
String overrideReference = contentHostingService.getReference(siteCollectionId);
String folderName = (String) state.getAttribute("folder_name");
if (StringUtils.isNoneBlank(reference, folderName)) {
context.put("reference", reference);
if (!reference.equals(overrideReference)) {
if (overrideReference != null && !reference.equals(overrideReference)) {
context.put("overrideReference", overrideReference);
}
context.put("folderName", folderName);
Expand All @@ -4890,7 +4890,7 @@ public String buildPermissionsPageContext(VelocityPortlet portlet, Context conte
context.put("permissionsLabel", rb.getString("list.fPerm"));

String toolId = toolManager.getCurrentPlacement().getId();
String startUrl = ServerConfigurationService.getPortalUrl() + "/site/" + siteId + "/tool/" + toolId + "?panel=Main";
String startUrl = ServerConfigurationService.getPortalUrl() + "/site/" + toolManager.getCurrentPlacement().getContext() + "/tool/" + toolId + "?panel=Main";
context.put("startPage", startUrl);

state.setAttribute (STATE_MODE, MODE_LIST);
Expand Down Expand Up @@ -6991,32 +6991,6 @@ public void doHierarchy(RunData data)
state.setAttribute(STATE_LIST_PREFERENCE, LIST_HIERARCHY);
}

// private static void resetCurrentMode(SessionState state)
// {
// String mode = (String) state.getAttribute(STATE_MODE);
// if(isStackEmpty(state))
// {
// if(MODE_HELPER.equals(mode))
// {
// cleanupState(state);
// state.setAttribute(STATE_RESOURCES_HELPER_MODE, MODE_ATTACHMENT_DONE);
// }
// else
// {
// state.setAttribute(STATE_MODE, MODE_LIST);
// state.removeAttribute(STATE_RESOURCES_HELPER_MODE);
// }
// return;
// }
// Map current_stack_frame = peekAtStack(state);
// String helper_mode = (String) current_stack_frame.get(STATE_RESOURCES_HELPER_MODE);
// if(helper_mode != null)
// {
// state.setAttribute(STATE_RESOURCES_HELPER_MODE, helper_mode);
// }
//
// }
//
/**
* Expand all the collection resources and put in EXPANDED_COLLECTIONS attribute.
*/
Expand Down
58 changes: 49 additions & 9 deletions webapi/src/main/java/org/sakaiproject/webapi/controllers/PermissionsController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,24 +83,64 @@ public Map<String, Object> getPermissions(@PathVariable String siteId, @PathVari

Site site = getSiteById(siteId);

// Get the site's AuthzGroup once for potential reuse
AuthzGroup siteAuthzGroup;
try {
siteAuthzGroup = authzGroupService.getAuthzGroup(siteRef);
} catch (GroupNotDefinedException ex) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "No site realm defined for ref " + siteRef);
}

AuthzGroup authzGroup;
try {
authzGroup = authzGroupService.getAuthzGroup(ref);
} catch (GroupNotDefinedException e) {
// Instructor editing a folder that doesn't have a realm yet
try {
authzGroup = authzGroupService.getAuthzGroup(siteRef);
} catch (GroupNotDefinedException ex) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "No realm defined for ref " + siteRef);
}
authzGroup = siteAuthzGroup;
}

AuthzGroup overrideAuthzGroup = null;
if (StringUtils.isNotBlank(overrideRef)) {
try {
overrideAuthzGroup = authzGroupService.getAuthzGroup(overrideRef);
} catch (GroupNotDefinedException e) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST, "No realm defined for override ref " + overrideRef);
String tempOverrideRef = overrideRef;
boolean done = false;
// We need to make sure the ref is for a content folder in this site
if (tempOverrideRef.matches("^/content/(group|group-user)/" + siteId + "/.*")) {
// Keep trying parent folders until we find one with permissions or reach site level
while (!done) {
try {
overrideAuthzGroup = authzGroupService.getAuthzGroup(tempOverrideRef);
done = true;
} catch (GroupNotDefinedException e) {
// Try parent folder - first check if we're already at site level
String siteRoot = "/content/group/" + siteId + "/";
String siteUserRoot = "/content/group-user/" + siteId + "/";
if (tempOverrideRef.equals(siteRoot) || tempOverrideRef.equals(siteUserRoot)) {
// At site level, use site's AuthzGroup
overrideAuthzGroup = siteAuthzGroup;
done = true;
} else {
// Remove the last folder segment but preserve trailing slash
String path = tempOverrideRef.substring(0, tempOverrideRef.length() - 1); // remove trailing slash
int lastSlash = path.lastIndexOf('/');
if (lastSlash > 0) {
tempOverrideRef = path.substring(0, lastSlash + 1); // restore trailing slash
log.debug("Trying parent folder for permissions: {}", tempOverrideRef);
} else {
// Shouldn't happen with our path pattern, but just in case
overrideAuthzGroup = siteAuthzGroup;
done = true;
}
}
}
}
} else {
// Not a content path, try it directly once
try {
overrideAuthzGroup = authzGroupService.getAuthzGroup(tempOverrideRef);
} catch (GroupNotDefinedException e) {
// Use site's AuthzGroup
overrideAuthzGroup = siteAuthzGroup;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion webcomponents/tool/src/main/frontend/packages/sakai-permissions/src/SakaiPermissions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ export class SakaiPermissions extends SakaiElement {

_loadPermissions() {

const url = `/api/sites/${portal.siteId}/permissions/${this.tool}?ref=${this.reference}${this.overrideReference ? `&overrideRef=${this.overrideReference}` : ""}`;
const url = `/api/sites/${portal.siteId}/permissions/${this.tool}?ref=${encodeURIComponent(this.reference)}${this.overrideReference ? `&overrideRef=${encodeURIComponent(this.overrideReference)}` : ""}`;
fetch(url, { cache: "no-cache" })
.then(res => {

Expand Down
25 changes: 16 additions & 9 deletions ...mponents/tool/src/main/frontend/packages/sakai-permissions/test/sakai-permissions.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ describe("sakai-permissions tests", () => {

it ("renders correctly", async () => {

fetchMock.get(data.permsUrl, data.perms);
// Encode the ref parameter in the mock URL for the initial load
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(`/site/${data.siteId}`)}`, data.perms);

const el = await fixture(html`
<sakai-permissions tool="tool">
Expand Down Expand Up @@ -80,7 +81,8 @@ describe("sakai-permissions tests", () => {

it ("tests the _handlePermissionClick method", async () => {

fetchMock.get(data.permsUrl, data.perms);
// Encode the ref parameter in the mock URL for the initial load
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(`/site/${data.siteId}`)}`, data.perms);

const el = await fixture(html`
<sakai-permissions tool="tool">
Expand Down Expand Up @@ -123,7 +125,7 @@ describe("sakai-permissions tests", () => {
const overrideRef = "override_ref";

// Mock up a 400 (bad request) response
const url = `/api/sites/${data.siteId}/permissions/tool?ref=${ref}&overrideRef=${overrideRef}`;
const url = `/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(ref)}&overrideRef=${encodeURIComponent(overrideRef)}`;
fetchMock.get(url, 400);

const consoleErrorStub = sinon.stub(console, "error");
Expand Down Expand Up @@ -153,7 +155,7 @@ describe("sakai-permissions tests", () => {
const overrideRef = "override_ref";
const overriddenPerms = { ...data.perms, locked: { maintain: [ "tool.create", "tool.delete" ] } };

const url = `/api/sites/${data.siteId}/permissions/tool?ref=${ref}&overrideRef=${overrideRef}`;
const url = `/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(ref)}&overrideRef=${encodeURIComponent(overrideRef)}`;
fetchMock.get(url, overriddenPerms);

const el = await fixture(html`
Expand All @@ -176,15 +178,15 @@ describe("sakai-permissions tests", () => {

const unsetPerms = { ...data.perms, on: { "maintain": [], "access": [] } };

fetchMock.get(data.permsUrl, unsetPerms);
// Encode the ref parameter in the mock URL for the initial load
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(`/site/${data.siteId}`)}`, unsetPerms);

// Mock the POST request for saving permissions
const saveUrl = `/api/sites/${data.siteId}/permissions`;
fetchMock.post(saveUrl, 200);

const el = await fixture(html`
<sakai-permissions tool="tool"
site-id="${data.siteId}"
fire-event>
</sakai-permissions>
`);
Expand Down Expand Up @@ -224,10 +226,14 @@ describe("sakai-permissions tests", () => {

it ("loads group permissions correctly", async () => {

fetchMock.get(data.permsUrl, data.perms);
const initialRef = `/site/${data.siteId}`;
// Encode the initial ref parameter in the mock URL
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(initialRef)}`, data.perms);

const el = await fixture(html`
<sakai-permissions tool="tool"
reference="${initialRef}" // Explicitly set initial reference
enable-groups
fire-event>
</sakai-permissions>
`);
Expand All @@ -251,7 +257,7 @@ describe("sakai-permissions tests", () => {
groups : data.groups,
};

fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${data.groups[0].reference}`, groupPerms);
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(data.groups[0].reference)}`, groupPerms);

groupPicker.dispatchEvent(new CustomEvent("groups-selected", { detail: { value: data.groups[0].reference }, bubbles: true }));

Expand All @@ -271,7 +277,8 @@ describe("sakai-permissions tests", () => {

it ("handles disable-groups correctly", async () => {

fetchMock.get(data.permsUrl, data.perms);
// Replace the incorrect mock with the proper URL pattern
fetchMock.get(`/api/sites/${data.siteId}/permissions/tool?ref=${encodeURIComponent(`/site/${data.siteId}`)}`, data.perms);

const el = await fixture(html`
<sakai-permissions tool="tool"
Expand Down
Loading