Skip to content

Commit c225a47

Browse files
committed
refactor(auth): drop unused OAuth2OTP transient $user field
The $user property added in this PR's earlier commits had only one writer (AuthService::finalizeRedemption) and no readers anywhere in the codebase. Since the field is not ORM-mapped, a future caller that re-loaded the OTP from DB and called getUser() would silently receive null — a footgun for no current benefit. Remove the field, its setUser/getUser accessors, and the now-unused Auth\User import from OAuth2OTP. Drop the matching setUser call site in finalizeRedemption and the unused mock expectation in the unit test. YAGNI: re-add (and bind to a defined consumer) when an actual reader materialises.
1 parent a0406ff commit c225a47

3 files changed

Lines changed: 3 additions & 19 deletions

File tree

app/Models/OAuth2/OAuth2OTP.php

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
use App\libs\Utils\PunnyCodeHelper;
1616
use App\Models\Utils\BaseEntity;
17-
use Auth\User;
1817
use Doctrine\ORM\Mapping AS ORM;
1918
use DateTime;
2019
use DateInterval;
@@ -119,12 +118,6 @@ class OAuth2OTP extends BaseEntity implements Identifier
119118
#[ORM\ManyToOne(targetEntity: \Models\OAuth2\Client::class, inversedBy: 'otp_grants', cascade: ['persist'])]
120119
private $client;
121120

122-
/**
123-
* @var User
124-
* this is a transient state
125-
*/
126-
private $user;
127-
128121
/**
129122
* OAuth2OTP constructor.
130123
* @param int $length
@@ -505,12 +498,4 @@ public function getUserId()
505498
{
506499
return $this->user_id;
507500
}
508-
509-
public function setUser(User $user): void{
510-
$this->user = $user;
511-
}
512-
513-
public function getUser():?User{
514-
return $this->user;
515-
}
516501
}

app/libs/Auth/AuthService.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,8 +199,9 @@ private function findAndValidateOTP(
199199
}
200200

201201
/**
202-
* Marks the OTP redeemed, attaches the user (transient), revokes sibling pending OTPs.
203-
* Entity methods short-circuit for inline OTPs — no special-casing needed here.
202+
* Marks the OTP redeemed, stores the resolved user id, and revokes sibling
203+
* pending OTPs. Entity methods short-circuit for inline OTPs — no special-
204+
* casing needed here.
204205
*
205206
* Concurrency: acquires a PESSIMISTIC_WRITE row lock and re-hydrates state
206207
* before checking redemption. This closes the validate→redeem race window:
@@ -227,7 +228,6 @@ private function finalizeRedemption(OAuth2OTP $otp, User $user, ?Client $client)
227228

228229
$otp->setAuthTime(time());
229230
$otp->setUserId($user->getId());
230-
$otp->setUser($user);
231231
$otp->redeem();
232232

233233
$grants2Revoke = $this->otp_repository->getByUserNameNotRedeemed($otp->getUserName(), $client);

tests/VerifyOTPChallengeTest.php

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,6 @@ public function testSucceedsAndConsumesOTPWhenSessionUserMatchesOTPUser(): void
149149
// finalizeRedemption mutations expected
150150
$otp->shouldReceive('setAuthTime')->once();
151151
$otp->shouldReceive('setUserId')->once()->with(100);
152-
$otp->shouldReceive('setUser')->once();
153152
$otp->shouldReceive('redeem')->once();
154153
$otp->shouldReceive('getId')->andReturn(7);
155154

0 commit comments

Comments
 (0)