diff --git a/center/src/main/java/com/microsoft/hydralab/center/controller/StorageController.java b/center/src/main/java/com/microsoft/hydralab/center/controller/StorageController.java index f2cc7205c..89d9a5bfa 100644 --- a/center/src/main/java/com/microsoft/hydralab/center/controller/StorageController.java +++ b/center/src/main/java/com/microsoft/hydralab/center/controller/StorageController.java @@ -57,12 +57,11 @@ public Result uploadFile(HttpServletRequest request, @RequestParam("file") MultipartFile uploadedFile, @RequestParam("fileUri") String fileUri) { String storageToken = request.getHeader("Authorization"); - if (storageToken != null) { - storageToken = storageToken.replaceAll("Bearer ", ""); - } else { + storageToken = extractBearerToken(storageToken); + if (storageToken == null) { return Result.error(HttpStatus.UNAUTHORIZED.value(), "Invalid visit with no auth code"); } - if (!storageTokenManageService.validateAccessToken(storageToken)) { + if (!storageTokenManageService.validateAccessToken(storageToken, Const.FilePermission.WRITE, fileUri)) { return Result.error(HttpStatus.UNAUTHORIZED.value(), "Unauthorized, error access token for storage actions."); } if (!LogUtils.isLegalStr(fileUri, Const.RegexString.STORAGE_FILE_REL_PATH, false)) { @@ -90,25 +89,22 @@ public void postDownloadFile(HttpServletRequest request, HttpServletResponse response, @RequestParam("fileUri") String fileUri) { String storageToken = request.getHeader("Authorization"); - if (storageToken != null) { - storageToken = storageToken.replaceAll("Bearer ", ""); - } else { + storageToken = extractBearerToken(storageToken); + if (storageToken == null) { throw new HydraLabRuntimeException(HttpStatus.UNAUTHORIZED.value(), "Invalid visit with no auth code"); } - if (!storageTokenManageService.validateAccessToken(storageToken)) { + boolean canRead = storageTokenManageService.validateAccessToken( + storageToken, Const.FilePermission.READ, fileUri); + boolean canWrite = storageTokenManageService.validateAccessToken( + storageToken, Const.FilePermission.WRITE, fileUri); + if (!canRead && !canWrite) { throw new HydraLabRuntimeException(HttpStatus.UNAUTHORIZED.value(), "Unauthorized, error access token for storage actions."); } if (!LogUtils.isLegalStr(fileUri, Const.RegexString.STORAGE_FILE_REL_PATH, false)) { throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path, file name should not include ';'!"); } - Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath(); - Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath(); - if (!filePath.startsWith(publicFolder + File.separator)) { - throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path"); - } - - File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri); + File file = LocalStorageIOUtil.resolveFilePath(fileUri).toFile(); if (!file.exists()) { throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), String.format("File %s not exist!", fileUri)); } @@ -136,9 +132,6 @@ public void getDownloadFile(HttpServletRequest request, if (token == null) { throw new HydraLabRuntimeException(HttpStatus.UNAUTHORIZED.value(), "Invalid visit with no auth code"); } - if (!storageTokenManageService.validateTokenVal(token)) { - throw new HydraLabRuntimeException(HttpStatus.UNAUTHORIZED.value(), "Unauthorized, error access token for storage actions."); - } final String appendPath = request.getAttribute(HandlerMapping.PATH_WITHIN_HANDLER_MAPPING_ATTRIBUTE).toString(); final String bestMatchingPattern = request.getAttribute(HandlerMapping.BEST_MATCHING_PATTERN_ATTRIBUTE).toString(); String fileUri = new AntPathMatcher().extractPathWithinPattern(bestMatchingPattern, appendPath); @@ -146,7 +139,10 @@ public void getDownloadFile(HttpServletRequest request, throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path, file name should not include ';'!"); } - File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri); + File file = LocalStorageIOUtil.resolveFilePath(fileUri).toFile(); + if (!storageTokenManageService.validateTokenVal(token, fileUri)) { + throw new HydraLabRuntimeException(HttpStatus.UNAUTHORIZED.value(), "Unauthorized, error access token for storage actions."); + } if (!file.exists()) { throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), String.format("File %s not exist!", fileUri)); } @@ -166,6 +162,15 @@ public void getDownloadFile(HttpServletRequest request, logger.info(String.format("Output file: %s , size: %d!", fileUri, resLen)); } + static String extractBearerToken(String authorizationHeader) { + String prefix = "Bearer "; + if (authorizationHeader == null || !authorizationHeader.startsWith(prefix)) { + return null; + } + String token = authorizationHeader.substring(prefix.length()); + return token.trim().isEmpty() ? null : token; + } + @GetMapping("/api/storage/getFileDownloadToken") public Result generateReadToken(@CurrentSecurityContext SysUser requestor, @QueryParam("fileUri") String fileUri) { @@ -174,7 +179,9 @@ public Result generateReadToken(@CurrentSecurityContext SysUser requestor, } if (fileUri.startsWith("/devices/screenshots/")) { - return Result.ok(storageTokenManageService.generateReadTokenForFile(requestor.getMailAddress(), "images" + fileUri).getToken()); + String screenshotStoragePath = getScreenshotStoragePath(fileUri); + return Result.ok(storageTokenManageService.generateReadTokenForFile( + requestor.getMailAddress(), screenshotStoragePath).getToken()); } String blobPath = fileUri; if (blobPath.startsWith("/")) { @@ -193,4 +200,17 @@ public Result generateReadToken(@CurrentSecurityContext SysUser requestor, return Result.ok(token); } + + static String getScreenshotStoragePath(String fileUri) { + String screenshotPrefix = "/devices/screenshots/"; + if (fileUri == null || !fileUri.startsWith(screenshotPrefix)) { + throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid screenshot path"); + } + Path screenshotRoot = Paths.get("images/devices/screenshots").normalize(); + Path screenshotPath = Paths.get("images").resolve(fileUri.substring(1)).normalize(); + if (screenshotPath.equals(screenshotRoot) || !screenshotPath.startsWith(screenshotRoot)) { + throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid screenshot path"); + } + return screenshotPath.toString().replace(File.separatorChar, '/'); + } } diff --git a/center/src/main/java/com/microsoft/hydralab/center/service/StorageTokenManageService.java b/center/src/main/java/com/microsoft/hydralab/center/service/StorageTokenManageService.java index b1dcbc2ae..353ef3694 100644 --- a/center/src/main/java/com/microsoft/hydralab/center/service/StorageTokenManageService.java +++ b/center/src/main/java/com/microsoft/hydralab/center/service/StorageTokenManageService.java @@ -7,14 +7,21 @@ import com.microsoft.hydralab.common.entity.common.StorageFileInfo; import com.microsoft.hydralab.common.file.AccessToken; import com.microsoft.hydralab.common.file.StorageServiceClientProxy; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageProperty; import com.microsoft.hydralab.common.util.Const; import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Service; import org.springframework.util.Assert; import javax.annotation.Resource; +import java.io.File; +import java.nio.charset.StandardCharsets; +import java.nio.file.InvalidPathException; +import java.nio.file.Paths; +import java.security.MessageDigest; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.function.Supplier; /** * @author Li Shen @@ -29,19 +36,15 @@ public class StorageTokenManageService { SysUserService sysUserService; @Resource UserTeamManagementService userTeamManagementService; - private final ConcurrentMap accessTokenMap = new ConcurrentHashMap<>(); + @Resource + LocalStorageProperty localStorageProperty; + private final ConcurrentMap accessTokenMap = new ConcurrentHashMap<>(); + private final ConcurrentMap tokenGrantMap = new ConcurrentHashMap<>(); public AccessToken generateReadToken(String uniqueId) { Assert.notNull(uniqueId, "The key of access token can't be null!"); - AccessToken accessToken = accessTokenMap.get(uniqueId); - - if (accessToken == null || storageServiceClientProxy.isAccessTokenExpired(accessToken)) { - accessToken = storageServiceClientProxy.generateAccessToken(Const.FilePermission.READ); - Assert.notNull(accessToken, "Generate access token with READ permission failed! Access token generated is null!"); - accessTokenMap.put(uniqueId, accessToken); - } - - return accessToken; + return getOrGenerateToken(uniqueId, Const.FilePermission.READ, null, + () -> storageServiceClientProxy.generateAccessToken(Const.FilePermission.READ)); } public boolean checkFileAuthorization(SysUser requestor, StorageFileInfo storageFileInfo) { @@ -70,52 +73,116 @@ public boolean checkFileAuthorization(SysUser requestor, StorageFileInfo storage public AccessToken generateReadTokenForFile(String uniqueId, String fileUri) { Assert.notNull(uniqueId, "The key of access token can't be null!"); Assert.notNull(fileUri, "The file URI can't be null!"); - AccessToken accessToken = accessTokenMap.get(uniqueId + fileUri); - - if (accessToken == null || storageServiceClientProxy.isAccessTokenExpired(accessToken)) { - accessToken = storageServiceClientProxy.generateAccessTokenForFile(Const.FilePermission.READ, fileUri); - Assert.notNull(accessToken, "Generate access token with READ permission failed! Access token generated is null!"); - accessTokenMap.put(uniqueId + fileUri, accessToken); - } - - return accessToken; + String normalizedFileUri = normalizeFileUri(fileUri); + return getOrGenerateToken(uniqueId, Const.FilePermission.READ, normalizedFileUri, + () -> storageServiceClientProxy.generateAccessTokenForFile(Const.FilePermission.READ, normalizedFileUri)); } public AccessToken generateWriteToken(String uniqueId) { Assert.notNull(uniqueId, "The key of access token can't be null!"); - AccessToken accessToken = accessTokenMap.get(uniqueId); + return getOrGenerateToken(uniqueId, Const.FilePermission.WRITE, null, + () -> storageServiceClientProxy.generateAccessToken(Const.FilePermission.WRITE)); + } + + public boolean validateAccessToken(String accessToken, String requiredPermission) { + return validateAccessToken(accessToken, requiredPermission, null); + } - if (accessToken == null || storageServiceClientProxy.isAccessTokenExpired(accessToken)) { - accessToken = storageServiceClientProxy.generateAccessToken(Const.FilePermission.WRITE); - Assert.notNull(accessToken, "Generate access token with WRITE permission failed! Access token generated is null!"); - accessTokenMap.put(uniqueId, accessToken); + public boolean validateAccessToken(String accessToken, String requiredPermission, String fileUri) { + if (StringUtils.isBlank(accessToken) || StringUtils.isBlank(requiredPermission)) { + return false; + } + if (tokensEqual(accessToken, localStorageProperty.getToken())) { + return true; } + return validateGrant(accessToken, requiredPermission, fileUri); + } + public boolean validateTokenVal(String token, String fileUri) { + if (StringUtils.isBlank(token)) { + return false; + } + return validateGrant("token=" + token, Const.FilePermission.READ, fileUri); + } + + private synchronized AccessToken getOrGenerateToken(String uniqueId, String permission, String fileUri, + Supplier tokenSupplier) { + String key = uniqueId + "|" + permission + "|" + StringUtils.defaultString(fileUri); + TokenGrant currentGrant = accessTokenMap.get(key); + if (currentGrant != null && !isGrantExpired(currentGrant)) { + return currentGrant.accessToken; + } + + AccessToken accessToken = tokenSupplier.get(); + Assert.notNull(accessToken, "Generate access token with " + permission + + " permission failed! Access token generated is null!"); + TokenGrant newGrant = new TokenGrant(accessToken, permission, fileUri); + TokenGrant previousGrant = accessTokenMap.put(key, newGrant); + if (previousGrant != null) { + tokenGrantMap.remove(previousGrant.accessToken.getToken(), previousGrant); + } + tokenGrantMap.put(accessToken.getToken(), newGrant); return accessToken; } - // todo: to be updated when needed: check token in the format of "token=xxx&expiryTime=yyy&permission=zzz" - public boolean validateAccessToken(String accessToken) { - return !StringUtils.isBlank(accessToken); + private boolean validateGrant(String token, String requiredPermission, String fileUri) { + TokenGrant grant = tokenGrantMap.get(token); + if (grant == null || !requiredPermission.equals(grant.permission)) { + return false; + } + if (isGrantExpired(grant)) { + tokenGrantMap.remove(token, grant); + return false; + } + if (grant.fileUri == null) { + return true; + } + try { + return grant.fileUri.equals(normalizeFileUri(fileUri)); + } catch (IllegalArgumentException e) { + return false; + } } - // todo: specify content - // for subfield "token" of AccessToken of storage type LOCAL. Differentiate validation method here as AccessToken is split by HTTP PATH EXTRACTION from frontend request already - public boolean validateTokenVal(String token) { - return !StringUtils.isBlank(token); + private boolean isGrantExpired(TokenGrant grant) { + return !Const.FilePermission.WRITE.equals(grant.permission) + && storageServiceClientProxy.isAccessTokenExpired(grant.accessToken); } - @Deprecated - public AccessToken temporaryGetReadSAS(String uniqueId) { - Assert.notNull(uniqueId, "The key of access token can't be null!"); - AccessToken accessToken = accessTokenMap.get(uniqueId); + private static String normalizeFileUri(String fileUri) { + if (StringUtils.isBlank(fileUri)) { + throw new IllegalArgumentException("The file URI can't be blank"); + } + try { + return Paths.get(fileUri).normalize().toString().replace(File.separatorChar, '/'); + } catch (InvalidPathException e) { + throw new IllegalArgumentException("Invalid file URI", e); + } + } - if (accessToken == null || storageServiceClientProxy.isAccessTokenExpired(accessToken)) { - accessToken = storageServiceClientProxy.generateAccessToken(Const.FilePermission.READ); - Assert.notNull(accessToken, "Current storage service doesn't config READ permission!"); - accessTokenMap.put(uniqueId, accessToken); + private static boolean tokensEqual(String first, String second) { + if (first == null || second == null) { + return false; } + return MessageDigest.isEqual(first.getBytes(StandardCharsets.UTF_8), second.getBytes(StandardCharsets.UTF_8)); + } + + private static final class TokenGrant { + private final AccessToken accessToken; + private final String permission; + private final String fileUri; + private TokenGrant(AccessToken accessToken, String permission, String fileUri) { + this.accessToken = accessToken; + this.permission = permission; + this.fileUri = fileUri; + } + } + + @Deprecated + public AccessToken temporaryGetReadSAS(String uniqueId) { + Assert.notNull(uniqueId, "The key of access token can't be null!"); + AccessToken accessToken = generateReadToken(uniqueId); accessToken.copySignature(); return accessToken; } diff --git a/center/src/main/java/com/microsoft/hydralab/center/util/LocalStorageIOUtil.java b/center/src/main/java/com/microsoft/hydralab/center/util/LocalStorageIOUtil.java index 0ec62cc0b..0ae68e362 100644 --- a/center/src/main/java/com/microsoft/hydralab/center/util/LocalStorageIOUtil.java +++ b/center/src/main/java/com/microsoft/hydralab/center/util/LocalStorageIOUtil.java @@ -30,12 +30,7 @@ private LocalStorageIOUtil() { } public static void copyUploadedStreamToFile(InputStream inputStream, String fileUri) { - Path publicFolder = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).normalize().toAbsolutePath(); - Path filePath = publicFolder.resolve(fileUri).normalize().toAbsolutePath(); - if (!filePath.startsWith(publicFolder + File.separator)) { - throw new HydraLabRuntimeException("Invalid file uri"); - } - File file = new File(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT + fileUri); + File file = resolveFilePath(fileUri).toFile(); File parentDirFile = new File(file.getParent()); if (!parentDirFile.exists() && !parentDirFile.mkdirs()) { throw new HydraLabRuntimeException(HttpStatus.INTERNAL_SERVER_ERROR.value(), "mkdirs failed!"); @@ -48,6 +43,18 @@ public static void copyUploadedStreamToFile(InputStream inputStream, String file } } + public static Path resolveFilePath(String fileUri) { + if (fileUri == null) { + throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path"); + } + Path storageRoot = Paths.get(Const.LocalStorageURL.CENTER_LOCAL_STORAGE_ROOT).toAbsolutePath().normalize(); + Path filePath = storageRoot.resolve(fileUri).toAbsolutePath().normalize(); + if (filePath.equals(storageRoot) || !filePath.startsWith(storageRoot)) { + throw new HydraLabRuntimeException(HttpStatus.BAD_REQUEST.value(), "Invalid file path"); + } + return filePath; + } + public static int copyDownloadedStreamToResponse(File file, OutputStream os) { int resLen; try (BufferedInputStream bis = new BufferedInputStream(new FileInputStream(file))) { diff --git a/center/src/main/resources/application.yml b/center/src/main/resources/application.yml index 7a0b6acac..6a88b2bd9 100644 --- a/center/src/main/resources/application.yml +++ b/center/src/main/resources/application.yml @@ -62,7 +62,7 @@ app: SASExpiryUpdate: ${BLOB_SAS_EXPIRY_UPDATE:10} timeUnit: ${TIME_UNIT:MINUTES} local: - token: ${CENTER_TOKEN:token=CENTER_LOCAL_STORAGE_TOKEN} + token: ${CENTER_TOKEN:} endpoint: ${LOCAL_STORAGE_ENDPOINT:http://localhost:9886/} fileExpiryDay: ${fileExpiryDay:-1} location: ${user.dir} diff --git a/center/src/test/java/com/microsoft/hydralab/center/controller/StorageControllerTest.java b/center/src/test/java/com/microsoft/hydralab/center/controller/StorageControllerTest.java new file mode 100644 index 000000000..b496538ec --- /dev/null +++ b/center/src/test/java/com/microsoft/hydralab/center/controller/StorageControllerTest.java @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package com.microsoft.hydralab.center.controller; + +import com.microsoft.hydralab.common.util.HydraLabRuntimeException; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class StorageControllerTest { + @Test + void acceptsOnlyStrictBearerAuthorizationHeader() { + assertEquals("token=issued", StorageController.extractBearerToken("Bearer token=issued")); + assertNull(StorageController.extractBearerToken("token=issued")); + assertNull(StorageController.extractBearerToken("BearerBearer token=issued")); + assertNull(StorageController.extractBearerToken("Bearer ")); + assertNull(StorageController.extractBearerToken("Bearer ")); + } + + @Test + void screenshotTokenPathCannotEscapeScreenshotStorage() { + assertEquals("images/devices/screenshots/device/screen.png", + StorageController.getScreenshotStoragePath("/devices/screenshots/device/screen.png")); + assertThrows(HydraLabRuntimeException.class, + () -> StorageController.getScreenshotStoragePath( + "/devices/screenshots/../../../pkgstore/private.apk")); + } +} diff --git a/center/src/test/java/com/microsoft/hydralab/center/service/StorageTokenManageServiceTest.java b/center/src/test/java/com/microsoft/hydralab/center/service/StorageTokenManageServiceTest.java new file mode 100644 index 000000000..1ae77fb80 --- /dev/null +++ b/center/src/test/java/com/microsoft/hydralab/center/service/StorageTokenManageServiceTest.java @@ -0,0 +1,104 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package com.microsoft.hydralab.center.service; + +import com.microsoft.hydralab.common.file.StorageServiceClientProxy; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageProperty; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageToken; +import com.microsoft.hydralab.common.util.Const; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertSame; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class StorageTokenManageServiceTest { + private StorageTokenManageService tokenService; + private StorageServiceClientProxy storageClient; + + @BeforeEach + void setUp() { + tokenService = new StorageTokenManageService(); + storageClient = mock(StorageServiceClientProxy.class); + tokenService.storageServiceClientProxy = storageClient; + + LocalStorageProperty localStorageProperty = new LocalStorageProperty(); + localStorageProperty.setToken("token=center-secret"); + tokenService.localStorageProperty = localStorageProperty; + } + + @Test + void rejectsTokensThatWereNotIssued() { + assertFalse(tokenService.validateAccessToken("token=made-up", Const.FilePermission.WRITE)); + assertFalse(tokenService.validateTokenVal("made-up", "packages/app.apk")); + } + + @Test + void enforcesPermissionForIssuedTokens() { + LocalStorageToken readToken = token("token=read"); + LocalStorageToken writeToken = token("token=write"); + when(storageClient.generateAccessToken(Const.FilePermission.READ)).thenReturn(readToken); + when(storageClient.generateAccessToken(Const.FilePermission.WRITE)).thenReturn(writeToken); + when(storageClient.isAccessTokenExpired(readToken)).thenReturn(false); + when(storageClient.isAccessTokenExpired(writeToken)).thenReturn(false); + + tokenService.generateReadToken("reader"); + tokenService.generateWriteToken("writer"); + + assertTrue(tokenService.validateAccessToken("token=read", Const.FilePermission.READ)); + assertFalse(tokenService.validateAccessToken("token=read", Const.FilePermission.WRITE)); + assertTrue(tokenService.validateAccessToken("token=write", Const.FilePermission.WRITE)); + } + + @Test + void fileTokenOnlyAuthorizesItsCanonicalPath() { + LocalStorageToken readToken = token("token=file-read"); + when(storageClient.generateAccessTokenForFile(Const.FilePermission.READ, "packages/app.apk")) + .thenReturn(readToken); + when(storageClient.isAccessTokenExpired(readToken)).thenReturn(false); + + tokenService.generateReadTokenForFile("reader", "packages/app.apk"); + + assertTrue(tokenService.validateTokenVal("file-read", "packages/app.apk")); + assertTrue(tokenService.validateTokenVal("file-read", "packages/folder/../app.apk")); + assertFalse(tokenService.validateTokenVal("file-read", "packages/other.apk")); + } + + @Test + void rejectsExpiredIssuedToken() { + LocalStorageToken readToken = token("token=expired"); + when(storageClient.generateAccessToken(Const.FilePermission.READ)).thenReturn(readToken); + when(storageClient.isAccessTokenExpired(readToken)).thenReturn(true); + + tokenService.generateReadToken("reader"); + + assertFalse(tokenService.validateAccessToken("token=expired", Const.FilePermission.READ)); + } + + @Test + void configuredCenterTokenIsHeaderOnly() { + assertTrue(tokenService.validateAccessToken("token=center-secret", Const.FilePermission.WRITE)); + assertTrue(tokenService.validateAccessToken("token=center-secret", Const.FilePermission.READ)); + assertFalse(tokenService.validateTokenVal("center-secret", "packages/app.apk")); + } + + @Test + void writeTokenRemainsStableForAgentsThatCannotRefreshIt() { + LocalStorageToken writeToken = token("token=agent-write"); + when(storageClient.generateAccessToken(Const.FilePermission.WRITE)).thenReturn(writeToken); + when(storageClient.isAccessTokenExpired(writeToken)).thenReturn(true); + + assertSame(tokenService.generateWriteToken("agent"), tokenService.generateWriteToken("agent")); + assertTrue(tokenService.validateAccessToken("token=agent-write", Const.FilePermission.WRITE)); + } + + private LocalStorageToken token(String value) { + LocalStorageToken token = new LocalStorageToken(); + token.setToken(value); + return token; + } +} diff --git a/center/src/test/java/com/microsoft/hydralab/center/util/LocalStorageIOUtilTest.java b/center/src/test/java/com/microsoft/hydralab/center/util/LocalStorageIOUtilTest.java new file mode 100644 index 000000000..c54da7836 --- /dev/null +++ b/center/src/test/java/com/microsoft/hydralab/center/util/LocalStorageIOUtilTest.java @@ -0,0 +1,31 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package com.microsoft.hydralab.center.util; + +import com.microsoft.hydralab.common.util.HydraLabRuntimeException; +import org.junit.jupiter.api.Test; + +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class LocalStorageIOUtilTest { + @Test + void resolvesPathWithinLocalStorageRoot() { + Path resolved = LocalStorageIOUtil.resolveFilePath("packages/app.apk"); + + assertEquals("app.apk", resolved.getFileName().toString()); + } + + @Test + void rejectsPathsOutsideLocalStorageRoot() { + assertThrows(HydraLabRuntimeException.class, + () -> LocalStorageIOUtil.resolveFilePath("../outside.txt")); + assertThrows(HydraLabRuntimeException.class, + () -> LocalStorageIOUtil.resolveFilePath("../local-backup/outside.txt")); + assertThrows(HydraLabRuntimeException.class, + () -> LocalStorageIOUtil.resolveFilePath("C:\\outside.txt")); + } +} diff --git a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageClientAdapter.java b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageClientAdapter.java index 7a2ee3d95..395625912 100644 --- a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageClientAdapter.java +++ b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageClientAdapter.java @@ -13,10 +13,13 @@ import org.springframework.util.Assert; import java.io.File; +import java.nio.file.Paths; +import java.time.Instant; +import java.time.temporal.ChronoUnit; import java.util.UUID; public class LocalStorageClientAdapter extends StorageServiceClient { - private boolean isInitiated = false; + private static final long TOKEN_EXPIRY_MINUTES = 120; private LocalStorageClient localStorageClient; Logger classLogger = LoggerFactory.getLogger(StorageServiceClient.class); @@ -32,9 +35,6 @@ public LocalStorageClientAdapter(StorageProperties storageProperties) { @Override public void updateAccessToken(AccessToken accessToken) { - if (isInitiated) { - return; - } if (!(accessToken instanceof LocalStorageToken)) { return; } @@ -42,25 +42,30 @@ public void updateAccessToken(AccessToken accessToken) { LocalStorageToken localStorageToken = (LocalStorageToken) accessToken; localStorageClient = new LocalStorageClient(localStorageToken); fileExpiryDay = localStorageToken.getFileExpiryDay(); - isInitiated = true; - classLogger.info("Init Agent local storage client successfully!"); + classLogger.info("Updated Agent local storage client access token successfully!"); } @Override public AccessToken generateAccessToken(String permissionType) { LocalStoragePermission permission = LocalStoragePermission.valueOf(permissionType); - // todo: generate token with specific permissions (WRITE/READ) and expiry time LocalStorageToken localStorageToken = new LocalStorageToken(); localStorageToken.setEndpoint(localStorageClient.getEndpoint()); localStorageToken.setToken("token=" + UUID.randomUUID()); localStorageToken.setFileExpiryDay(fileExpiryDay); + localStorageToken.setPermission(permission.name()); + long expiresAt = permission == LocalStoragePermission.WRITE + ? Long.MAX_VALUE + : Instant.now().plus(TOKEN_EXPIRY_MINUTES, ChronoUnit.MINUTES).getEpochSecond(); + localStorageToken.setExpiresAtEpochSecond(expiresAt); return localStorageToken; } @Override public AccessToken generateAccessTokenForFile(String permissionType, String fileUri) { - return generateAccessToken(permissionType); + LocalStorageToken localStorageToken = (LocalStorageToken) generateAccessToken(permissionType); + localStorageToken.setFileUri(Paths.get(fileUri).normalize().toString().replace(File.separatorChar, '/')); + return localStorageToken; } @Override @@ -69,8 +74,7 @@ public boolean isAccessTokenExpired(AccessToken accessToken) { LocalStorageToken localStorageToken = (LocalStorageToken) accessToken; Assert.notNull(localStorageToken, "The localStorageToken can't be null!"); - // todo: check if the token is expired - return true; + return localStorageToken.getExpiresAtEpochSecond() <= Instant.now().getEpochSecond(); } @Override diff --git a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageProperty.java b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageProperty.java index 7734618e7..6ccfcf71d 100644 --- a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageProperty.java +++ b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageProperty.java @@ -9,6 +9,8 @@ import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.stereotype.Component; +import java.util.UUID; + /** * @author Li Shen * @date 3/6/2023 @@ -20,6 +22,14 @@ @Component public class LocalStorageProperty extends StorageProperties { private String endpoint; - private String token; + private String token = generateToken(); private int fileExpiryDay; + + public void setToken(String token) { + this.token = token == null || token.trim().isEmpty() ? generateToken() : token; + } + + private static String generateToken() { + return "token=" + UUID.randomUUID(); + } } diff --git a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageToken.java b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageToken.java index 51153161a..67595bfaa 100644 --- a/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageToken.java +++ b/common/src/main/java/com/microsoft/hydralab/common/file/impl/local/LocalStorageToken.java @@ -18,6 +18,9 @@ public class LocalStorageToken implements AccessToken { private String token; private String endpoint; private int fileExpiryDay; + private String permission; + private String fileUri; + private long expiresAtEpochSecond; @Override public String getToken() { diff --git a/common/src/test/java/com/microsoft/hydralab/common/file/local/LocalStorageClientAdapterTest.java b/common/src/test/java/com/microsoft/hydralab/common/file/local/LocalStorageClientAdapterTest.java new file mode 100644 index 000000000..466e316ca --- /dev/null +++ b/common/src/test/java/com/microsoft/hydralab/common/file/local/LocalStorageClientAdapterTest.java @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +package com.microsoft.hydralab.common.file.local; + +import com.microsoft.hydralab.common.file.AccessToken; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageClientAdapter; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageProperty; +import com.microsoft.hydralab.common.file.impl.local.LocalStorageToken; +import com.microsoft.hydralab.common.util.Const; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class LocalStorageClientAdapterTest { + @Test + void generatedTokensCarryPermissionScopeAndExpiry() { + LocalStorageClientAdapter adapter = adapter(); + + LocalStorageToken token = (LocalStorageToken) adapter.generateAccessTokenForFile( + Const.FilePermission.READ, "packages/app.apk"); + + assertEqualsPermissionAndScope(token); + assertFalse(adapter.isAccessTokenExpired(token)); + } + + @Test + void blankConfiguredTokenGetsUnpredictableDefault() { + LocalStorageProperty first = new LocalStorageProperty(); + LocalStorageProperty second = new LocalStorageProperty(); + first.setToken(""); + second.setToken(""); + + assertTrue(first.getToken().startsWith("token=")); + assertTrue(second.getToken().startsWith("token=")); + assertNotEquals(first.getToken(), second.getToken()); + } + + private LocalStorageClientAdapter adapter() { + LocalStorageProperty property = new LocalStorageProperty(); + property.setEndpoint("http://localhost:9886/"); + property.setToken("token=center-secret"); + property.setFileExpiryDay(-1); + return new LocalStorageClientAdapter(property); + } + + private void assertEqualsPermissionAndScope(LocalStorageToken token) { + assertTrue(token.getToken().startsWith("token=")); + assertTrue(token.getExpiresAtEpochSecond() > 0); + assertEquals(Const.FilePermission.READ, token.getPermission()); + assertEquals("packages/app.apk", token.getFileUri()); + } +}