Skip to content

Commit

Permalink
Disable AAD and Azure Blob local auth (#662)
Browse files Browse the repository at this point in the history
<!-- Please provide brief information about the PR, what it contains &
its purpose, new behaviors after the change. And let us know here if you
need any help: https://github.com/microsoft/HydraLab/issues/new -->

## Description
1. Switch access token to id token
2. disable blob local auth and merge 6 blobs to 1

![image](https://github.com/user-attachments/assets/eab6f437-e07a-4568-b8a4-492326872567)

<!-- A few words to explain your changes -->

### Linked GitHub issue ID: #  

## Pull Request Checklist
<!-- Put an x in the boxes that apply. This is simply a reminder of what
we are going to look for before merging your code. -->

- [ ] Tests for the changes have been added (for bug fixes / features)
- [x] Code compiles correctly with all tests are passed.
- [x] I've read the [contributing
guide](https://github.com/microsoft/HydraLab/blob/main/CONTRIBUTING.md#making-changes-to-the-code)
and followed the recommended practices.
- [ ] [Wikis](https://github.com/microsoft/HydraLab/wiki) or
[README](https://github.com/microsoft/HydraLab/blob/main/README.md) have
been reviewed and added / updated if needed (for bug fixes / features)

### Does this introduce a breaking change?
*If this introduces a breaking change for Hydra Lab users, please
describe the impact and migration path.*

- [ ] Yes
- [ ] No

## How you tested it
*Please make sure the change is tested, you can test it by adding UTs,
do local test and share the screenshots, etc.*


Please check the type of change your PR introduces:
- [ ] Bugfix
- [ ] Feature
- [ ] Technical design
- [ ] Build related changes
- [x] Refactoring (no functional changes, no api changes)
- [ ] Code style update (formatting, renaming) or Documentation content
changes
- [ ] Other (please describe): 

### Feature UI screenshots or Technical design diagrams
*If this is a relatively large or complex change, kick it off by drawing
the tech design with PlantUML and explaining why you chose the solution
you did and what alternatives you considered, etc...*
  • Loading branch information
zhou9584 authored Jul 15, 2024
1 parent 1274f78 commit 4dc5eb4
Show file tree
Hide file tree
Showing 9 changed files with 93 additions and 39 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.springframework.security.core.annotation.CurrentSecurityContext;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestParam;
import org.springframework.web.bind.annotation.ResponseBody;
Expand Down Expand Up @@ -72,6 +73,31 @@ public void getAccessToken(@RequestParam("code") String code, HttpServletRequest
response.sendRedirect(redirectUrl);// CodeQL [java/unvalidated-url-redirection] False Positive: Has verified the string by regular expression
}

@PostMapping(value = {"/api/auth"}, produces = MediaType.APPLICATION_JSON_VALUE)
public void getIdToken(@RequestParam("code") String code,
@RequestParam("id_token") String idToken,
HttpServletRequest request,
HttpServletResponse response) throws IOException {
String redirectUrl = Const.FrontEndPath.INDEX_PATH;
if (idToken == null) {
response.sendRedirect(authUtil.getLoginUrl());
return;
}

securityUserService.addSessionAndUserAuth(authUtil.getLoginUserName(idToken), idToken, request.getSession());

String state = request.getParameter("state");
String prefix = Const.FrontEndPath.INDEX_PATH + "?" + Const.FrontEndPath.REDIRECT_PARAM + "=";

if (StringUtils.isNotEmpty(state) && state.startsWith(prefix)) {
String newUrl = state.replace(prefix, "");
if (LogUtils.isLegalStr(newUrl, Const.RegexString.URL, false)) {
redirectUrl = state;
}
}
response.sendRedirect(redirectUrl);// CodeQL [java/unvalidated-url-redirection] False Positive: Has verified the string by regular expression
}

/**
* Authenticated USER: all
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.net.URLEncoder;
import java.util.HashMap;
import java.util.Map;
import java.util.UUID;

@Component
public class AuthUtil {
Expand Down Expand Up @@ -76,10 +77,9 @@ public boolean isIgnore(String requestUrl) {
*/
public boolean verifyToken(String token) {
JSONObject userInfo = decodeAccessToken(token);
if (clientId != null && userInfo != null && clientId.equals(userInfo.getString("appid"))) {
if (clientId != null && userInfo != null && clientId.equals(userInfo.getString("aud"))) {
return true;
}

return false;
}

Expand All @@ -106,7 +106,7 @@ public String getLoginUserName(String accessToken) {
String username = "";
JSONObject userInfo = decodeAccessToken(accessToken);
if (userInfo != null) {
username = userInfo.getString("unique_name");
username = userInfo.getString("email");
}
return username;
}
Expand All @@ -126,7 +126,9 @@ public String getLoginUserDisplayName(String accessToken) {
* @return
*/
public String getLoginUrl() {
String loginUrl = authorizationUri + "?client_id=" + clientId + "&response_type=code&redirect_uri=" + redirectUri + "&response_mode=query&scope=" + scope;
String loginUrl = authorizationUri + "?client_id=" + clientId +
"&response_type=code+id_token&redirect_uri=" + redirectUri +
"&response_mode=form_post&nonce="+ UUID.randomUUID() +"&scope=" + scope;
return loginUrl;
}

Expand Down
3 changes: 2 additions & 1 deletion center/src/main/resources/application.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ app:
storage:
type: ${STORAGE_TYPE:LOCAL} # current available options: LOCAL (default), AZURE
azure:
connection: ${BLOB_CONNECTION_STR:}
endpoint: ${BLOB_ENDPOINT:}
container: ${BLOB_CONTAINER:}
fileExpiryDay: ${fileExpiryDay:6}
CDNUrl: ${CDN_URL:}
SASExpiryTimeFront: ${BLOB_SAS_EXPIRY_FRONT:120}
Expand Down
2 changes: 1 addition & 1 deletion common/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ dependencies {
compile group: 'org.jsoup', name: 'jsoup', version: '1.10.1'
// https://docs.microsoft.com/en-us/graph/sdks/sdk-installation?context=graph%2Fapi%2F1.0&view=graph-rest-1.0
compile 'com.microsoft.graph:microsoft-graph:5.4.0'
compile 'com.azure:azure-identity:1.9.2'
implementation 'com.azure:azure-identity:1.13.0'
//blob storage
compile 'com.azure:azure-storage-blob:12.23.0'
//Apk analysis
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
package com.microsoft.hydralab.common.file.impl.azure;

import com.azure.core.credential.AzureSasCredential;
import com.azure.core.credential.TokenCredential;
import com.azure.core.util.Context;
import com.azure.identity.DefaultAzureCredentialBuilder;
import com.azure.storage.blob.BlobClient;
import com.azure.storage.blob.BlobContainerClient;
import com.azure.storage.blob.BlobServiceClient;
Expand All @@ -13,10 +15,8 @@
import com.azure.storage.blob.models.BlobProperties;
import com.azure.storage.blob.models.BlobStorageException;
import com.azure.storage.blob.models.PublicAccessType;
import com.azure.storage.common.sas.AccountSasPermission;
import com.azure.storage.common.sas.AccountSasResourceType;
import com.azure.storage.common.sas.AccountSasService;
import com.azure.storage.common.sas.AccountSasSignatureValues;
import com.azure.storage.blob.models.UserDelegationKey;
import com.azure.storage.blob.sas.BlobServiceSasSignatureValues;
import com.google.common.net.MediaType;
import com.microsoft.hydralab.common.entity.common.StorageFileInfo;
import com.microsoft.hydralab.common.file.AccessToken;
Expand All @@ -36,6 +36,8 @@
public class AzureBlobClientAdapter extends StorageServiceClient {
private static boolean isAuthedBySAS = true;
private BlobServiceClient blobServiceClient;

private String containerName;
Logger classLogger = LoggerFactory.getLogger(AzureBlobClientAdapter.class);
private long SASExpiryUpdate;
private SASData sasDataInUse = null;
Expand All @@ -50,7 +52,14 @@ public AzureBlobClientAdapter(StorageProperties storageProperties) {
SASExpiryUpdate = azureBlobProperty.getSASExpiryUpdate();
SASPermission.READ.setExpiryTime(azureBlobProperty.getSASExpiryTimeFront(), azureBlobProperty.getTimeUnit());
SASPermission.WRITE.setExpiryTime(azureBlobProperty.getSASExpiryTimeAgent(), azureBlobProperty.getTimeUnit());
blobServiceClient = new BlobServiceClientBuilder().connectionString(azureBlobProperty.getConnection()).buildClient();
TokenCredential defaultAzureCredential = new DefaultAzureCredentialBuilder().build();
blobServiceClient = new BlobServiceClientBuilder()
.endpoint(azureBlobProperty.getEndpoint())
.credential(defaultAzureCredential)
.buildClient();
containerName = azureBlobProperty.getContainer();
// init container
getContainer();
fileExpiryDay = azureBlobProperty.getFileExpiryDay();
cdnUrl = azureBlobProperty.getCDNUrl();
isAuthedBySAS = false;
Expand Down Expand Up @@ -100,9 +109,10 @@ public StorageFileInfo download(File downloadToFile, StorageFileInfo fileInfo) {
return fileInfo;
}

private void buildClientBySAS(SASData sasData) {
public void buildClientBySAS(SASData sasData) {
AzureSasCredential azureSasCredential = new AzureSasCredential(sasData.getToken());
blobServiceClient = new BlobServiceClientBuilder().endpoint(sasData.getEndpoint()).credential(azureSasCredential).buildClient();
containerName = sasData.getContainer();
fileExpiryDay = sasData.getFileExpiryDay();
cdnUrl = sasData.getCdnUrl();
isConnected = true;
Expand All @@ -116,12 +126,14 @@ private void checkBlobClientUpdate() {
}
}

private BlobContainerClient getContainer(String containerName) {
private BlobContainerClient getContainer() {
BlobContainerClient blobContainerClient;
try {
blobContainerClient = blobServiceClient.getBlobContainerClient(containerName);
classLogger.info("Get a BlobContainerClient for container {}", containerName);
if (!blobContainerClient.exists()) {
// If the container doesn't exist, create it.
// If the client is authed by SAS, we don't have permission to create a container.
if (!isAuthedBySAS && !blobContainerClient.exists()) {
classLogger.info("Container {} doesn't exist, will try to create it.", containerName);
blobContainerClient.create();
}
Expand All @@ -134,22 +146,23 @@ private BlobContainerClient getContainer(String containerName) {

public SASData generateSAS(SASPermission sasPermission) {
Assert.isTrue(!isAuthedBySAS, "The client was init by SAS and can't generate SAS!");

SASData sasData = new SASData();
AccountSasService services = AccountSasService.parse(sasPermission.serviceStr);
AccountSasResourceType resourceTypes = AccountSasResourceType.parse(sasPermission.resourceStr);
AccountSasPermission permissions = AccountSasPermission.parse(sasPermission.permissionStr);
OffsetDateTime expiryTime = OffsetDateTime.ofInstant(Instant.now().plus(sasPermission.expiryTime, sasPermission.timeUnit), ZoneId.systemDefault());

AccountSasSignatureValues sasSignatureValues = new AccountSasSignatureValues(expiryTime, permissions,
services, resourceTypes);

sasData.setToken(blobServiceClient.generateAccountSas(sasSignatureValues));
UserDelegationKey userDelegationKey = blobServiceClient.getUserDelegationKey(OffsetDateTime.ofInstant(Instant.now(), ZoneId.systemDefault()), expiryTime);
BlobServiceSasSignatureValues blobServiceSasSignatureValues = new BlobServiceSasSignatureValues(expiryTime, sasPermission.permission);
BlobContainerClient blobContainerClient = blobServiceClient.getBlobContainerClient("testlocalauth");
if (!blobContainerClient.exists()) {
blobContainerClient.create();
}
String sas = blobContainerClient.generateUserDelegationSas(blobServiceSasSignatureValues, userDelegationKey);
sasData.setToken(sas);
sasData.setExpiredTime(expiryTime);
sasData.setEndpoint(blobServiceClient.getAccountUrl());
sasData.setSasPermission(sasPermission);
sasData.setFileExpiryDay(fileExpiryDay);
sasData.setCdnUrl(cdnUrl);
sasData.setContainer(containerName);
return sasData;
}

Expand All @@ -171,7 +184,7 @@ public String uploadFileToBlob(File uploadFile, String containerName, String blo
logger = classLogger;
}

BlobClient blobClient = getContainer(containerName).getBlobClient(blobFilePath);
BlobClient blobClient = getContainer().getBlobClient(containerName + "/" + blobFilePath);
if (uploadFile.getName().endsWith(MediaType.MP4_VIDEO.subtype())) {
BlobHttpHeaders headers = new BlobHttpHeaders();
headers.setContentType(MediaType.MP4_VIDEO.toString());
Expand Down Expand Up @@ -200,7 +213,7 @@ public BlobProperties downloadFileFromBlob(File downloadToFile, String container
if (!saveDir.exists()) {
Assert.isTrue(saveDir.mkdirs(), "mkdirs fail in downloadFileFromUrl");
}
BlobClient blobClient = getContainer(containerName).getBlobClient(blobFilePath);
BlobClient blobClient = getContainer().getBlobClient(containerName + "/" + blobFilePath);
return blobClient.downloadToFile(downloadToFile.getAbsolutePath(), true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@
@ConfigurationProperties(prefix = "app.storage.azure")
@Component
public class AzureBlobProperty extends StorageProperties {
private String connection;
private String endpoint;
private String container;
private long SASExpiryTimeFront;
private long SASExpiryTimeAgent;
private long SASExpiryUpdate;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ public class SASData implements AccessToken {
private String signature;
private String token;
private String endpoint;
private String container;
private OffsetDateTime expiredTime;
private int fileExpiryDay;
private String cdnUrl;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,29 @@
// Licensed under the MIT License.
package com.microsoft.hydralab.common.file.impl.azure;

import com.azure.storage.blob.sas.BlobContainerSasPermission;

import java.time.temporal.ChronoUnit;

public enum SASPermission {
/**
* Define permission
*/
WRITE("b", "co", "war"),
READ("b", "o", "r");
WRITE(true, true),
READ(true, false);

public final String serviceStr, resourceStr, permissionStr;
public long expiryTime;
public ChronoUnit timeUnit;

SASPermission(String serviceStr, String resourceStr, String permissionStr) {
this.serviceStr = serviceStr;
this.resourceStr = resourceStr;
this.permissionStr = permissionStr;
public final BlobContainerSasPermission permission = new BlobContainerSasPermission();

SASPermission(boolean read, boolean write) {
if (read) {
this.permission.setReadPermission(true);
}
if (write) {
this.permission.setWritePermission(true);
}
}

public void setExpiryTime(long expiryTime, String unit) {
Expand All @@ -29,9 +35,9 @@ public void setExpiryTime(long expiryTime, String unit) {
@Override
public String toString() {
return "SASPermission{" +
"serviceStr='" + serviceStr + '\'' +
", resourceStr='" + resourceStr + '\'' +
", permissionStr='" + permissionStr + '\'' +
"expiryTime=" + expiryTime +
", timeUnit=" + timeUnit +
", permission=" + permission +
'}';
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import com.microsoft.hydralab.common.file.StorageServiceClient;
import com.microsoft.hydralab.common.file.impl.azure.AzureBlobClientAdapter;
import com.microsoft.hydralab.common.file.impl.azure.AzureBlobProperty;
import com.microsoft.hydralab.common.file.impl.azure.SASData;
import com.microsoft.hydralab.common.test.BaseTest;
import com.microsoft.hydralab.common.util.Const;
import com.microsoft.hydralab.common.util.ThreadUtils;
Expand All @@ -30,23 +31,26 @@ class BlobClientTest extends BaseTest {

@BeforeAll
void initBlob() {
String connectionString = null;
String endpoint = null;
String container = null;
try {
Dotenv dotenv = Dotenv.load();
connectionString = dotenv.get("BLOB_CONNECTION_STRING");
endpoint = dotenv.get("BLOB_ENDPOINT");
container = dotenv.get("BLOB_CONTAINER");
logger.info("Get connectionString from env file successfully!");
} catch (Exception e) {
logger.error("Get connectionString from env file failed!", e);
}

property.setConnection(connectionString);
property.setEndpoint(endpoint);
property.setContainer(container);
property.setFileExpiryDay(6);
property.setSASExpiryTimeAgent(30);
property.setSASExpiryTimeFront(5);
property.setSASExpiryUpdate(0);
property.setTimeUnit("SECONDS");

if (StringUtils.isBlank(connectionString)) {
if (StringUtils.isBlank(endpoint)) {
storageServiceClient = new MockAzureBlobClient(property);
} else {
storageServiceClient = new AzureBlobClientAdapter(property);
Expand Down

0 comments on commit 4dc5eb4

Please sign in to comment.