Skip to content
This repository has been archived by the owner on Nov 22, 2023. It is now read-only.

Commit

Permalink
Added overloaded permission checks at DAO level
Browse files Browse the repository at this point in the history
  • Loading branch information
@john-shieh
john-shieh committed Jun 29, 2022
1 parent 46d85e0 commit 8a801a9
Show file tree
Hide file tree
Showing 3 changed files with 140 additions and 12 deletions.
46 changes: 46 additions & 0 deletions server/src/main/java/keywhiz/service/daos/SecretController.java
View file
Expand Up @@ -108,6 +108,12 @@ public List<SanitizedSecret> getSanitizedSecrets(@Nullable Long expireMaxTime, @
.collect(toList());
}

public List<SanitizedSecret> getSanitizedSecrets(@Nullable Long expireMaxTime, @Nullable Group group, Object principal) {
return secretDAO.getSecrets(expireMaxTime, group, null, null, null, principal).stream()
.map(SanitizedSecret::fromSecretSeriesAndContent)
.collect(toList());
}

/**
* @param expireMaxTime timestamp for farthest expiry to include
* @return all existing sanitized secrets and their groups matching criteria.
Expand Down Expand Up @@ -211,6 +217,14 @@ public List<SanitizedSecret> getSecretsBatched(int idx, int num, boolean newestF
.collect(toList());
}

public List<SanitizedSecret> getSecretsBatched(int idx, int num, boolean newestFirst, Object principal) {
checkArgument(idx >= 0, "Index must be positive when getting batched secret names!");
checkArgument(num >= 0, "Num must be positive when getting batched secret names!");
return secretDAO.getSecretsBatched(idx, num, newestFirst, principal).stream()
.map(SanitizedSecret::fromSecretSeriesAndContent)
.collect(toList());
}

public SecretBuilder builder(String name, String secret, String creator, long expiry) {
checkArgument(!name.isEmpty());
checkArgument(!secret.isEmpty());
Expand Down Expand Up @@ -334,6 +348,22 @@ public Secret create() {
return transformer.transform(secretDAO.getSecretByName(name).get());
}

public Secret create(Object principal) {
secretDAO.createSecret(
name,
ownerName,
encryptedSecret,
hmac,
creator,
metadata,
expiry,
description,
type,
generationOptions,
principal);
return transformer.transform(secretDAO.getSecretByName(name, principal).get());
}

public Secret createOrUpdate() {
secretDAO.createOrUpdateSecret(
name,
Expand All @@ -348,5 +378,21 @@ public Secret createOrUpdate() {
generationOptions);
return transformer.transform(secretDAO.getSecretByName(name).get());
}

public Secret createOrUpdate(Object principal) {
secretDAO.createOrUpdateSecret(
name,
ownerName,
encryptedSecret,
hmac,
creator,
metadata,
expiry,
description,
type,
generationOptions,
principal);
return transformer.transform(secretDAO.getSecretByName(name, principal).get());
}
}
}
92 changes: 87 additions & 5 deletions server/src/main/java/keywhiz/service/daos/SecretDAO.java
View file
Expand Up @@ -34,6 +34,8 @@
import keywhiz.service.daos.SecretContentDAO.SecretContentDAOFactory;
import keywhiz.service.daos.SecretSeriesDAO.SecretSeriesDAOFactory;
import keywhiz.service.exceptions.ConflictException;
import keywhiz.service.permissions.Action;
import keywhiz.service.permissions.PermissionCheck;
import org.joda.time.DateTime;
import org.jooq.Configuration;
import org.jooq.DSLContext;
Expand Down Expand Up @@ -71,6 +73,7 @@ public class SecretDAO {
private final SecretSeriesDAOFactory secretSeriesDAOFactory;
private final GroupDAO.GroupDAOFactory groupDAOFactory;
private final ContentCryptographer cryptographer;
private final PermissionCheck permissionCheck;

// this is the maximum length of a secret name, so that it will still fit in the 255 char limit
// of the database field if it is deleted and auto-renamed
Expand All @@ -85,12 +88,14 @@ public SecretDAO(
SecretContentDAOFactory secretContentDAOFactory,
SecretSeriesDAOFactory secretSeriesDAOFactory,
GroupDAO.GroupDAOFactory groupDAOFactory,
ContentCryptographer cryptographer) {
ContentCryptographer cryptographer,
PermissionCheck permissionCheck) {
this.dslContext = dslContext;
this.secretContentDAOFactory = secretContentDAOFactory;
this.secretSeriesDAOFactory = secretSeriesDAOFactory;
this.groupDAOFactory = groupDAOFactory;
this.cryptographer = cryptographer;
this.permissionCheck = permissionCheck;
}

@VisibleForTesting
Expand Down Expand Up @@ -157,6 +162,23 @@ public long createSecret(
});
}

public long createSecret(
String name,
String ownerName,
String encryptedSecret,
String hmac,
String creator,
Map<String, String> metadata,
long expiry,
String description,
@Nullable String type,
@Nullable Map<String, String> generationOptions,
Object principal) {
permissionCheck.checkAllowedOrThrow(principal, Action.ADD, null);

return createSecret(name, ownerName, encryptedSecret, hmac, creator, metadata, expiry, description, type, generationOptions);
}

@VisibleForTesting
public long createOrUpdateSecret(
String name,
Expand Down Expand Up @@ -216,6 +238,29 @@ public long createOrUpdateSecret(
});
}

public long createOrUpdateSecret(
String name,
String owner,
String encryptedSecret,
String hmac,
String creator,
Map<String, String> metadata,
long expiry,
String description,
@Nullable String type,
@Nullable Map<String, String> generationOptions,
Object principal) {
Optional<SecretSeries> secretSeries = secretSeriesDAOFactory.using(dslContext.configuration())
.getSecretSeriesByName(name);
if (secretSeries.isPresent()) {
permissionCheck.checkAllowedOrThrow(principal, Action.UPDATE, secretSeries.get());
} else {
permissionCheck.checkAllowedOrThrow(principal, Action.CREATE, null);
}

return createOrUpdateSecret(name, owner, encryptedSecret, hmac, creator, metadata, expiry, description, type, generationOptions);
}

@VisibleForTesting
public long partialUpdateSecret(String name, String creator,
PartialUpdateSecretRequestV2 request) {
Expand Down Expand Up @@ -359,6 +404,14 @@ public Optional<SecretSeriesAndContent> getSecretByName(String name) {
return Optional.empty();
}

public Optional<SecretSeriesAndContent> getSecretByName(String name, Object principal) {
Optional<SecretSeries> series = secretSeriesDAOFactory.using(dslContext.configuration())
.getSecretSeriesByName(name);
permissionCheck.checkAllowedOrThrow(principal, Action.READ, series.get());

return getSecretByName(name);
}

/**
* @param names of secrets series to look up secrets by.
* @return Secrets matching input parameters.
Expand Down Expand Up @@ -419,6 +472,14 @@ public ImmutableList<SecretSeriesAndContent> getSecrets(@Nullable Long expireMax
});
}

public ImmutableList<SecretSeriesAndContent> getSecrets(@Nullable Long expireMaxTime,
@Nullable Group group, @Nullable Long expireMinTime, @Nullable String minName,
@Nullable Integer limit, Object principal) {
permissionCheck.checkAllowedOrThrow(principal, Action.READ, null);

return getSecrets(expireMaxTime, group, expireMinTime, minName, limit);
}

/**
* @return A list of id, name
*/
Expand Down Expand Up @@ -456,6 +517,13 @@ public ImmutableList<SecretSeriesAndContent> getSecretsBatched(int idx, int num,
});
}

public ImmutableList<SecretSeriesAndContent> getSecretsBatched(int idx, int num,
boolean newestFirst, Object principal) {
permissionCheck.checkAllowedOrThrow(principal, Action.READ, null);

return getSecretsBatched(idx, num, newestFirst);
}

/**
* @param name of secret series to look up secrets by.
* @param versionIdx the first index to select in a list of versions sorted by creation time
Expand Down Expand Up @@ -560,6 +628,14 @@ public void deleteSecretsByName(String name) {
.deleteSecretSeriesByName(name);
}

public void deleteSecretsByName(String name, Object principal) {
Optional<SecretSeries> series = secretSeriesDAOFactory.using(dslContext.configuration())
.getSecretSeriesByName(name);
permissionCheck.checkAllowedOrThrow(principal, Action.DELETE, series.get());

deleteSecretsByName(name);
}

/**
* Renames the secret, specified by the secret id, to the name provided
* We check to make sure there are no other secrets that have the same name - if so,
Expand Down Expand Up @@ -668,20 +744,23 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
private final SecretSeriesDAOFactory secretSeriesDAOFactory;
private final GroupDAO.GroupDAOFactory groupDAOFactory;
private final ContentCryptographer cryptographer;
private final PermissionCheck permissionCheck;

@Inject public SecretDAOFactory(
DSLContext jooq,
@Readonly DSLContext readonlyJooq,
SecretContentDAOFactory secretContentDAOFactory,
SecretSeriesDAOFactory secretSeriesDAOFactory,
GroupDAO.GroupDAOFactory groupDAOFactory,
ContentCryptographer cryptographer) {
ContentCryptographer cryptographer,
PermissionCheck permissionCheck) {
this.jooq = jooq;
this.readonlyJooq = readonlyJooq;
this.secretContentDAOFactory = secretContentDAOFactory;
this.secretSeriesDAOFactory = secretSeriesDAOFactory;
this.groupDAOFactory = groupDAOFactory;
this.cryptographer = cryptographer;
this.permissionCheck = permissionCheck;
}

@Override public SecretDAO readwrite() {
Expand All @@ -690,7 +769,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretContentDAOFactory,
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer);
cryptographer,
permissionCheck);
}

@Override public SecretDAO readonly() {
Expand All @@ -699,7 +779,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretContentDAOFactory,
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer);
cryptographer,
permissionCheck);
}

@Override public SecretDAO using(Configuration configuration) {
Expand All @@ -709,7 +790,8 @@ public static class SecretDAOFactory implements DAOFactory<SecretDAO> {
secretContentDAOFactory,
secretSeriesDAOFactory,
groupDAOFactory,
cryptographer);
cryptographer,
permissionCheck);
}
}
}
14 changes: 7 additions & 7 deletions server/src/main/java/keywhiz/service/resources/automation/v2/SecretResource.java
View file
Expand Up @@ -135,7 +135,7 @@ public Response createSecret(@Auth AutomationClient automationClient,

Secret secret;
try {
secret = builder.create();
secret = builder.create(automationClient);
} catch (DataAccessException e) {
logger.info(format("Cannot create secret %s", name), e);
throw new ConflictException(format("Cannot create secret %s.", name));
Expand Down Expand Up @@ -181,7 +181,7 @@ public Response createOrUpdateSecret(@Auth AutomationClient automationClient,
.withMetadata(request.metadata())
.withType(request.type());

builder.createOrUpdate();
builder.createOrUpdate(automationClient);

Map<String, String> extraInfo = new HashMap<>();
if (request.description() != null) {
Expand Down Expand Up @@ -269,11 +269,11 @@ public Iterable<String> secretListing(@Auth AutomationClient automationClient,
throw new BadRequestException(
"Index and num must both be positive when retrieving batched secrets!");
}
return secretControllerReadOnly.getSecretsBatched(idx, num, newestFirst).stream()
return secretControllerReadOnly.getSecretsBatched(idx, num, newestFirst, automationClient).stream()
.map(SanitizedSecret::name)
.collect(toList());
}
return secretControllerReadOnly.getSanitizedSecrets(null, null).stream()
return secretControllerReadOnly.getSanitizedSecrets(null, null, automationClient).stream()
.map(SanitizedSecret::name)
.collect(toSet());
}
Expand Down Expand Up @@ -301,9 +301,9 @@ public Iterable<SanitizedSecret> secretListingV2(@Auth AutomationClient automati
throw new BadRequestException(
"Index and num must both be positive when retrieving batched secrets!");
}
return secretControllerReadOnly.getSecretsBatched(idx, num, newestFirst);
return secretControllerReadOnly.getSecretsBatched(idx, num, newestFirst, automationClient);
}
return secretControllerReadOnly.getSanitizedSecrets(null, null);
return secretControllerReadOnly.getSanitizedSecrets(null, null, automationClient);
}

/**
Expand Down Expand Up @@ -749,7 +749,7 @@ public Response deleteSecretSeries(@Auth AutomationClient automationClient,
// Get the groups for this secret so they can be restored manually if necessary
Set<String> groups = aclDAO.getGroupsFor(secret).stream().map(Group::getName).collect(toSet());

secretDAO.deleteSecretsByName(name);
secretDAO.deleteSecretsByName(name, automationClient);

// Record the deletion in the audit log
Map<String, String> extraInfo = new HashMap<>();
Expand Down

0 comments on commit 8a801a9

Please sign in to comment.