diff --git a/contract/openapi.yaml b/contract/openapi.yaml index 90f6434..7dd3586 100644 --- a/contract/openapi.yaml +++ b/contract/openapi.yaml @@ -24,7 +24,8 @@ components: type: "string" detail: {} id: - description: "활동 행 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다." + description: "활동 행의 공개 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다." + format: "uuid" type: "string" ip: type: @@ -370,6 +371,11 @@ components: type: - "string" - "null" + createdByName: + description: "매핑을 만든 관리자 이름" + type: + - "string" + - "null" ctMax: description: "동시 연결 상한 오버라이드 (null = 에이전트 기본, 0 = 해제)" format: "int32" @@ -421,6 +427,11 @@ components: type: - "string" - "null" + suspendedByName: + description: "정지한 관리자 이름 (자동 정지면 null)" + type: + - "string" + - "null" suspendedReason: description: "정지 사유 (SUSPENDED일 때)" type: @@ -956,7 +967,8 @@ components: type: "string" detail: {} id: - description: "감사 로그 행 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다." + description: "감사 로그 행의 공개 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다." + format: "uuid" type: "string" ip: type: @@ -1276,9 +1288,7 @@ components: displayName: maxLength: 100 minLength: 0 - type: - - "string" - - "null" + type: "string" extraNote: maxLength: 2000 minLength: 0 @@ -1312,6 +1322,7 @@ components: format: "uuid" type: "string" required: + - "displayName" - "orgId" - "purpose" - "type" @@ -3063,9 +3074,7 @@ components: format: "date-time" type: "string" displayName: - type: - - "string" - - "null" + type: "string" extraNote: type: - "string" @@ -3126,6 +3135,7 @@ components: type: "string" required: - "createdAt" + - "displayName" - "id" - "orgName" - "purpose" @@ -4169,6 +4179,10 @@ components: type: - "string" - "null" + actorName: + type: + - "string" + - "null" changedAt: format: "date-time" type: "string" @@ -4523,6 +4537,9 @@ components: grantedImageId: format: "uuid" type: "string" + grantedImageName: + description: "승인된 OS 이미지의 표시 이름. 카탈로그에서 내려간 이미지도 이름이 남습니다." + type: "string" grantedMemoryMb: format: "int32" type: "integer" @@ -4534,9 +4551,15 @@ components: type: - "string" - "null" + nodeName: + description: "배치된 노드의 이름. nodeId가 있을 때 함께 있습니다." + type: + - "string" + - "null" required: - "grantedDiskGb" - "grantedImageId" + - "grantedImageName" - "grantedMemoryMb" - "grantedVcpu" type: "object" @@ -4659,6 +4682,11 @@ components: type: - "string" - "null" + flavorName: + description: "요청한 사양 프리셋의 표시 이름. flavorId가 있을 때 함께 있습니다." + type: + - "string" + - "null" granted: anyOf: - $ref: "#/components/schemas/VmGrantedSpecResponse" @@ -4666,6 +4694,9 @@ components: imageId: format: "uuid" type: "string" + imageName: + description: "요청한 OS 이미지의 표시 이름. 카탈로그에서 내려간 이미지도 이름이 남습니다." + type: "string" reqDiskGb: format: "int32" type: "integer" @@ -4685,6 +4716,7 @@ components: - "null" required: - "imageId" + - "imageName" - "reqDiskGb" - "reqMemoryMb" - "reqVcpu" @@ -4987,7 +5019,7 @@ info: description: "부산대학교 클라우드 플랫폼 Pickle의 REST API. 인증은 JWT Bearer, 오류 응답은 RFC 9457 problem+json(Problem\ \ 스키마)을 따릅니다." title: "Pickle API" - version: "0.38.0" + version: "0.39.0" openapi: "3.1.0" paths: /admin/announcements: diff --git a/src/main/java/kr/ac/pusan/pickle/access/ResourceAccessGrantService.java b/src/main/java/kr/ac/pusan/pickle/access/ResourceAccessGrantService.java index 498c389..e70d628 100644 --- a/src/main/java/kr/ac/pusan/pickle/access/ResourceAccessGrantService.java +++ b/src/main/java/kr/ac/pusan/pickle/access/ResourceAccessGrantService.java @@ -11,6 +11,7 @@ import kr.ac.pusan.pickle.access.dto.ResourceAccessGrantView; import kr.ac.pusan.pickle.access.dto.ResourceAccessListResponse; import kr.ac.pusan.pickle.access.dto.UpdateResourceAccessGrantRequest; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.common.error.ApiException; import kr.ac.pusan.pickle.common.error.ErrorCodes; @@ -60,12 +61,13 @@ public class ResourceAccessGrantService { private final WorkspaceRepository workspaceRepository; private final UserRepository userRepository; private final AuditService auditService; + private final AuditIds auditIds; public ResourceAccessGrantService(List adapters, ResourceAccessResolver resolver, ResourceAccessGrantRepository grantRepository, WorkspaceMemberRepository workspaceMemberRepository, WorkspaceRepository workspaceRepository, UserRepository userRepository, - AuditService auditService) { + AuditService auditService, AuditIds auditIds) { this.adapters = adapters.stream() .collect(Collectors.toMap(ResourceTypeAdapter::type, Function.identity())); this.resolver = resolver; @@ -74,6 +76,7 @@ public ResourceAccessGrantService(List adapters, this.workspaceRepository = workspaceRepository; this.userRepository = userRepository; this.auditService = auditService; + this.auditIds = auditIds; } @Transactional(readOnly = true) @@ -203,10 +206,10 @@ private void audit(AuthenticatedUser actor, Managed managed, GrantChange change, ResourceAccessGrant grant, ResourceRole previousRole, String ip) { ResourceAccessAudit names = managed.adapter().accessAudit(); Map detail = new LinkedHashMap<>(); - detail.put("grantId", grant.getId()); + detail.put("grantId", grant.getPublicId()); detail.put("granteeType", grant.getGranteeType().name()); if (grant.getUserId() != null) { - detail.put("granteeUserId", grant.getUserId()); + detail.put("granteeUserId", auditIds.user(grant.getUserId())); } detail.put("role", grant.getRole().name()); if (previousRole != null) { diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminService.java index 586147c..a197881 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminService.java @@ -146,10 +146,21 @@ public UserSummaryResponse updateUser(AuthenticatedUser actor, UUID userId, auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.USER_ROLE_UPDATE, "user", user.getPublicId(), Map.of("previousRole", previousRole.name(), "role", user.getRole().name(), - "orgId", user.getOrgId() == null ? "null" : String.valueOf(user.getOrgId())), ip); + "orgId", orgPublicIdOrNull(user.getOrgId())), ip); return UserSummaryResponse.from(user); } + /** + * The organisation the account now belongs to, named publicly. Map.of + * refuses a null value, so an account under no organisation keeps the + * literal {@code "null"} this field has always carried there. + */ + private String orgPublicIdOrNull(Long orgId) { + return orgId == null ? "null" + : orgRepository.findById(orgId).map(org -> org.getPublicId().toString()) + .orElse("null"); + } + private static String normalize(String description) { return Texts.blankToNull(description); } diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminTaskService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminTaskService.java index 4663006..59fc654 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminTaskService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminTaskService.java @@ -8,6 +8,7 @@ import java.util.function.Function; import java.util.stream.Collectors; import kr.ac.pusan.pickle.admin.dto.AdminTaskResponse; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.auth.dto.MessageResponse; import kr.ac.pusan.pickle.common.error.ApiException; @@ -58,11 +59,13 @@ public class AdminTaskService { private final DeleteVmJob deleteVmJob; private final JobScheduler jobScheduler; private final AuditService auditService; + private final AuditIds auditIds; public AdminTaskService(ProvisioningTaskRepository taskRepository, VmRepository vmRepository, OrgRepository orgRepository, WorkspaceRepository workspaceRepository, ProvisioningService provisioningService, - DeleteVmJob deleteVmJob, JobScheduler jobScheduler, AuditService auditService) { + DeleteVmJob deleteVmJob, JobScheduler jobScheduler, AuditService auditService, + AuditIds auditIds) { this.taskRepository = taskRepository; this.vmRepository = vmRepository; this.orgRepository = orgRepository; @@ -71,6 +74,7 @@ public AdminTaskService(ProvisioningTaskRepository taskRepository, VmRepository this.deleteVmJob = deleteVmJob; this.jobScheduler = jobScheduler; this.auditService = auditService; + this.auditIds = auditIds; } /** @@ -149,7 +153,7 @@ public void afterCommit() { }); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.TASK_RETRY, "provisioning_task", task.getPublicId(), - Map.of("vmId", vmId, "kind", task.getKind().name()), ip); + Map.of("vmId", auditIds.vm(vmId), "kind", task.getKind().name()), ip); return new MessageResponse("작업 재시도를 접수했습니다. 잠시 후 작업 상태가 갱신됩니다."); } diff --git a/src/main/java/kr/ac/pusan/pickle/admin/AdminUserQueryService.java b/src/main/java/kr/ac/pusan/pickle/admin/AdminUserQueryService.java index abbaa55..06e7e43 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/AdminUserQueryService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/AdminUserQueryService.java @@ -166,19 +166,21 @@ public UserAdminDetailResponse getUser(AuthenticatedUser actor, UUID userId) { memberships, activeVmCount, statusChanges); } - /** Resolves each transition's actor email in one batch. */ + /** Resolves each transition's actor in one batch: id, email and name. */ private List mapStatusChanges(List changes) { List actorIds = changes.stream().map(UserStatusChange::getActorId) .filter(id -> id != null).distinct().toList(); - List actors = userRepository.findAllById(actorIds); - Map emails = actors.stream() - .collect(Collectors.toMap(User::getId, User::getEmail)); - Map publicIds = actors.stream() - .collect(Collectors.toMap(User::getId, User::getPublicId)); + Map actors = userRepository.findAllById(actorIds).stream() + .collect(Collectors.toMap(User::getId, java.util.function.Function.identity())); return changes.stream() - .map(change -> new UserStatusChangeResponse(change.getFromStatus(), change.getToStatus(), - publicIds.get(change.getActorId()), emails.get(change.getActorId()), - change.getReason(), change.getChangedAt())) + .map(change -> { + User actor = actors.get(change.getActorId()); + return new UserStatusChangeResponse(change.getFromStatus(), change.getToStatus(), + actor == null ? null : actor.getPublicId(), + actor == null ? null : actor.getEmail(), + actor == null ? null : actor.getName(), + change.getReason(), change.getChangedAt()); + }) .toList(); } diff --git a/src/main/java/kr/ac/pusan/pickle/admin/ApprovalService.java b/src/main/java/kr/ac/pusan/pickle/admin/ApprovalService.java index 86351e3..51c65a8 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/ApprovalService.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/ApprovalService.java @@ -13,6 +13,7 @@ import kr.ac.pusan.pickle.access.ResourceType; import kr.ac.pusan.pickle.admin.dto.ApproveRequestRequest; import kr.ac.pusan.pickle.admin.dto.RejectRequestRequest; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.common.error.ApiException; import kr.ac.pusan.pickle.common.error.ErrorCodes; @@ -68,6 +69,7 @@ public class ApprovalService { private final UserRepository userRepository; private final OrgRepository orgRepository; private final AuditService auditService; + private final AuditIds auditIds; private final NotificationService notificationService; public ApprovalService(RequestRepository requestRepository, RequestReviewRepository reviewRepository, @@ -76,7 +78,7 @@ public ApprovalService(RequestRepository requestRepository, RequestReviewReposit WorkspaceRepository workspaceRepository, ResourceAccessGrantRepository grantRepository, UserRepository userRepository, OrgRepository orgRepository, - AuditService auditService, NotificationService notificationService) { + AuditService auditService, AuditIds auditIds, NotificationService notificationService) { this.requestRepository = requestRepository; this.reviewRepository = reviewRepository; this.assembler = assembler; @@ -88,6 +90,7 @@ public ApprovalService(RequestRepository requestRepository, RequestReviewReposit this.userRepository = userRepository; this.orgRepository = orgRepository; this.auditService = auditService; + this.auditIds = auditIds; this.notificationService = notificationService; } @@ -208,7 +211,8 @@ public RequestDetailResponse reject(AuthenticatedUser actor, UUID requestId, reviewRepository.save(RequestReview.reject(request.getId(), actor.id(), form.comment().strip())); request.setStatus(RequestStatus.REJECTED); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.REQUEST_REJECT, - "request", request.getPublicId(), Map.of("workspaceId", request.getWorkspaceId()), ip); + "request", request.getPublicId(), + Map.of("workspaceId", auditIds.workspace(request.getWorkspaceId())), ip); notificationService.publish(request.getRequesterId(), NotificationEvent.REQUEST_REJECTED, Map.of("requestId", request.getPublicId(), "comment", form.comment().strip(), "type", request.getResourceType().name()), null); diff --git a/src/main/java/kr/ac/pusan/pickle/admin/dto/UserStatusChangeResponse.java b/src/main/java/kr/ac/pusan/pickle/admin/dto/UserStatusChangeResponse.java index 86344fa..a8a2e22 100644 --- a/src/main/java/kr/ac/pusan/pickle/admin/dto/UserStatusChangeResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/admin/dto/UserStatusChangeResponse.java @@ -11,6 +11,7 @@ public record UserStatusChangeResponse( UserStatus toStatus, @Nullable UUID actorId, @Nullable String actorEmail, + @Nullable String actorName, @Nullable String reason, Instant changedAt) { } diff --git a/src/main/java/kr/ac/pusan/pickle/audit/AuditIds.java b/src/main/java/kr/ac/pusan/pickle/audit/AuditIds.java new file mode 100644 index 0000000..84d843a --- /dev/null +++ b/src/main/java/kr/ac/pusan/pickle/audit/AuditIds.java @@ -0,0 +1,114 @@ +package kr.ac.pusan.pickle.audit; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.UUID; +import org.jspecify.annotations.Nullable; +import org.springframework.jdbc.core.JdbcTemplate; +import org.springframework.jdbc.core.RowCallbackHandler; +import org.springframework.stereotype.Component; + +/** + * Public identifiers for audit {@code detail}, resolved from the internal key a + * call site happens to be holding. + * + *

{@code detail} is free-form jsonb and reaches ordinary users through + * {@code /me/activity}, so an internal row number in it leaks exactly what a + * public id exists to stop leaking. Most call sites have the entity in hand and + * call {@code getPublicId()} directly; this is for the ones that only ever had a + * {@code long} — a mapping id just allocated, a VM id carried on a session, a + * foreign key read off another row.

+ * + *

One indexed primary-key lookup per call, on a path that already writes a + * row. Not a general-purpose translator: an id belongs here only because it is + * about to be written into an audit payload.

+ */ +@Component +public class AuditIds { + + private final JdbcTemplate jdbcTemplate; + + public AuditIds(JdbcTemplate jdbcTemplate) { + this.jdbcTemplate = jdbcTemplate; + } + + public @Nullable UUID user(@Nullable Long id) { + return one("select public_id from users where id = ?", id); + } + + public @Nullable UUID org(@Nullable Long id) { + return one("select public_id from orgs where id = ?", id); + } + + public @Nullable UUID workspace(@Nullable Long id) { + return one("select public_id from workspaces where id = ?", id); + } + + public @Nullable UUID vm(@Nullable Long id) { + return one("select public_id from vms where id = ?", id); + } + + public @Nullable UUID node(@Nullable Long id) { + return one("select public_id from nodes where id = ?", id); + } + + public @Nullable UUID osImage(@Nullable Long id) { + return one("select public_id from os_images where id = ?", id); + } + + public @Nullable UUID relay(@Nullable Long id) { + return one("select public_id from relays where id = ?", id); + } + + public @Nullable UUID portMapping(@Nullable Long id) { + return one("select public_id from port_mappings where id = ?", id); + } + + public @Nullable UUID sshKey(@Nullable Long id) { + return one("select public_id from user_ssh_keys where id = ?", id); + } + + /** Same order as the input, so a list stays aligned with its siblings. */ + public List users(Collection ids) { + return many("select id, public_id from users where id in ", ids); + } + + /** Same order as the input, so a list stays aligned with its siblings. */ + public List sshKeys(Collection ids) { + return many("select id, public_id from user_ssh_keys where id in ", ids); + } + + // ── internals ────────────────────────────────────────────────────────── + + /** + * Null in, null out: several of these come from nullable foreign keys, and + * an audit write must not fail because the thing it describes is gone. + */ + private @Nullable UUID one(String sql, @Nullable Long id) { + if (id == null) { + return null; + } + return jdbcTemplate.query(sql, rs -> rs.next() ? rs.getObject(1, UUID.class) : null, id); + } + + /** + * One statement for the whole list. {@code sqlPrefix} is a constant from a + * method above and the only text appended to it is the placeholder list, so + * nothing caller-supplied ever reaches the SQL. + */ + private List many(String sqlPrefix, Collection ids) { + if (ids.isEmpty()) { + return List.of(); + } + String placeholders = "(" + String.join(", ", Collections.nCopies(ids.size(), "?")) + ")"; + Map found = new HashMap<>(); + jdbcTemplate.query(sqlPrefix + placeholders, (RowCallbackHandler) rs -> + found.put(rs.getLong(1), rs.getObject(2, UUID.class)), + ids.toArray()); + return ids.stream().map(found::get).filter(Objects::nonNull).toList(); + } +} diff --git a/src/main/java/kr/ac/pusan/pickle/audit/AuditQueryService.java b/src/main/java/kr/ac/pusan/pickle/audit/AuditQueryService.java index 13e3e43..2501018 100644 --- a/src/main/java/kr/ac/pusan/pickle/audit/AuditQueryService.java +++ b/src/main/java/kr/ac/pusan/pickle/audit/AuditQueryService.java @@ -85,10 +85,11 @@ public PageResponse myActivity(long actorId, String actio params.add(size); params.add((long) page * size); List content = jdbcTemplate.query(""" - select a.id, a.action, a.target_type, a.target_id, a.detail, a.ip, a.created_at + select a.public_id, a.action, a.target_type, a.target_id, a.detail, a.ip, + a.created_at from audit_logs a""" + where + " order by a.created_at desc, a.id desc limit ? offset ?", - (rs, rowNum) -> new ActivityEntryResponse(String.valueOf(rs.getLong("id")), + (rs, rowNum) -> new ActivityEntryResponse(rs.getObject("public_id", UUID.class), rs.getString("action"), rs.getString("target_type"), rs.getString("target_id"), detailOf(rs.getString("detail")), rs.getString("ip"), @@ -135,14 +136,15 @@ public PageResponse adminAudit(AuthenticatedUser actor, params.add((long) page * size); // u.public_id, not a.actor_id: the actor is reported by the identifier // the API names accounts with. actor_id itself stays the internal key - // the join runs on. - List content = jdbcTemplate.query("select a.id, u.public_id " - + "as actor_public_id, " + // the join runs on. Same for a.public_id — a.id orders the page and + // never leaves the server. + List content = jdbcTemplate.query("select a.public_id, " + + "u.public_id as actor_public_id, " + "a.actor_role, a.action, a.target_type, a.target_id, a.detail, a.ip, " + "a.created_at, u.email as actor_email, u.name as actor_name, " + ACTOR_ORG_NAME + " as org_name" + base + where + " order by a.created_at desc, a.id desc limit ? offset ?", - (rs, rowNum) -> new AuditLogViewResponse(String.valueOf(rs.getLong("id")), + (rs, rowNum) -> new AuditLogViewResponse(rs.getObject("public_id", UUID.class), rs.getObject("actor_public_id", UUID.class), rs.getString("actor_email"), rs.getString("actor_name"), rs.getString("actor_role"), rs.getString("action"), rs.getString("target_type"), diff --git a/src/main/java/kr/ac/pusan/pickle/audit/AuditService.java b/src/main/java/kr/ac/pusan/pickle/audit/AuditService.java index 5ed1a8b..550b33b 100644 --- a/src/main/java/kr/ac/pusan/pickle/audit/AuditService.java +++ b/src/main/java/kr/ac/pusan/pickle/audit/AuditService.java @@ -165,10 +165,16 @@ public AuditService(JdbcTemplate jdbcTemplate, ObjectMapper objectMapper, /** * {@code targetId} is the target row's public identifier: the column - * is text and holds a UUID string. Targets that have no public identity - * (a refresh token, a settings key) pass null and put whatever identifies - * them in {@code detail}. Rows written before V78 hold the internal number - * of the day as text and mean what they always meant. + * is text and holds a UUID string. Targets that have no public identity (a + * refresh token, a settings key) pass null; a settings key names itself in + * {@code detail}, while a refresh token puts nothing there at all — its row + * number identified it to nobody, and the actor and action already say whose + * session did what. Rows written before V78 hold the internal number of the + * day as text and mean what they always meant. + * + *

{@code detail} reaches ordinary users through {@code /me/activity}, so + * an id in it is a public id or it is not written: {@link AuditIds} resolves + * one where the call site holds only an internal key.

*/ @Transactional(propagation = Propagation.REQUIRES_NEW) public void record(Long actorId, String actorRole, String action, String targetType, UUID targetId, diff --git a/src/main/java/kr/ac/pusan/pickle/audit/dto/ActivityEntryResponse.java b/src/main/java/kr/ac/pusan/pickle/audit/dto/ActivityEntryResponse.java index 79f888f..936c90c 100644 --- a/src/main/java/kr/ac/pusan/pickle/audit/dto/ActivityEntryResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/audit/dto/ActivityEntryResponse.java @@ -2,13 +2,14 @@ import io.swagger.v3.oas.annotations.media.Schema; import java.time.Instant; +import java.util.UUID; import org.jspecify.annotations.Nullable; import tools.jackson.databind.JsonNode; /** Contract {@code ActivityEntry}: one self-view audit row (login history included). */ public record ActivityEntryResponse( - @Schema(description = "활동 행 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다.") - String id, + @Schema(description = "활동 행의 공개 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다.") + UUID id, String action, @Nullable String targetType, @Nullable String targetId, diff --git a/src/main/java/kr/ac/pusan/pickle/audit/dto/AuditLogViewResponse.java b/src/main/java/kr/ac/pusan/pickle/audit/dto/AuditLogViewResponse.java index 7e45eb8..dcc872c 100644 --- a/src/main/java/kr/ac/pusan/pickle/audit/dto/AuditLogViewResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/audit/dto/AuditLogViewResponse.java @@ -13,8 +13,8 @@ * internal actors (e.g. the SSH gateway) stamp roles outside the user enum. */ public record AuditLogViewResponse( - @Schema(description = "감사 로그 행 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다.") - String id, + @Schema(description = "감사 로그 행의 공개 식별자. 목록 렌더링용 키이며 어떤 조회 파라미터도 아닙니다.") + UUID id, @Nullable UUID actorId, @Nullable String actorEmail, @Nullable String actorName, diff --git a/src/main/java/kr/ac/pusan/pickle/auth/AuthService.java b/src/main/java/kr/ac/pusan/pickle/auth/AuthService.java index 64b7723..72b114c 100644 --- a/src/main/java/kr/ac/pusan/pickle/auth/AuthService.java +++ b/src/main/java/kr/ac/pusan/pickle/auth/AuthService.java @@ -316,7 +316,7 @@ public AuthResult refresh(String rawToken, String ip, String userAgent) { // Theft signal: reuse of a rotated token revokes the whole chain. refreshTokenService.revokeChainFrom(current.getId()); auditService.record(current.getUserId(), null, AuditService.AUTH_REFRESH_REUSE_DETECTED, - "refresh_token", null, Map.of("refreshTokenId", current.getId()), ip); + "refresh_token", null, Map.of(), ip); throw refreshTokenInvalid(); } if (current.isExpired(Instant.now())) { @@ -337,7 +337,7 @@ public AuthResult refresh(String rawToken, String ip, String userAgent) { // Lost a race with another rotation of the same token: reuse. refreshTokenService.revokeChainFrom(current.getId()); auditService.record(current.getUserId(), null, AuditService.AUTH_REFRESH_REUSE_DETECTED, - "refresh_token", null, Map.of("refreshTokenId", current.getId()), ip); + "refresh_token", null, Map.of(), ip); return refreshTokenInvalid(); }); return new AuthResult( @@ -353,7 +353,7 @@ public void logout(String rawToken, String ip) { refreshTokenService.findByRawToken(rawToken).ifPresent(token -> { refreshTokenService.revoke(token.getId()); auditService.record(token.getUserId(), null, AuditService.AUTH_LOGOUT, - "refresh_token", null, Map.of("refreshTokenId", token.getId()), ip); + "refresh_token", null, Map.of(), ip); }); } diff --git a/src/main/java/kr/ac/pusan/pickle/campusip/CampusIpRequestService.java b/src/main/java/kr/ac/pusan/pickle/campusip/CampusIpRequestService.java index a7b68d9..a045e01 100644 --- a/src/main/java/kr/ac/pusan/pickle/campusip/CampusIpRequestService.java +++ b/src/main/java/kr/ac/pusan/pickle/campusip/CampusIpRequestService.java @@ -10,6 +10,7 @@ import java.util.regex.Pattern; import kr.ac.pusan.pickle.access.ResourceRole; import kr.ac.pusan.pickle.access.VmAccessService; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.campusip.dto.AdminCampusIpRequestView; import kr.ac.pusan.pickle.campusip.dto.CampusIpRequestView; @@ -69,13 +70,14 @@ public class CampusIpRequestService { private final kr.ac.pusan.pickle.orgs.OrgRepository orgRepository; private final NotificationService notificationService; private final AuditService auditService; + private final AuditIds auditIds; private final ObjectMapper objectMapper; public CampusIpRequestService(CampusIpRequestRepository requestRepository, VmRepository vmRepository, VmAccessService vmAccessService, UserRepository userRepository, kr.ac.pusan.pickle.orgs.OrgRepository orgRepository, NotificationService notificationService, - AuditService auditService, ObjectMapper objectMapper) { + AuditService auditService, AuditIds auditIds, ObjectMapper objectMapper) { this.requestRepository = requestRepository; this.vmRepository = vmRepository; this.vmAccessService = vmAccessService; @@ -83,6 +85,7 @@ public CampusIpRequestService(CampusIpRequestRepository requestRepository, this.orgRepository = orgRepository; this.notificationService = notificationService; this.auditService = auditService; + this.auditIds = auditIds; this.objectMapper = objectMapper; } @@ -123,7 +126,7 @@ public CampusIpRequestView create(AuthenticatedUser actor, UUID publicVmId, null); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.CAMPUS_IP_REQUEST, "campus_ip_request", created.getPublicId(), - Map.of("vmId", vmId, "ports", ports), ip); + Map.of("vmId", vm.getPublicId(), "ports", ports), ip); return toView(created, vm.getPublicId(), Map.of(actor.id(), actor.publicId())); } @@ -144,7 +147,7 @@ public void cancel(AuthenticatedUser actor, UUID publicVmId, UUID publicRequestI requestRepository.delete(request); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.CAMPUS_IP_CANCEL, "campus_ip_request", request.getPublicId(), - Map.of("vmId", vmId), ip); + Map.of("vmId", vm.getPublicId()), ip); } // ── admin ops ──────────────────────────────────────────────────────────── @@ -216,7 +219,7 @@ public AdminCampusIpRequestView updateStatus(AuthenticatedUser actor, UUID publi notificationArgs(request, vm), null); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.CAMPUS_IP_STATUS_UPDATE, "campus_ip_request", request.getPublicId(), - Map.of("vmId", request.getVmId(), "from", from.name(), "to", to.name()), ip); + Map.of("vmId", auditIds.vm(request.getVmId()), "from", from.name(), "to", to.name()), ip); User requester = userRepository.findById(request.getRequestedBy()).orElse(null); Map userIds = userPublicIds(List.of(request)); return new AdminCampusIpRequestView(request.getPublicId(), diff --git a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java index e5aec11..69e2406 100644 --- a/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java +++ b/src/main/java/kr/ac/pusan/pickle/common/openapi/OpenApiConfig.java @@ -41,7 +41,7 @@ public class OpenApiConfig { /** Contract version served in {@code info.version}; bump on any contract change. */ - public static final String CONTRACT_VERSION = "0.38.0"; + public static final String CONTRACT_VERSION = "0.39.0"; /** Name of the bearer-JWT security scheme in the published spec. */ private static final String BEARER_SCHEME = "bearerAuth"; diff --git a/src/main/java/kr/ac/pusan/pickle/provisioning/ProvisionVmJob.java b/src/main/java/kr/ac/pusan/pickle/provisioning/ProvisionVmJob.java index f6e8204..de21c88 100644 --- a/src/main/java/kr/ac/pusan/pickle/provisioning/ProvisionVmJob.java +++ b/src/main/java/kr/ac/pusan/pickle/provisioning/ProvisionVmJob.java @@ -570,14 +570,44 @@ private void finalizeVm(ProvisioningTask task, Vm vm) { String ip = Optional.ofNullable(vm.getIpAllocationId()) .flatMap(allocationRepository::findById) .map(a -> hostAddress(a.getIp())).orElse(null); + String imageName = imageRepository.findById(vm.getImageId()) + .map(OsImage::getDisplayName).orElse(null); vmEventRepository.save(new VmEvent(vm.getId(), VmEventType.CREATE, null, - "프로비저닝 완료 (vmid " + vm.getProxmoxVmid() + ", ip " + ip + ")")); + completedDetail(imageName, ip))); publishCreated(vm, ip); } taskRepository.complete(task.getId(), now); log.info("provision vm {} finished (vmid {})", vm.getId(), vm.getProxmoxVmid()); } + /** + * The owner's timeline entry for a finished provision: the OS the VM runs + * and the address it answers on, which are the two facts an owner + * recognises their own machine by. + * + *

The Proxmox vmid stood where the image name does now. It is a + * hypervisor number — nothing the owner can look up, ask about or act on — + * and it counts up from 100000 across the whole cluster, so every completed + * provision quietly told its owner how many the platform had ever done. The + * IP stays: it is the VM's own address, the owner sees it from inside the + * guest anyway, and it is what they identify the machine by in a log.

+ * + *

A clause whose value is missing is left out rather than printed empty — + * an image row that is gone, or a VM that ended up with no allocation, + * should shorten the sentence, not put {@code null} in front of a user.

+ */ + private static String completedDetail(String imageName, String ip) { + List parts = new ArrayList<>(2); + if (imageName != null && !imageName.isBlank()) { + parts.add("이미지 " + imageName); + } + if (ip != null && !ip.isBlank()) { + parts.add("ip " + ip); + } + return parts.isEmpty() ? COMPLETED_DETAIL + : COMPLETED_DETAIL + " (" + String.join(", ", parts) + ")"; + } + // --- failure handling (retry & compensation) ----------------- private void handleFailure(long taskId, long vmId, Exception e) { diff --git a/src/main/java/kr/ac/pusan/pickle/relay/AdminPortMappingService.java b/src/main/java/kr/ac/pusan/pickle/relay/AdminPortMappingService.java index 9121089..be65fd0 100644 --- a/src/main/java/kr/ac/pusan/pickle/relay/AdminPortMappingService.java +++ b/src/main/java/kr/ac/pusan/pickle/relay/AdminPortMappingService.java @@ -8,6 +8,7 @@ import java.util.UUID; import java.util.function.Function; import java.util.stream.Collectors; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.auth.dto.MessageResponse; import kr.ac.pusan.pickle.common.error.ApiException; @@ -18,11 +19,13 @@ import kr.ac.pusan.pickle.notification.NotificationService; import kr.ac.pusan.pickle.relay.dto.AdminPortMappingResponse; import kr.ac.pusan.pickle.security.AuthenticatedUser; +import kr.ac.pusan.pickle.user.User; import kr.ac.pusan.pickle.vm.Vm; import kr.ac.pusan.pickle.vm.VmEvent; import kr.ac.pusan.pickle.vm.VmEventRepository; import kr.ac.pusan.pickle.vm.VmEventType; import kr.ac.pusan.pickle.vm.VmRepository; +import org.jspecify.annotations.Nullable; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageRequest; import org.springframework.data.domain.Sort; @@ -67,6 +70,7 @@ public class AdminPortMappingService { private final RelayGenerations relayGenerations; private final NotificationService notificationService; private final AuditService auditService; + private final AuditIds auditIds; private final VmEventRepository vmEventRepository; private final PortForwardingService portForwardingService; @@ -74,7 +78,7 @@ public AdminPortMappingService(PortMappingRepository portMappingRepository, RelayRepository relayRepository, VmRepository vmRepository, kr.ac.pusan.pickle.user.UserRepository userRepository, RelayGenerations relayGenerations, NotificationService notificationService, - AuditService auditService, VmEventRepository vmEventRepository, + AuditService auditService, AuditIds auditIds, VmEventRepository vmEventRepository, PortForwardingService portForwardingService) { this.portMappingRepository = portMappingRepository; this.relayRepository = relayRepository; @@ -83,6 +87,7 @@ public AdminPortMappingService(PortMappingRepository portMappingRepository, this.relayGenerations = relayGenerations; this.notificationService = notificationService; this.auditService = auditService; + this.auditIds = auditIds; this.vmEventRepository = vmEventRepository; this.portForwardingService = portForwardingService; } @@ -118,7 +123,7 @@ public PageResponse list(UUID publicRelayId, UUID publ relays.values().forEach(relay -> failedByRelay.put(relay.getId(), portForwardingService.failedIds(relay))); - Map userIds = userPublicIds(result.getContent()); + Map actors = actors(result.getContent()); List views = result.getContent().stream().map(mapping -> { Relay relay = relays.get(mapping.getRelayId()); Vm vm = vms.get(mapping.getVmId()); @@ -128,14 +133,17 @@ public PageResponse list(UUID publicRelayId, UUID publ vm != null ? vm.getPublicId() : null, vm != null ? vm.getName() : null, mapping.getProto(), mapping.getPublicPort(), mapping.getTargetPort(), mapping.getStatus(), - mapping.getSuspendedReason(), userIds.get(mapping.getSuspendedBy()), + mapping.getSuspendedReason(), + publicIdOf(actors.get(mapping.getSuspendedBy())), + nameOf(actors.get(mapping.getSuspendedBy())), relay == null ? PortForwardApplyState.PENDING : PortForwardingService.applyState(mapping, relay.getAppliedGeneration(), failedByRelay.getOrDefault(relay.getId(), Set.of())), mapping.getCtMax(), mapping.getNewConnRate(), mapping.getNewConnBurst(), mapping.getPerSourceRate(), mapping.getPerSourceBurst(), - userIds.get(mapping.getCreatedBy()), mapping.getCreatedAt()); + publicIdOf(actors.get(mapping.getCreatedBy())), + nameOf(actors.get(mapping.getCreatedBy())), mapping.getCreatedAt()); }).toList(); return PageResponse.of(views, result); } @@ -166,8 +174,8 @@ public AdminPortMappingResponse suspend(AuthenticatedUser actor, UUID mappingId, } auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.PORT_MAPPING_SUSPEND, "port_mapping", mapping.getPublicId(), - Map.of("auto", false, "relayId", mapping.getRelayId(), - "vmId", mapping.getVmId(), "reason", reason), ip); + Map.of("auto", false, "relayId", auditIds.relay(mapping.getRelayId()), + "vmId", auditIds.vm(mapping.getVmId()), "reason", reason), ip); return toResponse(mapping); } @@ -185,7 +193,8 @@ public AdminPortMappingResponse unsuspend(AuthenticatedUser actor, UUID mappingI mapping.setLastChangeGeneration(generation); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.PORT_MAPPING_UNSUSPEND, "port_mapping", mapping.getPublicId(), - Map.of("relayId", mapping.getRelayId(), "vmId", mapping.getVmId()), ip); + Map.of("relayId", auditIds.relay(mapping.getRelayId()), + "vmId", auditIds.vm(mapping.getVmId())), ip); return toResponse(mapping); } @@ -212,7 +221,8 @@ public MessageResponse delete(AuthenticatedUser actor, UUID mappingId, String ip } auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.PORT_MAPPING_DELETE, "port_mapping", mapping.getPublicId(), - Map.of("relayId", mapping.getRelayId(), "vmId", mapping.getVmId(), + Map.of("relayId", auditIds.relay(mapping.getRelayId()), + "vmId", auditIds.vm(mapping.getVmId()), "proto", mapping.getProto().name(), "publicPort", mapping.getPublicPort()), ip); return new MessageResponse("매핑을 삭제했습니다. 잠시 후 릴레이에서 제거됩니다."); @@ -287,7 +297,8 @@ public AdminPortMappingResponse updateGuards(AuthenticatedUser actor, UUID mappi mapping.setLastChangeGeneration(generation); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.PORT_MAPPING_GUARDS_UPDATE, "port_mapping", mapping.getPublicId(), - Map.of("relayId", mapping.getRelayId(), "vmId", mapping.getVmId(), + Map.of("relayId", auditIds.relay(mapping.getRelayId()), + "vmId", auditIds.vm(mapping.getVmId()), "changes", changes), ip); return toResponse(mapping); } @@ -298,29 +309,43 @@ public AdminPortMappingResponse updateGuards(AuthenticatedUser actor, UUID mappi private AdminPortMappingResponse toResponse(PortMapping mapping) { Relay relay = relayRepository.findById(mapping.getRelayId()).orElseThrow(); Vm vm = vmRepository.findById(mapping.getVmId()).orElse(null); - Map userIds = userPublicIds(List.of(mapping)); + Map actors = actors(List.of(mapping)); return new AdminPortMappingResponse(mapping.getPublicId(), relay.getPublicId(), relay.getName(), vm != null ? vm.getPublicId() : null, vm != null ? vm.getName() : null, mapping.getProto(), mapping.getPublicPort(), mapping.getTargetPort(), mapping.getStatus(), - mapping.getSuspendedReason(), userIds.get(mapping.getSuspendedBy()), + mapping.getSuspendedReason(), + publicIdOf(actors.get(mapping.getSuspendedBy())), + nameOf(actors.get(mapping.getSuspendedBy())), PortForwardingService.applyState(mapping, relay.getAppliedGeneration(), portForwardingService.failedIds(relay)), mapping.getCtMax(), mapping.getNewConnRate(), mapping.getNewConnBurst(), mapping.getPerSourceRate(), mapping.getPerSourceBurst(), - userIds.get(mapping.getCreatedBy()), mapping.getCreatedAt()); + publicIdOf(actors.get(mapping.getCreatedBy())), + nameOf(actors.get(mapping.getCreatedBy())), mapping.getCreatedAt()); } /** Batch account join for {@code createdBy}/{@code suspendedBy}. */ - private Map userPublicIds(List mappings) { + /** + * The actors named on these mappings, kept whole: the response reports each + * one by id and by name, and this is the query that already has both. + */ + private Map actors(List mappings) { List ids = java.util.stream.Stream.concat( mappings.stream().map(PortMapping::getCreatedBy), mappings.stream().map(PortMapping::getSuspendedBy)) .filter(java.util.Objects::nonNull).distinct().toList(); return ids.isEmpty() ? Map.of() : userRepository.findAllById(ids).stream() - .collect(Collectors.toMap(kr.ac.pusan.pickle.user.User::getId, - kr.ac.pusan.pickle.user.User::getPublicId)); + .collect(Collectors.toMap(User::getId, Function.identity())); + } + + private static @Nullable UUID publicIdOf(@Nullable User user) { + return user == null ? null : user.getPublicId(); + } + + private static @Nullable String nameOf(@Nullable User user) { + return user == null ? null : user.getName(); } private PortMapping requireMapping(UUID mappingId) { diff --git a/src/main/java/kr/ac/pusan/pickle/relay/PortForwardingService.java b/src/main/java/kr/ac/pusan/pickle/relay/PortForwardingService.java index fbfe930..76a7d4a 100644 --- a/src/main/java/kr/ac/pusan/pickle/relay/PortForwardingService.java +++ b/src/main/java/kr/ac/pusan/pickle/relay/PortForwardingService.java @@ -8,6 +8,7 @@ import java.util.UUID; import kr.ac.pusan.pickle.access.ResourceRole; import kr.ac.pusan.pickle.access.VmAccessService; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.auth.RateLimitService; import kr.ac.pusan.pickle.auth.dto.MessageResponse; @@ -72,6 +73,7 @@ public class PortForwardingService { private final IpAddressResolver ipAddressResolver; private final VmEventRepository vmEventRepository; private final AuditService auditService; + private final AuditIds auditIds; private final NotificationService notificationService; private final JdbcTemplate jdbcTemplate; private final ObjectMapper objectMapper; @@ -82,7 +84,7 @@ public PortForwardingService(VmRepository vmRepository, PortMappingRepository portMappingRepository, RelayGenerations relayGenerations, SettingsService settingsService, RateLimitService rateLimitService, IpAddressResolver ipAddressResolver, VmEventRepository vmEventRepository, - AuditService auditService, NotificationService notificationService, + AuditService auditService, AuditIds auditIds, NotificationService notificationService, JdbcTemplate jdbcTemplate, ObjectMapper objectMapper) { this.vmRepository = vmRepository; this.vmAccessService = vmAccessService; @@ -94,6 +96,7 @@ public PortForwardingService(VmRepository vmRepository, this.ipAddressResolver = ipAddressResolver; this.vmEventRepository = vmEventRepository; this.auditService = auditService; + this.auditIds = auditIds; this.notificationService = notificationService; this.jdbcTemplate = jdbcTemplate; this.objectMapper = objectMapper; @@ -150,7 +153,7 @@ public PortForwardingView create(AuthenticatedUser actor, UUID publicVmId, request.proto() + " 공개 포트 할당 → 대상 포트 " + request.targetPort())); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_PORT_FORWARD_CREATE, "vm", vm.getPublicId(), - Map.of("mappingId", mappingId, "relayId", relay.getId(), + Map.of("mappingId", auditIds.portMapping(mappingId), "relayId", relay.getPublicId(), "proto", request.proto().name(), "targetPort", request.targetPort()), ip); return toView(portMappingRepository.findById(mappingId).orElseThrow()); } @@ -171,7 +174,7 @@ public MessageResponse delete(AuthenticatedUser actor, UUID publicVmId, mapping.getProto() + " " + mapping.getPublicPort() + " 공개 해제")); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_PORT_FORWARD_DELETE, "vm", vm.getPublicId(), - Map.of("mappingId", mapping.getId(), "relayId", mapping.getRelayId(), + Map.of("mappingId", mapping.getPublicId(), "relayId", auditIds.relay(mapping.getRelayId()), "proto", mapping.getProto().name(), "publicPort", mapping.getPublicPort()), ip); return new MessageResponse("포트 포워딩 삭제를 접수했습니다. 잠시 후 릴레이에서 제거됩니다."); diff --git a/src/main/java/kr/ac/pusan/pickle/relay/RelaySyncService.java b/src/main/java/kr/ac/pusan/pickle/relay/RelaySyncService.java index fd8bafb..35cc788 100644 --- a/src/main/java/kr/ac/pusan/pickle/relay/RelaySyncService.java +++ b/src/main/java/kr/ac/pusan/pickle/relay/RelaySyncService.java @@ -7,6 +7,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.common.text.Texts; import kr.ac.pusan.pickle.notification.NotificationEvent; @@ -84,16 +85,18 @@ select r.mapping_generation, m.id, lower(m.proto) as proto, private final SettingsService settingsService; private final NotificationService notificationService; private final AuditService auditService; + private final AuditIds auditIds; private final ObjectMapper objectMapper; public RelaySyncService(JdbcTemplate jdbcTemplate, RelayGenerations relayGenerations, SettingsService settingsService, NotificationService notificationService, - AuditService auditService, ObjectMapper objectMapper) { + AuditService auditService, AuditIds auditIds, ObjectMapper objectMapper) { this.jdbcTemplate = jdbcTemplate; this.relayGenerations = relayGenerations; this.settingsService = settingsService; this.notificationService = notificationService; this.auditService = auditService; + this.auditIds = auditIds; this.objectMapper = objectMapper; } @@ -199,7 +202,7 @@ private void accumulateCounters(long relayId, // bigint totals against a lying or corrupted agent. auditService.record(null, AuditService.ACTOR_ROLE_RELAY, AuditService.RELAY_SYNC_VIOLATION, "relay", relayPublicId(relayId), - Map.of("kind", "counter_sanity", "mappingId", mappingId, + Map.of("kind", "counter_sanity", "mappingId", auditIds.portMapping(mappingId), "maxReported", String.valueOf(raw.max())), null); log.warn("relay {} reported an insane counter for mapping {} (max {})", relayId, mappingId, raw.max()); @@ -293,7 +296,7 @@ private void autoSuspend(long relayId, long mappingId, long connsPerMin, long mb notificationService.publish(notificationService.sysAdminIds(), NotificationEvent.PORT_MAPPING_SUSPENDED, args, "pm_auto_suspend:" + mappingId); auditService.recordAfterCommit(null, AuditService.ACTOR_ROLE_RELAY, AuditService.PORT_MAPPING_SUSPEND, - "port_mapping", mappingPublicId, Map.of("auto", true, "relayId", relayId, + "port_mapping", mappingPublicId, Map.of("auto", true, "relayId", relayPublicId(relayId), "connsPerMin", connsPerMin, "mbytesPerMin", mbytesPerMin, "connsLimit", connsLimit, "mbytesLimit", mbytesLimit), null); log.warn("port mapping {} auto-suspended (conns/min {} vs {}, MB/min {} vs {})", diff --git a/src/main/java/kr/ac/pusan/pickle/relay/dto/AdminPortMappingResponse.java b/src/main/java/kr/ac/pusan/pickle/relay/dto/AdminPortMappingResponse.java index cefc9c7..ff30fdb 100644 --- a/src/main/java/kr/ac/pusan/pickle/relay/dto/AdminPortMappingResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/relay/dto/AdminPortMappingResponse.java @@ -25,6 +25,9 @@ public record AdminPortMappingResponse( @Nullable @Schema(description = "정지한 관리자 id (자동 정지면 null)") UUID suspendedBy, + @Nullable + @Schema(description = "정지한 관리자 이름 (자동 정지면 null)") + String suspendedByName, PortForwardApplyState applyState, @Nullable @Schema(description = "동시 연결 상한 오버라이드 (null = 에이전트 기본, 0 = 해제)") @@ -42,5 +45,8 @@ public record AdminPortMappingResponse( @Schema(description = "출발지별 버스트 오버라이드") Integer perSourceBurst, @Nullable UUID createdBy, + @Nullable + @Schema(description = "매핑을 만든 관리자 이름") + String createdByName, Instant createdAt) { } diff --git a/src/main/java/kr/ac/pusan/pickle/request/Request.java b/src/main/java/kr/ac/pusan/pickle/request/Request.java index cffe297..c1a0dde 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/Request.java +++ b/src/main/java/kr/ac/pusan/pickle/request/Request.java @@ -69,8 +69,13 @@ public class Request { @Column(name = "req_end_date") private LocalDate reqEndDate; - /** Requester-chosen name for the resource; seeds its settings at approval. */ - @Column(name = "display_name") + /** + * Requester-chosen name for the resource; seeds its settings at approval. + * Required on every request, whatever the resource type — a reference to + * this request is reported by name as well as by public id, and a UUID on + * its own cannot be read, remembered or spoken. + */ + @Column(name = "display_name", nullable = false) private String displayName; @Enumerated(EnumType.STRING) diff --git a/src/main/java/kr/ac/pusan/pickle/request/RequestAssembler.java b/src/main/java/kr/ac/pusan/pickle/request/RequestAssembler.java index 6a0f48c..6c930ae 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/RequestAssembler.java +++ b/src/main/java/kr/ac/pusan/pickle/request/RequestAssembler.java @@ -7,7 +7,6 @@ import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; -import java.util.UUID; import kr.ac.pusan.pickle.access.ResourceType; import kr.ac.pusan.pickle.inventory.Node; import kr.ac.pusan.pickle.inventory.NodeRepository; @@ -86,18 +85,19 @@ public List toDetails(List requests) { .findByRequestIdIn(idsOfType(requests, ResourceType.VM)).stream() .collect(Collectors.toMap(VmRequestDetail::getRequestId, Function.identity())); - // The per-type spec reports its catalog references by public id, so the - // rows behind them are batched here beside the display-name joins. - Map imageIds = publicIds(osImageRepository.findAllById(ids(Stream.concat( + // The per-type spec reports each catalog reference by public id AND by + // name, so the rows behind them are batched here beside the display-name + // joins — the same rows either way, kept whole rather than reduced to an + // id, because the name has to come from somewhere and this is already + // the query that has it. + Map images = byId(osImageRepository.findAllById(ids(Stream.concat( vmDetails.values().stream().map(VmRequestDetail::getImageId), vmDetails.values().stream().map(VmRequestDetail::getGrantedImageId)))), - OsImage::getId, OsImage::getPublicId); - Map flavorIds = publicIds(vmFlavorRepository.findAllById(ids( - vmDetails.values().stream().map(VmRequestDetail::getFlavorId))), - VmFlavor::getId, VmFlavor::getPublicId); - Map nodeIds = publicIds(nodeRepository.findAllById(ids( - vmDetails.values().stream().map(VmRequestDetail::getNodeId))), - Node::getId, Node::getPublicId); + OsImage::getId); + Map flavors = byId(vmFlavorRepository.findAllById(ids( + vmDetails.values().stream().map(VmRequestDetail::getFlavorId))), VmFlavor::getId); + Map nodes = byId(nodeRepository.findAllById(ids( + vmDetails.values().stream().map(VmRequestDetail::getNodeId))), Node::getId); List details = new ArrayList<>(requests.size()); for (Request request : requests) { @@ -121,10 +121,10 @@ public List toDetails(List requests) { ? RequestReviewResponse.from(review, users.get(review.getReviewerId())) : null, vmDetail != null ? VmRequestSpecResponse.from(vmDetail, - imageIds.get(vmDetail.getImageId()), - flavorIds.get(vmDetail.getFlavorId()), - imageIds.get(vmDetail.getGrantedImageId()), - nodeIds.get(vmDetail.getNodeId())) : null, + images.get(vmDetail.getImageId()), + flavors.get(vmDetail.getFlavorId()), + images.get(vmDetail.getGrantedImageId()), + nodes.get(vmDetail.getNodeId())) : null, request.getCreatedAt(), request.getUpdatedAt())); } return details; @@ -137,11 +137,6 @@ private static List idsOfType(List requests, ResourceType type) { .toList(); } - private static Map publicIds(List rows, Function idOf, - Function publicIdOf) { - return rows.stream().collect(Collectors.toMap(idOf, publicIdOf)); - } - private static Set ids(Stream stream) { return stream.filter(java.util.Objects::nonNull).collect(Collectors.toSet()); } diff --git a/src/main/java/kr/ac/pusan/pickle/request/RequestService.java b/src/main/java/kr/ac/pusan/pickle/request/RequestService.java index 0724527..f2fd01e 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/RequestService.java +++ b/src/main/java/kr/ac/pusan/pickle/request/RequestService.java @@ -8,6 +8,7 @@ import java.util.function.Function; import java.util.stream.Collectors; import kr.ac.pusan.pickle.access.ResourceType; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.common.error.ApiException; import kr.ac.pusan.pickle.common.error.ErrorCodes; @@ -51,12 +52,13 @@ public class RequestService { private final WorkspaceMemberRepository workspaceMemberRepository; private final OrgRepository orgRepository; private final AuditService auditService; + private final AuditIds auditIds; private final NotificationService notificationService; public RequestService(RequestRepository requestRepository, RequestAssembler assembler, List handlers, WorkspaceRepository workspaceRepository, WorkspaceMemberRepository workspaceMemberRepository, OrgRepository orgRepository, - AuditService auditService, NotificationService notificationService) { + AuditService auditService, AuditIds auditIds, NotificationService notificationService) { this.requestRepository = requestRepository; this.assembler = assembler; this.handlers = handlers.stream() @@ -65,6 +67,7 @@ public RequestService(RequestRepository requestRepository, RequestAssembler asse this.workspaceMemberRepository = workspaceMemberRepository; this.orgRepository = orgRepository; this.auditService = auditService; + this.auditIds = auditIds; this.notificationService = notificationService; } @@ -107,13 +110,13 @@ public RequestDetailResponse create(AuthenticatedUser actor, CreateRequestReques Request saved = requestRepository.save(new Request(form.type(), workspace.getId(), org.getId(), actor.id(), form.purpose().strip(), Texts.blankToNull(form.courseOrProject()), Texts.blankToNull(form.extraNote()), form.reqStartDate(), form.reqEndDate(), - Texts.blankToNull(form.displayName()))); + form.displayName().strip())); handler.saveDetail(saved, form); Map auditArgs = new LinkedHashMap<>(); auditArgs.put("type", form.type().name()); - auditArgs.put("workspaceId", workspace.getId()); - auditArgs.put("orgId", org.getId()); + auditArgs.put("workspaceId", workspace.getPublicId()); + auditArgs.put("orgId", org.getPublicId()); auditArgs.putAll(handler.submitAuditArgs(saved)); auditService.record(actor.id(), actor.role().name(), AuditService.REQUEST_CREATE, "request", saved.getPublicId(), auditArgs, ip); @@ -191,7 +194,8 @@ public RequestDetailResponse cancel(AuthenticatedUser actor, UUID requestId, Str } request.setStatus(RequestStatus.CANCELED); auditService.record(actor.id(), actor.role().name(), AuditService.REQUEST_CANCEL, - "request", request.getPublicId(), Map.of("workspaceId", request.getWorkspaceId()), ip); + "request", request.getPublicId(), + Map.of("workspaceId", auditIds.workspace(request.getWorkspaceId())), ip); return assembler.toDetail(request); } diff --git a/src/main/java/kr/ac/pusan/pickle/request/dto/CreateRequestRequest.java b/src/main/java/kr/ac/pusan/pickle/request/dto/CreateRequestRequest.java index 2ab1fab..5d98a11 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/dto/CreateRequestRequest.java +++ b/src/main/java/kr/ac/pusan/pickle/request/dto/CreateRequestRequest.java @@ -44,9 +44,11 @@ public record CreateRequestRequest( @Nullable LocalDate reqEndDate, - // 선택 입력 — 리소스 표시명을 신청 단계에서 지정. - @Size(max = 100, message = "표시명은 100자 이하여야 합니다.") - @Nullable String displayName, + // 신청하는 리소스의 이름. 종류를 가리지 않고 필수이며, 이 신청을 가리키는 + // 응답은 어디서나 식별자 옆에 이 이름을 함께 싣는다. + @NotBlank(message = "리소스 이름을 입력해 주세요.") + @Size(max = 100, message = "리소스 이름은 100자 이하여야 합니다.") + String displayName, /** Required when {@code type} is VM, ignored otherwise. */ @Valid @Nullable CreateVmRequestSpec vm) { diff --git a/src/main/java/kr/ac/pusan/pickle/request/dto/RequestDetailResponse.java b/src/main/java/kr/ac/pusan/pickle/request/dto/RequestDetailResponse.java index 9b0c4be..9e8ecba 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/dto/RequestDetailResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/request/dto/RequestDetailResponse.java @@ -31,7 +31,7 @@ public record RequestDetailResponse( @Nullable String extraNote, @Nullable LocalDate reqStartDate, @Nullable LocalDate reqEndDate, - @Nullable String displayName, + String displayName, RequestStatus status, @Nullable RequestReviewResponse review, @Nullable VmRequestSpecResponse vm, diff --git a/src/main/java/kr/ac/pusan/pickle/request/vm/VmRequestSupport.java b/src/main/java/kr/ac/pusan/pickle/request/vm/VmRequestSupport.java index 88b44cd..c0ceb21 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/vm/VmRequestSupport.java +++ b/src/main/java/kr/ac/pusan/pickle/request/vm/VmRequestSupport.java @@ -114,7 +114,9 @@ public void saveDetail(Request request, CreateRequestRequest form) { @Override public Map submitAuditArgs(Request request) { VmRequestDetail detail = detail(request); - return Map.of("imageId", detail.getImageId(), "reqVcpu", detail.getReqVcpu(), + return Map.of("imageId", imageRepository.findById(detail.getImageId()) + .map(OsImage::getPublicId).orElse(null), + "reqVcpu", detail.getReqVcpu(), "reqMemoryMb", detail.getReqMemoryMb(), "reqDiskGb", detail.getReqDiskGb()); } @@ -192,19 +194,20 @@ public Materialized materialize(Request request, ApproveRequestRequest form, Aut // vm_settings row; audited via the request.approve entry. The seeder // sanitizes, so it returns what was actually stored (null when the name // collapsed to nothing and no row was written). - String storedDisplayName = request.getDisplayName() != null - ? vmSettingsService.initializeDisplayName(vm.getId(), request.getDisplayName(), - request.getRequesterId()) - : null; + // Every request carries a name, so there is always one to seed; the + // seeder still answers null when sanitizing leaves nothing behind. + String storedDisplayName = vmSettingsService.initializeDisplayName(vm.getId(), + request.getDisplayName(), request.getRequesterId()); long vmId = vm.getId(); Map auditArgs = new LinkedHashMap<>(); - auditArgs.put("vmId", vmId); + auditArgs.put("vmId", vm.getPublicId()); auditArgs.put("hostname", hostname); auditArgs.put("grantedVcpu", spec.grantedVcpu()); auditArgs.put("grantedMemoryMb", spec.grantedMemoryMb()); auditArgs.put("grantedDiskGb", spec.grantedDiskGb()); - auditArgs.put("nodeId", nodeId); + auditArgs.put("nodeId", nodeRepository.findById(nodeId) + .map(kr.ac.pusan.pickle.inventory.Node::getPublicId).orElse(null)); if (storedDisplayName != null) { // Records the seeded display name's provenance (initializeDisplayName // itself does not audit — this entry is the audit trail). The stored diff --git a/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmGrantedSpecResponse.java b/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmGrantedSpecResponse.java index 34410dc..2cdd48d 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmGrantedSpecResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmGrantedSpecResponse.java @@ -1,13 +1,23 @@ package kr.ac.pusan.pickle.request.vm.dto; +import io.swagger.v3.oas.annotations.media.Schema; import org.jspecify.annotations.Nullable; import java.util.UUID; -/** Contract schema {@code VmGrantedSpec}: what the reviewer granted for a VM. */ +/** + * Contract schema {@code VmGrantedSpec}: what the reviewer granted for a VM. + * Each reference carries its name for the same reason the requested spec does — + * the grant outlives the catalog entry it points at. + */ public record VmGrantedSpecResponse( Integer grantedVcpu, Integer grantedMemoryMb, Integer grantedDiskGb, UUID grantedImageId, - @Nullable UUID nodeId) { + @Schema(description = "승인된 OS 이미지의 표시 이름. 카탈로그에서 내려간 이미지도 이름이 남습니다.") + String grantedImageName, + @Nullable UUID nodeId, + @Nullable + @Schema(description = "배치된 노드의 이름. nodeId가 있을 때 함께 있습니다.") + String nodeName) { } diff --git a/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmRequestSpecResponse.java b/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmRequestSpecResponse.java index 22683ac..3fbddb5 100644 --- a/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmRequestSpecResponse.java +++ b/src/main/java/kr/ac/pusan/pickle/request/vm/dto/VmRequestSpecResponse.java @@ -1,5 +1,9 @@ package kr.ac.pusan.pickle.request.vm.dto; +import io.swagger.v3.oas.annotations.media.Schema; +import kr.ac.pusan.pickle.inventory.Node; +import kr.ac.pusan.pickle.inventory.OsImage; +import kr.ac.pusan.pickle.inventory.VmFlavor; import kr.ac.pusan.pickle.request.vm.VmRequestDetail; import java.util.UUID; import org.jspecify.annotations.Nullable; @@ -8,10 +12,25 @@ * Contract schema {@code VmRequestSpec}: what a VM request asked for, reported * under {@code vm} in the request detail. {@code granted} is null until the * request is approved. + * + *

Every catalog reference carries its name beside its id. The request is a + * historical record and the catalog is not: an image retired since the request + * was filed is no longer in the catalog the client fetched, so a client that + * resolves the name for itself has nothing to resolve it against and the + * reference renders as unknown. The name travels with the reference for that + * reason.

+ * + *

Each name is present exactly when its id is — both come from the same row, + * and catalog rows are retired by status, never deleted.

*/ public record VmRequestSpecResponse( UUID imageId, + @Schema(description = "요청한 OS 이미지의 표시 이름. 카탈로그에서 내려간 이미지도 이름이 남습니다.") + String imageName, @Nullable UUID flavorId, + @Nullable + @Schema(description = "요청한 사양 프리셋의 표시 이름. flavorId가 있을 때 함께 있습니다.") + String flavorName, int reqVcpu, int reqMemoryMb, int reqDiskGb, @@ -21,13 +40,18 @@ public record VmRequestSpecResponse( @Nullable String rootDomain, @Nullable VmGrantedSpecResponse granted) { - public static VmRequestSpecResponse from(VmRequestDetail detail, UUID imageId, UUID flavorId, - UUID grantedImageId, UUID nodeId) { + public static VmRequestSpecResponse from(VmRequestDetail detail, OsImage image, + @Nullable VmFlavor flavor, OsImage grantedImage, @Nullable Node node) { VmGrantedSpecResponse granted = detail.getGrantedVcpu() != null ? new VmGrantedSpecResponse(detail.getGrantedVcpu(), detail.getGrantedMemoryMb(), - detail.getGrantedDiskGb(), grantedImageId, nodeId) + detail.getGrantedDiskGb(), grantedImage.getPublicId(), + grantedImage.getDisplayName(), + node == null ? null : node.getPublicId(), + node == null ? null : node.getName()) : null; - return new VmRequestSpecResponse(imageId, flavorId, + return new VmRequestSpecResponse(image.getPublicId(), image.getDisplayName(), + flavor == null ? null : flavor.getPublicId(), + flavor == null ? null : flavor.getDisplayName(), detail.getReqVcpu(), detail.getReqMemoryMb(), detail.getReqDiskGb(), detail.getSpecReason(), detail.getDesiredSlug(), detail.getDesiredSubdomain(), detail.getRootDomain(), granted); diff --git a/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewayRouteService.java b/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewayRouteService.java index 6509a72..7b1e8e2 100644 --- a/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewayRouteService.java +++ b/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewayRouteService.java @@ -242,7 +242,7 @@ public static List splitHostKeys(String stored) { private static final class Context { private final RouteRequest request; private final String gatewayPeer; - private Long identifiedKeyId; + private java.util.UUID identifiedKeyPublicId; Context(RouteRequest request, String gatewayPeer) { this.request = request; @@ -266,7 +266,7 @@ String fingerprint() { } void identify(UserSshKey key) { - this.identifiedKeyId = key.getId(); + this.identifiedKeyPublicId = key.getPublicId(); } Map detail(String reason) { @@ -280,8 +280,8 @@ Map detail(String reason) { if (request.publicKeyFingerprint() != null && !request.publicKeyFingerprint().isBlank()) { detail.put("fingerprint", request.publicKeyFingerprint()); } - if (identifiedKeyId != null) { - detail.put("keyId", identifiedKeyId); + if (identifiedKeyPublicId != null) { + detail.put("keyId", identifiedKeyPublicId); } if (request.connectionId() != null && !request.connectionId().isBlank()) { detail.put("connectionId", request.connectionId()); diff --git a/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionService.java b/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionService.java index 2ac291c..8456403 100644 --- a/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionService.java +++ b/src/main/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionService.java @@ -8,6 +8,7 @@ import java.util.Map; import java.util.Set; import java.util.UUID; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.sshgw.dto.SessionRequest; import kr.ac.pusan.pickle.sshkey.UserSshKey; @@ -61,12 +62,15 @@ public class SshGatewaySessionService { private final VmRepository vmRepository; private final UserSshKeyRepository sshKeyRepository; private final AuditService auditService; + private final AuditIds auditIds; public SshGatewaySessionService(VmRepository vmRepository, - UserSshKeyRepository sshKeyRepository, AuditService auditService) { + UserSshKeyRepository sshKeyRepository, AuditService auditService, + AuditIds auditIds) { this.vmRepository = vmRepository; this.sshKeyRepository = sshKeyRepository; this.auditService = auditService; + this.auditIds = auditIds; } /** @@ -94,15 +98,15 @@ public void recordSession(SessionRequest request, String gatewayPeer) { if (owners.size() == 1) { // Sound attribution: the signer is one of these keys, all one owner. actorId = owners.iterator().next(); - detail.put("userId", actorId); + detail.put("userId", auditIds.user(actorId)); detail.put("fingerprints", fingerprintsOf(resolved)); - detail.put("keyIds", keyIdsOf(resolved)); + detail.put("keyIds", auditIds.sshKeys(keyIdsOf(resolved))); bumpLastUsed(resolved); } else if (owners.size() >= 2) { // Framing vector: candidates span owners; the plugin can't prove the // signer, so attribute to no one. detail.put("ambiguous", true); - detail.put("candidateUserIds", new ArrayList<>(owners)); + detail.put("candidateUserIds", auditIds.users(owners)); detail.put("fingerprints", fingerprintsOf(resolved)); } else { // Zero resolve — all keys revoked mid-connection. Best-effort miss. diff --git a/src/main/java/kr/ac/pusan/pickle/terminal/TerminalService.java b/src/main/java/kr/ac/pusan/pickle/terminal/TerminalService.java index 68cce0d..c7882e3 100644 --- a/src/main/java/kr/ac/pusan/pickle/terminal/TerminalService.java +++ b/src/main/java/kr/ac/pusan/pickle/terminal/TerminalService.java @@ -229,7 +229,7 @@ public void sessionStart(String sessionId, String clientIp) { // detail carries lifecycle metadata ONLY — never frame/keystroke content. Map detail = new LinkedHashMap<>(); detail.put("sessionId", sessionId); - detail.put("vmId", s.vmId()); + detail.put("vmId", vmPublicId(s.vmId())); detail.put("clientIp", clientIp); auditService.record(s.userId(), roleName(s.userRole()), AuditService.TERMINAL_SESSION_START, "vm", vmPublicId(s.vmId()), detail, clientIp); @@ -250,7 +250,7 @@ public void sessionEnd(TerminalSessionEndRequest request) { // detail carries counts/reasons ONLY — never frame/keystroke content. Map detail = new LinkedHashMap<>(); detail.put("sessionId", request.sessionId()); - detail.put("vmId", s.vmId()); + detail.put("vmId", vmPublicId(s.vmId())); detail.put("reason", request.reason()); detail.put("durationSeconds", request.durationSeconds()); detail.put("bytesIn", request.bytesIn()); @@ -341,7 +341,7 @@ public void terminate(AuthenticatedUser actor, String sessionId, String ip) { if (known.isPresent()) { Map detail = new LinkedHashMap<>(); detail.put("sessionId", sessionId); - detail.put("vmId", known.get().vmId()); + detail.put("vmId", vmPublicId(known.get().vmId())); auditService.record(actor.id(), actor.role().name(), AuditService.TERMINAL_FORCE_TERMINATE, "vm", vmPublicId(known.get().vmId()), detail, ip); diff --git a/src/main/java/kr/ac/pusan/pickle/vm/VmDeletionService.java b/src/main/java/kr/ac/pusan/pickle/vm/VmDeletionService.java index 1b95e4d..0285c14 100644 --- a/src/main/java/kr/ac/pusan/pickle/vm/VmDeletionService.java +++ b/src/main/java/kr/ac/pusan/pickle/vm/VmDeletionService.java @@ -13,6 +13,7 @@ import kr.ac.pusan.pickle.access.VmAccessService; import kr.ac.pusan.pickle.admin.dto.ForceDeleteVmRequest; import kr.ac.pusan.pickle.admin.dto.ScheduleVmDeletionRequest; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.auth.dto.MessageResponse; import kr.ac.pusan.pickle.common.error.ApiException; @@ -86,6 +87,7 @@ public class VmDeletionService { private final JobScheduler jobScheduler; private final DeleteVmJob deleteVmJob; private final AuditService auditService; + private final AuditIds auditIds; private final NotificationService notificationService; private final ProvisioningTaskRepository provisioningTaskRepository; private final PublishingTeardownService publishingTeardown; @@ -95,7 +97,7 @@ public class VmDeletionService { public VmDeletionService(VmRepository vmRepository, WorkspaceMemberRepository workspaceMemberRepository, VmAccessService vmAccessService, UserRepository userRepository, VmEventRepository vmEventRepository, SettingsService settingsService, IpamService ipamService, JobScheduler jobScheduler, - DeleteVmJob deleteVmJob, AuditService auditService, + DeleteVmJob deleteVmJob, AuditService auditService, AuditIds auditIds, NotificationService notificationService, ProvisioningTaskRepository provisioningTaskRepository, PublishingTeardownService publishingTeardown, @@ -110,6 +112,7 @@ public VmDeletionService(VmRepository vmRepository, WorkspaceMemberRepository wo this.jobScheduler = jobScheduler; this.deleteVmJob = deleteVmJob; this.auditService = auditService; + this.auditIds = auditIds; this.notificationService = notificationService; this.provisioningTaskRepository = provisioningTaskRepository; this.publishingTeardown = publishingTeardown; @@ -141,8 +144,8 @@ public VmDeletionResponse selfDelete(AuthenticatedUser actor, UUID publicVmId, S vmEventRepository.save(new VmEvent(vmId, VmEventType.SELF_DELETE, actor.id(), "삭제 접수 — " + KST.format(scheduledFor) + " (KST) 파기 예정")); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_SELF_DELETE, - "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", vm.getOrgId(), - "workspaceId", vm.getWorkspaceId(), "scheduledFor", scheduledFor.toString()), ip); + "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", auditIds.org(vm.getOrgId()), + "workspaceId", auditIds.workspace(vm.getWorkspaceId()), "scheduledFor", scheduledFor.toString()), ip); // Best-effort graceful shutdown; its failure never touches the schedule. enqueueAfterCommit(() -> deleteVmJob.gracefulShutdown(vmId)); @@ -178,8 +181,8 @@ private VmDeletionResponse deleteErrorVmImmediately(AuthenticatedUser actor, Vm vmEventRepository.save(new VmEvent(vm.getId(), VmEventType.DELETE, actor.id(), "VM 파기 완료 — ERROR 상태(파기할 게스트 없음), IP 회수")); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_SELF_DELETE, - "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", vm.getOrgId(), - "workspaceId", vm.getWorkspaceId(), "immediate", true), ip); + "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", auditIds.org(vm.getOrgId()), + "workspaceId", auditIds.workspace(vm.getWorkspaceId()), "immediate", true), ip); return new VmDeletionResponse(VmDeleteKind.SELF, now, now, actor.publicId(), null, false); } @@ -211,8 +214,8 @@ public VmDeletionResponse scheduleDeletion(AuthenticatedUser actor, UUID publicV vmEventRepository.save(new VmEvent(vmId, VmEventType.SCHEDULE_DELETE, actor.id(), "관리자 삭제 접수 — " + KST.format(request.scheduledFor()) + " (KST), 사유: " + reason)); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_SCHEDULE_DELETE, - "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", vm.getOrgId(), - "workspaceId", vm.getWorkspaceId(), + "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", auditIds.org(vm.getOrgId()), + "workspaceId", auditIds.workspace(vm.getWorkspaceId()), "scheduledFor", request.scheduledFor().toString(), "reason", reason), ip); notificationService.publish(recipients(vm, false), NotificationEvent.VM_DELETE_SCHEDULED, Map.of("vmId", vm.getPublicId(), "vmName", vm.getName(), "reason", reason, @@ -256,8 +259,8 @@ public MessageResponse cancelScheduledDeletion(AuthenticatedUser actor, UUID pub : "관리자 삭제 취소")); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_CANCEL_SCHEDULED_DELETE, "vm", vm.getPublicId(), - Map.of("name", vm.getName(), "orgId", vm.getOrgId(), - "workspaceId", vm.getWorkspaceId(), "canceledKind", vm.getDeleteKind().name()), ip); + Map.of("name", vm.getName(), "orgId", auditIds.org(vm.getOrgId()), + "workspaceId", auditIds.workspace(vm.getWorkspaceId()), "canceledKind", vm.getDeleteKind().name()), ip); notificationService.publish(recipients(vm, false), NotificationEvent.VM_DELETE_CANCELED, Map.of("vmId", vm.getPublicId(), "vmName", vm.getName()), null); return new MessageResponse("삭제가 취소되었습니다."); @@ -303,8 +306,8 @@ public MessageResponse forceDelete(AuthenticatedUser actor, UUID publicVmId, overrodeProtection ? "강제 삭제 접수 — 삭제 보호 오버라이드, 즉시 강제 종료 후 파기" : "강제 삭제 접수 — 즉시 강제 종료 후 파기")); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.VM_FORCE_DELETE, - "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", vm.getOrgId(), - "workspaceId", vm.getWorkspaceId(), "overrodeProtection", overrodeProtection), ip); + "vm", vm.getPublicId(), Map.of("name", vm.getName(), "orgId", auditIds.org(vm.getOrgId()), + "workspaceId", auditIds.workspace(vm.getWorkspaceId()), "overrodeProtection", overrodeProtection), ip); enqueueAfterCommit(() -> deleteVmJob.deleteVm(vmId)); notificationService.publish(recipients(vm, true), NotificationEvent.VM_DELETE_FORCE, Map.of("vmId", vm.getPublicId(), "vmName", vm.getName()), null); diff --git a/src/main/java/kr/ac/pusan/pickle/workspace/WorkspaceService.java b/src/main/java/kr/ac/pusan/pickle/workspace/WorkspaceService.java index d9af756..dbb8d1c 100644 --- a/src/main/java/kr/ac/pusan/pickle/workspace/WorkspaceService.java +++ b/src/main/java/kr/ac/pusan/pickle/workspace/WorkspaceService.java @@ -6,6 +6,7 @@ import java.util.stream.Collectors; import kr.ac.pusan.pickle.access.AccessGranteeType; import kr.ac.pusan.pickle.access.ResourceAccessGrantRepository; +import kr.ac.pusan.pickle.audit.AuditIds; import kr.ac.pusan.pickle.audit.AuditService; import kr.ac.pusan.pickle.common.error.ApiException; import kr.ac.pusan.pickle.common.error.ErrorCodes; @@ -50,12 +51,13 @@ public class WorkspaceService { private final UserRepository userRepository; private final RequestRepository requestRepository; private final AuditService auditService; + private final AuditIds auditIds; private final NotificationService notificationService; private final List resourceAdapters; public WorkspaceService(WorkspaceRepository workspaceRepository, WorkspaceMemberRepository workspaceMemberRepository, ResourceAccessGrantRepository grantRepository, UserRepository userRepository, - RequestRepository requestRepository, AuditService auditService, + RequestRepository requestRepository, AuditService auditService, AuditIds auditIds, NotificationService notificationService, List resourceAdapters) { this.workspaceRepository = workspaceRepository; this.workspaceMemberRepository = workspaceMemberRepository; @@ -63,6 +65,7 @@ public WorkspaceService(WorkspaceRepository workspaceRepository, WorkspaceMember this.userRepository = userRepository; this.requestRepository = requestRepository; this.auditService = auditService; + this.auditIds = auditIds; this.notificationService = notificationService; this.resourceAdapters = resourceAdapters; } @@ -166,7 +169,8 @@ public WorkspaceMemberResponse addMember(AuthenticatedUser actor, UUID publicWor } auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.WORKSPACE_MEMBER_ADD, "workspace", workspace.getPublicId(), - Map.of("userId", target.getId(), "email", target.getEmail(), "role", member.getRole().name()), ip); + Map.of("userId", target.getPublicId(), "email", target.getEmail(), + "role", member.getRole().name()), ip); return WorkspaceMemberResponse.from(member, target); } @@ -202,7 +206,7 @@ public WorkspaceMemberResponse updateMemberRole(AuthenticatedUser actor, UUID pu User targetUser = userRepository.findById(targetUserId).orElseThrow(WorkspaceService::memberNotFound); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.WORKSPACE_MEMBER_UPDATE, "workspace", workspace.getPublicId(), - Map.of("userId", targetUserId, "previousRole", previousRole.name(), + Map.of("userId", targetUser.getPublicId(), "previousRole", previousRole.name(), "role", target.getRole().name()), ip); return WorkspaceMemberResponse.from(target, targetUser); } @@ -238,7 +242,7 @@ public void removeMember(AuthenticatedUser actor, UUID publicWorkspaceId, UUID p int revokedGrants = revokeGrantsOnWorkspaceResources(workspaceId, targetUserId); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.WORKSPACE_MEMBER_REMOVE, "workspace", workspace.getPublicId(), - Map.of("userId", targetUserId, "previousRole", target.getRole().name(), + Map.of("userId", auditIds.user(targetUserId), "previousRole", target.getRole().name(), "selfLeave", selfLeave, "revokedGrants", revokedGrants), ip); } @@ -301,7 +305,7 @@ public void delete(AuthenticatedUser actor, UUID publicWorkspaceId, String ip) { locked.setStatus(RequestStatus.CANCELED); auditService.recordAfterCommit(actor.id(), actor.role().name(), AuditService.REQUEST_CANCEL, "request", locked.getPublicId(), - Map.of("workspaceId", workspaceId, "reason", "workspace_deleted"), ip); + Map.of("workspaceId", workspace.getPublicId(), "reason", "workspace_deleted"), ip); } // Counted after the request rows are locked, not before: an approval diff --git a/src/main/resources/db/migration/V79__audit_public_id_and_request_display_name.sql b/src/main/resources/db/migration/V79__audit_public_id_and_request_display_name.sql new file mode 100644 index 0000000..67b2709 --- /dev/null +++ b/src/main/resources/db/migration/V79__audit_public_id_and_request_display_name.sql @@ -0,0 +1,59 @@ +-- Two identifiers a user should never have been shown, and one name a request +-- should always have carried. +-- +-- V78 gave 23 tables a public_id and left audit_logs out, on the reading that +-- its row id is a rendering key nobody queries by. The consequence was missed: +-- that key is rendered into the `id` field of both audit responses, and one of +-- them is an ordinary user's own history. A sequential number there discloses +-- the platform's total audit volume and its growth rate to anyone who reads +-- their own activity page twice -- the same leak V78 closed everywhere else. +-- 4,121 rows carried it at the time this was written. +-- +-- The application role has UPDATE, DELETE and TRUNCATE revoked on this table +-- (V7), and adding a column whose default is volatile rewrites it. The rewrite +-- is DDL performed by the table owner and needs no UPDATE privilege: verified +-- on a pg_dump copy of the development database, running as the `pickle` role, +-- where UPDATE on audit_logs is refused ("permission denied for table +-- audit_logs") and this statement succeeds, filling every existing row with a +-- distinct value. V78 used the same pattern on 23 tables. + +alter table audit_logs add column public_id uuid not null default gen_random_uuid(); +create unique index audit_logs_public_id_uidx on audit_logs (public_id); + +comment on column audit_logs.public_id is + '감사 로그 행의 공개 식별자. 응답의 id 필드가 담는 값이며, 내부 순번(id)은 서버 밖으로 나가지 않는다. 목록 렌더링용 키일 뿐 조회 파라미터가 아닌 성격은 그대로다.'; + +-- A request's display name becomes mandatory. It is the name of the resource +-- being asked for -- for every resource type, not just a VM -- and every +-- response that carries a request reference now carries it beside the id, +-- because a UUID cannot be read, remembered or spoken. Optional since it was +-- introduced, it was set on 6 of 84 rows. +-- +-- Each backfill source is a fact already on record rather than a stand-in: +-- 1. the name already stored, where there is one (6 rows); +-- 2. the name of the VM created from the request -- the resource the request +-- produced is the thing the name names (67 rows); +-- 3. the requester's own `purpose`, cut to the 100 characters the API accepts, +-- for a request that never produced a resource (11 rows: 10 canceled, 1 +-- still submitted). `purpose` is NOT NULL and blank on no row, and it is +-- what the requester themselves wrote about what they were asking for, so +-- it names the request more truthfully than anything generated would. +-- The trailing literal is unreachable while (3) holds and exists only so that +-- the NOT NULL below cannot fail a deployment on some row no source covers. +-- The subquery is ordered because nothing stops two VMs sharing a request_id; +-- none do today, and a backfill should not depend on that staying true. + +update requests r + set display_name = coalesce( + nullif(btrim(r.display_name), ''), + nullif(btrim((select v.name from vms v + where v.request_id = r.id + order by v.id limit 1)), ''), + nullif(left(btrim(r.purpose), 100), ''), + '이름 없는 신청') + where r.display_name is null or btrim(r.display_name) = ''; + +alter table requests alter column display_name set not null; + +comment on column requests.display_name is + '신청한 리소스의 표시명. 신청 시 필수 입력이며, 이 신청을 가리키는 모든 응답이 공개 식별자 옆에 함께 싣는다. 이 마이그레이션 이전 행은 만들어진 VM의 이름, 그것도 없으면 신청자가 적은 사용 목적의 앞 100자로 채웠다.'; diff --git a/src/test/java/kr/ac/pusan/pickle/ProvisioningEndToEndTest.java b/src/test/java/kr/ac/pusan/pickle/ProvisioningEndToEndTest.java index 9ab2050..8e3aed0 100644 --- a/src/test/java/kr/ac/pusan/pickle/ProvisioningEndToEndTest.java +++ b/src/test/java/kr/ac/pusan/pickle/ProvisioningEndToEndTest.java @@ -170,6 +170,7 @@ void flowFromSignupToProvisionedVm() throws Exception { "workspaceId", pub("workspaces", workspaceId), "orgId", pub("orgs", orgId), "purpose", "종단 검증용 서버", + "displayName", "종단 검증 서버", "vm", Map.of( "imageId", pub("os_images", imageId), "flavorId", pub("vm_flavors", flavorId), @@ -248,6 +249,18 @@ void flowFromSignupToProvisionedVm() throws Exception { assertThat(jdbcTemplate.queryForObject( "select count(*) from vm_events where vm_id = ? and type = 'CREATE'", Long.class, vmId)).isEqualTo(1); + // The owner reads this entry on their VM's timeline, so it names the OS + // the machine runs and the address it answers on — never the Proxmox + // vmid, which the owner cannot act on and which counts up across the + // cluster, telling them how many the platform had ever provisioned. + String createDetail = jdbcTemplate.queryForObject( + "select detail from vm_events where vm_id = ? and type = 'CREATE'", + String.class, vmId); + assertThat(createDetail) + .contains(jdbcTemplate.queryForObject( + "select display_name from os_images where id = ?", String.class, imageId)) + .contains(EXPECTED_IP) + .doesNotContain(String.valueOf(VMID)); // creation notice is a notifications row; the dispatcher emails it // (running it directly — the 1-minute recurring schedule is too slow // for the test, and the CAS claim makes the direct call race-safe) diff --git a/src/test/java/kr/ac/pusan/pickle/access/VmAccessGrantApiTest.java b/src/test/java/kr/ac/pusan/pickle/access/VmAccessGrantApiTest.java index 0ca3439..7446183 100644 --- a/src/test/java/kr/ac/pusan/pickle/access/VmAccessGrantApiTest.java +++ b/src/test/java/kr/ac/pusan/pickle/access/VmAccessGrantApiTest.java @@ -379,9 +379,13 @@ void aWorkspaceOwnerLettingThemselvesInIsRecordedAsBreakGlass() throws Exception userGrant(workspaceOwner.getId(), "MEMBER")); assertThat(auditCount(AuditService.VM_ACCESS_GRANT_ADD, vmId)).isEqualTo(1); assertThat(auditCount(AuditService.VM_ACCESS_BREAK_GLASS, vmId)).isEqualTo(1); - // the marker carries the same detail as the change it shadows + // the marker carries the same detail as the change it shadows. Both ids + // in it are public ones: this detail reaches its subject through their + // own activity feed, so an internal row number here is a leak. assertThat(auditDetail(AuditService.VM_ACCESS_BREAK_GLASS, vmId, "grantId")) - .isEqualTo(String.valueOf(selfGrantId)); + .isEqualTo(pub("resource_access_grants", selfGrantId).toString()); + assertThat(auditDetail(AuditService.VM_ACCESS_BREAK_GLASS, vmId, "granteeUserId")) + .isEqualTo(workspaceOwner.getPublicId().toString()); assertThat(auditDetail(AuditService.VM_ACCESS_BREAK_GLASS, vmId, "role")) .isEqualTo("MEMBER"); @@ -717,6 +721,7 @@ private long submitRequest(String token) throws Exception { body.put("workspaceId", pub("workspaces", workspaceId)); body.put("orgId", pub("orgs", orgId)); body.put("purpose", "접근 권한 테스트용 신청"); + body.put("displayName", "접근 권한 테스트 서버"); body.put("vm", vm); String response = mockMvc.perform(post("/api/v1/requests") .header("Authorization", "Bearer " + token) diff --git a/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java b/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java index e1dd59f..bd1f442 100644 --- a/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java +++ b/src/test/java/kr/ac/pusan/pickle/admin/AdminInventoryTest.java @@ -184,6 +184,7 @@ insert into workspace_members (workspace_id, user_id, role) .content(""" {"type": "VM", "workspaceId": "%s", "orgId": "%s", "purpose": "은퇴 OS 이미지 거부 확인", + "displayName": "은퇴 이미지 확인", "vm": {"imageId": "%s", "flavorId": "%s", "reqVcpu": 2, "reqMemoryMb": 2048, "reqDiskGb": 20}} """.formatted(pub("workspaces", workspaceId), pub("orgs", orgId), diff --git a/src/test/java/kr/ac/pusan/pickle/admin/ApprovalTest.java b/src/test/java/kr/ac/pusan/pickle/admin/ApprovalTest.java index 0e70a9a..08b8027 100644 --- a/src/test/java/kr/ac/pusan/pickle/admin/ApprovalTest.java +++ b/src/test/java/kr/ac/pusan/pickle/admin/ApprovalTest.java @@ -687,13 +687,13 @@ private long submit(String token, long workspaceId, long orgId, String displayNa vm.put("reqMemoryMb", flavor.getMemoryMb()); vm.put("reqDiskGb", flavor.getDiskGb()); Map body = new HashMap<>(); - if (displayName != null) { - body.put("displayName", displayName); - } body.put("type", "VM"); body.put("workspaceId", pub("workspaces", workspaceId)); body.put("orgId", pub("orgs", orgId)); body.put("purpose", "승인 흐름 테스트"); + // Every request carries a name. Callers whose subject is the name pass + // their own; the rest take this one so the body is valid. + body.put("displayName", displayName != null ? displayName : "승인 테스트 서버"); body.put("vm", vm); String response = postJson("/api/v1/requests", token, body) .andExpect(status().isCreated()) diff --git a/src/test/java/kr/ac/pusan/pickle/audit/AuditReadApiTest.java b/src/test/java/kr/ac/pusan/pickle/audit/AuditReadApiTest.java index 81d0602..5a5ea6b 100644 --- a/src/test/java/kr/ac/pusan/pickle/audit/AuditReadApiTest.java +++ b/src/test/java/kr/ac/pusan/pickle/audit/AuditReadApiTest.java @@ -3,6 +3,7 @@ import kr.ac.pusan.pickle.support.RequestFixtures; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.hamcrest.Matchers.not; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.UUID; @@ -130,6 +131,39 @@ void myActivityIsStrictlySelfScoped() throws Exception { mockMvc.perform(get("/api/v1/me/activity")).andExpect(status().isUnauthorized()); } + /** + * The row's own number never leaves the server. Both audit responses type + * {@code id} as an opaque rendering key, which is exactly why nothing + * objected while it carried the sequential {@code audit_logs.id} — and a + * user reading their own activity twice could read the platform's total + * audit volume and its growth rate off it. + */ + @Test + void auditRowsAreIdentifiedByTheirPublicIdNotTheRowNumber() throws Exception { + String action = "test." + runTag + ".vmdel"; + Long rowNumber = jdbcTemplate.queryForObject( + "select id from audit_logs where action = ? and actor_id = ?", + Long.class, action, self.getId()); + UUID publicId = jdbcTemplate.queryForObject( + "select public_id from audit_logs where id = ?", UUID.class, rowNumber); + + // the user's own history + mockMvc.perform(get("/api/v1/me/activity?action=" + action) + .header("Authorization", "Bearer " + selfToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0].id").value(publicId.toString())) + .andExpect(jsonPath("$.content[0].id") + .value(not(String.valueOf(rowNumber)))); + // and the same row through the admin view + mockMvc.perform(get("/api/v1/admin/audit?action=" + action + + "&actorEmail=aud.self@pusan.ac.kr") + .header("Authorization", "Bearer " + sysAdminToken)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content[0].id").value(publicId.toString())) + .andExpect(jsonPath("$.content[0].id") + .value(not(String.valueOf(rowNumber)))); + } + @Test void adminAuditScopesOrgAdminByDerivedMembershipInSql() throws Exception { String actionFilter = "?action=test." + runTag + ".vmdel&size=100"; diff --git a/src/test/java/kr/ac/pusan/pickle/request/RequestTest.java b/src/test/java/kr/ac/pusan/pickle/request/RequestTest.java index b52615f..01b4b32 100644 --- a/src/test/java/kr/ac/pusan/pickle/request/RequestTest.java +++ b/src/test/java/kr/ac/pusan/pickle/request/RequestTest.java @@ -356,7 +356,7 @@ void emptyOsCatalogListsNothingAndRefusesEverySubmission() throws Exception { } @Test - void displayNameIsLengthCappedAndEchoed() throws Exception { + void resourceNameIsRequiredLengthCappedAndEchoed() throws Exception { long workspaceId = createTeam(requesterToken, "vmr-dname-x1"); // over 100 chars → 422 (bean validation) @@ -366,7 +366,7 @@ void displayNameIsLengthCappedAndEchoed() throws Exception { .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) .andExpect(jsonPath("$.errors[0].field").value("displayName")); - // echoed in the detail; omitted → null + // echoed in the detail String response = postJson("/api/v1/requests", requesterToken, with(validBody(workspaceId), "displayName", "데이터분석 실습 서버")) .andExpect(status().isCreated()) @@ -378,9 +378,19 @@ void displayNameIsLengthCappedAndEchoed() throws Exception { .andExpect(status().isOk()) .andExpect(jsonPath("$.displayName").value("데이터분석 실습 서버")); - postJson("/api/v1/requests", requesterToken, validBody(workspaceId)) - .andExpect(status().isCreated()) - .andExpect(jsonPath("$.displayName").value((Object) null)); + // the name is what the resource will be called, so a request cannot be + // filed without one — omitted and blank are the same field error + Map nameless = validBody(workspaceId); + nameless.remove("displayName"); + postJson("/api/v1/requests", requesterToken, nameless) + .andExpect(status().isUnprocessableContent()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.errors[0].field").value("displayName")); + postJson("/api/v1/requests", requesterToken, + with(validBody(workspaceId), "displayName", " ")) + .andExpect(status().isUnprocessableContent()) + .andExpect(jsonPath("$.code").value("VALIDATION_FAILED")) + .andExpect(jsonPath("$.errors[0].field").value("displayName")); } @Test @@ -562,6 +572,7 @@ private Map bodyFor(long workspaceId, VmFlavor flavor) { body.put("workspaceId", pub("workspaces", workspaceId)); body.put("orgId", org.getPublicId()); body.put("purpose", "수업 실습용 서버"); + body.put("displayName", "실습 서버"); body.put("vm", vm); return body; } diff --git a/src/test/java/kr/ac/pusan/pickle/security/PublicIdentifierTest.java b/src/test/java/kr/ac/pusan/pickle/security/PublicIdentifierTest.java index 353acb0..b218976 100644 --- a/src/test/java/kr/ac/pusan/pickle/security/PublicIdentifierTest.java +++ b/src/test/java/kr/ac/pusan/pickle/security/PublicIdentifierTest.java @@ -2,6 +2,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; @@ -10,6 +11,7 @@ import java.util.UUID; import kr.ac.pusan.pickle.support.AccessGrantFixtures; import kr.ac.pusan.pickle.support.EmbeddedPostgresConfig; +import kr.ac.pusan.pickle.support.ReauthTestSupport; import kr.ac.pusan.pickle.support.SeedFixtures; import kr.ac.pusan.pickle.user.User; import kr.ac.pusan.pickle.user.UserRepository; @@ -20,6 +22,7 @@ import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.webmvc.test.autoconfigure.AutoConfigureMockMvc; import org.springframework.context.annotation.Import; +import org.springframework.http.MediaType; import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.test.context.ActiveProfiles; @@ -100,6 +103,48 @@ void everyIdTheApiHandsBackIsAUuidAndNeverTheRowNumber() throws Exception { assertThat(body).doesNotContain("\"id\":" + vmId); } + /** + * The audit {@code detail} is free-form jsonb, so nothing about it is typed + * and no compiler notices when a call site puts a row number in one. It is + * also the one payload an ordinary user reads about themselves, through + * {@code /me/activity}. This walks every audit row the suite has written and + * asserts the property directly: under a key that names an identifier, the + * value is never a number. + */ + @Test + void noAuditDetailNamesSomethingByItsRowNumber() throws Exception { + // an audited action whose detail carries an id-shaped key, so the sweep + // below can never pass by having nothing to look at + mockMvc.perform(post("/api/v1/workspaces/" + + SeedFixtures.publicId(jdbcTemplate, "workspaces", workspaceId) + "/members") + .header("Authorization", "Bearer " + ownerToken) + .header("X-Reauth-Token", ReauthTestSupport.seededReauthFor( + jdbcTemplate, jwtService, ownerToken)) + .contentType(MediaType.APPLICATION_JSON) + .content("{\"email\": \"" + outsider.getEmail() + "\", \"role\": \"MEMBER\"}")) + .andExpect(status().isCreated()); + + Long inspected = jdbcTemplate.queryForObject(""" + select count(*) from audit_logs a, lateral jsonb_each(a.detail) kv + where a.detail is not null and (kv.key like '%Id' or kv.key like '%Ids') + """, Long.class); + assertThat(inspected).isPositive(); + + List numeric = jdbcTemplate.queryForList(""" + select a.action || '.' || kv.key || ' = ' || kv.value::text + from audit_logs a, lateral jsonb_each(a.detail) kv + where a.detail is not null + and (kv.key like '%Id' or kv.key like '%Ids') + and (jsonb_typeof(kv.value) = 'number' + or (jsonb_typeof(kv.value) = 'array' + and exists (select 1 from jsonb_array_elements(kv.value) e + where jsonb_typeof(e) = 'number'))) + """, String.class); + assertThat(numeric) + .as("audit detail entries naming something by its row number") + .isEmpty(); + } + @Test void theRowNumberIsNoLongerAnAddress() throws Exception { // A well-formed bigint is not a well-formed identifier any more, so the @@ -190,8 +235,9 @@ private long createVm(long owningWorkspaceId, String hostname) { long imageId = jdbcTemplate.queryForObject("select min(id) from os_images", Long.class); long orgId = SeedFixtures.seedOrgId(jdbcTemplate); long requestId = jdbcTemplate.queryForObject(""" - insert into requests (resource_type, workspace_id, org_id, requester_id, purpose) - values ('VM', ?, ?, ?, '공개 식별자 확인') + insert into requests (resource_type, workspace_id, org_id, requester_id, purpose, + display_name) + values ('VM', ?, ?, ?, '공개 식별자 확인', '공개 식별자 확인') returning id """, Long.class, owningWorkspaceId, orgId, owner.getId()); String unique = hostname + "-" + Instant.now().toEpochMilli(); diff --git a/src/test/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionTest.java b/src/test/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionTest.java index eaa0e5d..6bda354 100644 --- a/src/test/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionTest.java +++ b/src/test/java/kr/ac/pusan/pickle/sshgw/SshGatewaySessionTest.java @@ -120,8 +120,11 @@ void candidatesSpanningTwoOwnersAreAmbiguousNullActor() throws Exception { Map row = latestSession(); assertThat(row.get("actor_id")).isNull(); + // the candidates are named publicly: this detail is readable by the very + // people it accuses, so it carries their public ids and not row numbers assertThat((String) row.get("detail")).contains("ambiguous") - .contains(String.valueOf(memberId)).contains(String.valueOf(otherId)); + .contains(pub("users", memberId).toString()) + .contains(pub("users", otherId).toString()); // framing prevention: no last_used_at bump on an ambiguous session assertThat(lastUsed(FP_MEMBER)).isNull(); assertThat(lastUsed(FP_OTHER)).isNull(); diff --git a/src/test/java/kr/ac/pusan/pickle/support/RequestFixtures.java b/src/test/java/kr/ac/pusan/pickle/support/RequestFixtures.java index 7c9e0db..64323cb 100644 --- a/src/test/java/kr/ac/pusan/pickle/support/RequestFixtures.java +++ b/src/test/java/kr/ac/pusan/pickle/support/RequestFixtures.java @@ -20,11 +20,15 @@ public static long insertVmRequest(JdbcTemplate jdbc, long workspaceId, long org long requesterId, String purpose, Long imageId, int vcpu, int memoryMb, int diskGb) { Long resolvedImageId = imageId != null ? imageId : jdbc.queryForObject("select min(id) from os_images", Long.class); + // display_name is mandatory on a request. The purpose stands in for it + // here for the same reason the backfill used it: it is what the fixture + // already says the request is for, so no test has to invent a name. long requestId = jdbc.queryForObject(""" - insert into requests (resource_type, workspace_id, org_id, requester_id, purpose) - values ('VM', ?, ?, ?, ?) + insert into requests (resource_type, workspace_id, org_id, requester_id, purpose, + display_name) + values ('VM', ?, ?, ?, ?, left(?, 100)) returning id - """, Long.class, workspaceId, orgId, requesterId, purpose); + """, Long.class, workspaceId, orgId, requesterId, purpose, purpose); jdbc.update(""" insert into vm_request_details (request_id, image_id, req_vcpu, req_memory_mb, req_disk_gb) values (?, ?, ?, ?, ?)