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
28 changes: 21 additions & 7 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- **Secret Sharing & Access Control (Phase 3)** (#182)
- **Secret CRUD API**: Full REST API for password manager functionality
- Create secrets with encrypted title, username, password, URL, notes (POST `/v1/secrets`)
- List user's secrets with pagination (GET `/v1/secrets`)
- View secret details (GET `/v1/secrets/{secret}`)
- Update secrets with automatic version incrementing (PATCH `/v1/secrets/{secret}`)
(#187)
- Create secrets with encrypted title, username, password, URL, notes
(POST `/v1/secrets`)
- List user's secrets with filter parameter: `all` (default), `owned`, `shared`
(via SecretShare) (GET `/v1/secrets?filter={type}`)
- Filter validation via `IndexSecretRequest` (rejects invalid filter values)
- Query optimization: Role IDs cached to avoid N+1 queries
- DRY implementation: Shared filter logic extracted to `Secret::scopeSharedWith()`
- Empty role array optimization: Skips `orWhereIn` when user has no roles
- View secret details with owner or share-based access (GET
`/v1/secrets/{secret}`)
- Update secrets with automatic version incrementing (PATCH
`/v1/secrets/{secret}`)
- Soft delete secrets (DELETE `/v1/secrets/{secret}`)
- Owner-based authorization via `SecretPolicy`
- Validation via `StoreSecretRequest` and `UpdateSecretRequest`
- 17 comprehensive Controller tests
- Authorization via `SecretPolicy` with 9 methods: viewAny, view, create,
update, delete, restore, forceDelete, share, viewShares
- Permission hierarchy: admin > write > read (via
`Secret::userHasPermission()`)
- Share-based access respects expiration dates and permission levels
- Validation via `StoreSecretRequest`, `UpdateSecretRequest`, `IndexSecretRequest`
- 22 comprehensive Controller tests covering CRUD + share-based access
scenarios
- **Secret Sharing API**: Grant/revoke access to secrets
- Grant read/write/admin access to users OR roles (POST `/v1/secrets/{secret}/shares`)
- List all shares for a secret (GET `/v1/secrets/{secret}/shares`)
Expand Down
21 changes: 18 additions & 3 deletions app/Http/Controllers/Api/V1/SecretController.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
namespace App\Http\Controllers\Api\V1;

use App\Http\Controllers\Controller;
use App\Http\Requests\IndexSecretRequest;
use App\Http\Requests\StoreSecretRequest;
use App\Http\Requests\UpdateSecretRequest;
use App\Models\Secret;
Expand Down Expand Up @@ -103,15 +104,29 @@ private function assignFields(Secret $secret, Request $request): bool
/**
* Display a listing of secrets accessible to the authenticated user.
*/
public function index(Request $request): JsonResponse
public function index(IndexSecretRequest $request): JsonResponse
{
$this->authorize('viewAny', Secret::class);

/** @var \App\Models\User $user */
$user = $request->user();

// Query secrets owned by user
$query = Secret::where('owner_id', $user->id);
// Cache role IDs to avoid N+1 queries
/** @var array<int> $roleIds */
$roleIds = $user->roles->pluck('id')->toArray();

// Build query based on filter parameter
$filter = $request->validated('filter', 'all');

// Use match expression with explicit return value capture
$query = match ($filter) {
'owned' => Secret::query()->where('owner_id', $user->id),
'shared' => Secret::query()->sharedWith($user, $roleIds),
default => Secret::query()->where(function ($q) use ($user, $roleIds) {
$q->where('owner_id', $user->id)
->orWhere(fn ($subQuery) => $subQuery->sharedWith($user, $roleIds));
}),
};

// Pagination
/** @var int $perPageInput */
Expand Down
53 changes: 53 additions & 0 deletions app/Http/Requests/IndexSecretRequest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

/*
* SPDX-FileCopyrightText: 2025 SecPal Contributors
*
* SPDX-License-Identifier: AGPL-3.0-or-later
*/

declare(strict_types=1);

namespace App\Http\Requests;

use Illuminate\Foundation\Http\FormRequest;
use Illuminate\Validation\Rule;

class IndexSecretRequest extends FormRequest
{
/**
* Determine if the user is authorized to make this request.
*/
public function authorize(): bool
{
return true;
}

/**
* Get the validation rules that apply to the request.
*
* @return array<string, \Illuminate\Contracts\Validation\ValidationRule|array<mixed>|string>
*/
public function rules(): array
{
return [
'filter' => [
'sometimes',
'string',
Rule::in(['all', 'owned', 'shared']),
],
];
}

/**
* Get custom messages for validator errors.
*
* @return array<string, string>
*/
public function messages(): array
{
return [
'filter.in' => 'The filter must be one of: all, owned, shared.',
];
}
}
28 changes: 28 additions & 0 deletions app/Models/Secret.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
* @method static \Illuminate\Database\Eloquent\Builder|Secret owned(string $userId)
* @method static \Illuminate\Database\Eloquent\Builder|Secret active()
* @method static \Illuminate\Database\Eloquent\Builder|Secret expired()
* @method static \Illuminate\Database\Eloquent\Builder|Secret sharedWith(User $user, array<int> $roleIds)
*/
class Secret extends Model
{
Expand Down Expand Up @@ -369,4 +370,31 @@ public function userHasPermission(User $user, string $permission): bool
default => false,
};
}

/**
* Scope query to secrets shared with the given user.
*
* Filters secrets that have active shares (not expired) where the user
* or any of the user's roles have access.
*
* @param \Illuminate\Database\Eloquent\Builder<Secret> $query
* @param array<int> $roleIds
* @return \Illuminate\Database\Eloquent\Builder<Secret>
*/
public function scopeSharedWith(\Illuminate\Database\Eloquent\Builder $query, User $user, array $roleIds): \Illuminate\Database\Eloquent\Builder
{
return $query->whereHas('shares', function ($q) use ($user, $roleIds) {
$q->where(function ($shareQuery) use ($user, $roleIds) {
$shareQuery->where('user_id', $user->id);
// Only add role check if user has roles (avoids empty array query)
if (! empty($roleIds)) {
$shareQuery->orWhereIn('role_id', $roleIds);
}
})
->where(function ($expiryQuery) {
$expiryQuery->whereNull('expires_at')
->orWhere('expires_at', '>', now());
});
});
}
}
20 changes: 20 additions & 0 deletions app/Policies/SecretPolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,24 @@ public function forceDelete(User $user, Secret $secret): bool
{
return $secret->owner_id === $user->id;
}

/**
* Determine whether the user can share the secret with others.
*
* Only the owner or users with admin permission can grant shares.
*/
public function share(User $user, Secret $secret): bool
{
return $secret->userHasPermission($user, 'admin');
}

/**
* Determine whether the user can view the shares of the secret.
*
* Only the owner or users with admin permission can see who has access.
*/
public function viewShares(User $user, Secret $secret): bool
{
return $secret->userHasPermission($user, 'admin');
}
}
111 changes: 111 additions & 0 deletions tests/Feature/Controllers/Api/V1/SecretControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -560,3 +560,114 @@
$response->assertForbidden();
});
});

describe('SecretController - Filter Parameter', function () {
test('filter=owned returns only owned secrets', function () {
// Arrange: Create owned secret
$ownSecret = createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $this->user->id,
'title_plain' => 'My Secret',
]);

// Create secret shared with user
$owner = User::factory()->create();
$sharedSecret = createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $owner->id,
'title_plain' => 'Shared Secret',
]);
\App\Models\SecretShare::create([
'secret_id' => $sharedSecret->id,
'user_id' => $this->user->id,
'permission' => 'read',
'granted_by' => $owner->id,
'granted_at' => now(),
]);

// Act
$response = getJson('/v1/secrets?filter=owned');

// Assert
$response->assertOk()
->assertJsonCount(1, 'data')
->assertJsonPath('data.0.title', 'My Secret');
});

test('filter=shared returns only shared secrets', function () {
// Arrange: Create owned secret
createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $this->user->id,
'title_plain' => 'My Secret',
]);

// Create secret shared with user
$owner = User::factory()->create();
$sharedSecret = createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $owner->id,
'title_plain' => 'Shared Secret',
]);
\App\Models\SecretShare::create([
'secret_id' => $sharedSecret->id,
'user_id' => $this->user->id,
'permission' => 'read',
'granted_by' => $owner->id,
'granted_at' => now(),
]);

// Act
$response = getJson('/v1/secrets?filter=shared');

// Assert
$response->assertOk()
->assertJsonCount(1, 'data')
->assertJsonPath('data.0.title', 'Shared Secret');
});

test('filter=all returns both owned and shared secrets', function () {
// Arrange: Create owned secret
createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $this->user->id,
'title_plain' => 'My Secret',
]);

// Create secret shared with user
$owner = User::factory()->create();
$sharedSecret = createTestSecret([
'tenant_id' => $this->tenant->id,
'owner_id' => $owner->id,
'title_plain' => 'Shared Secret',
]);
\App\Models\SecretShare::create([
'secret_id' => $sharedSecret->id,
'user_id' => $this->user->id,
'permission' => 'read',
'granted_by' => $owner->id,
'granted_at' => now(),
]);

// Act
$response = getJson('/v1/secrets?filter=all');

// Assert
$response->assertOk()
->assertJsonCount(2, 'data');
});

test('filter parameter rejects invalid values', function () {
// Act
$response = getJson('/v1/secrets?filter=invalid');

// Assert
$response->assertStatus(422)
->assertJsonValidationErrors(['filter'])
->assertJsonFragment([
'filter' => [
'The filter must be one of: all, owned, shared.',
],
]);
});
});
Loading