You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jspwiki.apache.org by ju...@apache.org on 2021/11/15 14:23:59 UTC
[jspwiki] 09/14: Ensure `getCookieFile( Engine, String )` only deletes files present on cookies' directory
This is an automated email from the ASF dual-hosted git repository.
juanpablo pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jspwiki.git
commit c4f4caebfced1b7ca03eec170976c69a767496a5
Author: Juan Pablo Santos RodrÃguez <ju...@gmail.com>
AuthorDate: Mon Nov 15 14:35:31 2021 +0100
Ensure `getCookieFile( Engine, String )` only deletes files present on cookies' directory
---
.../login/CookieAuthenticationLoginModule.java | 71 +++++++++-------------
1 file changed, 30 insertions(+), 41 deletions(-)
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 4ef8f8c..336da6d 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
@@ -47,7 +47,7 @@ import java.util.UUID;
/**
- * Logs in an user based on a cookie stored in the user's computer. The cookie
+ * Logs in a user based on a cookie stored in the user's computer. The cookie
* information is stored in the <code>jspwiki.workDir</code>, under the directory
* {@value #COOKIE_DIR}. For security purposes it is a very, very good idea
* to prevent access to this directory by everyone except the web server process;
@@ -97,45 +97,39 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
/**
* Describes how often we scrub the cookieDir directory.
*/
- private static final long SCRUB_PERIOD = 60 * 60 * 1000L; // In milliseconds
+ private static final long SCRUB_PERIOD = 60 * 60 * 1_000L; // In milliseconds
/**
- * @see javax.security.auth.spi.LoginModule#login()
* {@inheritDoc}
+ *
+ * @see javax.security.auth.spi.LoginModule#login()
*/
@Override
public boolean login() throws LoginException {
// Otherwise, let's go and look for the cookie!
final HttpRequestCallback hcb = new HttpRequestCallback();
final WikiEngineCallback wcb = new WikiEngineCallback();
-
final Callback[] callbacks = new Callback[] { hcb, wcb };
try {
m_handler.handle( callbacks );
-
final HttpServletRequest request = hcb.getRequest();
final String uid = getLoginCookie( request );
if( uid != null ) {
final Engine engine = wcb.getEngine();
final File cookieFile = getCookieFile( engine, uid );
-
if( cookieFile != null && cookieFile.exists() && cookieFile.canRead() ) {
-
- try( final Reader in = new BufferedReader( new InputStreamReader(Files.newInputStream( cookieFile.toPath() ), StandardCharsets.UTF_8 ) ) ) {
+ try( final Reader in = new BufferedReader( new InputStreamReader( Files.newInputStream( cookieFile.toPath() ), StandardCharsets.UTF_8 ) ) ) {
final String username = FileUtil.readContents( in );
-
if( log.isDebugEnabled() ) {
- log.debug( "Logged in cookie authenticated name=" + username );
+ log.debug( "Logged in cookie authenticated name={}", username );
}
// If login succeeds, commit these principals/roles
m_principals.add( new WikiPrincipal( username, WikiPrincipal.LOGIN_NAME ) );
- //
- // Tag the file so that we know that it has been accessed recently.
- //
+ // Tag the file so that we know that it has been accessed recently.
return cookieFile.setLastModified( System.currentTimeMillis() );
} catch( final IOException e ) {
@@ -152,7 +146,6 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
log.error( message, e );
throw new LoginException( message );
}
-
return false;
}
@@ -160,40 +153,41 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
* Attempts to locate the cookie file.
*
* @param engine Engine
- * @param uid An unique ID fetched from the user cookie
+ * @param uid An unique ID fetched from the user cookie
* @return A File handle, or null, if there was a problem.
*/
private static File getCookieFile( final Engine engine, final String uid ) {
final File cookieDir = new File( engine.getWorkDir(), COOKIE_DIR );
-
if( !cookieDir.exists() ) {
cookieDir.mkdirs();
}
-
if( !cookieDir.canRead() ) {
- log.error( "Cannot read from cookie directory!" + cookieDir.getAbsolutePath() );
+ log.error( "Cannot read from cookie directory! {}", cookieDir.getAbsolutePath() );
return null;
}
-
if( !cookieDir.canWrite() ) {
- log.error( "Cannot write to cookie directory!" + cookieDir.getAbsolutePath() );
+ log.error( "Cannot write to cookie directory! {}", cookieDir.getAbsolutePath() );
return null;
}
- //
// Scrub away old files
- //
final long now = System.currentTimeMillis();
-
if( now > ( c_lastScrubTime + SCRUB_PERIOD ) ) {
scrub( TextUtil.getIntegerProperty( engine.getWikiProperties(), PROP_LOGIN_EXPIRY_DAYS, DEFAULT_EXPIRY_DAYS ), cookieDir );
c_lastScrubTime = now;
}
- //
// Find the cookie file
- //
- return new File( cookieDir, uid );
+ final File file = new File( cookieDir, uid );
+ try {
+ if( file.getCanonicalPath().startsWith( cookieDir.getCanonicalPath() ) ) {
+ return file;
+ }
+ } catch( final IOException e ) {
+ log.error( "Problem retrieving login cookie, returning null: {}", e.getMessage() );
+ return null;
+ }
+ return null;
}
/**
@@ -226,13 +220,11 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
// Write the cookie content to the cookie store file.
try( final Writer out = new BufferedWriter( new OutputStreamWriter( Files.newOutputStream( cf.toPath() ), StandardCharsets.UTF_8 ) ) ) {
FileUtil.copyContents( new StringReader( username ), out );
-
if( log.isDebugEnabled() ) {
- log.debug( "Created login cookie for user " + username + " for " + days + " days" );
+ log.debug( "Created login cookie for user {} for {} days", username, days );
}
-
} catch( final IOException ex ) {
- log.error( "Unable to create cookie file to store user id: " + uid );
+ log.error( "Unable to create cookie file to store user id: {}", uid );
}
}
}
@@ -248,15 +240,12 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
final Cookie userId = getLoginCookie( "" );
userId.setMaxAge( 0 );
response.addCookie( userId );
-
final String uid = getLoginCookie( request );
-
if( uid != null ) {
final File cf = getCookieFile( engine, uid );
-
if( cf != null ) {
if( !cf.delete() ) {
- log.debug( "Error deleting cookie login " + uid );
+ log.debug( "Error deleting cookie login {}", uid );
}
}
}
@@ -265,12 +254,12 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
/**
* Helper function to get secure LOGIN cookie
*
- * @param: value of the cookie
+ * @param value of the cookie
*/
private static Cookie getLoginCookie( final String value ) {
final Cookie c = new Cookie( LOGIN_COOKIE_NAME, value );
- c.setHttpOnly( true ); //no browser access
- c.setSecure( true ); //only access via encrypted https allowed
+ c.setHttpOnly( true ); // no browser access
+ c.setSecure( true ); // only access via encrypted https allowed
return c;
}
@@ -280,8 +269,8 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
* However, if the user has logged in during the expiry period, the expiry is
* reset, and the cookie file left here.
*
- * @param days
- * @param cookieDir
+ * @param days number of days that the cookie will survive
+ * @param cookieDir cookie directory
*/
private static synchronized void scrub( final int days, final File cookieDir ) {
log.debug( "Scrubbing cookieDir..." );
@@ -296,12 +285,12 @@ public class CookieAuthenticationLoginModule extends AbstractLoginModule {
if( f.delete() ) {
deleteCount++;
} else {
- log.debug( "Error deleting cookie login with index " + i );
+ log.debug( "Error deleting cookie login with index {}", i );
}
}
}
- log.debug( "Removed " + deleteCount + " obsolete cookie logins" );
+ log.debug( "Removed {} obsolete cookie logins", deleteCount );
}
}