From 078e3b3013a7c74d618eba375986e7f2fadcbaa1 Mon Sep 17 00:00:00 2001 From: johnniang Date: Tue, 11 Feb 2020 19:57:07 +0800 Subject: [PATCH] Remove temp token mechanism --- .../admin/api/BackupController.java | 1 + .../filter/AbstractAuthenticationFilter.java | 49 ++----------- .../halo/app/security/util/SecurityUtils.java | 9 --- .../app/service/impl/BackupServiceImpl.java | 70 +++++++------------ 4 files changed, 32 insertions(+), 97 deletions(-) diff --git a/src/main/java/run/halo/app/controller/admin/api/BackupController.java b/src/main/java/run/halo/app/controller/admin/api/BackupController.java index e75d7f4b0..b250f2da0 100644 --- a/src/main/java/run/halo/app/controller/admin/api/BackupController.java +++ b/src/main/java/run/halo/app/controller/admin/api/BackupController.java @@ -68,6 +68,7 @@ public class BackupController { contentType = request.getServletContext().getMimeType(backupResource.getFile().getAbsolutePath()); } catch (IOException e) { log.warn("Could not determine file type", e); + // Ignore this error } return ResponseEntity.ok() diff --git a/src/main/java/run/halo/app/security/filter/AbstractAuthenticationFilter.java b/src/main/java/run/halo/app/security/filter/AbstractAuthenticationFilter.java index 04c16a954..2423858fb 100644 --- a/src/main/java/run/halo/app/security/filter/AbstractAuthenticationFilter.java +++ b/src/main/java/run/halo/app/security/filter/AbstractAuthenticationFilter.java @@ -9,14 +9,11 @@ import org.springframework.util.Assert; import org.springframework.web.filter.OncePerRequestFilter; import run.halo.app.cache.StringCacheStore; import run.halo.app.config.properties.HaloProperties; -import run.halo.app.exception.ForbiddenException; import run.halo.app.exception.NotInstallException; import run.halo.app.model.properties.PrimaryProperties; -import run.halo.app.model.support.HaloConst; import run.halo.app.security.context.SecurityContextHolder; import run.halo.app.security.handler.AuthenticationFailureHandler; import run.halo.app.security.handler.DefaultAuthenticationFailureHandler; -import run.halo.app.security.util.SecurityUtils; import run.halo.app.service.OptionService; import javax.servlet.FilterChain; @@ -24,7 +21,10 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; /** * Abstract authentication filter. @@ -157,11 +157,6 @@ public abstract class AbstractAuthenticationFilter extends OncePerRequestFilter return; } - if (checkForTempToken(request)) { - filterChain.doFilter(request, response); - return; - } - try { // Do authenticate doAuthenticate(request, response, filterChain); @@ -170,41 +165,7 @@ public abstract class AbstractAuthenticationFilter extends OncePerRequestFilter } } - private boolean checkForTempToken(HttpServletRequest request) { - // Get token from request - String tempToken = getTokenFromRequest(request, HaloConst.TEMP_TOKEN, HaloConst.TEMP_TOKEN); - - if (StringUtils.isEmpty(tempToken)) { - return false; - } - - String tempTokenKey = SecurityUtils.buildTempTokenKey(tempToken); - // Check the token - Optional tokenCountOptional = cacheStore.getAny(tempTokenKey, Integer.class); - - if (!tokenCountOptional.isPresent()) { - // If the token is not found - throw new ForbiddenException("The temporary token has been expired").setErrorData(tempToken); - } - - log.info("Got valid temp token: [{}]", tempToken); - - int count = tokenCountOptional.get(); - // TODO May cause unsafe thread, fixing next time - // Count down - count--; - if (count <= 0) { - // If count is less than 0, then clear this temp token - cacheStore.delete(tempTokenKey); - } else { - // Put the less count - cacheStore.put(tempTokenKey, String.valueOf(count)); - } - - return true; - } - - String getTokenFromRequest(@NonNull HttpServletRequest request, @NonNull String tokenQueryName, @NonNull String tokenHeaderName) { + protected String getTokenFromRequest(@NonNull HttpServletRequest request, @NonNull String tokenQueryName, @NonNull String tokenHeaderName) { Assert.notNull(request, "Http servlet request must not be null"); Assert.hasText(tokenQueryName, "Token query name must not be blank"); Assert.hasText(tokenHeaderName, "Token header name must not be blank"); diff --git a/src/main/java/run/halo/app/security/util/SecurityUtils.java b/src/main/java/run/halo/app/security/util/SecurityUtils.java index 7fb1b54d5..a12d9c981 100644 --- a/src/main/java/run/halo/app/security/util/SecurityUtils.java +++ b/src/main/java/run/halo/app/security/util/SecurityUtils.java @@ -26,9 +26,6 @@ public class SecurityUtils { private final static String REFRESH_TOKEN_CACHE_PREFIX = "halo.admin.refresh_token."; - private final static String TEMP_TOKEN_CACHE_PREFIX = "halo.temp.token."; - - private SecurityUtils() { } @@ -60,10 +57,4 @@ public class SecurityUtils { return TOKEN_REFRESH_CACHE_PREFIX + refreshToken; } - @NonNull - public static String buildTempTokenKey(@NonNull String tempToken) { - Assert.hasText(tempToken, "Temporary token must not be blank"); - - return TEMP_TOKEN_CACHE_PREFIX + tempToken; - } } diff --git a/src/main/java/run/halo/app/service/impl/BackupServiceImpl.java b/src/main/java/run/halo/app/service/impl/BackupServiceImpl.java index d99aa92a5..c88733553 100644 --- a/src/main/java/run/halo/app/service/impl/BackupServiceImpl.java +++ b/src/main/java/run/halo/app/service/impl/BackupServiceImpl.java @@ -6,7 +6,6 @@ import lombok.extern.slf4j.Slf4j; import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.time.DateFormatUtils; -import org.apache.http.client.utils.URIBuilder; import org.json.JSONObject; import org.springframework.core.io.Resource; import org.springframework.core.io.UrlResource; @@ -24,7 +23,6 @@ import run.halo.app.model.dto.post.BasePostDetailDTO; import run.halo.app.model.entity.Post; import run.halo.app.model.entity.Tag; import run.halo.app.model.support.HaloConst; -import run.halo.app.security.util.SecurityUtils; import run.halo.app.service.BackupService; import run.halo.app.service.OptionService; import run.halo.app.service.PostService; @@ -34,7 +32,6 @@ import run.halo.app.utils.HaloUtils; import java.io.File; import java.io.IOException; import java.net.MalformedURLException; -import java.net.URISyntaxException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.NoSuchFileException; @@ -42,17 +39,10 @@ import java.nio.file.Path; import java.nio.file.Paths; import java.time.LocalDateTime; import java.time.format.DateTimeFormatter; -import java.util.ArrayList; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.concurrent.TimeUnit; +import java.util.*; import java.util.stream.Collectors; import java.util.stream.Stream; -import static run.halo.app.model.support.HaloConst.TEMP_TOKEN; -import static run.halo.app.model.support.HaloConst.TEMP_TOKEN_EXPIRATION; - /** * Backup service implementation. * @@ -191,8 +181,14 @@ public class BackupServiceImpl implements BackupService { @Override public List listHaloBackups() { + // Ensure the parent folder exist + Path backupParentPath = Paths.get(haloProperties.getBackupDir()); + if (Files.notExists(backupParentPath)) { + return Collections.emptyList(); + } + // Build backup dto - try (Stream subPathStream = Files.list(Paths.get(haloProperties.getBackupDir()))) { + try (Stream subPathStream = Files.list(backupParentPath)) { return subPathStream .filter(backupPath -> StringUtils.startsWithIgnoreCase(backupPath.getFileName().toString(), HaloConst.HALO_BACKUP_PREFIX)) .map(this::buildBackupDto) @@ -236,19 +232,32 @@ public class BackupServiceImpl implements BackupService { public Resource loadFileAsResource(String fileName) { Assert.hasText(fileName, "Backup file name must not be blank"); - // Get backup file path - Path backupFilePath = Paths.get(haloProperties.getBackupDir(), fileName).normalize(); + Path backupParentPath = Paths.get(haloProperties.getBackupDir()); + try { + if (Files.notExists(backupParentPath)) { + // Create backup parent path if it does not exists + Files.createDirectories(backupParentPath); + } + + // Get backup file path + Path backupFilePath = Paths.get(haloProperties.getBackupDir(), fileName).normalize(); + + // Check directory traversal + run.halo.app.utils.FileUtils.checkDirectoryTraversal(backupParentPath, backupFilePath); + // Build url resource Resource backupResource = new UrlResource(backupFilePath.toUri()); if (!backupResource.exists()) { - // If the backup resouce is not exist + // If the backup resource is not exist throw new NotFoundException("The file " + fileName + " was not found"); } // Return the backup resource return backupResource; } catch (MalformedURLException e) { throw new NotFoundException("The file " + fileName + " was not found", e); + } catch (IOException e) { + throw new ServiceException("Failed to create backup parent path: " + backupParentPath, e); } } @@ -271,8 +280,6 @@ public class BackupServiceImpl implements BackupService { backup.setFileSize(Files.size(backupPath)); } catch (IOException e) { throw new ServiceException("Failed to access file " + backupPath, e); - } catch (URISyntaxException e) { - throw new ServiceException("Failed to generate download link for file: " + backupPath, e); } return backup; @@ -284,36 +291,11 @@ public class BackupServiceImpl implements BackupService { * @param filename filename must not be blank * @return download url */ - private String buildDownloadUrl(@NonNull String filename) throws URISyntaxException { + private String buildDownloadUrl(@NonNull String filename) { Assert.hasText(filename, "File name must not be blank"); // Composite http url - String backupFullUrl = HaloUtils.compositeHttpUrl(optionService.getBlogBaseUrl(), "api/admin/backups/halo", filename); - - // Get temp token - String tempToken = cacheStore.get(buildBackupTokenKey(filename)).orElseGet(() -> { - String token = buildTempToken(1); - // Cache this projection - cacheStore.putIfAbsent(buildBackupTokenKey(filename), token, TEMP_TOKEN_EXPIRATION.toDays(), TimeUnit.DAYS); - return token; - }); - - return new URIBuilder(backupFullUrl).addParameter(TEMP_TOKEN, tempToken).toString(); + return HaloUtils.compositeHttpUrl(optionService.getBlogBaseUrl(), "api/admin/backups/halo", filename); } - private String buildBackupTokenKey(String backupFileName) { - return BACKUP_TOKEN_KEY_PREFIX + backupFileName; - } - - private String buildTempToken(@NonNull Object value) { - Assert.notNull(value, "Temp token value must not be null"); - - // Generate temp token - String tempToken = HaloUtils.randomUUIDWithoutDash(); - - // Cache the token - cacheStore.putIfAbsent(SecurityUtils.buildTempTokenKey(tempToken), value.toString(), TEMP_TOKEN_EXPIRATION.toDays(), TimeUnit.DAYS); - - return tempToken; - } }