Skip to content

Commit

Permalink
Merge pull request #2428 from opencb/TASK-5923
Browse files Browse the repository at this point in the history 
TASK-5923 - Organization returns authentication origins secretKey
  • Loading branch information
@pfurio
pfurio authored Apr 18, 2024
2 parents bdc444d + 220cb88 commit 2048bc0
Show file tree
Hide file tree
Showing 19 changed files with 339 additions and 123 deletions.
17 changes: 11 additions & 6 deletions ...cga-app/src/main/java/org/opencb/opencga/app/migrations/v3_0_0/OrganizationMigration.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -377,17 +377,22 @@ protected void run() throws Exception {
if (configuration.getAuthentication() != null && CollectionUtils.isNotEmpty(configuration.getAuthentication().getAuthenticationOrigins())) {
for (AuthenticationOrigin authenticationOrigin : configuration.getAuthentication().getAuthenticationOrigins()) {
if (authenticationOrigin.getType().equals(AuthenticationOrigin.AuthenticationType.OPENCGA)
&& authenticationOrigin.getId().equals(CatalogAuthenticationManager.INTERNAL)) {
&& "internal".equals(authenticationOrigin.getId())) {
continue;
}
authenticationOrigin.setAlgorithm(algorithm);
authenticationOrigin.setSecretKey(secretKey);
authenticationOrigin.setExpiration(3600);
authOrigins.add(convertToDocument(authenticationOrigin));
Document authOriginDoc = convertToDocument(authenticationOrigin);
authOriginDoc.put("algorithm", algorithm);
authOriginDoc.put("secretKey", secretKey);
authOriginDoc.put("expiration", 3600L);
authOrigins.add(authOriginDoc);
}
}
}
authOrigins.add(convertToDocument(CatalogAuthenticationManager.createRandomInternalAuthenticationOrigin()));
Document authOriginDoc = convertToDocument(CatalogAuthenticationManager.createOpencgaAuthenticationOrigin());
authOriginDoc.put("id", "internal");
authOriginDoc.put("algorithm", algorithm);
authOriginDoc.put("secretKey", secretKey);
authOriginDoc.put("expiration", 3600L);

// Set organization counter, owner and authOrigins
orgCol.updateOne(Filters.eq("id", organizationId), Updates.combine(
Expand Down
64 changes: 64 additions & 0 deletions ...main/java/org/opencb/opencga/app/migrations/v3_1_0/AuthOriginSimplificationMigration.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
package org.opencb.opencga.app.migrations.v3_1_0;

import com.mongodb.client.model.Filters;
import com.mongodb.client.model.Projections;
import com.mongodb.client.model.UpdateOneModel;
import com.mongodb.client.model.Updates;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.bson.Document;
import org.bson.conversions.Bson;
import org.opencb.opencga.catalog.db.api.OrganizationDBAdaptor;
import org.opencb.opencga.catalog.db.mongodb.OrganizationMongoDBAdaptorFactory;
import org.opencb.opencga.catalog.migration.Migration;
import org.opencb.opencga.catalog.migration.MigrationTool;
import org.opencb.opencga.core.common.PasswordUtils;

import java.util.List;

@Migration(id = "hide_secret_key", description = "Hide secret key #TASK-5923", version = "3.1.0",
language = Migration.MigrationLanguage.JAVA, domain = Migration.MigrationDomain.CATALOG, date = 20240410)
public class AuthOriginSimplificationMigration extends MigrationTool {

@Override
protected void run() throws Exception {
Bson query = Filters.exists("configuration.token", false);
Bson projection = Projections.include("_id", "configuration");
migrateCollection(OrganizationMongoDBAdaptorFactory.ORGANIZATION_COLLECTION, query, projection, (orgDocument, bulk) -> {
Document configuration = orgDocument.get(OrganizationDBAdaptor.QueryParams.CONFIGURATION.key(), Document.class);
if (configuration != null) {
String secretKey = null;
List<Document> authenticationOrigins = configuration.getList("authenticationOrigins", Document.class);
if (CollectionUtils.isNotEmpty(authenticationOrigins)) {
for (Document authenticationOrigin : authenticationOrigins) {
if (authenticationOrigin.getString("type").equals("OPENCGA")) {
secretKey = authenticationOrigin.getString("secretKey");
if (authenticationOrigin.getString("id").equals("internal")) {
authenticationOrigin.put("id", "OPENCGA");
}
}
authenticationOrigin.remove("secretKey");
authenticationOrigin.remove("algorithm");
authenticationOrigin.remove("expiration");
}
}
secretKey = StringUtils.isNotEmpty(secretKey) ? secretKey : PasswordUtils.getStrongRandomPassword(32);
Document token = new Document()
.append("secretKey", secretKey)
.append("algorithm", "HS256")
.append("expiration", 3600L);
configuration.put("token", token);
bulk.add(new UpdateOneModel<>(
Filters.eq("_id", orgDocument.get("_id")),
Updates.set("configuration", configuration)));
}
});

// Change authOrigin id from all "internal" users
getMongoCollection(OrganizationMongoDBAdaptorFactory.USER_COLLECTION)
.updateMany(
new Document("account.authentication.id", "internal"),
Updates.set("account.authentication.id", "OPENCGA"));
}

}
21 changes: 11 additions & 10 deletions ...ain/java/org/opencb/opencga/catalog/auth/authentication/CatalogAuthenticationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import org.opencb.opencga.catalog.exceptions.CatalogException;
import org.opencb.opencga.catalog.utils.ParamUtils;
import org.opencb.opencga.core.common.MailUtils;
import org.opencb.opencga.core.common.PasswordUtils;
import org.opencb.opencga.core.config.AuthenticationOrigin;
import org.opencb.opencga.core.config.Email;
import org.opencb.opencga.core.models.user.AuthenticationResponse;
Expand All @@ -43,19 +42,24 @@
*/
public class CatalogAuthenticationManager extends AuthenticationManager {

// TODO: Remove INTERNAL field and its usages after several releases (TASK-5923)
@Deprecated
public static final String INTERNAL = "internal";
public static final String OPENCGA = "OPENCGA";
private final Email emailConfig;

private final DBAdaptorFactory dbAdaptorFactory;

public CatalogAuthenticationManager(DBAdaptorFactory dbAdaptorFactory, Email emailConfig, String secretKeyString, long expiration) {
public CatalogAuthenticationManager(DBAdaptorFactory dbAdaptorFactory, Email emailConfig, String algorithm, String secretKeyString,
long expiration) {
super(expiration);

this.emailConfig = emailConfig;
this.dbAdaptorFactory = dbAdaptorFactory;

Key secretKey = this.converStringToKeyObject(secretKeyString, SignatureAlgorithm.HS256.getJcaName());
this.jwtManager = new JwtManager(SignatureAlgorithm.HS256.getValue(), secretKey);
SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.valueOf(algorithm);
Key secretKey = this.converStringToKeyObject(secretKeyString, signatureAlgorithm.getJcaName());
this.jwtManager = new JwtManager(signatureAlgorithm.getValue(), secretKey);

this.logger = LoggerFactory.getLogger(CatalogAuthenticationManager.class);
}
Expand Down Expand Up @@ -136,12 +140,9 @@ public OpenCGAResult resetPassword(String organizationId, String userId) throws
return result;
}

public static AuthenticationOrigin createRandomInternalAuthenticationOrigin() {
public static AuthenticationOrigin createOpencgaAuthenticationOrigin() {
return new AuthenticationOrigin()
.setId(CatalogAuthenticationManager.INTERNAL)
.setType(AuthenticationOrigin.AuthenticationType.OPENCGA)
.setAlgorithm("HS256")
.setExpiration(3600L)
.setSecretKey(PasswordUtils.getStrongRandomPassword(32));
.setId(CatalogAuthenticationManager.OPENCGA)
.setType(AuthenticationOrigin.AuthenticationType.OPENCGA);
}
}
7 changes: 4 additions & 3 deletions ...c/main/java/org/opencb/opencga/catalog/auth/authentication/LDAPAuthenticationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ public class LDAPAuthenticationManager extends AuthenticationManager {
private String host;
private boolean ldaps;

public LDAPAuthenticationManager(AuthenticationOrigin authenticationOrigin, String secretKeyString, long expiration) {
public LDAPAuthenticationManager(AuthenticationOrigin authenticationOrigin, String algorithm, String secretKeyString, long expiration) {
super(expiration);
this.logger = LoggerFactory.getLogger(LDAPAuthenticationManager.class);
this.host = authenticationOrigin.getHost();
Expand Down Expand Up @@ -107,8 +107,9 @@ public LDAPAuthenticationManager(AuthenticationOrigin authenticationOrigin, Stri

logger.info("Init LDAP AuthenticationManager. Host: {}, env:{}", host, envToStringRedacted(getDefaultEnv()));

Key secretKey = this.converStringToKeyObject(secretKeyString, SignatureAlgorithm.HS256.getJcaName());
this.jwtManager = new JwtManager(SignatureAlgorithm.HS256.getValue(), secretKey);
SignatureAlgorithm signatureAlgorithm = SignatureAlgorithm.valueOf(algorithm);
Key secretKey = this.converStringToKeyObject(secretKeyString, signatureAlgorithm.getJcaName());
this.jwtManager = new JwtManager(signatureAlgorithm.getValue(), secretKey);
}

protected static String envToStringRedacted(Hashtable<String, Object> env) {
Expand Down
14 changes: 10 additions & 4 deletions ...main/java/org/opencb/opencga/catalog/auth/authentication/azure/AuthenticationFactory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,22 +41,28 @@ public void configureOrganizationAuthenticationManager(Organization organization
Email email = new Email();

Map<String, AuthenticationManager> tmpAuthenticationManagerMap = new HashMap<>();

long expiration = organization.getConfiguration().getToken().getExpiration();
String algorithm = organization.getConfiguration().getToken().getAlgorithm();
String secretKey = organization.getConfiguration().getToken().getSecretKey();

if (organization.getConfiguration() != null
&& CollectionUtils.isNotEmpty(organization.getConfiguration().getAuthenticationOrigins())) {
for (AuthenticationOrigin authOrigin : organization.getConfiguration().getAuthenticationOrigins()) {
if (authOrigin.getId() != null) {
switch (authOrigin.getType()) {
case LDAP:
tmpAuthenticationManagerMap.put(authOrigin.getId(),
new LDAPAuthenticationManager(authOrigin, authOrigin.getSecretKey(), authOrigin.getExpiration()));
new LDAPAuthenticationManager(authOrigin, algorithm, secretKey, expiration));
break;
case AzureAD:
tmpAuthenticationManagerMap.put(authOrigin.getId(), new AzureADAuthenticationManager(authOrigin));
break;
case OPENCGA:
tmpAuthenticationManagerMap.put(authOrigin.getId(),
new CatalogAuthenticationManager(catalogDBAdaptorFactory, email, authOrigin.getSecretKey(),
authOrigin.getExpiration()));
CatalogAuthenticationManager catalogAuthenticationManager =
new CatalogAuthenticationManager(catalogDBAdaptorFactory, email, algorithm, secretKey, expiration);
tmpAuthenticationManagerMap.put(CatalogAuthenticationManager.INTERNAL, catalogAuthenticationManager);
tmpAuthenticationManagerMap.put(CatalogAuthenticationManager.OPENCGA, catalogAuthenticationManager);
break;
default:
logger.warn("Unexpected authentication origin type '{}' for id '{}' found in organization '{}'. "
Expand Down
3 changes: 2 additions & 1 deletion ...log/src/main/java/org/opencb/opencga/catalog/auth/authorization/AuthorizationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package org.opencb.opencga.catalog.auth.authorization;

import org.opencb.opencga.catalog.exceptions.CatalogAuthorizationException;
import org.opencb.opencga.catalog.exceptions.CatalogException;
import org.opencb.opencga.catalog.utils.ParamUtils;
import org.opencb.opencga.core.api.ParamConstants;
Expand Down Expand Up @@ -118,7 +119,7 @@ default boolean isOpencgaAdministrator(JwtPayload payload) throws CatalogExcepti
return isOpencgaAdministrator(payload.getOrganization(), payload.getUserId());
}

boolean isOpencgaAdministrator(String organization, String userId) throws CatalogException;
boolean isOpencgaAdministrator(String organization, String userId) throws CatalogAuthorizationException;

default void checkIsOpencgaAdministrator(JwtPayload payload) throws CatalogException {
checkIsOpencgaAdministrator(payload, null);
Expand Down
19 changes: 12 additions & 7 deletions .../main/java/org/opencb/opencga/catalog/auth/authorization/CatalogAuthorizationManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,11 @@ public void checkCanEditProject(String organizationId, long projectId, String us
}

@Override
public void checkCanViewOrganization(String organizationId, String userId) throws CatalogException {
public void checkCanViewOrganization(String organizationId, String userId) throws CatalogAuthorizationException {
if (isOpencgaAdministrator(organizationId, userId)) {
return;
}
// Check user belongs to organization
if (!dbAdaptorFactory.getCatalogUserDBAdaptor(organizationId).exists(userId)) {
throw new CatalogAuthorizationException("Permission denied. User '" + userId + "' does not belong to the organization.");
}
checkUserExists(organizationId, userId);
}

@Override
Expand Down Expand Up @@ -220,11 +217,11 @@ public void checkCanCreateUpdateDeleteVariableSets(String organizationId, long s
}

@Override
public boolean isOpencgaAdministrator(String organization, String userId) throws CatalogException {
public boolean isOpencgaAdministrator(String organization, String userId) throws CatalogAuthorizationException {
if (ParamConstants.ADMIN_ORGANIZATION.equals(organization) || userId.startsWith(ParamConstants.ADMIN_ORGANIZATION + ":")) {
// Check user exists in ADMIN ORGANIZATION
String user = userId.replace(ParamConstants.ADMIN_ORGANIZATION + ":", "");
dbAdaptorFactory.getCatalogUserDBAdaptor(ParamConstants.ADMIN_ORGANIZATION).checkId(user);
checkUserExists(ParamConstants.ADMIN_ORGANIZATION, user);
return true;
}
return false;
Expand Down Expand Up @@ -1014,6 +1011,14 @@ public void removePermissionRule(String organizationId, long studyId, String per
====================================
*/

private void checkUserExists(String organizationId, String userId) throws CatalogAuthorizationException {
try {
dbAdaptorFactory.getCatalogUserDBAdaptor(organizationId).checkId(userId);
} catch (CatalogException e) {
throw new CatalogAuthorizationException("User '" + userId + "' not authorized to see Org '" + organizationId + "'.", e);
}
}

/**
* Retrieves the groupId where the members belongs to.
*
Expand Down
2 changes: 2 additions & 0 deletions opencga-catalog/src/main/java/org/opencb/opencga/catalog/db/api/OrganizationDBAdaptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ enum QueryParams implements QueryParam {
INTERNAL_MIGRATION_EXECUTIONS("internal.migrationExecutions", OBJECT, ""),
CONFIGURATION("configuration", OBJECT, ""),
CONFIGURATION_AUTHENTICATION_ORIGINS("configuration.authenticationOrigins", OBJECT, ""),
CONFIGURATION_AUTHENTICATION_ORIGINS_OPTIONS("configuration.authenticationOrigins.options", OBJECT, ""),
CONFIGURATION_TOKEN("configuration.token", OBJECT, ""),
CREATION_DATE("creationDate", DATE, ""),
MODIFICATION_DATE("modificationDate", DATE, ""),
PROJECTS("projects", OBJECT, ""),
Expand Down
2 changes: 1 addition & 1 deletion opencga-catalog/src/main/java/org/opencb/opencga/catalog/db/mongodb/UserMongoDBAdaptor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -434,7 +434,7 @@ public OpenCGAResult nativeGet(Query query, QueryOptions options) throws Catalog
public OpenCGAResult update(Query query, ObjectMap parameters, QueryOptions queryOptions) throws CatalogDBException {
Map<String, Object> userParameters = new HashMap<>();

final String[] acceptedParams = {QueryParams.NAME.key(), QueryParams.EMAIL.key(), QueryParams.ORGANIZATION.key()};
final String[] acceptedParams = {QueryParams.NAME.key(), QueryParams.EMAIL.key()};
filterStringParams(parameters, userParameters, acceptedParams);

if (parameters.containsKey(QueryParams.INTERNAL_STATUS_ID.key())) {
Expand Down
44 changes: 44 additions & 0 deletions ...a/org/opencb/opencga/catalog/db/mongodb/iterators/OrganizationCatalogMongoDBIterator.java
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package org.opencb.opencga.catalog.db.mongodb.iterators;

import com.mongodb.client.ClientSession;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.lang3.StringUtils;
import org.bson.Document;
import org.opencb.commons.datastore.core.Query;
Expand Down Expand Up @@ -135,7 +136,50 @@ private void fetchNextBatch() {
}
}

// TODO: Remove this code after several releases (TASK-5923)
recoverSecretKeyTask5923(organizationDocument);

organizationListBuffer.add(organizationDocument);
}
}

// TODO: Remove this method after several releases (TASK-5923)
/**
* This method should only be necessary in order to be able to run the migration for TASK-5923 because OpenCGA looks for the config.
* This configuration was changed in TASK-5923, therefore, OpenCGA would be unable to initialise without it.
* Once the migration is run, this code will do nothing, so it should be removed in the future.
*
* @param organizationDocument Organization document.
*/
private void recoverSecretKeyTask5923(Document organizationDocument) {
Document configuration = organizationDocument.get(OrganizationDBAdaptor.QueryParams.CONFIGURATION.key(), Document.class);
if (configuration != null) {
String secretKey = null;
String algorithm = null;
long expiration = 3600L;
List<Document> authenticationOrigins = configuration.getList("authenticationOrigins", Document.class);
if (CollectionUtils.isNotEmpty(authenticationOrigins)) {
for (Document authenticationOrigin : authenticationOrigins) {
if (authenticationOrigin.getString("type").equals("OPENCGA")) {
secretKey = authenticationOrigin.getString("secretKey");
algorithm = authenticationOrigin.getString("algorithm");
Number expiration1 = authenticationOrigin.get("expiration", Number.class);
expiration = expiration1 != null ? expiration1.longValue() : expiration;

// authenticationOrigin.put("id", "OPENCGA");
}
}
if (StringUtils.isNotEmpty(secretKey)) {
Document token = configuration.get("token", Document.class);
if (token == null || StringUtils.isEmpty(token.getString("secretKey"))) {
token = new Document("secretKey", secretKey)
.append("algorithm", algorithm)
.append("expiration", expiration);
configuration.put("token", token);
}
}
}
}
}

}
Loading

0 comments on commit 2048bc0

Please sign in to comment.