diff --git a/Dockerfile b/Dockerfile index 4a66b30..7b1a1a7 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,8 @@ FROM gitgud.foo/thegrind/laravel-base:latest +COPY ./hook.sh /app/hook.sh # Get the app in there -COPY . /app \ No newline at end of file +COPY . /app + +ENV ENABLE_QUEUE_WORKER=true +ENV ENABLE_SCHEDULER=true \ No newline at end of file diff --git a/app/Http/Controllers/OIDCController.php b/app/Http/Controllers/OIDCController.php index c24b1e8..d1fd509 100644 --- a/app/Http/Controllers/OIDCController.php +++ b/app/Http/Controllers/OIDCController.php @@ -135,7 +135,7 @@ class OIDCController extends Controller $builder = $builder ->issuedBy(config('app.url')) ->permittedFor($client->client_id) - ->relatedTo((string) $user->id) + ->relatedTo((string) $user->uuid) ->issuedAt($issuedAt) ->expiresAt($issuedAt->modify('+5 minutes')) ->withClaim('email', $user->email); @@ -195,7 +195,7 @@ class OIDCController extends Controller return response()->json([ - 'sub' => (string) $user->id, + 'sub' => (string) $user->uuid, 'email' => $user->email, 'name' => $user->name, 'preferred_username' => $user->preferred_username, diff --git a/app/Models/User.php b/app/Models/User.php index a5d5b07..8921fae 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -14,6 +14,17 @@ class User extends Authenticatable /** @use HasFactory<\Database\Factories\UserFactory> */ use HasFactory, Notifiable; + protected static function boot() + { + parent::boot(); + + static::creating(function ($user) { + if (empty($user->uuid)) { + $user->uuid = Str::uuid(); + } + }); + } + /** * The attributes that are mass assignable. * @@ -26,7 +37,8 @@ class User extends Authenticatable 'avatar', 'preferred_username', 'is_admin', - 'auto_approve_apps' + 'auto_approve_apps', + 'uuid' ]; /** diff --git a/database/migrations/2025_08_02_231720_add_uuid_to_users_table.php b/database/migrations/2025_08_02_231720_add_uuid_to_users_table.php new file mode 100644 index 0000000..d25a480 --- /dev/null +++ b/database/migrations/2025_08_02_231720_add_uuid_to_users_table.php @@ -0,0 +1,43 @@ +uuid('uuid')->nullable()->after('id'); + }); + + // Generate UUIDs for existing users + DB::table('users')->whereNull('uuid')->get()->each(function ($user) { + DB::table('users') + ->where('id', $user->id) + ->update(['uuid' => (string) Str::uuid()]); + }); + + // Add unique index + Schema::table('users', function (Blueprint $table) { + $table->unique('uuid'); + }); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + Schema::table('users', function (Blueprint $table) { + $table->dropUnique(['uuid']); + $table->dropColumn('uuid'); + }); + } +}; diff --git a/hooks.sh b/hook.sh similarity index 100% rename from hooks.sh rename to hook.sh diff --git a/tests/Feature/OIDCControllerTest.php b/tests/Feature/OIDCControllerTest.php index d05ea66..697b1e8 100644 --- a/tests/Feature/OIDCControllerTest.php +++ b/tests/Feature/OIDCControllerTest.php @@ -263,7 +263,7 @@ describe('OIDC Token Endpoint', function () { // Verify ID token is valid JWT $parser = new Parser(new JoseEncoder()); $idToken = $parser->parse($data['id_token']); - expect($idToken->claims()->get('sub'))->toBe((string) $user->id); + expect($idToken->claims()->get('sub'))->toBe((string) $user->uuid); expect($idToken->claims()->get('email'))->toBe($user->email); }); @@ -424,7 +424,7 @@ describe('OIDC UserInfo Endpoint', function () { $response->assertStatus(200) ->assertJson([ - 'sub' => (string) $user->id, + 'sub' => (string) $user->uuid, 'email' => $user->email, 'name' => $user->name, 'preferred_username' => $user->preferred_username, diff --git a/tests/Feature/OIDCUuidSubClaimTest.php b/tests/Feature/OIDCUuidSubClaimTest.php new file mode 100644 index 0000000..f4a2bdb --- /dev/null +++ b/tests/Feature/OIDCUuidSubClaimTest.php @@ -0,0 +1,178 @@ +create(); + $app = Application::factory()->create(); + + // Store authorization code + $code = 'test-auth-code'; + Cache::put("auth_code:$code", [ + 'user_id' => $user->id, + 'client_id' => $app->id, + 'scope' => 'openid profile email', + ], now()->addMinutes(5)); + + $this->actingAs($user); + + $response = $this->post(route('auth.token'), [ + 'grant_type' => 'authorization_code', + 'code' => $code, + 'redirect_uri' => $app->redirect_uri, + 'client_id' => $app->client_id, + 'client_secret' => $app->client_secret, + ]); + + $response->assertStatus(200); + $data = $response->json(); + + // Parse the JWT ID token + $parser = new Parser(new JoseEncoder()); + $idToken = $parser->parse($data['id_token']); + + // Verify that the 'sub' claim contains the user's UUID, not the ID + expect($idToken->claims()->get('sub'))->toBe((string) $user->uuid); + expect($idToken->claims()->get('sub'))->not->toBe((string) $user->id); + expect(Str::isUuid($idToken->claims()->get('sub')))->toBeTrue(); +}); + +test('userinfo endpoint sub claim uses user UUID', function () { + $user = User::factory()->create([ + 'name' => 'Test User', + 'email' => 'test@example.com', + 'preferred_username' => 'testuser', + 'avatar' => 'avatar.jpg' + ]); + + $app = Application::factory()->create(); + + // Create a valid access token + $accessToken = 'valid-access-token'; + AuthenticationToken::create([ + 'user_id' => $user->id, + 'application_id' => $app->id, + 'token' => $accessToken, + 'issued_at' => now(), + 'expires_at' => now()->addHour(), + 'ip' => '127.0.0.1', + 'user_agent' => 'Test Agent' + ]); + + $response = $this->get(route('auth.userinfo'), [ + 'Authorization' => 'Bearer ' . $accessToken + ]); + + $response->assertStatus(200); + $data = $response->json(); + + // Verify that the 'sub' field contains the user's UUID, not the ID + expect($data['sub'])->toBe((string) $user->uuid); + expect($data['sub'])->not->toBe((string) $user->id); + expect(Str::isUuid($data['sub']))->toBeTrue(); + + // Verify other fields are still correct + expect($data['email'])->toBe($user->email); + expect($data['name'])->toBe($user->name); + expect($data['preferred_username'])->toBe($user->preferred_username); +}); + +test('JWT sub claim is consistent across multiple tokens for same user', function () { + $user = User::factory()->create(); + $app = Application::factory()->create(); + + // Generate first token + $code1 = 'test-auth-code-1'; + Cache::put("auth_code:$code1", [ + 'user_id' => $user->id, + 'client_id' => $app->id, + 'scope' => 'openid profile email', + ], now()->addMinutes(5)); + + $response1 = $this->post(route('auth.token'), [ + 'grant_type' => 'authorization_code', + 'code' => $code1, + 'redirect_uri' => $app->redirect_uri, + 'client_id' => $app->client_id, + 'client_secret' => $app->client_secret, + ]); + + // Generate second token + $code2 = 'test-auth-code-2'; + Cache::put("auth_code:$code2", [ + 'user_id' => $user->id, + 'client_id' => $app->id, + 'scope' => 'openid profile email', + ], now()->addMinutes(5)); + + $response2 = $this->post(route('auth.token'), [ + 'grant_type' => 'authorization_code', + 'code' => $code2, + 'redirect_uri' => $app->redirect_uri, + 'client_id' => $app->client_id, + 'client_secret' => $app->client_secret, + ]); + + $parser = new Parser(new JoseEncoder()); + $idToken1 = $parser->parse($response1->json()['id_token']); + $idToken2 = $parser->parse($response2->json()['id_token']); + + // Both tokens should have the same 'sub' claim (user's UUID) + expect($idToken1->claims()->get('sub'))->toBe($idToken2->claims()->get('sub')); + expect($idToken1->claims()->get('sub'))->toBe((string) $user->uuid); +}); + +test('different users have different UUID sub claims', function () { + $user1 = User::factory()->create(); + $user2 = User::factory()->create(); + $app = Application::factory()->create(); + + // Generate token for user 1 + $code1 = 'test-auth-code-1'; + Cache::put("auth_code:$code1", [ + 'user_id' => $user1->id, + 'client_id' => $app->id, + 'scope' => 'openid profile email', + ], now()->addMinutes(5)); + + $response1 = $this->post(route('auth.token'), [ + 'grant_type' => 'authorization_code', + 'code' => $code1, + 'redirect_uri' => $app->redirect_uri, + 'client_id' => $app->client_id, + 'client_secret' => $app->client_secret, + ]); + + // Generate token for user 2 + $code2 = 'test-auth-code-2'; + Cache::put("auth_code:$code2", [ + 'user_id' => $user2->id, + 'client_id' => $app->id, + 'scope' => 'openid profile email', + ], now()->addMinutes(5)); + + $response2 = $this->post(route('auth.token'), [ + 'grant_type' => 'authorization_code', + 'code' => $code2, + 'redirect_uri' => $app->redirect_uri, + 'client_id' => $app->client_id, + 'client_secret' => $app->client_secret, + ]); + + $parser = new Parser(new JoseEncoder()); + $idToken1 = $parser->parse($response1->json()['id_token']); + $idToken2 = $parser->parse($response2->json()['id_token']); + + // Each user should have different 'sub' claims + expect($idToken1->claims()->get('sub'))->not->toBe($idToken2->claims()->get('sub')); + expect($idToken1->claims()->get('sub'))->toBe((string) $user1->uuid); + expect($idToken2->claims()->get('sub'))->toBe((string) $user2->uuid); +}); \ No newline at end of file diff --git a/tests/Feature/UserUuidTest.php b/tests/Feature/UserUuidTest.php new file mode 100644 index 0000000..e5e4b87 --- /dev/null +++ b/tests/Feature/UserUuidTest.php @@ -0,0 +1,79 @@ +create(); + + expect($user->uuid)->not->toBeNull(); + expect(Str::isUuid((string) $user->uuid))->toBeTrue(); +}); + +test('user UUID is unique', function () { + $user1 = User::factory()->create(); + $user2 = User::factory()->create(); + + expect($user1->uuid)->not->toBe($user2->uuid); +}); + +test('user UUID cannot be duplicated', function () { + $user1 = User::factory()->create(); + + expect(function () use ($user1) { + User::create([ + 'name' => 'Test User', + 'email' => 'test2@example.com', + 'password' => bcrypt('password'), + 'uuid' => $user1->uuid + ]); + })->toThrow(\Illuminate\Database\QueryException::class); +}); + +test('user UUID can be manually set during creation', function () { + $customUuid = Str::uuid(); + + $user = User::create([ + 'name' => 'Test User', + 'email' => 'test@example.com', + 'password' => bcrypt('password'), + 'uuid' => $customUuid + ]); + + expect($user->uuid)->toBe($customUuid); +}); + +test('user UUID is not overwritten if already set', function () { + $customUuid = Str::uuid(); + + $user = new User([ + 'name' => 'Test User', + 'email' => 'test@example.com', + 'password' => bcrypt('password'), + 'uuid' => $customUuid + ]); + + $user->save(); + + expect($user->uuid)->toBe($customUuid); +}); + +test('existing users can have UUIDs added via migration', function () { + // This test verifies that the migration properly adds UUIDs to existing users + // We can't easily test the migration directly, but we can test that users + // created without UUIDs in the factory would get them via the boot method + + $user = new User([ + 'name' => 'Test User', + 'email' => 'test@example.com', + 'password' => bcrypt('password'), + ]); + + // Don't set UUID, let the boot method handle it + $user->save(); + + expect($user->uuid)->not->toBeNull(); + expect(Str::isUuid((string) $user->uuid))->toBeTrue(); +}); \ No newline at end of file