fix(api): align team-scoped responses

This commit is contained in:
Andras Bacsai
2026-08-20 12:40:13 +02:00
parent 13a577a731
commit c9b857884b
6 changed files with 106 additions and 17 deletions
@@ -15,9 +15,9 @@ use OpenApi\Attributes as OA;
class GithubController extends Controller
{
private function removeSensitiveData($githubApp)
private function removeSensitiveData(GithubApp $githubApp, int $teamId)
{
if (request()->attributes->get('can_read_sensitive', false) === true) {
if (request()->attributes->get('can_read_sensitive', false) === true && $githubApp->team_id === $teamId) {
$githubApp->makeVisible([
'client_secret',
'webhook_secret',
@@ -97,8 +97,8 @@ class GithubController extends Controller
->orWhere('is_system_wide', true);
})->get();
$githubApps = $githubApps->map(function ($app) {
return $this->removeSensitiveData($app);
$githubApps = $githubApps->map(function ($app) use ($teamId) {
return $this->removeSensitiveData($app, $teamId);
});
return response()->json($githubApps);
@@ -13,9 +13,9 @@ use OpenApi\Attributes as OA;
class GitlabController extends Controller
{
private function removeSensitiveData(GitlabApp $gitlabApp)
private function removeSensitiveData(GitlabApp $gitlabApp, int $teamId)
{
if (request()->attributes->get('can_read_sensitive', false) === true) {
if (request()->attributes->get('can_read_sensitive', false) === true && $gitlabApp->team_id === $teamId) {
$gitlabApp->makeVisible([
'client_secret',
'webhook_token',
@@ -108,8 +108,8 @@ class GitlabController extends Controller
->orWhere('is_system_wide', true);
})->get();
$gitlabApps = $gitlabApps->map(function ($app) {
return $this->removeSensitiveData($app);
$gitlabApps = $gitlabApps->map(function ($app) use ($teamId) {
return $this->removeSensitiveData($app, $teamId);
});
return response()->json($gitlabApps);
@@ -280,7 +280,7 @@ class GitlabController extends Controller
'gitlab_app_name' => $gitlabApp->name,
]);
return response()->json($this->removeSensitiveData($gitlabApp->fresh()), 201);
return response()->json($this->removeSensitiveData($gitlabApp->fresh(), $teamId), 201);
} catch (\Throwable $e) {
return handleError($e);
}
@@ -441,7 +441,7 @@ class GitlabController extends Controller
return response()->json([
'message' => 'GitLab app updated successfully',
'data' => $this->removeSensitiveData($gitlabApp->fresh()),
'data' => $this->removeSensitiveData($gitlabApp->fresh(), $teamId),
]);
} catch (ModelNotFoundException $e) {
return response()->json([
+9 -7
View File
@@ -56,7 +56,7 @@ class TeamController extends Controller
if (is_null($teamId)) {
return invalidTokenResponse();
}
$teams = auth()->user()->teams->sortBy('id');
$teams = auth()->user()->teams->where('id', $teamId)->values();
$teams = $teams->map(function ($team) {
return $this->removeSensitiveData($team);
});
@@ -100,13 +100,14 @@ class TeamController extends Controller
)]
public function team_by_id(Request $request)
{
$id = $request->id;
$teamId = getTeamIdFromToken();
if (is_null($teamId)) {
return invalidTokenResponse();
}
$teams = auth()->user()->teams;
$team = $teams->where('id', $id)->first();
if ((int) $request->id !== (int) $teamId) {
return response()->json(['message' => 'Team not found.'], 404);
}
$team = auth()->user()->teams->where('id', $teamId)->first();
if (is_null($team)) {
return response()->json(['message' => 'Team not found.'], 404);
}
@@ -159,13 +160,14 @@ class TeamController extends Controller
)]
public function members_by_id(Request $request)
{
$id = $request->id;
$teamId = getTeamIdFromToken();
if (is_null($teamId)) {
return invalidTokenResponse();
}
$teams = auth()->user()->teams;
$team = $teams->where('id', $id)->first();
if ((int) $request->id !== (int) $teamId) {
return response()->json(['message' => 'Team not found.'], 404);
}
$team = auth()->user()->teams->where('id', $teamId)->first();
if (is_null($team)) {
return response()->json(['message' => 'Team not found.'], 404);
}
@@ -197,6 +197,39 @@ describe('GET /api/v1/github-apps', function () {
]);
});
test('does not return system-wide github app secrets owned by another team', function () {
$otherTeam = Team::factory()->create();
$otherPrivateKey = PrivateKey::create([
'name' => 'System Key',
'private_key' => validGithubAppsApiPrivateKey(),
'team_id' => $otherTeam->id,
]);
GithubApp::create([
'name' => 'Foreign System GitHub App',
'api_url' => 'https://api.github.com',
'html_url' => 'https://github.com',
'app_id' => 11111,
'installation_id' => 22222,
'client_id' => 'system-client-id',
'client_secret' => 'foreign-client-secret',
'webhook_secret' => 'foreign-webhook-secret',
'private_key_id' => $otherPrivateKey->id,
'team_id' => $otherTeam->id,
'is_system_wide' => true,
]);
$sensitiveToken = createGithubAppsApiToken($this, ['read', 'read:sensitive']);
$response = $this->withToken($sensitiveToken)
->getJson('/api/v1/github-apps')
->assertSuccessful()
->assertJsonFragment(['name' => 'Foreign System GitHub App']);
expect($response->json('0'))
->not->toHaveKey('client_secret')
->not->toHaveKey('webhook_secret');
});
test('does not return other teams github apps', function () {
// Create a GitHub app for this team
GithubApp::create([
+30
View File
@@ -64,6 +64,36 @@ describe('GET /api/v1/gitlab-apps', function () {
expect($response->json('0'))->not->toHaveKey('client_secret')
->and($response->json('0'))->not->toHaveKey('webhook_token');
});
test('does not return system-wide gitlab app secrets owned by another team', function () {
$otherTeam = Team::factory()->create();
GitlabApp::create([
'name' => 'Foreign System GitLab',
'api_url' => 'https://gitlab.com/api/v4',
'html_url' => 'https://gitlab.com',
'client_id' => 'foreign-client-id',
'client_secret' => 'foreign-client-secret',
'webhook_token' => 'foreign-webhook-token',
'access_token' => 'foreign-access-token',
'refresh_token' => 'foreign-refresh-token',
'team_id' => $otherTeam->id,
'is_system_wide' => true,
]);
session(['currentTeam' => $this->team]);
$sensitiveToken = $this->user->createToken('sensitive-token', ['read', 'read:sensitive'])->plainTextToken;
$response = $this->withToken($sensitiveToken)
->getJson('/api/v1/gitlab-apps')
->assertSuccessful()
->assertJsonFragment(['name' => 'Foreign System GitLab']);
expect($response->json('0'))
->not->toHaveKey('client_secret')
->not->toHaveKey('webhook_token')
->not->toHaveKey('access_token')
->not->toHaveKey('refresh_token');
});
});
describe('POST /api/v1/gitlab-apps', function () {
@@ -8,6 +8,9 @@ use Illuminate\Foundation\Testing\RefreshDatabase;
uses(RefreshDatabase::class);
beforeEach(function () {
config()->set('app.maintenance.driver', 'file');
config()->set('cache.default', 'array');
InstanceSettings::unguarded(fn () => InstanceSettings::updateOrCreate(['id' => 0], ['is_api_enabled' => true]));
$this->team = Team::factory()->create(['name' => 'Token Team']);
@@ -27,6 +30,27 @@ function teamTokenApiHeaders(string $bearerToken): array
}
describe('token team endpoints', function () {
test('legacy team endpoints are restricted to the token team', function () {
$otherTeam = Team::factory()->create(['name' => 'Other Team']);
$otherMember = User::factory()->create();
$otherTeam->members()->attach($this->user->id, ['role' => 'owner']);
$otherTeam->members()->attach($otherMember->id, ['role' => 'member']);
$this->withHeaders(teamTokenApiHeaders($this->bearerToken))
->getJson('/api/v1/teams')
->assertOk()
->assertJsonCount(1)
->assertJsonPath('0.id', $this->team->id);
$this->withHeaders(teamTokenApiHeaders($this->bearerToken))
->getJson("/api/v1/teams/{$otherTeam->id}")
->assertNotFound();
$this->withHeaders(teamTokenApiHeaders($this->bearerToken))
->getJson("/api/v1/teams/{$otherTeam->id}/members")
->assertNotFound();
});
test('GET /team returns the token team', function () {
$this->withHeaders(teamTokenApiHeaders($this->bearerToken))
->getJson('/api/v1/team')