You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jspwiki.apache.org by br...@apache.org on 2020/02/15 12:18:03 UTC

[jspwiki] branch master updated: Fix a few minor SonarCloud issues.

This is an automated email from the ASF dual-hosted git repository.

brushed pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jspwiki.git


The following commit(s) were added to refs/heads/master by this push:
     new 9be0a19  Fix a few minor  SonarCloud issues.
9be0a19 is described below

commit 9be0a190e5508c726858d1831ecfbd116b09b1b9
Author: brushed <di...@gmail.com>
AuthorDate: Sat Feb 15 13:17:50 2020 +0100

    Fix a few minor  SonarCloud issues.
---
 ChangeLog.md                                       |  2 +
 .../src/main/java/org/apache/wiki/WikiContext.java |  9 ++--
 .../login/CookieAuthenticationLoginModule.java     | 63 ++++++++++++++--------
 3 files changed, 48 insertions(+), 26 deletions(-)

diff --git a/ChangeLog.md b/ChangeLog.md
index a22f71e..7bde688 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -23,6 +23,8 @@ under the License.
 
 * AttachmentManager:  fix the order of processing.  Added a few extra unit tests.
 
+* Few minor SonarCloud fixes
+
 **2020-01-28  Juan Pablo Santos (juanpablo AT apache DOT org)**
 
 * _2.11.0-M7-git-06_
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 13a2d11..65f1821 100644
--- a/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java
+++ b/jspwiki-main/src/main/java/org/apache/wiki/WikiContext.java
@@ -741,11 +741,12 @@ public class WikiContext implements Cloneable, Command {
         //  Figure out which template we should be using for this page.
         String template = null;
         if ( request != null ) {
-            template = request.getParameter( "skin" );
-
-            if( template != null ) {
-                template = template.replaceAll("\\p{Punct}", "");
+            String skin = request.getParameter( "skin" );
+            if( skin != null )
+            {
+                template = skin.replaceAll("\\p{Punct}", "");
             }
+
         }
 
         // If request doesn't supply the value, extract from wiki page
diff --git a/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java b/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java
index bdc28ce..f4615a0 100644
--- a/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java
+++ b/jspwiki-main/src/main/java/org/apache/wiki/auth/login/CookieAuthenticationLoginModule.java
@@ -14,7 +14,7 @@
     "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
     KIND, either express or implied.  See the License for the
     specific language governing permissions and limitations
-    under the License.    
+    under the License.
  */
 package org.apache.wiki.auth.login;
 
@@ -100,14 +100,13 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
 
     /**
      * @see javax.security.auth.spi.LoginModule#login()
-     * 
+     *
      * {@inheritDoc}
      */
     public boolean login() throws LoginException
     {
         // Otherwise, let's go and look for the cookie!
         HttpRequestCallback hcb = new HttpRequestCallback();
-        //UserDatabaseCallback ucb = new UserDatabaseCallback();
         WikiEngineCallback wcb  = new WikiEngineCallback();
 
         Callback[] callbacks = new Callback[]
@@ -127,11 +126,12 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
 
                 if( cookieFile != null && cookieFile.exists() && cookieFile.canRead() )
                 {
-                    Reader in = null;
 
                     try
+                    (
+                        Reader in = new BufferedReader( new InputStreamReader( new FileInputStream( cookieFile ), "UTF-8" ) );
+                    )
                     {
-                        in = new BufferedReader( new InputStreamReader( new FileInputStream( cookieFile ), "UTF-8" ) );
                         String username = FileUtil.readContents( in );
 
                         if ( log.isDebugEnabled() )
@@ -145,18 +145,13 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
                         //
                         //  Tag the file so that we know that it has been accessed recently.
                         //
-                        cookieFile.setLastModified( System.currentTimeMillis() );
+                        return cookieFile.setLastModified( System.currentTimeMillis() );
 
-                        return true;
                     }
                     catch( IOException e )
                     {
                         return false;
                     }
-                    finally
-                    {
-                        if( in != null ) in.close();
-                    }
                 }
             }
         }
@@ -220,8 +215,7 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
         //
         //  Find the cookie file
         //
-        File cookieFile = new File( cookieDir, uid );
-        return cookieFile;
+        return new File( cookieDir, uid );
     }
 
     /**
@@ -232,9 +226,7 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
      */
     private static String getLoginCookie(HttpServletRequest request)
     {
-        String cookie = HttpUtil.retrieveCookieValue( request, LOGIN_COOKIE_NAME );
-
-        return cookie;
+        return HttpUtil.retrieveCookieValue( request, LOGIN_COOKIE_NAME );
     }
 
     /**
@@ -248,19 +240,23 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
     public static void setLoginCookie( final WikiEngine engine, final HttpServletResponse response, final String username ) {
         final UUID uid = UUID.randomUUID();
         final int days = TextUtil.getIntegerProperty( engine.getWikiProperties(), PROP_LOGIN_EXPIRY_DAYS, DEFAULT_EXPIRY_DAYS );
-        final Cookie userId = new Cookie( LOGIN_COOKIE_NAME, uid.toString() );
+        final Cookie userId = getLoginCookie( uid.toString() );
         userId.setMaxAge( days * 24 * 60 * 60 );
         response.addCookie( userId );
 
         final File cf = getCookieFile( engine, uid.toString() );
         if( cf != null ) {
             //  Write the cookie content to the cookie store file.
-            try( final Writer out = new BufferedWriter( new OutputStreamWriter( new FileOutputStream(cf), StandardCharsets.UTF_8 ) ) ) {
+            try(
+                final Writer out = new BufferedWriter( new OutputStreamWriter( new FileOutputStream(cf), StandardCharsets.UTF_8 ) )
+            )
+            {
                 FileUtil.copyContents( new StringReader(username), out );
 
                 if( log.isDebugEnabled() ) {
                     log.debug( "Created login cookie for user "+username+" for "+days+" days" );
                 }
+
             } catch( final IOException ex ) {
                 log.error( "Unable to create cookie file to store user id: " + uid );
             }
@@ -276,7 +272,7 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
      */
     public static void clearLoginCookie( WikiEngine engine, HttpServletRequest request, HttpServletResponse response )
     {
-        Cookie userId = new Cookie( LOGIN_COOKIE_NAME, "" );
+        Cookie userId = getLoginCookie( "" );
         userId.setMaxAge( 0 );
         response.addCookie( userId );
 
@@ -288,12 +284,29 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
 
             if( cf != null )
             {
-                cf.delete();
+                if( !cf.delete() )
+                {
+                    log.debug("Error deleting cookie login "+uid);
+                }
             }
         }
     }
 
     /**
+     *  Helper function to get secure LOGIN cookie
+     *
+     * @param:  value of the cookie
+     */
+    private static Cookie getLoginCookie( String value )
+    {
+        Cookie c = new Cookie( LOGIN_COOKIE_NAME, value );
+        c.setHttpOnly(true);  //no browser access
+        c.setSecure(true); //only access via encrypted https allowed
+        return c;
+    }
+
+
+    /**
      *  Goes through the cookie directory and removes any obsolete files.
      *  The scrubbing takes place one day after the cookie was supposed to expire.
      *  However, if the user has logged in during the expiry period, the expiry is
@@ -320,8 +333,14 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule
 
             if( lastModified < obsoleteDateLimit )
             {
-                f.delete();
-                deleteCount++;
+                if( f.delete() )
+                {
+                    deleteCount++;
+                }
+                else
+                {
+                    log.debug("Error deleting cookie login with index "+i);
+                }
             }
         }