diff --git a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/main/java/org/xwiki/wiki/user/script/WikiUserManagerScriptService.java b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/main/java/org/xwiki/wiki/user/script/WikiUserManagerScriptService.java index f2212e1..674e143 100644 --- a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/main/java/org/xwiki/wiki/user/script/WikiUserManagerScriptService.java +++ b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/main/java/org/xwiki/wiki/user/script/WikiUserManagerScriptService.java @@ -30,6 +30,7 @@ import org.xwiki.component.annotation.Component; import org.xwiki.context.Execution; import org.xwiki.model.reference.DocumentReference; +import org.xwiki.model.reference.DocumentReferenceResolver; import org.xwiki.model.reference.EntityReferenceSerializer; import org.xwiki.model.reference.WikiReference; import org.xwiki.script.service.ScriptService; @@ -75,6 +76,9 @@ @Inject private EntityReferenceSerializer entityReferenceSerializer; + + @Inject + private DocumentReferenceResolver documentReferenceResolver; /** * Provides access to the current context. @@ -126,33 +130,81 @@ public UserScope getUserScope(String wikiId) } /** + * Check that all required permissions are respected by both the script and the user. + * + * @param wikiId the id of the wiki concerned by the operation + * + * @throws AccessDeniedException if the permissions are not respected + */ + private void checkRights(String wikiId) throws AccessDeniedException + { + checkRights(wikiId, null); + } + + /** + * Check that all required permissions are respected by both the script and the user. + * + * @param wikiId the id of the wiki concerned by the operation + * @param user the user concerned by the operation + * + * @throws AccessDeniedException if the permissions are not respected + */ + private void checkRights(String wikiId, DocumentReference user) throws AccessDeniedException + { + XWikiContext context = xcontextProvider.get(); + + // Does the script author have the admin right? + // + // The goal is to avoid that a non-granted user writes a script, which could be executed by an administrator, + // which uses this script service to perform "nasty" operations, like being invited to a sub-wiki. + // + // By the past, we checked for the programing right, but it was too restrictive, as it make impossible to + // a user without programing rights to create a wiki and then invite some peoples in it. + authorizationManager.checkAccess(Right.ADMIN, context.getDoc().getAuthorReference(), + context.getDoc().getDocumentReference()); + + // Is the user concerned by the operation? + if (user != null && user.equals(context.getUserReference())) { + // If the user is concerned, then she has the right to perform this operation. + return; + } + + // Does the current user have the admin right? + authorizationManager.checkAccess(Right.ADMIN, context.getUserReference(), new WikiReference(wikiId)); + } + + /** + * Check that all required permissions are respected by both the script and the user concerned by a candidacy. + * + * @param candidacy the candidacy concerned by the operation + * + * @throws AccessDeniedException if the permissions are not respected + */ + private void checkRights(MemberCandidacy candidacy) throws AccessDeniedException + { + checkRights(candidacy.getWikiId(), documentReferenceResolver.resolve(candidacy.getUserId())); + } + + + /** * @param wikiId Id of the wiki to change * @param scope scope to set * @return true if it succeed */ public boolean setUserScope(String wikiId, String scope) { - XWikiContext xcontext = xcontextProvider.get(); - boolean success = true; try { - // Check if the current script has the programing rights - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - // Check the right - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Do the job + checkRights(wikiId); wikiUserManager.setUserScope(wikiId, UserScope.valueOf(scope.toUpperCase())); + return true; } catch (WikiUserManagerException e) { setLastError(e); - success = false; } catch (AccessDeniedException e) { setLastError(e); - success = false; } catch (IllegalArgumentException e) { setLastError(e); - success = false; } - return success; + return false; } /** @@ -184,27 +236,18 @@ public MembershipType getMembershipType(String wikiId) */ public boolean setMembershipType(String wikiId, String type) { - XWikiContext xcontext = xcontextProvider.get(); - boolean success = true; try { - // Check if the current script has the programing rights - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - // Check the right - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Do the job + checkRights(wikiId); wikiUserManager.setMembershipType(wikiId, MembershipType.valueOf(type.toUpperCase())); + return true; } catch (WikiUserManagerException e) { setLastError(e); - success = false; } catch (AccessDeniedException e) { setLastError(e); - success = false; } catch (IllegalArgumentException e) { setLastError(e); - success = false; } - return success; + return false; } /** @@ -247,14 +290,8 @@ public Boolean isMember(String userId, String wikiId) */ public boolean addMember(String userId, String wikiId) { - XWikiContext xcontext = xcontextProvider.get(); try { - // Check if the current script has the programing rights - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - // Check the right - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Add the member + checkRights(wikiId); wikiUserManager.addMember(userId, wikiId); } catch (AccessDeniedException e) { setLastError(e); @@ -276,14 +313,8 @@ public boolean addMember(String userId, String wikiId) */ public boolean addMembers(Collection userIds, String wikiId) { - XWikiContext xcontext = xcontextProvider.get(); try { - // Check if the current script has the programing rights - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - // Check the right - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Add the member + checkRights(wikiId); wikiUserManager.addMembers(userIds, wikiId); } catch (AccessDeniedException e) { setLastError(e); @@ -305,14 +336,8 @@ public boolean addMembers(Collection userIds, String wikiId) */ public boolean removeMember(String userId, String wikiId) { - XWikiContext xcontext = xcontextProvider.get(); try { - // Check if the current script has the programing rights - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - // Check the right - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Add the member + checkRights(wikiId); wikiUserManager.removeMember(userId, wikiId); } catch (AccessDeniedException e) { setLastError(e); @@ -415,17 +440,7 @@ public Boolean hasPendingInvitation(DocumentReference user, String wikiId) } try { - // Check if the current user is userId and if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - - // If the current user is not concerned by the invitation, he must be admin of the subwiki - if (!xcontext.getUserReference().equals(user)) { - authorizationManager.checkAccess(Right.ADMIN, - xcontext.getUserReference(), new WikiReference(wikiId)); - } - // Do the job + checkRights(wikiId, user); return wikiUserManager.hasPendingInvitation(user, wikiId); } catch (AccessDeniedException e) { setLastError(e); @@ -449,18 +464,7 @@ public Boolean hasPendingRequest(DocumentReference user, String wikiId) } try { - // Check if the passed user is the current user and if the current document has the programming rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - - // If the current user is not concerned by the invitation, he must be admin of the subwiki to have pending - // requests. - if (!xcontext.getUserReference().equals(user)) { - authorizationManager.checkAccess(Right.ADMIN, - xcontext.getUserReference(), new WikiReference(wikiId)); - } - // Do the job + checkRights(wikiId, user); return wikiUserManager.hasPendingRequest(user, wikiId); } catch (AccessDeniedException e) { setLastError(e); @@ -573,13 +577,7 @@ public MemberCandidacy askToJoin(String userId, String wikiId, String message) public boolean acceptRequest(MemberCandidacy request, String message, String privateComment) { try { - // Check if the current user is userId and if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - authorizationManager.checkAccess(Right.ADMIN, - xcontext.getUserReference(), new WikiReference(request.getWikiId())); - // Do the job + checkRights(request); wikiUserManager.acceptRequest(request, message, privateComment); return true; } catch (WikiUserManagerException e) { @@ -602,13 +600,7 @@ public boolean acceptRequest(MemberCandidacy request, String message, String pri public boolean refuseRequest(MemberCandidacy request, String message, String privateComment) { try { - // Check if the current user is userId and if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - authorizationManager.checkAccess(Right.ADMIN, - xcontext.getUserReference(), new WikiReference(request.getWikiId())); - // Do the job + checkRights(request); wikiUserManager.refuseRequest(request, message, privateComment); return true; } catch (WikiUserManagerException e) { @@ -629,17 +621,7 @@ public boolean refuseRequest(MemberCandidacy request, String message, String pri public boolean cancelCandidacy(MemberCandidacy candidacy) { try { - // Check if the current user is userId and if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - - String currentUser = entityReferenceSerializer.serialize(xcontext.getUserReference()); - if (!candidacy.getUserId().equals(currentUser)) { - authorizationManager.checkAccess(Right.ADMIN, - xcontext.getUserReference(), new WikiReference(candidacy.getWikiId())); - } - // Do the job + checkRights(candidacy); wikiUserManager.cancelCandidacy(candidacy); return true; } catch (WikiUserManagerException e) { @@ -663,12 +645,7 @@ public MemberCandidacy invite(String userId, String wikiId, String message) { // Invite try { - // Check if the current user is userId and if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - authorizationManager.checkAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference()); - authorizationManager.checkAccess(Right.ADMIN, xcontext.getUserReference(), new WikiReference(wikiId)); - // Do the job + checkRights(wikiId); return wikiUserManager.invite(userId, wikiId, message); } catch (WikiUserManagerException e) { setLastError(e); @@ -687,28 +664,17 @@ public MemberCandidacy invite(String userId, String wikiId, String message) */ public boolean acceptInvitation(MemberCandidacy invitation, String message) { - // Check if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - if (!authorizationManager.hasAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference())) { - return false; - } - - // Check right - if (!canSeeCandidacy(invitation)) { - // TODO - return false; - } - - // Accept invitation try { + checkRights(invitation); wikiUserManager.acceptInvitation(invitation, message); + return true; + } catch (AccessDeniedException e) { + setLastError(e); } catch (WikiUserManagerException e) { setLastError(e); - return false; } - - return true; + + return false; } /** @@ -720,27 +686,18 @@ public boolean acceptInvitation(MemberCandidacy invitation, String message) */ public boolean refuseInvitation(MemberCandidacy invitation, String message) { - // Check if the current script has the programing rights - XWikiContext xcontext = xcontextProvider.get(); - if (!authorizationManager.hasAccess(Right.PROGRAM, xcontext.getDoc().getAuthorReference(), - xcontext.getDoc().getDocumentReference())) { - return false; - } - - // Check right - if (!canSeeCandidacy(invitation)) { - // TODO - return false; - } // Accept invitation try { + checkRights(invitation); wikiUserManager.refuseInvitation(invitation, message); + return true; + } catch (AccessDeniedException e) { + setLastError(e); } catch (WikiUserManagerException e) { setLastError(e); - return false; } - return true; + return false; } } diff --git a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/test/java/org/xwiki/wiki/user/script/WikiUserManagerScriptServiceTest.java b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/test/java/org/xwiki/wiki/user/script/WikiUserManagerScriptServiceTest.java index 881741d..49035e3 100644 --- a/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/test/java/org/xwiki/wiki/user/script/WikiUserManagerScriptServiceTest.java +++ b/xwiki-platform-core/xwiki-platform-wiki/xwiki-platform-wiki-user/xwiki-platform-wiki-user-script/src/test/java/org/xwiki/wiki/user/script/WikiUserManagerScriptServiceTest.java @@ -123,17 +123,17 @@ public void setUp() throws Exception } /** - * @return the exception expected when the current script has the not the programing right + * @return the exception expected when the current script has the not the admin right */ - private Exception currentScriptHasNotProgrammingRight() throws AccessDeniedException + private Exception currentScriptHasNotAdminRight() throws AccessDeniedException { DocumentReference authorDocRef = new DocumentReference("mainWiki", "XWiki", "Admin"); when(currentDoc.getAuthorReference()).thenReturn(authorDocRef); DocumentReference currentDocRef = new DocumentReference("subwiki", "Test", "test"); when(currentDoc.getDocumentReference()).thenReturn(currentDocRef); - Exception exception = new AccessDeniedException(Right.PROGRAM, authorDocRef, currentDocRef); - doThrow(exception).when(authorizationManager).checkAccess(Right.PROGRAM, authorDocRef, currentDocRef); + Exception exception = new AccessDeniedException(Right.ADMIN, authorDocRef, currentDocRef); + doThrow(exception).when(authorizationManager).checkAccess(Right.ADMIN, authorDocRef, currentDocRef); return exception; } @@ -179,7 +179,7 @@ public void setUserScope() throws Exception public void setUserScopeWithoutPR() throws Exception { // Current script has not the programming right - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); boolean result = mocker.getComponentUnderTest().setUserScope("subwiki", "LOCAL_ONLY"); assertEquals(false, result); @@ -245,7 +245,7 @@ public void setMembershipType() throws Exception public void setMembershipTypeWithoutPR() throws Exception { // Current script has not the programming right - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); boolean result = mocker.getComponentUnderTest().setMembershipType("subwiki", "INVITE"); assertEquals(false, result); @@ -336,7 +336,7 @@ public void addMember() throws Exception public void addMemberWithoutPR() throws Exception { // Current script has not the programming right - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); boolean result = mocker.getComponentUnderTest().addMember("xwiki:XWiki.UserA", "subwiki"); assertEquals(false, result); @@ -368,7 +368,7 @@ public void addMembers() throws Exception public void addMembersWithoutPR() throws Exception { // Current script has not the programming right - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); Collection userIds = new ArrayList(); boolean result = mocker.getComponentUnderTest().addMembers(userIds, "subwiki"); @@ -401,7 +401,7 @@ public void removeMember() throws Exception public void removeMemberWithoutPR() throws Exception { // Current script has not the programming right - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); boolean result = mocker.getComponentUnderTest().removeMember("xwiki:XWiki.UserA", "subwiki"); assertEquals(false, result); @@ -649,7 +649,7 @@ public void hasPendingInvitationWhenError() throws Exception public void hasPendingInvitationWhenNoPR() throws Exception { String wikiId = "subwiki"; - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); DocumentReference userToTest = new DocumentReference("mainWiki", "XWiki", "UserABC"); // Test @@ -698,7 +698,7 @@ public void hasPendingRequestWhenError() throws Exception public void hasPendingRequestWhenNoPR() throws Exception { String wikiId = "subwiki"; - Exception exception = currentScriptHasNotProgrammingRight(); + Exception exception = currentScriptHasNotAdminRight(); DocumentReference userToTest = new DocumentReference("mainWiki", "XWiki", "UserABC"); // Test