Skip to content
Open
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
8 changes: 6 additions & 2 deletions app/nexus/internal/route/acl/policy/delete.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,13 @@ func RouteDeletePolicy(
return err
}

policyID := request.ID
// TODO: Issue #250 - SDK uses 'id' field, but we're using name as primary key.
// This is a temporary workaround. The SDK's PolicyDeleteRequest.ID field should
// be renamed to 'name' in a future SDK update.
// See: https://github.com/spiffe/spike/issues/250
policyName := request.ID

deleteErr := state.DeletePolicy(policyID)
deleteErr := state.DeletePolicy(policyName)
if deleteErr != nil {
return net.RespondWithHTTPError(deleteErr, w, reqres.PolicyDeleteResponse{})
}
Expand Down
21 changes: 12 additions & 9 deletions app/nexus/internal/route/acl/policy/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,20 @@ import (
state "github.com/spiffe/spike/app/nexus/internal/state/base"
)

// RouteGetPolicy handles HTTP requests to retrieve a specific policy by its ID.
// It processes the request body to fetch detailed information about a single
// policy.
// RouteGetPolicy handles HTTP requests to retrieve a specific policy by its
// name. It processes the request body to fetch detailed information about a
// single policy.
//
// The function expects a JSON request body containing:
// - ID: unique identifier of the policy to retrieve
// - Name: name of the policy to retrieve
//
// On success, it returns the complete policy object. If the policy is not
// found, it returns a "not found" error. For other errors, it returns an
// internal server error.
//
// Parameters:
// - w: HTTP response writer for sending the response
// - r: HTTP request containing the policy ID to retrieve
// - r: HTTP request containing the policy name to retrieve
// - audit: Audit entry for logging the policy read action
//
// Returns:
Expand All @@ -38,14 +38,13 @@ import (
// Example request body:
//
// {
// "id": "policy-123"
// "name": "example-policy"
// }
//
// Example success response:
//
// {
// "policy": {
// "id": "policy-123",
// "name": "example-policy",
// "spiffe_id_pattern": "^spiffe://example\.org/.*/service",
// "path_pattern": "^api/",
Expand Down Expand Up @@ -93,9 +92,13 @@ func RouteGetPolicy(
return err
}

policyID := request.ID
// TODO: Issue #250 - SDK uses 'id' field, but we're using name as primary key.
// This is a temporary workaround. The SDK's PolicyReadRequest.ID field should
// be renamed to 'name' in a future SDK update.
// See: https://github.com/spiffe/spike/issues/250
policyName := request.ID

policy, policyErr := state.GetPolicy(policyID)
policy, policyErr := state.GetPolicy(policyName)
if policyErr != nil {
return net.RespondWithHTTPError(policyErr, w, reqres.PolicyReadResponse{})
}
Expand Down
36 changes: 18 additions & 18 deletions app/nexus/internal/state/backend/memory/memory.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ type Store struct {
secretStore *kv.KV // In-memory key-value store for secrets
secretMu sync.RWMutex // Mutex protecting secret operations

policies map[string]*data.Policy // In-memory map of policies by ID
policies map[string]*data.Policy // In-memory map of policies by name
policyMu sync.RWMutex // Mutex protecting policy operations

cipher cipher.AEAD // Encryption cipher (for interface compatibility)
Expand Down Expand Up @@ -189,8 +189,8 @@ func (s *Store) LoadAllSecrets(_ context.Context) (

// StorePolicy stores a policy in the store.
//
// This method is thread-safe and validates that the policy has a non-empty ID
// before storing. If a policy with the same ID already exists, it is
// This method is thread-safe and validates that the policy has a non-empty
// name before storing. If a policy with the same name already exists, it is
// replaced.
//
// Parameters:
Expand All @@ -200,43 +200,43 @@ func (s *Store) LoadAllSecrets(_ context.Context) (
//
// Returns:
// - *sdkErrors.SDKError: nil on success, or sdkErrors.ErrEntityInvalid if
// the policy ID is empty
// the policy name is empty
func (s *Store) StorePolicy(
_ context.Context, policy data.Policy,
) *sdkErrors.SDKError {
s.policyMu.Lock()
defer s.policyMu.Unlock()

if policy.ID == "" {
if policy.Name == "" {
failErr := *sdkErrors.ErrEntityInvalid.Clone()
failErr.Msg = "policy ID cannot be empty"
failErr.Msg = "policy name cannot be empty"
return &failErr
}

s.policies[policy.ID] = &policy
s.policies[policy.Name] = &policy
return nil
}

// LoadPolicy retrieves a policy from the store by its ID.
// LoadPolicy retrieves a policy from the store by its name.
//
// This method is thread-safe and returns the policy if it exists.
//
// Parameters:
// - context.Context: Context for cancellation (ignored in this
// implementation)
// - id: The unique identifier of the policy to retrieve
// - name: The name of the policy to retrieve
//
// Returns:
// - *data.Policy: The policy if found, nil otherwise
// - *sdkErrors.SDKError: nil on success, or sdkErrors.ErrEntityNotFound if
// the policy does not exist
func (s *Store) LoadPolicy(
_ context.Context, id string,
_ context.Context, name string,
) (*data.Policy, *sdkErrors.SDKError) {
s.policyMu.RLock()
defer s.policyMu.RUnlock()

policy, exists := s.policies[id]
policy, exists := s.policies[name]
if !exists {
return nil, sdkErrors.ErrEntityNotFound
}
Expand All @@ -254,7 +254,7 @@ func (s *Store) LoadPolicy(
// implementation)
//
// Returns:
// - map[string]*data.Policy: A map of policy IDs to policies
// - map[string]*data.Policy: A map of policy names to policies
// - *sdkErrors.SDKError: Always returns nil for in-memory storage
func (s *Store) LoadAllPolicies(
_ context.Context,
Expand All @@ -264,30 +264,30 @@ func (s *Store) LoadAllPolicies(

// Create a copy to avoid race conditions
result := make(map[string]*data.Policy, len(s.policies))
for id, policy := range s.policies {
result[id] = policy
for name, policy := range s.policies {
result[name] = policy
}

return result, nil
}

// DeletePolicy removes a policy from the store by its ID.
// DeletePolicy removes a policy from the store by its name.
//
// This method is thread-safe and removes the policy if it exists. If the
// policy does not exist, this is a no-op (no error is returned).
//
// Parameters:
// - context.Context: Context for cancellation (ignored in this
// implementation)
// - id: The unique identifier of the policy to delete
// - name: The name of the policy to delete
//
// Returns:
// - *sdkErrors.SDKError: Always returns nil for in-memory storage
func (s *Store) DeletePolicy(_ context.Context, id string) *sdkErrors.SDKError {
func (s *Store) DeletePolicy(_ context.Context, name string) *sdkErrors.SDKError {
s.policyMu.Lock()
defer s.policyMu.Unlock()

delete(s.policies, id)
delete(s.policies, name)
return nil
}

Expand Down
53 changes: 19 additions & 34 deletions app/nexus/internal/state/backend/memory/memory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,8 +261,7 @@ func TestInMemoryStore_StoreAndLoadPolicy(t *testing.T) {

// Create a test policy
policy := data.Policy{
ID: "test-policy-1",
Name: "Test Policy",
Name: "test-policy-1",
SPIFFEIDPattern: "^spiffe://example\\.org/app/.*$",
PathPattern: "^app/secrets/.*$",
Permissions: []data.PolicyPermission{data.PermissionRead, data.PermissionWrite},
Expand All @@ -275,7 +274,7 @@ func TestInMemoryStore_StoreAndLoadPolicy(t *testing.T) {
}

// Load the policy
loadedPolicy, loadErr := store.LoadPolicy(ctx, policy.ID)
loadedPolicy, loadErr := store.LoadPolicy(ctx, policy.Name)
if loadErr != nil {
t.Errorf("LoadPolicy failed: %v", loadErr)
}
Expand All @@ -286,10 +285,6 @@ func TestInMemoryStore_StoreAndLoadPolicy(t *testing.T) {
}

// Verify the loaded policy matches the stored one
if loadedPolicy.ID != policy.ID {
t.Errorf("Expected ID '%s', got '%s'", policy.ID, loadedPolicy.ID)
}

if loadedPolicy.Name != policy.Name {
t.Errorf("Expected Name '%s', got '%s'", policy.Name, loadedPolicy.Name)
}
Expand All @@ -310,15 +305,14 @@ func TestInMemoryStore_StoreAndLoadPolicy(t *testing.T) {
}
}

func TestInMemoryStore_StorePolicyEmptyID(t *testing.T) {
func TestInMemoryStore_StorePolicyEmptyName(t *testing.T) {
testCipher := createTestCipher(t)
store := NewInMemoryStore(testCipher, 10)
ctx := context.Background()

// Create a policy with empty ID
// Create a policy with empty Name
policy := data.Policy{
ID: "", // Empty ID
Name: "Test Policy",
Name: "", // Empty Name
SPIFFEIDPattern: "^spiffe://example\\.org/app/.*$",
PathPattern: "^app/secrets/.*$",
Permissions: []data.PolicyPermission{data.PermissionRead},
Expand All @@ -327,7 +321,7 @@ func TestInMemoryStore_StorePolicyEmptyID(t *testing.T) {
// Store the policy - should fail with ErrEntityInvalid
storeErr := store.StorePolicy(ctx, policy)
if storeErr == nil {
t.Error("Expected error when storing policy with empty ID")
t.Error("Expected error when storing policy with empty name")
}

if storeErr != nil && !storeErr.Is(sdkErrors.ErrEntityInvalid) {
Expand Down Expand Up @@ -365,22 +359,19 @@ func TestInMemoryStore_LoadAllPolicies(t *testing.T) {
// Store multiple policies
policies := map[string]data.Policy{
"policy-1": {
ID: "policy-1",
Name: "Read Policy",
Name: "policy-1",
SPIFFEIDPattern: "^spiffe://example\\.org/reader/.*$",
PathPattern: "^read/.*$",
Permissions: []data.PolicyPermission{data.PermissionRead},
},
"policy-2": {
ID: "policy-2",
Name: "Write Policy",
Name: "policy-2",
SPIFFEIDPattern: "^spiffe://example\\.org/writer/.*$",
PathPattern: "^write/.*$",
Permissions: []data.PolicyPermission{data.PermissionWrite},
},
"policy-3": {
ID: "policy-3",
Name: "Admin Policy",
Name: "policy-3",
SPIFFEIDPattern: "^spiffe://example\\.org/admin/.*$",
PathPattern: "^secrets/.*$",
Permissions: []data.PolicyPermission{data.PermissionRead,
Expand All @@ -392,7 +383,7 @@ func TestInMemoryStore_LoadAllPolicies(t *testing.T) {
for _, policy := range policies {
storeErr := store.StorePolicy(ctx, policy)
if storeErr != nil {
t.Errorf("Failed to store policy %s: %v", policy.ID, storeErr)
t.Errorf("Failed to store policy %s: %v", policy.Name, storeErr)
}
}

Expand All @@ -407,26 +398,21 @@ func TestInMemoryStore_LoadAllPolicies(t *testing.T) {
}

// Verify each policy was loaded correctly
for id, expectedPolicy := range policies {
loadedPolicy, exists := allPolicies[id]
for name, expectedPolicy := range policies {
loadedPolicy, exists := allPolicies[name]
if !exists {
t.Errorf("Expected policy with ID %s to exist", id)
t.Errorf("Expected policy with name %s to exist", name)
continue
}

if loadedPolicy.ID != expectedPolicy.ID {
t.Errorf("ID mismatch for %s: expected %s, got %s",
id, expectedPolicy.ID, loadedPolicy.ID)
}

if loadedPolicy.Name != expectedPolicy.Name {
t.Errorf("Name mismatch for %s: expected %s, got %s",
id, expectedPolicy.Name, loadedPolicy.Name)
name, expectedPolicy.Name, loadedPolicy.Name)
}

if !reflect.DeepEqual(loadedPolicy.Permissions, expectedPolicy.Permissions) {
t.Errorf("Permissions mismatch for %s: expected %v, got %v",
id, expectedPolicy.Permissions, loadedPolicy.Permissions)
name, expectedPolicy.Permissions, loadedPolicy.Permissions)
}
}
}
Expand Down Expand Up @@ -454,8 +440,7 @@ func TestInMemoryStore_DeletePolicy(t *testing.T) {

// Create and store test policy
policy := data.Policy{
ID: "deletable-policy",
Name: "Deletable Policy",
Name: "deletable-policy",
SPIFFEIDPattern: "^spiffe://example\\.org/temp/.*$",
PathPattern: "^secrets/temp/.*$",
Permissions: []data.PolicyPermission{data.PermissionRead},
Expand All @@ -467,19 +452,19 @@ func TestInMemoryStore_DeletePolicy(t *testing.T) {
}

// Verify policy exists
loadedPolicy, loadErr := store.LoadPolicy(ctx, policy.ID)
loadedPolicy, loadErr := store.LoadPolicy(ctx, policy.Name)
if loadErr != nil || loadedPolicy == nil {
t.Fatal("Policy should exist before deletion")
}

// Delete the policy
deleteErr := store.DeletePolicy(ctx, policy.ID)
deleteErr := store.DeletePolicy(ctx, policy.Name)
if deleteErr != nil {
t.Errorf("DeletePolicy failed: %v", deleteErr)
}

// Verify policy no longer exists (LoadPolicy returns ErrEntityNotFound)
deletedPolicy, loadAfterDeleteErr := store.LoadPolicy(ctx, policy.ID)
deletedPolicy, loadAfterDeleteErr := store.LoadPolicy(ctx, policy.Name)
if loadAfterDeleteErr == nil || !loadAfterDeleteErr.Is(sdkErrors.ErrEntityNotFound) {
t.Errorf("Expected ErrEntityNotFound after deletion, got: %v", loadAfterDeleteErr)
}
Expand Down
Loading