From 206839e6c126ab7d5e8bd7db9ef4f4322f8b5326 Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Sun, 7 Dec 2025 09:34:53 -0500 Subject: [PATCH 1/6] JSPWIKI-130 addressing part 2 of the reported issues, work in progress --- UPGRADING | 22 +++++- .../java/org/apache/wiki/api/core/Engine.java | 7 +- .../java/org/apache/wiki/WikiContext.java | 2 +- .../java/org/apache/wiki/WikiSession.java | 8 +- .../wiki/auth/AuthorizationManager.java | 3 +- .../auth/DefaultAuthorizationManager.java | 4 +- .../apache/wiki/auth/DefaultUserManager.java | 74 +++++++++++++------ .../apache/wiki/auth/user/UserDatabase.java | 4 + .../wiki/auth/AuthorizationManagerTest.java | 4 +- .../org/apache/wiki/plugin/IfPluginTest.java | 4 +- 10 files changed, 99 insertions(+), 33 deletions(-) diff --git a/UPGRADING b/UPGRADING index a14f33ece9..f136ff8892 100644 --- a/UPGRADING +++ b/UPGRADING @@ -1,5 +1,5 @@ -Apache JSPWiki 2.12.0 - Upgrading Notes +Apache JSPWiki 3.0.0 - Upgrading Notes ================================================== Licensed to the Apache Software Foundation (ASF) under one @@ -21,6 +21,26 @@ Apache JSPWiki 2.12.0 - Upgrading Notes The license file can be found in LICENSE. + +Upgrading JSPWiki to 3.0.0 +--------------------------- +Please see https://jspwiki-wiki.apache.org/Wiki.jsp?page=NewIn3.0.0 for details + +1. New requirements + * Java 17 needed to run JSPWiki + +2. Backwards incompatible changes: + * Page level access controls (i.e. [ALLOW edit/view/etc User/Role] ) logic has been changed including the IF plugin. + See details with JSPWIKI-130. + + To maintain the behavior of JSPWiki 2.X use the following setting in jspwiki-custom.properties. + jspwiki.security.useOldPageAccessControlLogic=true + This is not recommended. + +3. Many new security features have been added and enabled by default. Please review the default + jspwiki.properties file + + Upgrading JSPWiki to 2.12.0 --------------------------- diff --git a/jspwiki-api/src/main/java/org/apache/wiki/api/core/Engine.java b/jspwiki-api/src/main/java/org/apache/wiki/api/core/Engine.java index a456c4d460..0dd704a22b 100644 --- a/jspwiki-api/src/main/java/org/apache/wiki/api/core/Engine.java +++ b/jspwiki-api/src/main/java/org/apache/wiki/api/core/Engine.java @@ -55,7 +55,12 @@ Licensed to the Apache Software Foundation (ASF) under one * {@code Context#getEngine()} method or through {@code Wiki.engine().find(..)} DSL methods. */ public interface Engine { - + /** + * see JSPWIKI-130 + * @since 3.0.0 + */ + String PROP_USE_2_X_ACL_LOGIC = "jspwiki.security.useOldPageAccessControlLogic"; + /** The default inlining pattern. Currently "*.png" */ String DEFAULT_INLINEPATTERN = "*.png"; diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java b/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java index bbecead58f..4e026dda9c 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java @@ -545,7 +545,7 @@ public Principal getCurrentUser() { // This shouldn't happen, really... return WikiPrincipal.GUEST; } - return m_session.getUserPrincipal(); + return m_session.getLoginPrincipal(); } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WikiSession.java b/jspwiki-main/src/main/java/org/apache/wiki/WikiSession.java index d14fe3de5b..d52eac0385 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiSession.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiSession.java @@ -417,7 +417,7 @@ protected void injectUserProfilePrincipals() { throw new IllegalStateException( "User database cannot be null." ); } try { - final UserProfile profile = database.find( searchId ); + final UserProfile profile = database.findByLoginName( searchId ); final Principal[] principals = database.getPrincipals( profile.getLoginName() ); for( final Principal principal : principals ) { // Add the Principal to the Subject @@ -426,6 +426,12 @@ protected void injectUserProfilePrincipals() { // Set the user principal if needed; we prefer FullName, but the WikiName will also work final boolean isFullNamePrincipal = ( principal instanceof WikiPrincipal && ( ( WikiPrincipal )principal ).getType().equals( WikiPrincipal.FULL_NAME ) ); + if (( principal instanceof WikiPrincipal && + ( ( WikiPrincipal )principal ).getType().equals( WikiPrincipal.LOGIN_NAME )) ){ + m_loginPrincipal = principal; + } + + if ( isFullNamePrincipal ) { m_userPrincipal = principal; } else if ( !( m_userPrincipal instanceof WikiPrincipal ) ) { diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/AuthorizationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/AuthorizationManager.java index bd3833759e..794c9ce96a 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/AuthorizationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/AuthorizationManager.java @@ -233,7 +233,8 @@ default boolean hasAccess( final Context context, final HttpServletResponse resp *
  • Finally, if a user cannot be found, manufacture and return a generic {@link org.apache.wiki.auth.acl.UnresolvedPrincipal}
    • * * - * @param name the name of the Principal to resolve + * @param name the name of the Principal to resolve. Note: as of v3.0.0, the + * underlying behavior has changed. Principals can be resolved via login names only. * @return the fully-resolved Principal */ Principal resolvePrincipal( final String name ); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java index 9a2c8aebbf..802b951aac 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java @@ -352,11 +352,9 @@ public Principal resolvePrincipal( final String name ) { // Ok, no luck---this must be a user principal final Principal[] principals; - final UserProfile profile; final UserDatabase db = m_engine.getManager( UserManager.class ).getUserDatabase(); try { - profile = db.find( name ); - principals = db.getPrincipals( profile.getLoginName() ); + principals = db.getPrincipals( name ); for( final Principal value : principals ) { principal = value; if( principal.getName().equals( name ) ) { diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java index d156170424..4e0fb46eb0 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java @@ -155,7 +155,7 @@ public UserProfile getUserProfile( final Session session ) { if ( session.isAuthenticated() ) { user = session.getUserPrincipal(); try { - profile = getUserDatabase().find( user.getName() ); + profile = getUserDatabase().findByWikiName( user.getName()); newProfile = false; } catch( final NoSuchPrincipalException e ) { LOG.debug(e.getMessage(), e); @@ -166,6 +166,8 @@ public UserProfile getUserProfile( final Session session ) { profile = getUserDatabase().newProfile(); if ( user != null ) { profile.setLoginName( user.getName() ); + } else { + LOG.warn("new profile however the user principal is null. this shouldn't happen"); } if ( !profile.isNew() ) { throw new IllegalStateException( "New profile should be marked 'new'. Check your UserProfile implementation." ); @@ -374,31 +376,61 @@ public void validateProfile( final Context context, final UserProfile profile ) UserProfile otherProfile; final String fullName = profile.getFullname(); final String loginName = profile.getLoginName(); + final String wikiName = profile.getWikiName(); final String email = profile.getEmail(); - // It's illegal to use as a full name someone else's login name - try { - otherProfile = getUserDatabase().find( fullName ); - if( otherProfile != null && !profile.equals( otherProfile ) && !fullName.equals( otherProfile.getFullname() ) ) { - final Object[] args = { fullName }; - session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalfullname" ), args ) ); + + if ("true".equalsIgnoreCase(m_engine.getWikiProperties().getProperty(Engine.PROP_USE_2_X_ACL_LOGIC, "false"))) { + // It's illegal to use as a full name someone else's login name + try { + otherProfile = getUserDatabase().find( fullName ); + if( otherProfile != null && !profile.equals( otherProfile ) && !fullName.equals( otherProfile.getFullname() ) ) { + final Object[] args = { fullName }; + session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalfullname" ), args ) ); + } + } catch( final NoSuchPrincipalException e ) { + LOG.debug(e.getMessage(), e); + /* It's clean */ } - } catch( final NoSuchPrincipalException e ) { - LOG.debug(e.getMessage(), e); - /* It's clean */ - } - // It's illegal to use as a login name someone else's full name - try { - otherProfile = getUserDatabase().find( loginName ); - if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { - final Object[] args = { loginName }; - session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalloginname" ), args ) ); + // It's illegal to use as a login name someone else's full name + try { + otherProfile = getUserDatabase().find( loginName ); + if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { + final Object[] args = { loginName }; + session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalloginname" ), args ) ); + } + } catch( final NoSuchPrincipalException e ) { + LOG.debug(e.getMessage(), e); + /* It's clean */ + } + } else { + //JSPWIKI-130, v3+ behavior + // It is legal to use as a full name someone else's login name + + // It's illegal to use as a login name someone else's full name + try { + otherProfile = getUserDatabase().findByLoginName(loginName ); + if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { + final Object[] args = { loginName }; + session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalloginname" ), args ) ); + } + } catch( final NoSuchPrincipalException e ) { + LOG.debug(e.getMessage(), e); + /* It's clean */ + } + try { + otherProfile = getUserDatabase().findByWikiName(wikiName ); + if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { + final Object[] args = { loginName }; + session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalloginname" ), args ) ); + } + } catch( final NoSuchPrincipalException e ) { + LOG.debug(e.getMessage(), e); + /* It's clean */ } - } catch( final NoSuchPrincipalException e ) { - LOG.debug(e.getMessage(), e); - /* It's clean */ } + // It's illegal to use multiple accounts with the same email if (email != null && email.trim().length() > 0) { @@ -499,7 +531,7 @@ public void service( final HttpServletRequest req, final HttpServletResponse res */ public UserProfile getUserInfo( final String uid ) throws NoSuchPrincipalException { if( m_manager != null ) { - return m_manager.getUserDatabase().findByWikiName( uid ); + return m_manager.getUserDatabase().findByUid( uid ); } throw new IllegalStateException( "The manager is offline." ); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java index f604e50244..bb9ebbd13d 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java @@ -77,7 +77,11 @@ public interface UserDatabase { * that supplied the name is unknown. * * @param index the login name, full name, or wiki name + * @deprecated depending on the use case, this API's usage can be dangerous. + * Recommend using other APIs for more explicit lookup types. see JSPWIKI-130 + * for additional details. */ + @Deprecated UserProfile find( String index ) throws NoSuchPrincipalException; /** diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthorizationManagerTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthorizationManagerTest.java index d015525f3b..07127dd324 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthorizationManagerTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthorizationManagerTest.java @@ -519,8 +519,8 @@ public void testResolveUsers() throws WikiException Assertions.fail( "Failed save: " + e.getLocalizedMessage() ); } Assertions.assertEquals( new WikiPrincipal( "authmanagertest", WikiPrincipal.LOGIN_NAME ), m_auth.resolvePrincipal( "authmanagertest" ) ); - Assertions.assertEquals( new WikiPrincipal( "AuthorizationManagerTest User", WikiPrincipal.FULL_NAME ), m_auth.resolvePrincipal( "AuthorizationManagerTest User" ) ); - Assertions.assertEquals( new WikiPrincipal( "AuthorizationManagerTestUser", WikiPrincipal.WIKI_NAME ), m_auth.resolvePrincipal( "AuthorizationManagerTestUser" ) ); + //Assertions.assertEquals( new WikiPrincipal( "AuthorizationManagerTest User", WikiPrincipal.FULL_NAME ), m_auth.resolvePrincipal( "AuthorizationManagerTest User" ) ); + //Assertions.assertEquals( new WikiPrincipal( "AuthorizationManagerTestUser", WikiPrincipal.WIKI_NAME ), m_auth.resolvePrincipal( "AuthorizationManagerTestUser" ) ); try { m_engine.getManager( UserManager.class ).getUserDatabase().deleteByLoginName( "authmanagertest" ); diff --git a/jspwiki-main/src/test/java/org/apache/wiki/plugin/IfPluginTest.java b/jspwiki-main/src/test/java/org/apache/wiki/plugin/IfPluginTest.java index b7a99ca8de..0f06c5a43b 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/plugin/IfPluginTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/plugin/IfPluginTest.java @@ -67,7 +67,7 @@ Context getJanneBasedWikiContextFor( final Page page ) throws WikiException { */ @Test void testIfPluginUserAllowed() throws WikiException { - final String src = "[{IfPlugin user='Janne Jalkanen'\n\nContent visible for Janne Jalkanen}]"; + final String src = "[{IfPlugin user='janne'\n\nContent visible for Janne Jalkanen}]"; final String expected = "

    Content visible for Janne Jalkanen

    \n"; testEngine.saveText( "Test", src ); @@ -85,7 +85,7 @@ void testIfPluginUserAllowed() throws WikiException { */ @Test void testIfPluginUserNotAllowed() throws WikiException { - final String src = "[{IfPlugin user='!Janne Jalkanen'\n\nContent NOT visible for Janne Jalkanen}]"; + final String src = "[{IfPlugin user='!janne'\n\nContent NOT visible for Janne Jalkanen}]"; final String expected = "\n"; testEngine.saveText( "Test", src ); From 579527c3a8a7c79ca141f1afe76ff4e7290fb9c0 Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Sun, 7 Dec 2025 09:35:28 -0500 Subject: [PATCH 2/6] JSPWIKI-130 addressing part 2 of the reported issues, work in progress --- .../src/main/java/org/apache/wiki/auth/DefaultUserManager.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java index 4e0fb46eb0..83d3913bb8 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java @@ -69,6 +69,7 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Properties; import java.util.ResourceBundle; import java.util.WeakHashMap; +import org.apache.wiki.auth.authorize.GroupManager; import tools.jackson.databind.ObjectMapper; import tools.jackson.databind.node.ObjectNode; @@ -419,6 +420,7 @@ public void validateProfile( final Context context, final UserProfile profile ) LOG.debug(e.getMessage(), e); /* It's clean */ } + m_engine.getManager(GroupManager.class).getGroupDatabase().groups(); try { otherProfile = getUserDatabase().findByWikiName(wikiName ); if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { From 565c8526b788cefa5667aa2a6cf2e36926b44c65 Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Sun, 7 Dec 2025 14:25:12 -0500 Subject: [PATCH 3/6] JSPWIKI-130 --- .../apache/wiki/auth/DefaultUserManager.java | 31 +++++++++++++++++-- .../auth/authorize/DefaultGroupManager.java | 22 +++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java index 83d3913bb8..9b6bbb8694 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java @@ -69,6 +69,7 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Properties; import java.util.ResourceBundle; import java.util.WeakHashMap; +import org.apache.wiki.auth.authorize.Group; import org.apache.wiki.auth.authorize.GroupManager; import tools.jackson.databind.ObjectMapper; import tools.jackson.databind.node.ObjectNode; @@ -414,13 +415,39 @@ public void validateProfile( final Context context, final UserProfile profile ) otherProfile = getUserDatabase().findByLoginName(loginName ); if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { final Object[] args = { loginName }; - session.addMessage( SESSION_MESSAGES, MessageFormat.format( rb.getString( "security.error.illegalloginname" ), args ) ); + session.addMessage( SESSION_MESSAGES, MessageFormat.format( + rb.getString( "security.error.illegalloginname" ), args ) ); } } catch( final NoSuchPrincipalException e ) { LOG.debug(e.getMessage(), e); /* It's clean */ } - m_engine.getManager(GroupManager.class).getGroupDatabase().groups(); + //it's illegal to use a username, email or wiki name as a group name + try { + Group[] groups = m_engine.getManager(GroupManager.class).getGroupDatabase().groups(); + for (Group grp : groups) { + if (grp.getName().equals(loginName)) { + final Object[] args = {loginName}; + session.addMessage(SESSION_MESSAGES, + MessageFormat.format(rb.getString("security.error.illegalloginname"), args)); + } + if (grp.getName().equals(wikiName)) { + final Object[] args = {wikiName}; + session.addMessage(SESSION_MESSAGES, + MessageFormat.format(rb.getString("security.error.illegalloginname"), args)); + } + if (grp.getName().equals(email)) { + final Object[] args = {email}; + session.addMessage(SESSION_MESSAGES, + MessageFormat.format(rb.getString("security.error.illegalloginname"), args)); + } + } + } catch (WikiSecurityException ex) { + //TODO i18n + session.addMessage(SESSION_MESSAGES, + "failed to query for groups " + ex.getMessage()); + } + //wiki names must be unique as well. try { otherProfile = getUserDatabase().findByWikiName(wikiName ); if( otherProfile != null && !profile.equals( otherProfile ) && !loginName.equals( otherProfile.getLoginName() ) ) { diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java index 446935f270..2b17909fa2 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java @@ -194,6 +194,7 @@ public Group parseGroup( String name, String memberLine, final boolean create ) if( create ) { name = "MyGroup"; } else { + //TODO i18n throw new WikiSecurityException( "Group name cannot be blank." ); } } else if( ArrayUtils.contains( Group.RESTRICTED_GROUPNAMES, name ) ) { @@ -224,6 +225,7 @@ public Group parseGroup( String name, String memberLine, final boolean create ) } catch( final NoSuchPrincipalException e ) { // It's a new group.... throw error if we don't create new ones if( !create ) { + //TODO i18n throw new NoSuchPrincipalException( "Group '" + name + "' does not exist." ); } } @@ -236,6 +238,26 @@ public Group parseGroup( String name, String memberLine, final boolean create ) group.add( new WikiPrincipal( member ) ); } } + + if ("true".equalsIgnoreCase(m_engine.getWikiProperties().getProperty(Engine.PROP_USE_2_X_ACL_LOGIC, "false"))) { + //check to ensure that the group name does not conflict with any existing user account login, email or wiki name + UserManager userManger = m_engine.getManager(UserManager.class); + try { userManger.getUserDatabase().findByEmail(name); + throw new WikiSecurityException( "Group name conflicts with a user account" ); + }catch (NoSuchPrincipalException e) { + //no issues here + } + try { userManger.getUserDatabase().findByLoginName(name); + throw new WikiSecurityException( "Group name conflicts with a user account" ); + }catch (NoSuchPrincipalException e) { + //no issues here + } + try { userManger.getUserDatabase().findByWikiName(name); + throw new WikiSecurityException( "Group name conflicts with a user account" ); + }catch (NoSuchPrincipalException e) { + //no issues here + } + } return group; } From 9fea75f165b5b2be313c3e539bd5d3b54764cf7a Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Sun, 7 Dec 2025 16:09:02 -0500 Subject: [PATCH 4/6] JSPWIKI-130 makes the first half of part 2 --- .../auth/authorize/DefaultGroupManager.java | 2 +- .../wiki/auth/user/AbstractUserDatabase.java | 2 ++ .../apache/wiki/auth/user/UserDatabase.java | 2 ++ .../wiki/auth/user/XMLUserDatabaseTest.java | 2 -- .../org/apache/wiki/plugin/GroupsTest.java | 36 +++++++++++++++---- 5 files changed, 35 insertions(+), 9 deletions(-) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java index 2b17909fa2..67e98f52cf 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/authorize/DefaultGroupManager.java @@ -239,7 +239,7 @@ public Group parseGroup( String name, String memberLine, final boolean create ) } } - if ("true".equalsIgnoreCase(m_engine.getWikiProperties().getProperty(Engine.PROP_USE_2_X_ACL_LOGIC, "false"))) { + if ("false".equalsIgnoreCase(m_engine.getWikiProperties().getProperty(Engine.PROP_USE_2_X_ACL_LOGIC, "false"))) { //check to ensure that the group name does not conflict with any existing user account login, email or wiki name UserManager userManger = m_engine.getManager(UserManager.class); try { userManger.getUserDatabase().findByEmail(name); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java index 3481642268..fbf0130fba 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java @@ -55,6 +55,8 @@ public abstract class AbstractUserDatabase implements UserDatabase { * that supplied the name is unknown. * * @param index the login name, full name, or wiki name + * @return non null + * @throws org.apache.wiki.auth.NoSuchPrincipalException * @see org.apache.wiki.auth.user.UserDatabase#find(java.lang.String) */ @Override diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java index bb9ebbd13d..68627f9ff2 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/UserDatabase.java @@ -77,6 +77,8 @@ public interface UserDatabase { * that supplied the name is unknown. * * @param index the login name, full name, or wiki name + * @return non null + * @throws org.apache.wiki.auth.NoSuchPrincipalException * @deprecated depending on the use case, this API's usage can be dangerous. * Recommend using other APIs for more explicit lookup types. see JSPWIKI-130 * for additional details. diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java index dc533f4063..121cb5179e 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/user/XMLUserDatabaseTest.java @@ -19,7 +19,6 @@ Licensed to the Apache Software Foundation (ASF) under one package org.apache.wiki.auth.user; import java.io.File; -import java.io.IOException; import org.apache.commons.lang3.ArrayUtils; import org.apache.wiki.TestEngine; import org.apache.wiki.WikiEngine; @@ -39,7 +38,6 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.Properties; import java.util.UUID; import org.apache.commons.io.FileUtils; -import org.apache.wiki.auth.authorize.XMLGroupDatabase; public class XMLUserDatabaseTest { diff --git a/jspwiki-main/src/test/java/org/apache/wiki/plugin/GroupsTest.java b/jspwiki-main/src/test/java/org/apache/wiki/plugin/GroupsTest.java index b5cb7cdb0a..03a0bd5c0a 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/plugin/GroupsTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/plugin/GroupsTest.java @@ -25,28 +25,34 @@ Licensed to the Apache Software Foundation (ASF) under one import java.util.UUID; import org.apache.commons.io.FileUtils; import org.apache.wiki.TestEngine; +import org.apache.wiki.WikiEngine; +import org.apache.wiki.api.exceptions.WikiException; +import org.apache.wiki.auth.authorize.GroupManager; import org.apache.wiki.auth.authorize.XMLGroupDatabase; import org.apache.wiki.pages.PageManager; import org.apache.wiki.render.RenderingManager; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; public class GroupsTest { - static TestEngine testEngine; - - @BeforeAll - public static void init() throws IOException { + private TestEngine testEngine; + private File target ; + @BeforeEach + public void init() throws IOException { final Properties props = TestEngine.getTestProperties(); - File target = new File("target/XMLUserDatabaseTest" + UUID.randomUUID().toString() + ".xml"); + target = new File("target/GroupsTest" + UUID.randomUUID().toString() + ".xml"); FileUtils.copyFile(new File("src/test/resources/groupdatabase.xml"), target); props.put(XMLGroupDatabase.PROP_DATABASE, target.getAbsolutePath()); - testEngine = TestEngine.build(); + testEngine = TestEngine.build(props); } + + @AfterEach public void tearDown() throws Exception { testEngine.getManager( PageManager.class ).deletePage( "Test" ); @@ -66,5 +72,23 @@ public void testTag() throws Exception { + "TV\n" , res ); } + + @Test + public void jspwiki130part2() throws Exception { + + GroupManager usermanger = testEngine.getManager(GroupManager.class); + + Assertions.assertThrows(WikiException.class, () -> { + usermanger.parseGroup("janne", "Al\nBob\nCookie", true); + }); + Assertions.assertThrows(WikiException.class, () -> { + usermanger.parseGroup("JanneJalkanen", "Al\nBob\nCookie", true); + }); + + Assertions.assertThrows(WikiException.class, () -> { + usermanger.parseGroup("janne@ecyrd.com", "Al\nBob\nCookie", true); + }); + + } } From 508922a01c68519bac77e912d79ce0efc7c8c02e Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Wed, 10 Dec 2025 20:22:09 -0500 Subject: [PATCH 5/6] JSPWIKI-130 addresses the final part of this jira, ready for peer review --- .../main/java/org/apache/wiki/WikiPage.java | 1 + .../auth/DefaultAuthenticationManager.java | 3 --- .../auth/DefaultAuthorizationManager.java | 24 +++++++++---------- .../apache/wiki/auth/DefaultUserManager.java | 4 +++- .../wiki/auth/user/AbstractUserDatabase.java | 14 +++++++---- .../wiki/auth/user/DummyUserDatabase.java | 1 + .../wiki/auth/user/JDBCUserDatabase.java | 1 + .../wiki/auth/user/XMLUserDatabase.java | 1 + .../wiki/auth/AuthenticationManagerTest.java | 8 +++---- .../wiki/auth/authorize/GroupManagerTest.java | 2 +- .../resources/jspwiki-testUserPolicy.policy | 2 +- 11 files changed, 35 insertions(+), 26 deletions(-) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/WikiPage.java b/jspwiki-main/src/main/java/org/apache/wiki/WikiPage.java index 44f3087954..404813d98c 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/WikiPage.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiPage.java @@ -355,6 +355,7 @@ public int compareTo( final Page page ) { * * {@inheritDoc} */ + @Override public boolean equals( final Object o ) { if( o instanceof WikiPage ) { final WikiPage wp = ( WikiPage )o; diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java index 7f33147c7f..1cf943525e 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthenticationManager.java @@ -48,15 +48,12 @@ Licensed to the Apache Software Foundation (ASF) under one import jakarta.servlet.http.HttpSession; import java.security.Principal; import java.util.Collections; -import java.util.Date; import java.util.HashMap; import java.util.HashSet; import java.util.Map; import java.util.Properties; import java.util.Set; import org.apache.wiki.WikiContext; -import org.apache.wiki.api.core.Context; -import org.apache.wiki.auth.user.DefaultUserProfile; import org.apache.wiki.auth.user.UserProfile; diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java index 802b951aac..8080de4217 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultAuthorizationManager.java @@ -335,35 +335,35 @@ public Principal resolvePrincipal( final String name ) { // Check built-in Roles first final Role role = new Role(name); if ( Role.isBuiltInRole( role ) ) { - return role; + return role; } // Check Authorizer Roles Principal principal = m_authorizer.findRole( name ); if ( principal != null ) { - return principal; + return principal; } // Check Groups principal = m_engine.getManager( GroupManager.class ).findRole( name ); if ( principal != null ) { - return principal; + return principal; } // Ok, no luck---this must be a user principal final Principal[] principals; final UserDatabase db = m_engine.getManager( UserManager.class ).getUserDatabase(); try { - principals = db.getPrincipals( name ); - for( final Principal value : principals ) { - principal = value; - if( principal.getName().equals( name ) ) { - return principal; - } - } + principals = db.getPrincipals( name ); + for( final Principal value : principals ) { + principal = value; + if( principal.getName().equals( name ) ) { + return principal; + } + } } catch( final NoSuchPrincipalException e ) { - // We couldn't find the user... - LOG.debug(e.getMessage(), e); + // We couldn't find the user... + LOG.debug(e.getMessage(), e); } // Ok, no luck---mark this as unresolved and move on return new UnresolvedPrincipal( name ); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java index 9b6bbb8694..86852c182d 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/DefaultUserManager.java @@ -205,7 +205,8 @@ public void setUserProfile( final Context context, final UserProfile profile ) t if( otherProfile != null && !otherProfile.equals( oldProfile ) ) { throw new DuplicateUserException( "security.error.login.taken", profile.getLoginName() ); } - } catch( final NoSuchPrincipalException e ) { + } catch (final NoSuchPrincipalException e) { + LOG.debug(e.getMessage(), e); } try { otherProfile = getUserDatabase().findByFullName( profile.getFullname() ); @@ -252,6 +253,7 @@ public void setUserProfile( final Context context, final UserProfile profile ) t fireEvent( WikiSecurityEvent.PROFILE_SAVE, session, profile ); } } + m_profiles.put( session, profile ); } /** {@inheritDoc} */ diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java index fbf0130fba..50bf3caf00 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/AbstractUserDatabase.java @@ -48,7 +48,7 @@ public abstract class AbstractUserDatabase implements UserDatabase { protected static final String SHA_PREFIX = "{SHA}"; protected static final String SSHA_PREFIX = "{SSHA}"; protected static final String SHA256_PREFIX = "{SHA-256}"; - + protected Engine m_engine; /** * Looks up and returns the first {@link UserProfile} in the user database that whose login name, full name, or wiki name matches the * supplied string. This method provides a "forgiving" search algorithm for resolving principal names when the exact profile attribute @@ -146,9 +146,13 @@ public Principal[] getPrincipals( final String identifier ) throws NoSuchPrincip if( profile.getLoginName() != null && !profile.getLoginName().isEmpty() ) { principals.add( new WikiPrincipal( profile.getLoginName(), WikiPrincipal.LOGIN_NAME ) ); } - if( profile.getFullname() != null && !profile.getFullname().isEmpty() ) { - principals.add( new WikiPrincipal( profile.getFullname(), WikiPrincipal.FULL_NAME ) ); + if ("true".equalsIgnoreCase(m_engine.getWikiProperties().getProperty(Engine.PROP_USE_2_X_ACL_LOGIC, "false"))) { + if( profile.getFullname() != null && !profile.getFullname().isEmpty() ) { + principals.add( new WikiPrincipal( profile.getFullname(), WikiPrincipal.FULL_NAME ) ); + } } + + if( profile.getWikiName() != null && !profile.getWikiName().isEmpty() ) { principals.add( new WikiPrincipal( profile.getWikiName(), WikiPrincipal.WIKI_NAME ) ); } @@ -161,7 +165,9 @@ public Principal[] getPrincipals( final String identifier ) throws NoSuchPrincip * @see org.apache.wiki.auth.user.UserDatabase#initialize(org.apache.wiki.api.core.Engine, java.util.Properties) */ @Override - public abstract void initialize( Engine engine, Properties props ) throws NoRequiredPropertyException, WikiSecurityException; + public void initialize( Engine engine, Properties props ) throws NoRequiredPropertyException, WikiSecurityException { + this.m_engine = engine; + } /** * Factory method that instantiates a new DefaultUserProfile with a new, distinct unique identifier. diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DummyUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DummyUserDatabase.java index af4aea6839..36d588c060 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DummyUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DummyUserDatabase.java @@ -111,6 +111,7 @@ public Principal[] getWikiNames() { */ @Override public void initialize( final Engine engine, final Properties props ) { + this.m_engine = engine; } /** diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/JDBCUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/JDBCUserDatabase.java index 9db7da6f11..6a4ff0c05f 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/JDBCUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/JDBCUserDatabase.java @@ -415,6 +415,7 @@ public Principal[] getWikiNames() throws WikiSecurityException { */ @Override public void initialize( final Engine engine, final Properties props ) throws NoRequiredPropertyException, WikiSecurityException { + m_engine = engine; final String jndiName = props.getProperty( PROP_DB_DATASOURCE, DEFAULT_DB_JNDI_NAME ); try { final Context initCtx = new InitialContext(); diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java index c3c0c39fa0..be23346e56 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/XMLUserDatabase.java @@ -179,6 +179,7 @@ public Principal[] getWikiNames() throws WikiSecurityException { /** {@inheritDoc} */ @Override public void initialize( final Engine engine, final Properties props ) throws NoRequiredPropertyException { + m_engine = engine; final File defaultFile; if( engine.getRootPath() == null ) { LOG.warn( "Cannot identify JSPWiki root path" ); diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthenticationManagerTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthenticationManagerTest.java index e682a42d6f..7950223e25 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthenticationManagerTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/AuthenticationManagerTest.java @@ -111,7 +111,7 @@ public void testCustomAuthorizer() throws Exception { Assertions.assertTrue( session.hasPrincipal( Role.AUTHENTICATED ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( Users.JANNE, WikiPrincipal.LOGIN_NAME ) ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "JanneJalkanen", WikiPrincipal.WIKI_NAME ) ) ); - Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); + //Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); Assertions.assertTrue( session.hasPrincipal( new Role( "AuthorizerRole") ) ); Assertions.assertFalse( session.hasPrincipal( new Role( "ContainerRole") ) ); Assertions.assertFalse( session.hasPrincipal( new Role( "DummyRole") ) ); @@ -123,7 +123,7 @@ public void testCustomAuthorizer() throws Exception { Assertions.assertTrue( session.hasPrincipal( Role.AUTHENTICATED ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( Users.JANNE, WikiPrincipal.LOGIN_NAME ) ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "JanneJalkanen", WikiPrincipal.WIKI_NAME ) ) ); - Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); + //Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); Assertions.assertTrue( session.hasPrincipal( new Role( "AuthorizerRole") ) ); Assertions.assertTrue( session.hasPrincipal( new Role( "ContainerRole") ) ); Assertions.assertFalse( session.hasPrincipal( new Role( "DummyRole") ) ); @@ -181,7 +181,7 @@ public void testLoginCustom() throws Exception { Assertions.assertTrue( session.hasPrincipal( Role.AUTHENTICATED ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( Users.JANNE, WikiPrincipal.LOGIN_NAME ) ) ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "JanneJalkanen", WikiPrincipal.WIKI_NAME ) ) ); - Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); + //Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "Janne Jalkanen", WikiPrincipal.FULL_NAME ) ) ); } @Test @@ -198,7 +198,7 @@ public void testLoginCustomWithGroup() throws Exception { // (ALL, AUTHENTICATED, login, fullname, wikiname Principals) final Session session = WikiSession.guestSession( m_engine ); m_auth.login( session, null, Users.JANNE, Users.JANNE_PASS ); - Assertions.assertEquals( 3, session.getPrincipals().length ); + Assertions.assertEquals( 2, session.getPrincipals().length ); Assertions.assertEquals( 2, session.getRoles().length ); Assertions.assertTrue( session.hasPrincipal( new WikiPrincipal( "JanneJalkanen", WikiPrincipal.WIKI_NAME ) ) ); diff --git a/jspwiki-main/src/test/java/org/apache/wiki/auth/authorize/GroupManagerTest.java b/jspwiki-main/src/test/java/org/apache/wiki/auth/authorize/GroupManagerTest.java index 6f5a605927..9adc8d2b11 100644 --- a/jspwiki-main/src/test/java/org/apache/wiki/auth/authorize/GroupManagerTest.java +++ b/jspwiki-main/src/test/java/org/apache/wiki/auth/authorize/GroupManagerTest.java @@ -78,7 +78,7 @@ public void setUp() throws Exception m_groupMgr.setGroup( m_session, group ); group = m_groupMgr.parseGroup( "Test2", "Bob", true ); m_groupMgr.setGroup( m_session, group ); - group = m_groupMgr.parseGroup( "Test3", "Fred Flintstone", true ); + group = m_groupMgr.parseGroup( "Test3", "Fred", true ); m_groupMgr.setGroup( m_session, group ); // We should see 3 events: 1 for each group add diff --git a/jspwiki-main/src/test/resources/jspwiki-testUserPolicy.policy b/jspwiki-main/src/test/resources/jspwiki-testUserPolicy.policy index 00e081422a..4b80a5bceb 100644 --- a/jspwiki-main/src/test/resources/jspwiki-testUserPolicy.policy +++ b/jspwiki-main/src/test/resources/jspwiki-testUserPolicy.policy @@ -23,7 +23,7 @@ grant principal org.apache.wiki.auth.authorize.Role "Authenticated" { permission org.apache.wiki.auth.permissions.PagePermission "*:*", "view"; }; -grant principal org.apache.wiki.auth.WikiPrincipal "Janne Jalkanen" { +grant principal org.apache.wiki.auth.WikiPrincipal "JanneJalkanen" { permission org.apache.wiki.auth.permissions.PagePermission "*:*", "edit,delete"; }; From 3f5811c43813a3a8ab7a99dbd023851f8a911872 Mon Sep 17 00:00:00 2001 From: Alex O'Ree Date: Sun, 14 Dec 2025 17:38:25 -0500 Subject: [PATCH 6/6] adding notes --- .../java/org/apache/wiki/auth/user/DefaultUserProfile.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DefaultUserProfile.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DefaultUserProfile.java index 9e6470c4cd..5383b2ed4a 100644 --- a/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DefaultUserProfile.java +++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/user/DefaultUserProfile.java @@ -71,6 +71,9 @@ public boolean equals( final Object o ) { final DefaultUserProfile u = ( DefaultUserProfile )o; return same( fullname, u.fullname ) && //same( password, u.password ) && + //Note: this used to compare the password for some reason, but that was causing + //issues when the user wanted to change their password. since this is called + //from DefaultUserManager#setProfile same( loginName, u.loginName ) && same( StringUtils.lowerCase( email ), StringUtils.lowerCase( u.email ) ) && same( wikiname, u.wikiname );