From 73fd8a2970308d6eb2ba9f201f73e56bacdda559 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 28 Feb 2024 16:40:26 -0500 Subject: [PATCH 01/13] Add fence_account_key table to db This will be used for Bond providers --- .../20240228_add_fence_account_key_table.yaml | 30 +++++++++++++++++++ .../db/changelog/db.changelog-master.yaml | 3 ++ 2 files changed, 33 insertions(+) create mode 100644 service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml diff --git a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml new file mode 100644 index 00000000..b4d77876 --- /dev/null +++ b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml @@ -0,0 +1,30 @@ +databaseChangeLog: + - changeSet: + id: "add_fence_account_key_table" + author: sehsan + changes: + - createTable: + tableName: fence_account_key + columns: + - column: + name: id + type: int + autoIncrement: true + constraints: + primaryKey: true + nullable: false + - column: + name: key_json + type: text + constraints: + nullable: false + - column: + name: expires_at + type: timestamp + constraints: + nullable: false + - column: + name: update_lock_timeout + type: timestamp + constraints: + nullable: false diff --git a/service/src/main/resources/db/changelog/db.changelog-master.yaml b/service/src/main/resources/db/changelog/db.changelog-master.yaml index 2bbc5c79..aa3e7402 100644 --- a/service/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/service/src/main/resources/db/changelog/db.changelog-master.yaml @@ -29,3 +29,6 @@ databaseChangeLog: - include: file: changesets/20220412_add_last_encrypt_timestamp_to_ssh_key_pair_table.yaml relativeToChangelogFile: true + - include: + file: changesets/20240228_add_fence_account_key_table.yaml + relativeToChangelogFile: true From 6c996e965c7b1d1aaaf8ecca09d275b86b3dec13 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 29 Feb 2024 12:37:46 -0500 Subject: [PATCH 02/13] Add distributed_lock table --- ...ence_account_key_and_dist_lock_table.yaml} | 19 +++++++++++++++++-- .../db/changelog/db.changelog-master.yaml | 2 +- 2 files changed, 18 insertions(+), 3 deletions(-) rename service/src/main/resources/db/changelog/changesets/{20240228_add_fence_account_key_table.yaml => 20240228_add_fence_account_key_and_dist_lock_table.yaml} (58%) diff --git a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml similarity index 58% rename from service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml rename to service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml index b4d77876..d1c8c7cc 100644 --- a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_table.yaml +++ b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml @@ -1,6 +1,6 @@ databaseChangeLog: - changeSet: - id: "add_fence_account_key_table" + id: "add_fence_account_key_and_dist_lock_table" author: sehsan changes: - createTable: @@ -23,8 +23,23 @@ databaseChangeLog: type: timestamp constraints: nullable: false + - createTable: + tableName: distributed_lock + columns: + - column: + name: lock_name + type: text + constraints: + primaryKey: true + primaryKeyName: pk_dist_lock - column: - name: update_lock_timeout + name: user_id + type: text + constraints: + primaryKey: true + primaryKeyName: pk_dist_lock + - column: + name: expires_at type: timestamp constraints: nullable: false diff --git a/service/src/main/resources/db/changelog/db.changelog-master.yaml b/service/src/main/resources/db/changelog/db.changelog-master.yaml index aa3e7402..a66b26a9 100644 --- a/service/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/service/src/main/resources/db/changelog/db.changelog-master.yaml @@ -30,5 +30,5 @@ databaseChangeLog: file: changesets/20220412_add_last_encrypt_timestamp_to_ssh_key_pair_table.yaml relativeToChangelogFile: true - include: - file: changesets/20240228_add_fence_account_key_table.yaml + file: changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml relativeToChangelogFile: true From 5b99a700390cbed7c2cfecde45f85166cee0ea0b Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 29 Feb 2024 15:32:03 -0500 Subject: [PATCH 03/13] Add models --- .../externalcreds/models/DistributedLock.java | 15 +++++++++++++++ .../externalcreds/models/FenceAccountKey.java | 16 ++++++++++++++++ 2 files changed, 31 insertions(+) create mode 100644 service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java create mode 100644 service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java diff --git a/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java b/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java new file mode 100644 index 00000000..0a8ee0bf --- /dev/null +++ b/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java @@ -0,0 +1,15 @@ +package bio.terra.externalcreds.models; + +import java.sql.Timestamp; +import org.immutables.value.Value; + +@Value.Immutable +public interface DistributedLock extends WithDistributedLock { + String getLockName(); + + String getUserId(); + + Timestamp getExpiresAt(); + + class Builder extends ImmutableDistributedLock.Builder {} +} diff --git a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java new file mode 100644 index 00000000..eb2075fd --- /dev/null +++ b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java @@ -0,0 +1,16 @@ +package bio.terra.externalcreds.models; + +import java.sql.Timestamp; +import java.util.Optional; +import org.immutables.value.Value; + +@Value.Immutable +public interface FenceAccountKey extends WithFenceAccountKey { + Integer getId(); + + Optional getKeyJson(); + + Timestamp getExpiresAt(); + + class Builder extends ImmutableFenceAccountKey.Builder {} +} From 0f250c9f7d0c56336d15e3f6993c1ebafde06031 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Thu, 29 Feb 2024 17:53:27 -0500 Subject: [PATCH 04/13] Add DistributedLockDAO --- .../dataAccess/DistributedLockDAO.java | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) create mode 100644 service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java new file mode 100644 index 00000000..5ceb0878 --- /dev/null +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java @@ -0,0 +1,80 @@ +package bio.terra.externalcreds.dataAccess; + +import bio.terra.externalcreds.models.DistributedLock; +import io.opentelemetry.instrumentation.annotations.WithSpan; +import java.util.Optional; +import lombok.extern.slf4j.Slf4j; +import org.springframework.dao.support.DataAccessUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.stereotype.Repository; + +@Repository +@Slf4j +public class DistributedLockDAO { + + private static final RowMapper DISTRIBUTED_LOCK_ROW_MAPPER = + ((rs, rowNum) -> + new DistributedLock.Builder() + .lockName(rs.getString("lock_name")) + .userId(rs.getString("user_id")) + .expiresAt(rs.getTimestamp("expires_at")) + .build()); + + final NamedParameterJdbcTemplate jdbcTemplate; + + public DistributedLockDAO(NamedParameterJdbcTemplate jdbcTemplate) { + this.jdbcTemplate = jdbcTemplate; + } + + /** + * @param lockName The name of the lock, e.g {provider}-createKey + * @param userId The Sam user id + * @return Optional An optional containing the distributed lock with this + * lockName and userId (or empty) + */ + @WithSpan + public Optional getDistributedLock(String lockName, String userId) { + var namedParameters = + new MapSqlParameterSource().addValue("lockName", lockName).addValue("userId", userId); + var query = "SELECT * FROM distributed_lock WHERE lock_name = :lockName AND user_id = :userId"; + return Optional.ofNullable( + DataAccessUtils.singleResult( + jdbcTemplate.query(query, namedParameters, DISTRIBUTED_LOCK_ROW_MAPPER))); + } + + /** + * @param distributedLock The DistributedLock to insert + * @return distributedLock The DistributedLock that was inserted + */ + @WithSpan + public DistributedLock upsertDistributedLock(DistributedLock distributedLock) { + var query = + "INSERT INTO distributed_lock (lock_name, user_id, expires_at)" + + " VALUES (:lockName, :userId, :expiresAt)"; + + var namedParameters = + new MapSqlParameterSource() + .addValue("lockName", distributedLock.getLockName()) + .addValue("userId", distributedLock.getUserId()) + .addValue("expiresAt", distributedLock.getExpiresAt()); + + jdbcTemplate.update(query, namedParameters); + return distributedLock; + } + + /** + * @param lockName The name of the lock, e.g {provider}-createKey + * @param userId The Sam user id + * @return boolean whether a distributed lock was found and deleted + */ + @WithSpan + public boolean deleteDistributedLock(String lockName, String userId) { + var query = "DELETE FROM distributed_lock WHERE lock_name = :lockName AND user_id = :userId"; + var namedParameters = + new MapSqlParameterSource().addValue("lockName", lockName).addValue("userId", userId); + + return jdbcTemplate.update(query, namedParameters) > 0; + } +} From 09c2d11b5e985e8e376f8a85f8d7c61f25562113 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 4 Mar 2024 11:41:07 -0500 Subject: [PATCH 05/13] Add linked_account_id to fence_account_key Needed to locate a key based on the user and provider name. --- .../bio/terra/externalcreds/models/FenceAccountKey.java | 2 ++ ...20240228_add_fence_account_key_and_dist_lock_table.yaml | 7 +++++++ 2 files changed, 9 insertions(+) diff --git a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java index eb2075fd..ebef018a 100644 --- a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java +++ b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java @@ -8,6 +8,8 @@ public interface FenceAccountKey extends WithFenceAccountKey { Integer getId(); + Integer getLinkedAccountId(); + Optional getKeyJson(); Timestamp getExpiresAt(); diff --git a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml index d1c8c7cc..e8d975f0 100644 --- a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml +++ b/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml @@ -13,6 +13,13 @@ databaseChangeLog: constraints: primaryKey: true nullable: false + - column: + name: linked_account_id + type: int + constraints: + nullable: false + references: linked_account(id) + foreignKeyName: fk_linked_account_id - column: name: key_json type: text From da62d3b285c9dd11cb9a1d0d5c9c52e7d2873082 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 4 Mar 2024 12:06:44 -0500 Subject: [PATCH 06/13] Add FenceAccountKeyDAO --- .../dataAccess/FenceAccountKeyDAO.java | 83 +++++++++++++++++++ .../externalcreds/models/FenceAccountKey.java | 3 +- 2 files changed, 84 insertions(+), 2 deletions(-) create mode 100644 service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java new file mode 100644 index 00000000..70db493a --- /dev/null +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -0,0 +1,83 @@ +package bio.terra.externalcreds.dataAccess; + +import bio.terra.externalcreds.models.FenceAccountKey; +import io.opentelemetry.instrumentation.annotations.WithSpan; +import java.nio.charset.StandardCharsets; +import java.util.*; +import lombok.extern.slf4j.Slf4j; +import org.springframework.dao.support.DataAccessUtils; +import org.springframework.jdbc.core.RowMapper; +import org.springframework.jdbc.core.namedparam.MapSqlParameterSource; +import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate; +import org.springframework.jdbc.support.GeneratedKeyHolder; +import org.springframework.stereotype.Repository; + +@Repository +@Slf4j +public class FenceAccountKeyDAO { + + private static final RowMapper FENCE_ACCOUNT_KEY_ROW_MAPPER = + ((rs, rowNum) -> + new FenceAccountKey.Builder() + .id(rs.getInt("id")) + .linkedAccountId(rs.getInt("linked_account_id")) + .keyJson(rs.getString("key_json")) + .expiresAt(rs.getTimestamp("expires_at")) + .build()); + + final NamedParameterJdbcTemplate jdbcTemplate; + + public FenceAccountKeyDAO(NamedParameterJdbcTemplate jdbcTemplate) { + this.jdbcTemplate = jdbcTemplate; + } + + @WithSpan + public Optional getFenceAccountKey(String userId, String providerName) { + var namedParameters = + new MapSqlParameterSource("userId", userId).addValue("providerName", providerName); + var query = + "SELECT p.* FROM fence_account_key p" + + " INNER JOIN linked_account la ON la.id = p.linked_account_id" + + " WHERE la.user_id = :userId" + + " AND la.provider_name = :providerName"; + return Optional.ofNullable( + DataAccessUtils.singleResult( + jdbcTemplate.query(query, namedParameters, FENCE_ACCOUNT_KEY_ROW_MAPPER))); + } + + @WithSpan + public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { + var query = + "INSERT INTO fence_account_key (linked_account_id, key_json, expires_at)" + + " VALUES (:linkedAccountId, :keyJson, :expiresAt)" + + " RETURNING id"; + + var namedParameters = + new MapSqlParameterSource() + .addValue("linkedAccountId", fenceAccountKey.getLinkedAccountId()) + .addValue("keyJson", base64(fenceAccountKey.getKeyJson())) + .addValue("expiresAt", fenceAccountKey.getExpiresAt()); + + // generatedKeyHolder will hold the id returned by the query as specified by the RETURNING + // clause + var generatedKeyHolder = new GeneratedKeyHolder(); + jdbcTemplate.update(query, namedParameters, generatedKeyHolder); + + return fenceAccountKey.withId(Objects.requireNonNull(generatedKeyHolder.getKey()).intValue()); + } + + /** + * @param linkedAccountId id of the linked account + * @return boolean whether a fence account key was deleted + */ + @WithSpan + public boolean deleteFenceAccountKey(int linkedAccountId) { + var namedParameters = new MapSqlParameterSource("linkedAccountId", linkedAccountId); + var query = "DELETE FROM fence_account_key WHERE linked_account_id = :linkedAccountId"; + return jdbcTemplate.update(query, namedParameters) > 0; + } + + private String base64(String keyJson) { + return Base64.getEncoder().encodeToString(keyJson.getBytes(StandardCharsets.UTF_8)); + } +} diff --git a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java index ebef018a..78576850 100644 --- a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java +++ b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java @@ -1,7 +1,6 @@ package bio.terra.externalcreds.models; import java.sql.Timestamp; -import java.util.Optional; import org.immutables.value.Value; @Value.Immutable @@ -10,7 +9,7 @@ public interface FenceAccountKey extends WithFenceAccountKey { Integer getLinkedAccountId(); - Optional getKeyJson(); + String getKeyJson(); Timestamp getExpiresAt(); From 2668876e2bda2d747ccbeae1fadbf63642f70047 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 4 Mar 2024 12:40:01 -0500 Subject: [PATCH 07/13] Add FenceAccountKeyDAO tests Also fix encoding/decoding key JSON --- .../dataAccess/FenceAccountKeyDAO.java | 10 ++- .../externalcreds/models/FenceAccountKey.java | 3 +- .../bio/terra/externalcreds/TestUtils.java | 9 +++ .../dataAccess/FenceAccountKeyDAOTest.java | 78 +++++++++++++++++++ 4 files changed, 96 insertions(+), 4 deletions(-) create mode 100644 service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index 70db493a..e4c98958 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -21,7 +21,7 @@ public class FenceAccountKeyDAO { new FenceAccountKey.Builder() .id(rs.getInt("id")) .linkedAccountId(rs.getInt("linked_account_id")) - .keyJson(rs.getString("key_json")) + .keyJson(base64Decode(rs.getString("key_json"))) .expiresAt(rs.getTimestamp("expires_at")) .build()); @@ -55,7 +55,7 @@ public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { var namedParameters = new MapSqlParameterSource() .addValue("linkedAccountId", fenceAccountKey.getLinkedAccountId()) - .addValue("keyJson", base64(fenceAccountKey.getKeyJson())) + .addValue("keyJson", base64Encode(fenceAccountKey.getKeyJson())) .addValue("expiresAt", fenceAccountKey.getExpiresAt()); // generatedKeyHolder will hold the id returned by the query as specified by the RETURNING @@ -77,7 +77,11 @@ public boolean deleteFenceAccountKey(int linkedAccountId) { return jdbcTemplate.update(query, namedParameters) > 0; } - private String base64(String keyJson) { + private static String base64Encode(String keyJson) { return Base64.getEncoder().encodeToString(keyJson.getBytes(StandardCharsets.UTF_8)); } + + private static String base64Decode(String encodedKeyJson) { + return new String(Base64.getDecoder().decode(encodedKeyJson), StandardCharsets.UTF_8); + } } diff --git a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java index 78576850..345a6cea 100644 --- a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java +++ b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java @@ -1,11 +1,12 @@ package bio.terra.externalcreds.models; import java.sql.Timestamp; +import java.util.Optional; import org.immutables.value.Value; @Value.Immutable public interface FenceAccountKey extends WithFenceAccountKey { - Integer getId(); + Optional getId(); Integer getLinkedAccountId(); diff --git a/service/src/test/java/bio/terra/externalcreds/TestUtils.java b/service/src/test/java/bio/terra/externalcreds/TestUtils.java index 3c628568..f00782c6 100644 --- a/service/src/test/java/bio/terra/externalcreds/TestUtils.java +++ b/service/src/test/java/bio/terra/externalcreds/TestUtils.java @@ -2,6 +2,7 @@ import bio.terra.externalcreds.config.ProviderProperties; import bio.terra.externalcreds.generated.model.Provider; +import bio.terra.externalcreds.models.FenceAccountKey; import bio.terra.externalcreds.models.GA4GHPassport; import bio.terra.externalcreds.models.GA4GHVisa; import bio.terra.externalcreds.models.LinkedAccount; @@ -56,6 +57,14 @@ public static GA4GHVisa createRandomVisa() { .build(); } + public static FenceAccountKey createRandomFenceAccountKey() { + return new FenceAccountKey.Builder() + .linkedAccountId(1) + .keyJson(UUID.randomUUID().toString()) + .expiresAt(getRandomTimestamp()) + .build(); + } + public static ProviderProperties createRandomProvider() { try { return ProviderProperties.create() diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java new file mode 100644 index 00000000..38b74808 --- /dev/null +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java @@ -0,0 +1,78 @@ +package bio.terra.externalcreds.dataAccess; + +import static org.junit.jupiter.api.Assertions.*; + +import bio.terra.externalcreds.BaseTest; +import bio.terra.externalcreds.TestUtils; +import java.util.Optional; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +class FenceAccountKeyDAOTest extends BaseTest { + + @Autowired private LinkedAccountDAO linkedAccountDAO; + @Autowired private FenceAccountKeyDAO fenceAccountKeyDAO; + + @Test + void testGetMissingFenceAccountKey() { + var shouldBeEmpty = + fenceAccountKeyDAO.getFenceAccountKey("nonexistent_user_id", "nonexistent_provider_name"); + assertEmpty(shouldBeEmpty); + } + + @Nested + class CreateFenceAccountKey { + + @Test + void testCreateAndGetFenceAccountKey() { + var savedAccount = + linkedAccountDAO.upsertLinkedAccount(TestUtils.createRandomLinkedAccount()); + assertPresent(savedAccount.getId()); + + var fenceAccountKey = TestUtils.createRandomFenceAccountKey(); + var savedFenceAccountKey = + fenceAccountKeyDAO.insertFenceAccountKey( + fenceAccountKey.withLinkedAccountId(savedAccount.getId().get())); + assertPresent(savedFenceAccountKey.getId()); + + assertEquals( + fenceAccountKey + .withId(savedFenceAccountKey.getId()) + .withLinkedAccountId(savedFenceAccountKey.getLinkedAccountId()) + .withKeyJson(savedFenceAccountKey.getKeyJson()), + savedFenceAccountKey); + + var loadedFenceAccountKey = + fenceAccountKeyDAO.getFenceAccountKey( + savedAccount.getUserId(), savedAccount.getProviderName()); + assertEquals(Optional.of(savedFenceAccountKey), loadedFenceAccountKey); + } + } + + @Nested + class DeleteFenceAccountKey { + + @Test + void testDeleteFenceAccountKey() { + var savedAccount = + linkedAccountDAO.upsertLinkedAccount(TestUtils.createRandomLinkedAccount()); + assertPresent(savedAccount.getId()); + fenceAccountKeyDAO.insertFenceAccountKey( + TestUtils.createRandomFenceAccountKey().withLinkedAccountId(savedAccount.getId().get())); + + assertPresent( + fenceAccountKeyDAO.getFenceAccountKey( + savedAccount.getUserId(), savedAccount.getProviderName())); + assertTrue(fenceAccountKeyDAO.deleteFenceAccountKey(savedAccount.getId().get())); + assertEmpty( + fenceAccountKeyDAO.getFenceAccountKey( + savedAccount.getUserId(), savedAccount.getProviderName())); + } + + @Test + void testDeleteNonexistentFenceAccountKey() { + assertFalse(fenceAccountKeyDAO.deleteFenceAccountKey(-1)); + } + } +} From 97c8c892077a901e6c5c1c9ef935f82f8bbcf610 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Mon, 4 Mar 2024 12:53:59 -0500 Subject: [PATCH 08/13] Add DistributedLockDAO tests --- .../dataAccess/DistributedLockDAOTest.java | 64 +++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java new file mode 100644 index 00000000..7cf73cd8 --- /dev/null +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java @@ -0,0 +1,64 @@ +package bio.terra.externalcreds.dataAccess; + +import static org.junit.jupiter.api.Assertions.*; + +import bio.terra.externalcreds.BaseTest; +import bio.terra.externalcreds.TestUtils; +import bio.terra.externalcreds.models.DistributedLock; +import java.util.Optional; +import java.util.UUID; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; + +class DistributedLockDAOTest extends BaseTest { + + @Autowired private DistributedLockDAO distributedLockDAO; + private final String testLockName = "provider-createKey"; + private final DistributedLock testDistributedLock = + new DistributedLock.Builder() + .lockName(testLockName) + .userId(UUID.randomUUID().toString()) + .expiresAt(TestUtils.getRandomTimestamp()) + .build(); + + @Test + void testGetMissingDistributedLock() { + var shouldBeEmpty = distributedLockDAO.getDistributedLock(testLockName, "nonexistent_user_id"); + assertEmpty(shouldBeEmpty); + } + + @Nested + class CreateDistributedLock { + + @Test + void testCreateAndGetDistributedLock() { + DistributedLock savedLock = distributedLockDAO.upsertDistributedLock(testDistributedLock); + assertEquals(testDistributedLock, savedLock); + var loadedDistributedLock = + distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId()); + assertEquals(Optional.of(savedLock), loadedDistributedLock); + } + } + + @Nested + class DeleteDistributedLock { + + @Test + void testDeleteDistributedLock() { + DistributedLock savedLock = distributedLockDAO.upsertDistributedLock(testDistributedLock); + assertPresent( + distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId())); + assertTrue( + distributedLockDAO.deleteDistributedLock(savedLock.getLockName(), savedLock.getUserId())); + assertEmpty( + distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId())); + } + + @Test + void testDeleteNonexistentDistributedLock() { + + assertFalse(distributedLockDAO.deleteDistributedLock(testLockName, "nonexistent_user_id")); + } + } +} From f0a6de36816a083c6647a2a6221a1f0fcf9c84bf Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 6 Mar 2024 09:42:40 -0500 Subject: [PATCH 09/13] Code review comments --- .../dataAccess/DistributedLockDAO.java | 5 +++-- .../dataAccess/FenceAccountKeyDAO.java | 16 ++++------------ ...d_fence_account_key_and_dist_lock_table.yaml} | 3 ++- .../db/changelog/db.changelog-master.yaml | 2 +- .../dataAccess/DistributedLockDAOTest.java | 4 ++-- 5 files changed, 12 insertions(+), 18 deletions(-) rename service/src/main/resources/db/changelog/changesets/{20240228_add_fence_account_key_and_dist_lock_table.yaml => 20240305_add_fence_account_key_and_dist_lock_table.yaml} (95%) diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java index 5ceb0878..ace576ba 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java @@ -38,7 +38,8 @@ public DistributedLockDAO(NamedParameterJdbcTemplate jdbcTemplate) { public Optional getDistributedLock(String lockName, String userId) { var namedParameters = new MapSqlParameterSource().addValue("lockName", lockName).addValue("userId", userId); - var query = "SELECT * FROM distributed_lock WHERE lock_name = :lockName AND user_id = :userId"; + var query = + "SELECT lock_name, user_id, expires_at FROM distributed_lock WHERE lock_name = :lockName AND user_id = :userId"; return Optional.ofNullable( DataAccessUtils.singleResult( jdbcTemplate.query(query, namedParameters, DISTRIBUTED_LOCK_ROW_MAPPER))); @@ -49,7 +50,7 @@ public Optional getDistributedLock(String lockName, String user * @return distributedLock The DistributedLock that was inserted */ @WithSpan - public DistributedLock upsertDistributedLock(DistributedLock distributedLock) { + public DistributedLock insertDistributedLock(DistributedLock distributedLock) { var query = "INSERT INTO distributed_lock (lock_name, user_id, expires_at)" + " VALUES (:lockName, :userId, :expiresAt)"; diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index e4c98958..a881d311 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -2,7 +2,6 @@ import bio.terra.externalcreds.models.FenceAccountKey; import io.opentelemetry.instrumentation.annotations.WithSpan; -import java.nio.charset.StandardCharsets; import java.util.*; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.support.DataAccessUtils; @@ -21,7 +20,7 @@ public class FenceAccountKeyDAO { new FenceAccountKey.Builder() .id(rs.getInt("id")) .linkedAccountId(rs.getInt("linked_account_id")) - .keyJson(base64Decode(rs.getString("key_json"))) + .keyJson(rs.getString("key_json")) .expiresAt(rs.getTimestamp("expires_at")) .build()); @@ -36,8 +35,8 @@ public Optional getFenceAccountKey(String userId, String provid var namedParameters = new MapSqlParameterSource("userId", userId).addValue("providerName", providerName); var query = - "SELECT p.* FROM fence_account_key p" - + " INNER JOIN linked_account la ON la.id = p.linked_account_id" + "SELECT fence.id, fence.linked_account_id, fence.key_json, fence.expires_at FROM fence_account_key fence" + + " INNER JOIN linked_account la ON la.id = fence.linked_account_id" + " WHERE la.user_id = :userId" + " AND la.provider_name = :providerName"; return Optional.ofNullable( @@ -55,7 +54,7 @@ public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { var namedParameters = new MapSqlParameterSource() .addValue("linkedAccountId", fenceAccountKey.getLinkedAccountId()) - .addValue("keyJson", base64Encode(fenceAccountKey.getKeyJson())) + .addValue("keyJson", fenceAccountKey.getKeyJson()) .addValue("expiresAt", fenceAccountKey.getExpiresAt()); // generatedKeyHolder will hold the id returned by the query as specified by the RETURNING @@ -77,11 +76,4 @@ public boolean deleteFenceAccountKey(int linkedAccountId) { return jdbcTemplate.update(query, namedParameters) > 0; } - private static String base64Encode(String keyJson) { - return Base64.getEncoder().encodeToString(keyJson.getBytes(StandardCharsets.UTF_8)); - } - - private static String base64Decode(String encodedKeyJson) { - return new String(Base64.getDecoder().decode(encodedKeyJson), StandardCharsets.UTF_8); - } } diff --git a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml b/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml similarity index 95% rename from service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml rename to service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml index e8d975f0..052943bc 100644 --- a/service/src/main/resources/db/changelog/changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml +++ b/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml @@ -20,9 +20,10 @@ databaseChangeLog: nullable: false references: linked_account(id) foreignKeyName: fk_linked_account_id + deleteCascade: true - column: name: key_json - type: text + type: jsonb constraints: nullable: false - column: diff --git a/service/src/main/resources/db/changelog/db.changelog-master.yaml b/service/src/main/resources/db/changelog/db.changelog-master.yaml index 99bc1dec..fe15c369 100644 --- a/service/src/main/resources/db/changelog/db.changelog-master.yaml +++ b/service/src/main/resources/db/changelog/db.changelog-master.yaml @@ -33,5 +33,5 @@ databaseChangeLog: file: changesets/20240304_provider_name_to_provider.yaml relativeToChangelogFile: true - include: - file: changesets/20240228_add_fence_account_key_and_dist_lock_table.yaml + file: changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml relativeToChangelogFile: true diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java index 7cf73cd8..d5ed08ee 100644 --- a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java @@ -33,7 +33,7 @@ class CreateDistributedLock { @Test void testCreateAndGetDistributedLock() { - DistributedLock savedLock = distributedLockDAO.upsertDistributedLock(testDistributedLock); + DistributedLock savedLock = distributedLockDAO.insertDistributedLock(testDistributedLock); assertEquals(testDistributedLock, savedLock); var loadedDistributedLock = distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId()); @@ -46,7 +46,7 @@ class DeleteDistributedLock { @Test void testDeleteDistributedLock() { - DistributedLock savedLock = distributedLockDAO.upsertDistributedLock(testDistributedLock); + DistributedLock savedLock = distributedLockDAO.insertDistributedLock(testDistributedLock); assertPresent( distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId())); assertTrue( From ccfc78fa243d7921f6d0b2e4b759ae7765ea6b5e Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 6 Mar 2024 09:50:09 -0500 Subject: [PATCH 10/13] Change providerName to provider --- .../externalcreds/dataAccess/FenceAccountKeyDAO.java | 10 ++++++---- .../dataAccess/FenceAccountKeyDAOTest.java | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index a881d311..9ca5b11e 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -1,5 +1,6 @@ package bio.terra.externalcreds.dataAccess; +import bio.terra.externalcreds.generated.model.Provider; import bio.terra.externalcreds.models.FenceAccountKey; import io.opentelemetry.instrumentation.annotations.WithSpan; import java.util.*; @@ -31,14 +32,16 @@ public FenceAccountKeyDAO(NamedParameterJdbcTemplate jdbcTemplate) { } @WithSpan - public Optional getFenceAccountKey(String userId, String providerName) { + public Optional getFenceAccountKey(String userId, Provider provider) { var namedParameters = - new MapSqlParameterSource("userId", userId).addValue("providerName", providerName); + new MapSqlParameterSource() + .addValue("userId", userId) + .addValue("provider", provider.name()); var query = "SELECT fence.id, fence.linked_account_id, fence.key_json, fence.expires_at FROM fence_account_key fence" + " INNER JOIN linked_account la ON la.id = fence.linked_account_id" + " WHERE la.user_id = :userId" - + " AND la.provider_name = :providerName"; + + " AND la.provider = :provider::provider_enum"; return Optional.ofNullable( DataAccessUtils.singleResult( jdbcTemplate.query(query, namedParameters, FENCE_ACCOUNT_KEY_ROW_MAPPER))); @@ -75,5 +78,4 @@ public boolean deleteFenceAccountKey(int linkedAccountId) { var query = "DELETE FROM fence_account_key WHERE linked_account_id = :linkedAccountId"; return jdbcTemplate.update(query, namedParameters) > 0; } - } diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java index 38b74808..8b89b618 100644 --- a/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java @@ -4,6 +4,7 @@ import bio.terra.externalcreds.BaseTest; import bio.terra.externalcreds.TestUtils; +import bio.terra.externalcreds.generated.model.Provider; import java.util.Optional; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -16,8 +17,7 @@ class FenceAccountKeyDAOTest extends BaseTest { @Test void testGetMissingFenceAccountKey() { - var shouldBeEmpty = - fenceAccountKeyDAO.getFenceAccountKey("nonexistent_user_id", "nonexistent_provider_name"); + var shouldBeEmpty = fenceAccountKeyDAO.getFenceAccountKey("nonexistent_user_id", Provider.RAS); assertEmpty(shouldBeEmpty); } @@ -45,7 +45,7 @@ void testCreateAndGetFenceAccountKey() { var loadedFenceAccountKey = fenceAccountKeyDAO.getFenceAccountKey( - savedAccount.getUserId(), savedAccount.getProviderName()); + savedAccount.getUserId(), savedAccount.getProvider()); assertEquals(Optional.of(savedFenceAccountKey), loadedFenceAccountKey); } } @@ -63,11 +63,11 @@ void testDeleteFenceAccountKey() { assertPresent( fenceAccountKeyDAO.getFenceAccountKey( - savedAccount.getUserId(), savedAccount.getProviderName())); + savedAccount.getUserId(), savedAccount.getProvider())); assertTrue(fenceAccountKeyDAO.deleteFenceAccountKey(savedAccount.getId().get())); assertEmpty( fenceAccountKeyDAO.getFenceAccountKey( - savedAccount.getUserId(), savedAccount.getProviderName())); + savedAccount.getUserId(), savedAccount.getProvider())); } @Test From fcdd68211524c1d6abf98ff0d93f159b23e29932 Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 6 Mar 2024 10:02:31 -0500 Subject: [PATCH 11/13] Fix tests --- .../bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java | 2 +- service/src/test/java/bio/terra/externalcreds/TestUtils.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index 9ca5b11e..fefb082c 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -51,7 +51,7 @@ public Optional getFenceAccountKey(String userId, Provider prov public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { var query = "INSERT INTO fence_account_key (linked_account_id, key_json, expires_at)" - + " VALUES (:linkedAccountId, :keyJson, :expiresAt)" + + " VALUES (:linkedAccountId, :keyJson::jsonb, :expiresAt)" + " RETURNING id"; var namedParameters = diff --git a/service/src/test/java/bio/terra/externalcreds/TestUtils.java b/service/src/test/java/bio/terra/externalcreds/TestUtils.java index 586eda2e..40574295 100644 --- a/service/src/test/java/bio/terra/externalcreds/TestUtils.java +++ b/service/src/test/java/bio/terra/externalcreds/TestUtils.java @@ -64,7 +64,7 @@ public static GA4GHVisa createRandomVisa() { public static FenceAccountKey createRandomFenceAccountKey() { return new FenceAccountKey.Builder() .linkedAccountId(1) - .keyJson(UUID.randomUUID().toString()) + .keyJson("{\"key\": \"value\"}") .expiresAt(getRandomTimestamp()) .build(); } From dded2a201f38e4c54f2bab1f200f186f9df5b44d Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 6 Mar 2024 11:47:33 -0500 Subject: [PATCH 12/13] Change Timestamp to Instant --- .../terra/externalcreds/dataAccess/DistributedLockDAO.java | 5 +++-- .../terra/externalcreds/dataAccess/FenceAccountKeyDAO.java | 5 +++-- .../java/bio/terra/externalcreds/models/DistributedLock.java | 4 ++-- .../java/bio/terra/externalcreds/models/FenceAccountKey.java | 4 ++-- service/src/test/java/bio/terra/externalcreds/TestUtils.java | 2 +- .../externalcreds/dataAccess/DistributedLockDAOTest.java | 2 +- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java index ace576ba..bd37b9fa 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/DistributedLockDAO.java @@ -2,6 +2,7 @@ import bio.terra.externalcreds.models.DistributedLock; import io.opentelemetry.instrumentation.annotations.WithSpan; +import java.sql.Timestamp; import java.util.Optional; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.support.DataAccessUtils; @@ -19,7 +20,7 @@ public class DistributedLockDAO { new DistributedLock.Builder() .lockName(rs.getString("lock_name")) .userId(rs.getString("user_id")) - .expiresAt(rs.getTimestamp("expires_at")) + .expiresAt(rs.getTimestamp("expires_at").toInstant()) .build()); final NamedParameterJdbcTemplate jdbcTemplate; @@ -59,7 +60,7 @@ public DistributedLock insertDistributedLock(DistributedLock distributedLock) { new MapSqlParameterSource() .addValue("lockName", distributedLock.getLockName()) .addValue("userId", distributedLock.getUserId()) - .addValue("expiresAt", distributedLock.getExpiresAt()); + .addValue("expiresAt", Timestamp.from(distributedLock.getExpiresAt())); jdbcTemplate.update(query, namedParameters); return distributedLock; diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index fefb082c..eb0536f6 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -3,6 +3,7 @@ import bio.terra.externalcreds.generated.model.Provider; import bio.terra.externalcreds.models.FenceAccountKey; import io.opentelemetry.instrumentation.annotations.WithSpan; +import java.sql.Timestamp; import java.util.*; import lombok.extern.slf4j.Slf4j; import org.springframework.dao.support.DataAccessUtils; @@ -22,7 +23,7 @@ public class FenceAccountKeyDAO { .id(rs.getInt("id")) .linkedAccountId(rs.getInt("linked_account_id")) .keyJson(rs.getString("key_json")) - .expiresAt(rs.getTimestamp("expires_at")) + .expiresAt(rs.getTimestamp("expires_at").toInstant()) .build()); final NamedParameterJdbcTemplate jdbcTemplate; @@ -58,7 +59,7 @@ public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { new MapSqlParameterSource() .addValue("linkedAccountId", fenceAccountKey.getLinkedAccountId()) .addValue("keyJson", fenceAccountKey.getKeyJson()) - .addValue("expiresAt", fenceAccountKey.getExpiresAt()); + .addValue("expiresAt", Timestamp.from(fenceAccountKey.getExpiresAt())); // generatedKeyHolder will hold the id returned by the query as specified by the RETURNING // clause diff --git a/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java b/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java index 0a8ee0bf..b462dd76 100644 --- a/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java +++ b/service/src/main/java/bio/terra/externalcreds/models/DistributedLock.java @@ -1,6 +1,6 @@ package bio.terra.externalcreds.models; -import java.sql.Timestamp; +import java.time.Instant; import org.immutables.value.Value; @Value.Immutable @@ -9,7 +9,7 @@ public interface DistributedLock extends WithDistributedLock { String getUserId(); - Timestamp getExpiresAt(); + Instant getExpiresAt(); class Builder extends ImmutableDistributedLock.Builder {} } diff --git a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java index 345a6cea..c4c69c33 100644 --- a/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java +++ b/service/src/main/java/bio/terra/externalcreds/models/FenceAccountKey.java @@ -1,6 +1,6 @@ package bio.terra.externalcreds.models; -import java.sql.Timestamp; +import java.time.Instant; import java.util.Optional; import org.immutables.value.Value; @@ -12,7 +12,7 @@ public interface FenceAccountKey extends WithFenceAccountKey { String getKeyJson(); - Timestamp getExpiresAt(); + Instant getExpiresAt(); class Builder extends ImmutableFenceAccountKey.Builder {} } diff --git a/service/src/test/java/bio/terra/externalcreds/TestUtils.java b/service/src/test/java/bio/terra/externalcreds/TestUtils.java index 40574295..4fe58f86 100644 --- a/service/src/test/java/bio/terra/externalcreds/TestUtils.java +++ b/service/src/test/java/bio/terra/externalcreds/TestUtils.java @@ -65,7 +65,7 @@ public static FenceAccountKey createRandomFenceAccountKey() { return new FenceAccountKey.Builder() .linkedAccountId(1) .keyJson("{\"key\": \"value\"}") - .expiresAt(getRandomTimestamp()) + .expiresAt(getRandomTimestamp().toInstant()) .build(); } diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java index d5ed08ee..6155b749 100644 --- a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java @@ -19,7 +19,7 @@ class DistributedLockDAOTest extends BaseTest { new DistributedLock.Builder() .lockName(testLockName) .userId(UUID.randomUUID().toString()) - .expiresAt(TestUtils.getRandomTimestamp()) + .expiresAt(TestUtils.getRandomTimestamp().toInstant()) .build(); @Test From b81b81dea2ab9b48136256b4242d86a6a1306aaf Mon Sep 17 00:00:00 2001 From: Saman Ehsan Date: Wed, 6 Mar 2024 12:18:03 -0500 Subject: [PATCH 13/13] Upsert fence account key --- .../dataAccess/FenceAccountKeyDAO.java | 6 +++- ...fence_account_key_and_dist_lock_table.yaml | 1 + .../dataAccess/DistributedLockDAOTest.java | 10 ++++++ .../dataAccess/FenceAccountKeyDAOTest.java | 34 +++++++++++++++++-- 4 files changed, 48 insertions(+), 3 deletions(-) diff --git a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java index eb0536f6..54df0f41 100644 --- a/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java +++ b/service/src/main/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAO.java @@ -49,10 +49,14 @@ public Optional getFenceAccountKey(String userId, Provider prov } @WithSpan - public FenceAccountKey insertFenceAccountKey(FenceAccountKey fenceAccountKey) { + public FenceAccountKey upsertFenceAccountKey(FenceAccountKey fenceAccountKey) { var query = "INSERT INTO fence_account_key (linked_account_id, key_json, expires_at)" + " VALUES (:linkedAccountId, :keyJson::jsonb, :expiresAt)" + + " ON CONFLICT (linked_account_id) DO UPDATE SET" + + " linked_account_id = excluded.linked_account_id," + + " key_json = excluded.key_json::jsonb," + + " expires_at = excluded.expires_at" + " RETURNING id"; var namedParameters = diff --git a/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml b/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml index 052943bc..418be03a 100644 --- a/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml +++ b/service/src/main/resources/db/changelog/changesets/20240305_add_fence_account_key_and_dist_lock_table.yaml @@ -18,6 +18,7 @@ databaseChangeLog: type: int constraints: nullable: false + unique: true references: linked_account(id) foreignKeyName: fk_linked_account_id deleteCascade: true diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java index 6155b749..8b2e92d7 100644 --- a/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/DistributedLockDAOTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.dao.DuplicateKeyException; class DistributedLockDAOTest extends BaseTest { @@ -39,6 +40,15 @@ void testCreateAndGetDistributedLock() { distributedLockDAO.getDistributedLock(savedLock.getLockName(), savedLock.getUserId()); assertEquals(Optional.of(savedLock), loadedDistributedLock); } + + @Test + void testCreateDuplicateDistributedLockConflictError() { + DistributedLock savedLock = distributedLockDAO.insertDistributedLock(testDistributedLock); + assertEquals(testDistributedLock, savedLock); + assertThrows( + DuplicateKeyException.class, + () -> distributedLockDAO.insertDistributedLock(testDistributedLock)); + } } @Nested diff --git a/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java index 8b89b618..359ed8be 100644 --- a/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java +++ b/service/src/test/java/bio/terra/externalcreds/dataAccess/FenceAccountKeyDAOTest.java @@ -32,7 +32,7 @@ void testCreateAndGetFenceAccountKey() { var fenceAccountKey = TestUtils.createRandomFenceAccountKey(); var savedFenceAccountKey = - fenceAccountKeyDAO.insertFenceAccountKey( + fenceAccountKeyDAO.upsertFenceAccountKey( fenceAccountKey.withLinkedAccountId(savedAccount.getId().get())); assertPresent(savedFenceAccountKey.getId()); @@ -48,6 +48,36 @@ void testCreateAndGetFenceAccountKey() { savedAccount.getUserId(), savedAccount.getProvider()); assertEquals(Optional.of(savedFenceAccountKey), loadedFenceAccountKey); } + + @Test + void testUpsertFenceAccountKey() { + var savedAccount = + linkedAccountDAO.upsertLinkedAccount(TestUtils.createRandomLinkedAccount()); + assertPresent(savedAccount.getId()); + + var fenceAccountKey = + TestUtils.createRandomFenceAccountKey().withLinkedAccountId(savedAccount.getId().get()); + var savedFenceAccountKey = fenceAccountKeyDAO.upsertFenceAccountKey(fenceAccountKey); + assertPresent(savedFenceAccountKey.getId()); + + assertEquals( + fenceAccountKey + .withId(savedFenceAccountKey.getId()) + .withLinkedAccountId(savedFenceAccountKey.getLinkedAccountId()) + .withKeyJson(savedFenceAccountKey.getKeyJson()), + savedFenceAccountKey); + + var newKey = "{\"key\": \"new value\"}"; + var updatedFenceAccountKey = + fenceAccountKeyDAO.upsertFenceAccountKey(fenceAccountKey.withKeyJson(newKey)); + + assertEquals( + fenceAccountKey + .withId(savedFenceAccountKey.getId()) + .withLinkedAccountId(savedFenceAccountKey.getLinkedAccountId()) + .withKeyJson(newKey), + updatedFenceAccountKey); + } } @Nested @@ -58,7 +88,7 @@ void testDeleteFenceAccountKey() { var savedAccount = linkedAccountDAO.upsertLinkedAccount(TestUtils.createRandomLinkedAccount()); assertPresent(savedAccount.getId()); - fenceAccountKeyDAO.insertFenceAccountKey( + fenceAccountKeyDAO.upsertFenceAccountKey( TestUtils.createRandomFenceAccountKey().withLinkedAccountId(savedAccount.getId().get())); assertPresent(