From c34d02549b6d595a38a33591d59beae764b419a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Coll=20Morag=C3=B3n?= Date: Wed, 30 Aug 2023 11:38:46 +0100 Subject: [PATCH 1/4] core: Fix cellbase validator to acknowledge tokens. #TASK-4913 --- .../core/cellbase/CellBaseValidator.java | 44 ++++++++++++++++++- .../core/cellbase/CellBaseValidatorTest.java | 35 +++++++++++++++ 2 files changed, 78 insertions(+), 1 deletion(-) diff --git a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java index 21f5c79fecd..43da58f396a 100644 --- a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java +++ b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java @@ -1,6 +1,8 @@ package org.opencb.opencga.core.cellbase; +import io.jsonwebtoken.JwtException; import org.apache.commons.lang3.StringUtils; +import org.opencb.biodata.models.variant.avro.VariantAnnotation; import org.opencb.cellbase.client.rest.CellBaseClient; import org.opencb.cellbase.core.config.SpeciesConfiguration; import org.opencb.cellbase.core.config.SpeciesProperties; @@ -8,6 +10,7 @@ import org.opencb.cellbase.core.result.CellBaseDataResponse; import org.opencb.cellbase.core.token.DataAccessTokenManager; import org.opencb.cellbase.core.token.DataAccessTokenSources; +import org.opencb.commons.datastore.core.QueryOptions; import org.opencb.opencga.core.common.VersionUtils; import org.opencb.opencga.core.config.storage.CellBaseConfiguration; import org.slf4j.Logger; @@ -15,6 +18,7 @@ import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.Comparator; import java.util.List; import java.util.stream.Collectors; @@ -46,6 +50,7 @@ private CellBaseClient newCellBaseClient(CellBaseConfiguration cellBaseConfigura toCellBaseSpeciesName(species), assembly, cellBaseConfiguration.getDataRelease(), + cellBaseConfiguration.getToken(), cellBaseConfiguration.toClientConfiguration()); } @@ -136,7 +141,7 @@ public void validate() throws IOException { validate(false); } - public CellBaseConfiguration validate(boolean autoComplete) throws IOException { + private CellBaseConfiguration validate(boolean autoComplete) throws IOException { CellBaseConfiguration cellBaseConfiguration = getCellBaseConfiguration(); String inputVersion = getVersion(); CellBaseDataResponse species; @@ -189,6 +194,38 @@ public CellBaseConfiguration validate(boolean autoComplete) throws IOException { } } } + if (getToken() != null) { + // Check it's supported + if (!supportsToken(serverVersion)) { + throw new IllegalArgumentException("Token not supported for cellbase " + + "url: '" + getURL() + "'" + + ", version: '" + inputVersion + "'"); + } + + // Check it's an actual token + DataAccessTokenManager tokenManager = new DataAccessTokenManager(); + try { + tokenManager.decode(getToken()); + } catch (JwtException e) { + throw new IllegalArgumentException("Malformed token for cellbase " + + "url: '" + getURL() + "'" + + ", version: '" + inputVersion + + "', species: '" + getSpecies() + + "', assembly: '" + getAssembly() + "'"); + } + + // Check it's a valid token + CellBaseDataResponse response = cellBaseClient.getVariantClient() + .getAnnotationByVariantIds(Collections.singletonList("1:1:N:C"), new QueryOptions(), true); + if (response.firstResult() == null) { + throw new IllegalArgumentException("Invalid token for cellbase " + + "url: '" + getURL() + "'" + + ", version: '" + inputVersion + + "', species: '" + getSpecies() + + "', assembly: '" + getAssembly() + "'"); + } + } + return cellBaseConfiguration; } @@ -231,6 +268,11 @@ public static boolean supportsDataReleaseActiveByDefaultIn(String serverVersion) return VersionUtils.isMinVersion("5.5.0", serverVersion, true); } + public static boolean supportsToken(String serverVersion) { + // Tokens support starts at version 5.4.0 + return VersionUtils.isMinVersion("5.4.0", serverVersion); + } + public String getVersionFromServerMajor() throws IOException { return major(getVersionFromServer()); } diff --git a/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java b/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java index 36db80985f0..38d91dd0fc8 100644 --- a/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java +++ b/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java @@ -1,6 +1,8 @@ package org.opencb.opencga.core.cellbase; +import org.apache.commons.lang3.StringUtils; import org.junit.Assert; +import org.junit.Assume; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -70,4 +72,37 @@ public void testNoActiveReleases() throws IOException { thrown.expectMessage("No active data releases found on cellbase"); CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.1", null, null), "mmusculus", "GRCm38", true); } + + @Test + public void testToken() throws IOException { + String token = System.getenv("CELLBASE_HGMD_TOKEN"); + Assume.assumeTrue(StringUtils.isNotEmpty(token)); + CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.4", null, token), "hsapiens", "grch38", true); + Assert.assertNotNull(validated.getToken()); + } + + @Test + public void testTokenNotSupported() throws IOException { + String token = System.getenv("CELLBASE_HGMD_TOKEN"); + Assume.assumeTrue(StringUtils.isNotEmpty(token)); + CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.1", null, token), "hsapiens", "grch38", true); + Assert.assertNotNull(validated.getToken()); + } + + @Test + public void testMalformedToken() throws IOException { + thrown.expectMessage("Malformed token for cellbase"); + String token = "MALFORMED_TOKEN"; + CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.4", null, token), "hsapiens", "grch38", true); + Assert.assertNotNull(validated.getToken()); + } + + @Test + public void testUnsignedToken() throws IOException { + thrown.expectMessage("Invalid token for cellbase"); + String token = "eyJhbGciOiJIUzI1NiJ9.eyJzb3VyY2VzIjp7ImhnbWQiOjkyMjMzNzIwMzY4NTQ3NzU4MDd9LCJ2ZXJzaW9uIjoiMS4wIiwic3ViIjoiWkVUVEEiLCJpYXQiOjE2OTMyMTY5MDd9.invalidsignature"; + CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.4", null, token), "hsapiens", "grch38", true); + Assert.assertNotNull(validated.getToken()); + } + } \ No newline at end of file From cf6c1f136519b50629146f53bbe242f55c79bc90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Coll=20Morag=C3=B3n?= Date: Wed, 30 Aug 2023 17:38:03 +0100 Subject: [PATCH 2/4] core: Do not validate empty string tokens. #TASK-4913 --- .../org/opencb/opencga/core/cellbase/CellBaseValidator.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java index 43da58f396a..d1738dedf02 100644 --- a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java +++ b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java @@ -194,7 +194,9 @@ private CellBaseConfiguration validate(boolean autoComplete) throws IOException } } } - if (getToken() != null) { + if (StringUtils.isEmpty(getToken())) { + cellBaseConfiguration.setToken(null); + } else { // Check it's supported if (!supportsToken(serverVersion)) { throw new IllegalArgumentException("Token not supported for cellbase " From b54ca18149aea527546f90261d0d0b96c05159e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Coll=20Morag=C3=B3n?= Date: Thu, 31 Aug 2023 10:05:31 +0100 Subject: [PATCH 3/4] core: Improve exception message #TASK-4913 --- .../opencga/analysis/variant/manager/VariantStorageManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java b/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java index 806fbb6d818..cf5164f727a 100644 --- a/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java +++ b/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java @@ -1225,7 +1225,7 @@ private R secureTool(String toolId, boolean isOperation, ObjectMap params, S throw e; } catch (Exception e) { exception = e; - throw new StorageEngineException("Error executing operation " + toolId, e); + throw new StorageEngineException("Error executing operation '" + toolId + "' : " + e.getMessage(), e); } finally { if (result instanceof DataResult) { auditAttributes.append("dbTime", ((DataResult) result).getTime()); From ba9e611655319bd6452113578a438a636dfd2009 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jacobo=20Coll=20Morag=C3=B3n?= Date: Thu, 31 Aug 2023 11:15:12 +0100 Subject: [PATCH 4/4] core: Fix validation test #TASK-4913 --- .../analysis/variant/manager/VariantStorageManager.java | 8 +++++++- .../opencb/opencga/core/cellbase/CellBaseValidator.java | 5 +++-- .../opencga/core/cellbase/CellBaseValidatorTest.java | 8 ++++++++ 3 files changed, 18 insertions(+), 3 deletions(-) diff --git a/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java b/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java index cf5164f727a..48b5cf87114 100644 --- a/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java +++ b/opencga-analysis/src/main/java/org/opencb/opencga/analysis/variant/manager/VariantStorageManager.java @@ -49,6 +49,7 @@ import org.opencb.opencga.catalog.managers.StudyManager; import org.opencb.opencga.core.api.ParamConstants; import org.opencb.opencga.core.cellbase.CellBaseValidator; +import org.opencb.opencga.core.common.ExceptionUtils; import org.opencb.opencga.core.common.UriUtils; import org.opencb.opencga.core.config.storage.CellBaseConfiguration; import org.opencb.opencga.core.config.storage.SampleIndexConfiguration; @@ -564,7 +565,10 @@ public OpenCGAResult setCellbaseConfiguration(String project, CellBaseConfi String annotationSaveId, String token) throws CatalogException, StorageEngineException { StopWatch stopwatch = StopWatch.createStarted(); - return secureOperationByProject("configureCellbase", project, new ObjectMap(), token, engine -> { + return secureOperationByProject("configureCellbase", project, new ObjectMap() + .append("cellbaseConfiguration", cellbaseConfiguration) + .append("annotate", annotate) + .append("annotationSaveId", annotationSaveId), token, engine -> { OpenCGAResult result = new OpenCGAResult<>(); result.setResultType(Job.class.getCanonicalName()); result.setResults(new ArrayList<>()); @@ -1237,6 +1241,8 @@ private R secureTool(String toolId, boolean isOperation, ObjectMap params, S if (exception != null) { auditAttributes.append("errorType", exception.getClass()); auditAttributes.append("errorMessage", exception.getMessage()); + auditAttributes.append("errorMessageFull", ExceptionUtils.prettyExceptionMessage(exception, false, true)); + auditAttributes.append("exceptionStackTrace", ExceptionUtils.prettyExceptionStackTrace(exception)); status = new AuditRecord.Status(AuditRecord.Status.Result.ERROR, new Error(-1, exception.getClass().getName(), exception.getMessage())); } else { diff --git a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java index d1738dedf02..467dabf7b75 100644 --- a/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java +++ b/opencga-core/src/main/java/org/opencb/opencga/core/cellbase/CellBaseValidator.java @@ -194,7 +194,8 @@ private CellBaseConfiguration validate(boolean autoComplete) throws IOException } } } - if (StringUtils.isEmpty(getToken())) { + String token = getToken(); + if (StringUtils.isEmpty(token)) { cellBaseConfiguration.setToken(null); } else { // Check it's supported @@ -207,7 +208,7 @@ private CellBaseConfiguration validate(boolean autoComplete) throws IOException // Check it's an actual token DataAccessTokenManager tokenManager = new DataAccessTokenManager(); try { - tokenManager.decode(getToken()); + tokenManager.decode(token); } catch (JwtException e) { throw new IllegalArgumentException("Malformed token for cellbase " + "url: '" + getURL() + "'" diff --git a/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java b/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java index 38d91dd0fc8..36026700237 100644 --- a/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java +++ b/opencga-core/src/test/java/org/opencb/opencga/core/cellbase/CellBaseValidatorTest.java @@ -85,10 +85,18 @@ public void testToken() throws IOException { public void testTokenNotSupported() throws IOException { String token = System.getenv("CELLBASE_HGMD_TOKEN"); Assume.assumeTrue(StringUtils.isNotEmpty(token)); + thrown.expectMessage("Token not supported"); CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.1", null, token), "hsapiens", "grch38", true); Assert.assertNotNull(validated.getToken()); } + @Test + public void testTokenEmpty() throws IOException { + String token = ""; + CellBaseConfiguration validated = CellBaseValidator.validate(new CellBaseConfiguration(ParamConstants.CELLBASE_URL, "v5.1", null, token), "hsapiens", "grch38", true); + Assert.assertNull(validated.getToken()); + } + @Test public void testMalformedToken() throws IOException { thrown.expectMessage("Malformed token for cellbase");