Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Track fix_transit_key_delete_inconsistency_20260309 Context

- [Specification](./spec.md)
- [Implementation Plan](./plan.md)
- [Metadata](./metadata.json)
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"track_id": "fix_transit_key_delete_inconsistency_20260309",
"type": "bug",
"status": "new",
"created_at": "2026-03-09T12:00:00Z",
"updated_at": "2026-03-09T12:00:00Z",
"description": "Fix Transit Key Delete Endpoint Inconsistency (DELETE /v1/transit/keys/:id` uses UUID while all other transit endpoints use `:name`. Make it consistent)"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Implementation Plan: Fix Transit Key Delete Endpoint Inconsistency

## Phase 1: Core Domain and Repository Updates
- [x] Task: Update `TransitKeyRepository` interface in `internal/transit/domain/repository.go` to change `Delete` signature from `transitKeyID uuid.UUID` to `name string`.
- [x] Task: Update `TransitKeyUseCase` interface in `internal/transit/usecase/interface.go` to change `Delete` signature from `transitKeyID uuid.UUID` to `name string`.
- [x] Task: Update PostgreSQL implementation in `internal/transit/repository/postgresql/postgresql_transit_key_repository.go` to soft-delete all versions by name.
- [x] Task: Update MySQL implementation in `internal/transit/repository/mysql/mysql_transit_key_repository.go` to soft-delete all versions by name.
- [x] Task: Update `TransitKeyUseCase` implementation in `internal/transit/usecase/transit_key_usecase.go` to use the new repository method.
- [x] Task: Update use case unit tests in `internal/transit/usecase/transit_key_usecase_test.go`.
- [x] Task: Update repository unit tests for both PostgreSQL and MySQL.
- [x] Task: Conductor - User Manual Verification 'Phase 1: Core Domain and Repository Updates' (Protocol in workflow.md)

## Phase 2: HTTP Handler and Routing Updates
- [x] Task: Update `TransitKeyHandler.DeleteHandler` in `internal/transit/http/transit_key_handler.go` to extract `name` from URL parameters instead of `id` (UUID).
- [x] Task: Update route registration in `internal/http/server.go` to change `DELETE /v1/transit/keys/:id` to `DELETE /v1/transit/keys/:name`.
- [x] Task: Update handler unit tests in `internal/transit/http/transit_key_handler_test.go`.
- [x] Task: Conductor - User Manual Verification 'Phase 2: HTTP Handler and Routing Updates' (Protocol in workflow.md)

## Phase 3: Integration Tests and Documentation
- [x] Task: Update integration tests in `test/integration/transit_flow_test.go` to use the transit key name for the DELETE request.
- [x] Task: Update `docs/openapi.yaml` to change `{id}` to `{name}` for the `DELETE /v1/transit/keys/{id}` endpoint and update its description.
- [x] Task: Update `docs/engines/transit.md` to ensure the Delete Transit Key section uses `:name`.
- [x] Task: Update `docs/concepts/api-fundamentals.md` and `docs/auth/policies.md` to change `:id` to `:name` for the transit delete endpoint.
- [x] Task: Conductor - User Manual Verification 'Phase 3: Integration Tests and Documentation' (Protocol in workflow.md)
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Specification: Fix Transit Key Delete Endpoint Inconsistency

## Overview
This track addresses an inconsistency in the Transit engine API. Currently, the `DELETE /v1/transit/keys/:id` endpoint uses a UUID (`:id`), while all other transit endpoints (GET, POST rotate/encrypt/decrypt) use the human-readable `:name`. This fix will align the DELETE endpoint with the rest of the API and the Tokenization engine's behavior.

## Functional Requirements
- Change the `DELETE /v1/transit/keys/:id` endpoint to `DELETE /v1/transit/keys/:name` in the router.
- The endpoint must accept a string `:name` instead of a UUID `:id`.
- Soft-delete all versions of the transit key associated with the given name.
- Update the `TransitKeyUseCase.Delete` method signature to accept `name string` instead of `transitKeyID uuid.UUID`.
- Update the `TransitKeyRepository.Delete` method signature to accept `name string` and update all matching records' `deleted_at` field.
- Update both PostgreSQL and MySQL repository implementations.
- Ensure proper authorization is maintained (requires `delete` capability on the path `/v1/transit/keys/:name`).
- Update the integration tests in `test/integration/transit_flow_test.go` to use `:name` instead of `:id` for deletion testing.

## Non-Functional Requirements
- Maintain API consistency across the platform.
- Ensure no regressions in other transit operations.

## Acceptance Criteria
- `DELETE /v1/transit/keys/:name` successfully soft-deletes all versions of the key.
- Integration tests confirm that all versions are marked as deleted.
- Unit tests for the handler, use case, and repository are updated/added.
- Documentation (`docs/openapi.yaml`, `docs/engines/transit.md`, `docs/concepts/api-fundamentals.md`, `docs/auth/policies.md`) reflects the change.

## Out of Scope
- Changing other engines' endpoints (Tokenization is already consistent).
- Hard-deletion of keys (remains a separate process/track).
2 changes: 1 addition & 1 deletion docs/auth/policies.md
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ Endpoint capability intent (quick map, condensed from [Capability matrix](../con
| --- | --- |
| `GET /v1/clients`, `GET /v1/audit-logs`, `POST /v1/tokenization/validate` | `read` |
| `POST /v1/clients`, `PUT /v1/clients/:id`, `POST /v1/transit/keys`, `POST /v1/tokenization/keys` | `write` |
| `DELETE /v1/token`, `DELETE /v1/clients/:id/tokens`, `DELETE /v1/clients/:id`, `DELETE /v1/transit/keys/:id`, `DELETE /v1/tokenization/keys/:id`, `POST /v1/tokenization/revoke` | `delete` |
| `DELETE /v1/token`, `DELETE /v1/clients/:id/tokens`, `DELETE /v1/clients/:id`, `DELETE /v1/transit/keys/:name`, `DELETE /v1/tokenization/keys/:id`, `POST /v1/tokenization/revoke` | `delete` |
| `POST /v1/secrets/*path`, `POST /v1/transit/keys/:name/encrypt`, `POST /v1/tokenization/keys/:name/tokenize` | `encrypt` |
| `GET /v1/secrets/*path`, `POST /v1/transit/keys/:name/decrypt`, `POST /v1/tokenization/detokenize` | `decrypt` |
| `POST /v1/transit/keys/:name/rotate`, `POST /v1/tokenization/keys/:name/rotate` | `rotate` |
Expand Down
4 changes: 2 additions & 2 deletions docs/concepts/api-fundamentals.md
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ First place to look:
- `GET /v1/transit/keys` requires `read`
- `POST /v1/transit/keys` requires `write`
- `POST /v1/transit/keys/:name/rotate` requires `rotate`
- `DELETE /v1/transit/keys/:id` requires `delete`
- `DELETE /v1/transit/keys/:name` requires `delete`
- `POST /v1/transit/keys/:name/encrypt` requires `encrypt`
- `POST /v1/transit/keys/:name/decrypt` requires `decrypt`
- `GET /v1/tokenization/keys` requires `read`
Expand Down Expand Up @@ -109,7 +109,7 @@ This section is the canonical capability-to-endpoint reference used by API docs
| `GET /v1/transit/keys` | `read` |
| `POST /v1/transit/keys` | `write` |
| `POST /v1/transit/keys/:name/rotate` | `rotate` |
| `DELETE /v1/transit/keys/:id` | `delete` |
| `DELETE /v1/transit/keys/:name` | `delete` |
| `POST /v1/transit/keys/:name/encrypt` | `encrypt` |
| `POST /v1/transit/keys/:name/decrypt` | `decrypt` |
| `GET /v1/tokenization/keys` | `read` |
Expand Down
48 changes: 20 additions & 28 deletions docs/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -520,31 +520,14 @@ paths:
$ref: "#/components/schemas/ErrorResponse"
"429":
$ref: "#/components/responses/TooManyRequests"
/v1/transit/keys/{name}/rotate:
post:
delete:
tags: [transit]
summary: Rotate transit key
summary: Delete transit key
security:
- bearerAuth: []
parameters:
- name: name
in: path
required: true
schema:
type: string
requestBody:
required: true
content:
application/json:
schema:
$ref: "#/components/schemas/TransitKeyRotateRequest"
responses:
"201":
description: New transit key version created
content:
application/json:
schema:
$ref: "#/components/schemas/TransitKeyResponse"
"204":
description: Deleted
"401":
$ref: "#/components/responses/Unauthorized"
"403":
Expand All @@ -559,22 +542,31 @@ paths:
$ref: "#/components/responses/ValidationError"
"429":
$ref: "#/components/responses/TooManyRequests"
/v1/transit/keys/{id}:
delete:
/v1/transit/keys/{name}/rotate:
post:
tags: [transit]
summary: Delete transit key
summary: Rotate transit key
security:
- bearerAuth: []
parameters:
- name: id
- name: name
in: path
required: true
schema:
type: string
format: uuid
requestBody:
required: true
content:
application/json:
schema:
$ref: "#/components/schemas/TransitKeyRotateRequest"
responses:
"204":
description: Deleted
"201":
description: New transit key version created
content:
application/json:
schema:
$ref: "#/components/schemas/TransitKeyResponse"
"401":
$ref: "#/components/responses/Unauthorized"
"403":
Expand Down
2 changes: 1 addition & 1 deletion internal/http/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ func (s *Server) registerTransitRoutes(
)

// Delete transit key
keys.DELETE("/:id",
keys.DELETE("/:name",
authHTTP.AuthorizationMiddleware(authDomain.DeleteCapability, auditLogUseCase, s.logger),
transitKeyHandler.DeleteHandler,
)
Expand Down
4 changes: 2 additions & 2 deletions internal/transit/domain/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ type TransitKeyRepository interface {
// Create stores a new transit key in the repository using transaction support from context.
Create(ctx context.Context, transitKey *TransitKey) error

// Delete soft deletes a transit key by marking it with DeletedAt timestamp.
Delete(ctx context.Context, transitKeyID uuid.UUID) error
// Delete soft deletes all versions of a transit key by name, marking them with DeletedAt timestamp.
Delete(ctx context.Context, name string) error

// GetByName retrieves the latest version of a transit key by name. Returns ErrTransitKeyNotFound if not found.
GetByName(ctx context.Context, name string) (*TransitKey, error)
Expand Down
21 changes: 11 additions & 10 deletions internal/transit/http/transit_key_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"strconv"

"github.com/gin-gonic/gin"
"github.com/google/uuid"

"github.com/allisson/secrets/internal/httputil"
"github.com/allisson/secrets/internal/transit/http/dto"
Expand Down Expand Up @@ -119,21 +118,23 @@ func (h *TransitKeyHandler) RotateHandler(c *gin.Context) {
c.JSON(http.StatusCreated, response)
}

// DeleteHandler soft deletes a transit key by ID.
// DELETE /v1/transit/keys/:id - Requires DeleteCapability on path /v1/transit/keys/:id.
// DeleteHandler soft deletes a transit key by name.
// DELETE /v1/transit/keys/:name - Requires DeleteCapability on path /v1/transit/keys/:name.
// Returns 204 No Content.
func (h *TransitKeyHandler) DeleteHandler(c *gin.Context) {
// Parse and validate UUID
transitKeyID, err := uuid.Parse(c.Param("id"))
if err != nil {
httputil.HandleBadRequestGin(c,
fmt.Errorf("invalid transit key ID format: must be a valid UUID"),
h.logger)
// Extract and validate name from URL parameter
name := c.Param("name")
if name == "" {
httputil.HandleBadRequestGin(
c,
fmt.Errorf("transit key name cannot be empty"),
h.logger,
)
return
}

// Call use case
if err := h.transitKeyUseCase.Delete(c.Request.Context(), transitKeyID); err != nil {
if err := h.transitKeyUseCase.Delete(c.Request.Context(), name); err != nil {
httputil.HandleErrorGin(c, err, h.logger)
return
}
Expand Down
34 changes: 9 additions & 25 deletions internal/transit/http/transit_key_handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -311,53 +311,37 @@ func TestTransitKeyHandler_RotateHandler(t *testing.T) {
}

func TestTransitKeyHandler_DeleteHandler(t *testing.T) {
t.Run("Success_ValidUUID", func(t *testing.T) {
t.Run("Success_ValidName", func(t *testing.T) {
handler, mockUseCase := setupTestTransitKeyHandler(t)

transitKeyID := uuid.Must(uuid.NewV7())
name := "test-key"

mockUseCase.EXPECT().
Delete(mock.Anything, transitKeyID).
Delete(mock.Anything, name).
Return(nil).
Once()

c, w := createTestContext(http.MethodDelete, fmt.Sprintf("/v1/transit/keys/%s", transitKeyID), nil)
c.Params = gin.Params{gin.Param{Key: "id", Value: transitKeyID.String()}}
c, w := createTestContext(http.MethodDelete, fmt.Sprintf("/v1/transit/keys/%s", name), nil)
c.Params = gin.Params{gin.Param{Key: "name", Value: name}}

handler.DeleteHandler(c)

assert.Equal(t, http.StatusNoContent, w.Code)
assert.Empty(t, w.Body.String())
})

t.Run("Error_InvalidUUID", func(t *testing.T) {
handler, _ := setupTestTransitKeyHandler(t)

c, w := createTestContext(http.MethodDelete, "/v1/transit/keys/invalid-uuid", nil)
c.Params = gin.Params{gin.Param{Key: "id", Value: "invalid-uuid"}}

handler.DeleteHandler(c)

assert.Equal(t, http.StatusBadRequest, w.Code)

var response map[string]interface{}
err := json.Unmarshal(w.Body.Bytes(), &response)
assert.NoError(t, err)
assert.Equal(t, "bad_request", response["error"])
})

t.Run("Error_TransitKeyNotFound", func(t *testing.T) {
handler, mockUseCase := setupTestTransitKeyHandler(t)

transitKeyID := uuid.Must(uuid.NewV7())
name := "nonexistent-key"

mockUseCase.EXPECT().
Delete(mock.Anything, transitKeyID).
Delete(mock.Anything, name).
Return(transitDomain.ErrTransitKeyNotFound).
Once()

c, w := createTestContext(http.MethodDelete, fmt.Sprintf("/v1/transit/keys/%s", transitKeyID), nil)
c.Params = gin.Params{gin.Param{Key: "id", Value: transitKeyID.String()}}
c, w := createTestContext(http.MethodDelete, fmt.Sprintf("/v1/transit/keys/%s", name), nil)
c.Params = gin.Params{gin.Param{Key: "name", Value: name}}

handler.DeleteHandler(c)

Expand Down
15 changes: 4 additions & 11 deletions internal/transit/repository/mysql/mysql_transit_key_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"errors"
"time"

"github.com/google/uuid"

cryptoDomain "github.com/allisson/secrets/internal/crypto/domain"
"github.com/allisson/secrets/internal/database"
apperrors "github.com/allisson/secrets/internal/errors"
Expand Down Expand Up @@ -52,18 +50,13 @@ func (m *MySQLTransitKeyRepository) Create(ctx context.Context, transitKey *tran
return nil
}

// Delete soft-deletes a transit key by setting its deleted_at timestamp.
func (m *MySQLTransitKeyRepository) Delete(ctx context.Context, transitKeyID uuid.UUID) error {
// Delete soft-deletes all versions of a transit key by name.
func (m *MySQLTransitKeyRepository) Delete(ctx context.Context, name string) error {
querier := database.GetTx(ctx, m.db)

query := `UPDATE transit_keys SET deleted_at = NOW() WHERE id = ?`

id, err := transitKeyID.MarshalBinary()
if err != nil {
return apperrors.Wrap(err, "failed to marshal transit key id")
}
query := `UPDATE transit_keys SET deleted_at = NOW() WHERE name = ? AND deleted_at IS NULL`

_, err = querier.ExecContext(ctx, query, id)
_, err := querier.ExecContext(ctx, query, name)
if err != nil {
return apperrors.Wrap(err, "failed to delete transit key")
}
Expand Down
Loading
Loading