Skip to content

DT-3056: Audit Library Card DAA Associations and Removals #2853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rushtong merged 28 commits into from
Apr 11, 2026
Merged

DT-3056: Audit Library Card DAA Associations and Removals #2853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
0000e08
feat: add audits to lc daa actions
rushtong Apr 3, 2026
7c83a6a
feat: add query for audit records
rushtong Apr 3, 2026
4f028e5
Merge branch 'develop' into gr-DT-3056-audit-lc-daa
rushtong Apr 3, 2026
b19ecbc
feat: add service/resource method and test cleanup
rushtong Apr 3, 2026
1584b65
feat: api docs
rushtong Apr 3, 2026
87862e9
Merge branch 'develop' into gr-DT-3056-audit-lc-daa
rushtong Apr 3, 2026
ca24ece
feat: sonar warnings
rushtong Apr 3, 2026
4911289
feat: rm duplicate test
rushtong Apr 3, 2026
36e7f88
feat: fix sonar warning
rushtong Apr 3, 2026
9c1374a
feat: fix id
rushtong Apr 3, 2026
d286944
feat: fix api doc
rushtong Apr 3, 2026
654044d
feat: fix path
rushtong Apr 3, 2026
757146d
feat: skip audit on conflict
rushtong Apr 3, 2026
ea2b29a
feat: test coverage and cleanup
rushtong Apr 4, 2026
c6ad6ea
Merge branch 'develop' into gr-DT-3056-audit-lc-daa
rushtong Apr 4, 2026
2ed3bdf
Apply suggestions from code review
rushtong Apr 4, 2026
3f25b19
feat: copilot feedback: include the library card user id
rushtong Apr 4, 2026
051bb48
feat: address sonar warnings
rushtong Apr 4, 2026
4435e9a
feat: test fix
rushtong Apr 4, 2026
70ec601
feat: spotless
rushtong Apr 4, 2026
676413f
feat: address sonar warnings
rushtong Apr 4, 2026
d63d5e1
feat: swagger api updates
rushtong Apr 4, 2026
b274109
feat: spotless
rushtong Apr 4, 2026
7d7fe56
feat: additional tests for on conflict handling
rushtong Apr 4, 2026
8123386
feat: bug fix for invalid requests and incorrect response when creati…
rushtong Apr 6, 2026
a4c2da3
Merge branch 'develop' into gr-DT-3056-audit-lc-daa
rushtong Apr 7, 2026
4e77909
feat: merge fix
rushtong Apr 7, 2026
c593a50
feat: minor fix for null LC population
rushtong Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/main/java/org/broadinstitute/consent/http/ConsentModule.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,7 @@ InstitutionService providesInstitutionService() {
@Provides
LibraryCardService providesLibraryCardService() {
return new LibraryCardService(
providesDaaDAO(),
providesLibraryCardDAO(),
providesInstitutionDAO(),
providesInstitutionService(),
Expand Down
49 changes: 41 additions & 8 deletions src/main/java/org/broadinstitute/consent/http/db/LibraryCardDAO.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@

import java.util.Date;
import java.util.List;
import org.broadinstitute.consent.http.db.mapper.LibraryCardDaaAuditMapper;
import org.broadinstitute.consent.http.db.mapper.LibraryCardReducer;
import org.broadinstitute.consent.http.db.mapper.LibraryCardWithDaaReducer;
import org.broadinstitute.consent.http.models.DataAccessAgreement;
import org.broadinstitute.consent.http.models.LibraryCard;
import org.broadinstitute.consent.http.models.LibraryCardDaaAudit;
import org.jdbi.v3.sqlobject.config.RegisterBeanMapper;
import org.jdbi.v3.sqlobject.config.RegisterRowMapper;
import org.jdbi.v3.sqlobject.customizer.Bind;
import org.jdbi.v3.sqlobject.customizer.BindList;
import org.jdbi.v3.sqlobject.customizer.BindList.EmptyHandling;
Expand Down Expand Up @@ -134,19 +137,49 @@ WITH daa_deletes AS (DELETE FROM lc_daa lc_daa WHERE lc_daa.lc_id = :libraryCard

@SqlUpdate(
"""
INSERT INTO lc_daa (lc_id, daa_id)
VALUES (:lcId, :daaId)
ON CONFLICT DO NOTHING
WITH lc_daa_insert AS (
INSERT INTO lc_daa (lc_id, daa_id)
VALUES (:lcId, :daaId)
ON CONFLICT DO NOTHING
RETURNING lc_id
)
INSERT INTO lc_daa_audit (daa_id, lc_id, lc_user_id, user_id, action, action_date)
SELECT :daaId, :lcId, :lcUserId, :userId, 'ADD', NOW()
WHERE EXISTS (SELECT 1 FROM lc_daa_insert)
""")
void createLibraryCardDaaRelation(@Bind("lcId") Integer lcId, @Bind("daaId") Integer daaId);
void createLibraryCardDaaRelation(
@Bind("lcUserId") Integer lcUserId,
@Bind("userId") Integer userId,
@Bind("lcId") Integer lcId,
@Bind("daaId") Integer daaId);

@SqlUpdate(
"""
DELETE FROM lc_daa
WHERE lc_id = :lcId
AND daa_id = :daaId
WITH lc_daa_delete AS (
DELETE FROM lc_daa
WHERE lc_id = :lcId
AND daa_id = :daaId
RETURNING lc_id
)
INSERT INTO lc_daa_audit (daa_id, lc_id, lc_user_id, user_id, action, action_date)
SELECT :daaId, :lcId, :lcUserId, :userId, 'REMOVE', NOW()
WHERE EXISTS (SELECT 1 FROM lc_daa_delete)
""")
void deleteLibraryCardDaaRelation(
@Bind("lcUserId") Integer lcUserId,
@Bind("userId") Integer userId,
@Bind("lcId") Integer lcId,
@Bind("daaId") Integer daaId);

@RegisterRowMapper(LibraryCardDaaAuditMapper.class)
@SqlQuery(
"""
SELECT *
FROM lc_daa_audit
WHERE lc_user_id = :lcUserId
ORDER BY action_date DESC
""")
void deleteLibraryCardDaaRelation(@Bind("lcId") Integer lcId, @Bind("daaId") Integer daaId);
List<LibraryCardDaaAudit> findAuditsByLcUserId(@Bind("lcUserId") Integer lcUserId);
Comment thread
rushtong marked this conversation as resolved.

/**
* Finds library cards by user emails.
Expand Down
23 changes: 23 additions & 0 deletions src/main/java/org/broadinstitute/consent/http/db/mapper/LibraryCardDaaAuditMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.broadinstitute.consent.http.db.mapper;

import java.sql.ResultSet;
import java.sql.SQLException;
import org.broadinstitute.consent.http.enumeration.AuditActions;
import org.broadinstitute.consent.http.models.LibraryCardDaaAudit;
import org.jdbi.v3.core.mapper.RowMapper;
import org.jdbi.v3.core.statement.StatementContext;

public class LibraryCardDaaAuditMapper implements RowMapper<LibraryCardDaaAudit>, RowMapperHelper {
@Override
public LibraryCardDaaAudit map(ResultSet rs, StatementContext ctx) throws SQLException {
int lcId = hasColumn(rs, "lc_id") ? rs.getInt("lc_id") : 0;
return new LibraryCardDaaAudit(
rs.getLong("id"),
rs.getInt("daa_id"),
lcId > 0 ? lcId : null,
rs.getInt("lc_user_id"),
rs.getInt("user_id"),
AuditActions.valueOf(rs.getString("action").toUpperCase()),
rs.getTimestamp("action_date").toInstant());
}
}
23 changes: 23 additions & 0 deletions src/main/java/org/broadinstitute/consent/http/models/LibraryCardDaaAudit.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
package org.broadinstitute.consent.http.models;

import java.time.Instant;
import org.broadinstitute.consent.http.enumeration.AuditActions;

/**
* @param id The audit ID
* @param daaId The Data Access Agreement ID
* @param lcId The Library Card ID
* @param lcUserId The Library Card User ID (the user associated with the library card at the time
* of the audit action)
* @param userId The User ID who initiated the action
* @param action The action performed (e.g., ADD, REMOVE)
* @param actionDate The action date
*/
public record LibraryCardDaaAudit(
Long id,
Integer daaId,
Integer lcId,
Integer lcUserId,
Integer userId,
AuditActions action,
Instant actionDate) {}
14 changes: 9 additions & 5 deletions src/main/java/org/broadinstitute/consent/http/resources/DaaResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,12 +126,15 @@ public Response createLibraryCardDaaRelation(
user.getLibraryCard() == null
? libraryCardService.createLibraryCardForSigningOfficial(user, authedUser)
: user.getLibraryCard();
libraryCardService.addDaaToLibraryCard(libraryCard.getId(), daaId);
libraryCardService.addDaaToLibraryCard(
user.getUserId(), authedUser.getUserId(), libraryCard.getId(), daaId);
LibraryCard updatedLibraryCard =
libraryCardService.findLibraryCardWithDaasById(libraryCard.getId());
Comment on lines +131 to +132
Copy link
Copy Markdown
Contributor Author

@rushtong rushtong Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixes a bug in the current implementation that returns the original, un-updated library card.

Copy link
Copy Markdown
Contributor Author

@rushtong rushtong Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a side note, I want to update the service layer to return the updated LC. I will do that in a separate PR to keep this one as focused as possible.

URI uri =
info.getBaseUriBuilder()
.replacePath("api/libraryCards/{libraryCardId}")
.build(libraryCard.getId());
return Response.ok().location(uri).entity(libraryCard).build();
return Response.ok().location(uri).entity(updatedLibraryCard).build();
} catch (Exception e) {
return createExceptionResponse(e);
}
Expand Down Expand Up @@ -194,7 +197,8 @@ public Response deleteDaaForUser(
return Response.status(Status.FORBIDDEN).build();
}
if (user.getLibraryCard() != null) {
libraryCardService.removeDaaFromLibraryCard(user.getLibraryCard().getId(), daaId);
libraryCardService.removeDaaFromLibraryCard(
user.getUserId(), authedUser.getUserId(), user.getLibraryCard().getId(), daaId);
}
return Response.ok().build();
} catch (Exception e) {
Expand Down Expand Up @@ -242,7 +246,7 @@ public Response bulkRemoveUsersFromDaa(
}
daaService.findById(daaId);
for (User user : users) {
libraryCardService.removeDaaFromUserLibraryCard(user, daaId);
libraryCardService.removeDaaFromUserLibraryCard(user, authedUser, daaId);
}
return Response.ok().build();
} catch (Exception e) {
Expand Down Expand Up @@ -286,7 +290,7 @@ public Response bulkRemoveDAAsFromUser(
}
List<DataAccessAgreement> daaList = daaService.findDAAsInJsonArray(json, "daaList");
for (DataAccessAgreement daa : daaList) {
libraryCardService.removeDaaFromUserLibraryCard(user, daa.getDaaId());
libraryCardService.removeDaaFromUserLibraryCard(user, authedUser, daa.getDaaId());
}
return Response.ok().build();
} catch (Exception e) {
Expand Down
60 changes: 45 additions & 15 deletions src/main/java/org/broadinstitute/consent/http/resources/LibraryCardResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,32 +13,40 @@
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import jakarta.ws.rs.core.Response;
import java.util.List;
import org.broadinstitute.consent.http.enumeration.UserRoles;
import org.broadinstitute.consent.http.models.AuthUser;
import org.broadinstitute.consent.http.models.DuosUser;
import org.broadinstitute.consent.http.models.LibraryCard;
import org.broadinstitute.consent.http.models.LibraryCardDaaAudit;
import org.broadinstitute.consent.http.models.User;
import org.broadinstitute.consent.http.service.LibraryCardService;
import org.broadinstitute.consent.http.service.UserService;
import org.broadinstitute.consent.http.service.feature.InstitutionAndLibraryCardEnforcement;
import org.broadinstitute.consent.http.util.gson.GsonUtil;

@Path("api/libraryCards")
public class LibraryCardResource extends Resource {

private final UserService userService;
private final LibraryCardService libraryCardService;
private final InstitutionAndLibraryCardEnforcement institutionAndLibraryCardEnforcement;

@Inject
public LibraryCardResource(UserService userService, LibraryCardService libraryCardService) {
public LibraryCardResource(
UserService userService,
LibraryCardService libraryCardService,
InstitutionAndLibraryCardEnforcement institutionAndLibraryCardEnforcement) {
this.userService = userService;
this.libraryCardService = libraryCardService;
this.institutionAndLibraryCardEnforcement = institutionAndLibraryCardEnforcement;
}

@GET
@Produces("application/json")
@Produces(MediaType.APPLICATION_JSON)
@RolesAllowed(ADMIN)
public Response getLibraryCards(@Auth AuthUser authUser) {
public Response getLibraryCards(@SuppressWarnings("unused") @Auth DuosUser duosUser) {
try {
List<LibraryCard> libraryCards = libraryCardService.findAllLibraryCards();
return Response.ok().entity(libraryCards).build();
Expand All @@ -48,10 +56,11 @@ public Response getLibraryCards(@Auth AuthUser authUser) {
}

@GET
@Produces("application/json")
@Produces(MediaType.APPLICATION_JSON)
@Path("/{id}")
@RolesAllowed(ADMIN)
public Response getLibraryCardById(@Auth AuthUser authUser, @PathParam("id") Integer id) {
public Response getLibraryCardById(
@SuppressWarnings("unused") @Auth DuosUser duosUser, @PathParam("id") Integer id) {
try {
LibraryCard libraryCard = libraryCardService.findLibraryCardById(id);
return Response.ok().entity(libraryCard).build();
Expand All @@ -61,11 +70,11 @@ public Response getLibraryCardById(@Auth AuthUser authUser, @PathParam("id") Int
}

@GET
@Produces("application/json")
@Produces(MediaType.APPLICATION_JSON)
@Path("/institution/{id}")
@RolesAllowed({ADMIN})
public Response getLibraryCardsByInstitutionId(
@Auth AuthUser authUser, @PathParam("id") Integer id) {
@SuppressWarnings("unused") @Auth DuosUser duosUser, @PathParam("id") Integer id) {
try {
List<LibraryCard> libraryCards = libraryCardService.findLibraryCardsByInstitutionId(id);
return Response.ok().entity(libraryCards).build();
Expand All @@ -75,12 +84,12 @@ public Response getLibraryCardsByInstitutionId(
}

@POST
@Consumes("application/json")
@Produces("application/json")
@Consumes(MediaType.APPLICATION_JSON)
@Produces(MediaType.APPLICATION_JSON)
@RolesAllowed({SIGNINGOFFICIAL})
public Response createLibraryCard(@Auth AuthUser authUser, String libraryCard) {
public Response createLibraryCard(@Auth DuosUser duosUser, String libraryCard) {
try {
User user = userService.findUserByEmail(authUser.getEmail());
User user = duosUser.getUser();
LibraryCard payload = GsonUtil.getInstance().fromJson(libraryCard, LibraryCard.class);
payload.setCreateUserId(user.getUserId());
LibraryCard newLibraryCard = libraryCardService.createLibraryCard(payload, user);
Expand All @@ -91,11 +100,11 @@ public Response createLibraryCard(@Auth AuthUser authUser, String libraryCard) {
}

@DELETE
@Produces("application/json")
@Produces(MediaType.APPLICATION_JSON)
@Path("/{id}")
@RolesAllowed({ADMIN, SIGNINGOFFICIAL})
public Response deleteLibraryCard(@Auth AuthUser authUser, @PathParam("id") Integer id) {
User user = userService.findUserByEmail(authUser.getEmail());
public Response deleteLibraryCard(@Auth DuosUser duosUser, @PathParam("id") Integer id) {
User user = duosUser.getUser();
LibraryCard card = libraryCardService.findLibraryCardById(id);
User lcUser = null;
try {
Expand All @@ -117,6 +126,27 @@ public Response deleteLibraryCard(@Auth AuthUser authUser, @PathParam("id") Inte
}
}

@GET
@Produces(MediaType.APPLICATION_JSON)
@Path("/history/{userId}")
@RolesAllowed({ADMIN, SIGNINGOFFICIAL})
public Response findLibraryCardDaaAuditsByUserId(
@Auth DuosUser duosUser, @PathParam("userId") Integer userId) {
try {
User libraryCardUser = userService.findUserById(userId);
User authedUser = duosUser.getUser();
if (!checkIsAdmin(authedUser)) {
institutionAndLibraryCardEnforcement.validateEmailsFromSameInstitution(
authedUser.getEmail(), libraryCardUser.getEmail());
}
List<LibraryCardDaaAudit> audits =
libraryCardService.findLibraryCardDaaAuditsByUserId(userId);
return Response.ok().entity(audits).build();
} catch (Exception e) {
return createExceptionResponse(e);
}
}

private boolean checkIsAdmin(User user) {
return user.getRoles().stream()
.anyMatch(role -> role.getName().equalsIgnoreCase(UserRoles.ADMIN.getRoleName()));
Expand Down
Loading
Loading