From 6bedec03443d96c082e7f8f8ebac5442815d0f8b Mon Sep 17 00:00:00 2001 From: romanetar Date: Thu, 25 Jun 2026 17:11:32 +0200 Subject: [PATCH 1/2] fix(speakers): honour payload bio on createMySpeaker instead of always overriding with member bio Signed-off-by: romanetar --- .../OAuth2SummitSpeakersApiController.php | 17 ++- tests/oauth2/OAuth2SummitSpeakersApiTest.php | 138 ++++++++++++++++++ 2 files changed, 147 insertions(+), 8 deletions(-) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php index 1ef9c6e4f..7c25bb340 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php @@ -1353,17 +1353,18 @@ public function createMySpeaker() 'notes' ]; - // set data from current member ... - $aux_payload = [ - 'member_id' => $current_member->getId(), + // Member fields are used as defaults only — user-submitted values take precedence. + // member_id is always forced from the authenticated user for security. + $member_defaults = [ 'first_name' => $current_member->getFirstName(), - 'last_name' => $current_member->getLastName(), - 'bio' => $current_member->getBio(), - 'twitter' => $current_member->getTwitterHandle(), - 'irc' => $current_member->getIrcHandle(), + 'last_name' => $current_member->getLastName(), + 'bio' => $current_member->getBio(), + 'twitter' => $current_member->getTwitterHandle(), + 'irc' => $current_member->getIrcHandle(), ]; - $payload = array_merge($payload, $aux_payload); + $payload = array_merge($member_defaults, $payload); + $payload['member_id'] = $current_member->getId(); $speaker = $this->service->addSpeaker(HTMLCleaner::cleanData($payload, $fields), $current_member); diff --git a/tests/oauth2/OAuth2SummitSpeakersApiTest.php b/tests/oauth2/OAuth2SummitSpeakersApiTest.php index 689373bf6..d498297f0 100644 --- a/tests/oauth2/OAuth2SummitSpeakersApiTest.php +++ b/tests/oauth2/OAuth2SummitSpeakersApiTest.php @@ -21,6 +21,9 @@ use models\summit\PresentationSpeaker; use utils\FilterParser; use models\summit\SpeakersSummitRegistrationPromoCode; +use models\main\Member; +use LaravelDoctrine\ORM\Facades\Registry; +use models\utils\SilverstripeBaseModel; final class OAuth2SummitSpeakersApiTest extends ProtectedApiTestCase { @@ -2016,4 +2019,139 @@ public function testGetCurrentSummitSpeakersActivitiesCountWithAcceptedPresentat $this->assertEquals($baseline + 1, $data->count); } + private function resetEmIfNeeded(): void + { + if (!self::$em->isOpen()) { + self::$em = Registry::resetManager(SilverstripeBaseModel::EntityManager); + } + } + + /** + * Regression test for: createMySpeaker must honour the bio submitted in the + * payload and must NOT silently overwrite it with the member's bio. + * + * Previously array_merge($payload, $aux_payload) put $aux_payload last so + * member->getBio() always won regardless of what the user submitted. + */ + public function testCreateMySpeakerPayloadBioTakesPrecedenceOverMemberBio() + { + // Create a fresh member (no speaker attached) with a known bio + $prefix = str_random(10); + $memberBio = "This is the OLD bio from the member/FNid profile."; + $payloadBio = "This is the NEW bio submitted by the user in the portal."; + + $newMember = new Member(); + $newMember->setEmail("test_bio_precedence_{$prefix}@example.com"); + $newMember->setFirstName("Bio"); + $newMember->setLastName("Precedence"); + $newMember->setActive(true); + $newMember->setEmailVerified(true); + $newMember->setUserExternalId(mt_rand()); + $newMember->setBio($memberBio); + self::$em->persist($newMember); + self::$em->flush(); + + // Authenticate as the new member + self::$service->setUserId($newMember->getUserExternalId()); + self::$service->setUserExternalId($newMember->getUserExternalId()); + self::$service->setUserEmail($newMember->getEmail()); + self::$service->setUserFirstName($newMember->getFirstName()); + self::$service->setUserLastName($newMember->getLastName()); + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + $response = $this->action( + "POST", + "OAuth2SummitSpeakersApiController@createMySpeaker", + [], + [], + [], + [], + $headers, + json_encode(['bio' => $payloadBio]) + ); + + // Restore authenticated member so tearDown works correctly + self::$service->setUserId(self::$member->getUserExternalId()); + self::$service->setUserExternalId(self::$member->getUserExternalId()); + self::$service->setUserEmail(self::$member->getEmail()); + self::$service->setUserFirstName(self::$member->getFirstName()); + self::$service->setUserLastName(self::$member->getLastName()); + + $this->assertResponseStatus(201); + $speaker = json_decode($response->getContent()); + $this->assertNotNull($speaker); + $this->assertEquals($payloadBio, $speaker->bio, + "Speaker bio must match what the user submitted, not the member/FNid bio."); + + // Cleanup — EM may have been closed by the BrowserKit HTTP simulation + $this->resetEmIfNeeded(); + self::$em->remove(self::$em->find(Member::class, $newMember->getId())); + self::$em->flush(); + } + + /** + * When no bio is submitted in the payload, createMySpeaker should fall back + * to the member's bio so that a first-time speaker gets a sensible default. + */ + public function testCreateMySpeakerFallsBackToMemberBioWhenNoneSubmitted() + { + $prefix = str_random(10); + $memberBio = "Default bio coming from the FNid member profile."; + + $newMember = new Member(); + $newMember->setEmail("test_bio_fallback_{$prefix}@example.com"); + $newMember->setFirstName("Bio"); + $newMember->setLastName("Fallback"); + $newMember->setActive(true); + $newMember->setEmailVerified(true); + $newMember->setUserExternalId(mt_rand()); + $newMember->setBio($memberBio); + self::$em->persist($newMember); + self::$em->flush(); + + self::$service->setUserId($newMember->getUserExternalId()); + self::$service->setUserExternalId($newMember->getUserExternalId()); + self::$service->setUserEmail($newMember->getEmail()); + self::$service->setUserFirstName($newMember->getFirstName()); + self::$service->setUserLastName($newMember->getLastName()); + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + // No 'bio' key in the payload — member bio should be used as default + $response = $this->action( + "POST", + "OAuth2SummitSpeakersApiController@createMySpeaker", + [], + [], + [], + [], + $headers, + json_encode(['title' => 'Engineer']) + ); + + self::$service->setUserId(self::$member->getUserExternalId()); + self::$service->setUserExternalId(self::$member->getUserExternalId()); + self::$service->setUserEmail(self::$member->getEmail()); + self::$service->setUserFirstName(self::$member->getFirstName()); + self::$service->setUserLastName(self::$member->getLastName()); + + $this->assertResponseStatus(201); + $speaker = json_decode($response->getContent()); + $this->assertNotNull($speaker); + $this->assertEquals($memberBio, $speaker->bio, + "When no bio is submitted, speaker bio must default to the member/FNid bio."); + + // Cleanup — EM may have been closed by the BrowserKit HTTP simulation + $this->resetEmIfNeeded(); + self::$em->remove(self::$em->find(Member::class, $newMember->getId())); + self::$em->flush(); + } + } \ No newline at end of file From bec9c2616f8ce827a2baed6b572d874e6dae24ef Mon Sep 17 00:00:00 2001 From: smarcet Date: Mon, 31 Aug 2026 17:16:58 -0300 Subject: [PATCH 2/2] fix(speakers): treat empty payload values as not sent on createMySpeaker Form clients commonly serialize untouched fields as empty strings. An empty or whitespace-only value for the defaultable keys (first_name, last_name, bio, twitter, irc) won the merge against the member defaults and created a speaker with an empty field instead of falling back to the member profile data. On create there is nothing to clear yet, so such values now count as not sent. The PUT path is untouched: empty strings there still clear the field. Adds a boundary regression test: a whitespace-only submitted bio falls back to the member bio (also covering the trim). --- .../OAuth2SummitSpeakersApiController.php | 8 +++ tests/oauth2/OAuth2SummitSpeakersApiTest.php | 69 +++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php index 7c25bb340..b5f8f1f67 100644 --- a/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php +++ b/app/Http/Controllers/Apis/Protected/Summit/OAuth2SummitSpeakersApiController.php @@ -1363,6 +1363,14 @@ public function createMySpeaker() 'irc' => $current_member->getIrcHandle(), ]; + // On create there is nothing to clear yet, so an empty or whitespace-only + // submitted value counts as "not sent" and the member default applies. + foreach (array_keys($member_defaults) as $key) { + if (isset($payload[$key]) && is_string($payload[$key]) && trim($payload[$key]) === '') { + unset($payload[$key]); + } + } + $payload = array_merge($member_defaults, $payload); $payload['member_id'] = $current_member->getId(); diff --git a/tests/oauth2/OAuth2SummitSpeakersApiTest.php b/tests/oauth2/OAuth2SummitSpeakersApiTest.php index d498297f0..4b8e30a33 100644 --- a/tests/oauth2/OAuth2SummitSpeakersApiTest.php +++ b/tests/oauth2/OAuth2SummitSpeakersApiTest.php @@ -2154,4 +2154,73 @@ public function testCreateMySpeakerFallsBackToMemberBioWhenNoneSubmitted() self::$em->flush(); } + /** + * Boundary case: form clients commonly serialize untouched fields as empty + * strings. On create, an empty or whitespace-only bio must count as "not + * sent" and fall back to the member bio - not create a speaker with an + * empty bio. + */ + public function testCreateMySpeakerEmptyBioFallsBackToMemberBio() + { + $prefix = str_random(10); + $memberBio = "Bio coming from the FNid member profile."; + + $newMember = new Member(); + $newMember->setEmail("test_bio_empty_{$prefix}@example.com"); + $newMember->setFirstName("Bio"); + $newMember->setLastName("EmptyString"); + $newMember->setActive(true); + $newMember->setEmailVerified(true); + $newMember->setUserExternalId(mt_rand()); + $newMember->setBio($memberBio); + self::$em->persist($newMember); + self::$em->flush(); + + self::$service->setUserId($newMember->getUserExternalId()); + self::$service->setUserExternalId($newMember->getUserExternalId()); + self::$service->setUserEmail($newMember->getEmail()); + self::$service->setUserFirstName($newMember->getFirstName()); + self::$service->setUserLastName($newMember->getLastName()); + + $headers = [ + "HTTP_Authorization" => " Bearer " . $this->access_token, + "CONTENT_TYPE" => "application/json", + ]; + + try { + // Whitespace-only bio - must be treated as "not sent" + $response = $this->action( + "POST", + "OAuth2SummitSpeakersApiController@createMySpeaker", + [], + [], + [], + [], + $headers, + json_encode(['bio' => ' ', 'title' => 'Engineer']) + ); + } finally { + // Restore authenticated member even if the request blows up, + // so the rest of the class does not run impersonated. + self::$service->setUserId(self::$member->getUserExternalId()); + self::$service->setUserExternalId(self::$member->getUserExternalId()); + self::$service->setUserEmail(self::$member->getEmail()); + self::$service->setUserFirstName(self::$member->getFirstName()); + self::$service->setUserLastName(self::$member->getLastName()); + } + + $this->assertResponseStatus(201); + $speaker = json_decode($response->getContent()); + $this->assertNotNull($speaker); + $this->assertEquals($memberBio, $speaker->bio, + "An empty/whitespace-only submitted bio must fall back to the member/FNid bio."); + + // Cleanup - EM may have been closed by the BrowserKit HTTP simulation + $this->resetEmIfNeeded(); + $createdSpeaker = self::$em->find(PresentationSpeaker::class, intval($speaker->id)); + if (!is_null($createdSpeaker)) self::$em->remove($createdSpeaker); + self::$em->remove(self::$em->find(Member::class, $newMember->getId())); + self::$em->flush(); + } + } \ No newline at end of file