HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Complex if statements in authentication method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
statementsauthenticationcomplexmethod

Problem

I had asked questions about my entire class which I'll flag for closing soon. It appeared to be a little too broad for 1 post so I chopped it up and will be asking a little more specific questions separately.

In my Application Layer I have a class AuthService containing the following method with a code structure that has gotten quite complex with if statements that I'm unable to re-factor a better way.

I'd appreciate anybody who could review this piece of code and give me suggestions on improving / optimizing the design of it. Any other suggestions are welcome as well.

```
public function findLoggedInUser()
{
if (($userData = $this->httpSession->getParameter('user')) && isset($userData['id'])) {
// User found in current http session
return $this->userMapper->findById($userData['id']);
} elseif ($httpSessionData = $this->httpCookie->getParameter('usess')) {
if (isset($httpSessionData['uid'], $httpSessionData['sno'], $httpSessionData['token'])) {
if ($user = $this->userMapper->findById($httpSessionData['uid'])) {
if ($httpSession = $this->httpSessionMapper->findByUserAndSeriesNo($user, $httpSessionData['sno'])) {
if (password_verify($httpSessionData['token'], (string) $httpSession->getTokenHash()) {
// Generate new http session entity tokens
$token['raw'] = new Token(CryptoCharGen::numeric(60));
$token['hash'] = new TokenHash(password_hash((string) $token['raw'], PASSWORD_BCRYPT, ['cost' => 4]));

// Update and persist http session entity with new token
$httpSession->setTokenHash($token['hash']);

$this->httpSessionMapper->update($httpSession);

// Issue new http cookie usess with updated http session recovery data
$this->httpCookie->setParameter('usess', [
'uid' =

Solution

Two suggestions on how to keep this code maintainable:

-
use early exits

-
keep conditions atomic (one if checks for one thing)

Example:

function findLoggedInUser()
{
    if (($userData = $this->httpSession->getParameter('user')) && isset($userData['id'])) {
        // User found in current http session
        return $this->userMapper->findById($userData['id']);
    }

    $httpSessionData = $this->httpCookie->getParameter('usess');
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if(!isset($httpSessionData['uid'], $httpSessionData['sno'], $httpSessionData['token'])) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $user = $this->userMapper->findById($httpSessionData['uid']);
    if(!$user) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $httpSession = $this->httpSessionMapper->findByUserAndSeriesNo($user, $httpSessionData['sno']));
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if (!password_verify($httpSessionData['token'], (string) $httpSession->getTokenHash()) {
        $this->httpSessionMapper->deleteAllByUser($user);
        $this->httpCookie->destroy('usess');
        return null;
    }

    // EVERYTHING'S FINE, DO THE JOB

    // Generate new http session entity tokens
    $token['raw']  = new Token(CryptoCharGen::numeric(60));
    $token['hash'] = new TokenHash(password_hash((string) $token['raw'], PASSWORD_BCRYPT, ['cost' => 4]));

    // Update and persist http session entity with new token
    $httpSession->setTokenHash($token['hash']);

    $this->httpSessionMapper->update($httpSession);

    // Issue new http cookie usess with updated http session recovery data
    $this->httpCookie->setParameter('usess', [
        'uid'   => (string) $httpSession->getUser()->getId(),
    'sno'   => (string) $httpSession->getSeriesNo(),
    'token' => (string) $token['raw']
        ], 60 * 60 * 24 * 90);

    // User has been found
    return $httpSession->getUser();
}


This way, the code remains readable and flexible for future modifications.

Code Snippets

function findLoggedInUser()
{
    if (($userData = $this->httpSession->getParameter('user')) && isset($userData['id'])) {
        // User found in current http session
        return $this->userMapper->findById($userData['id']);
    }

    $httpSessionData = $this->httpCookie->getParameter('usess');
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if(!isset($httpSessionData['uid'], $httpSessionData['sno'], $httpSessionData['token'])) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $user = $this->userMapper->findById($httpSessionData['uid']);
    if(!$user) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    $httpSession = $this->httpSessionMapper->findByUserAndSeriesNo($user, $httpSessionData['sno']));
    if(!$httpSessionData) {
        $this->httpCookie->destroy('usess');
        return null;
    }

    if (!password_verify($httpSessionData['token'], (string) $httpSession->getTokenHash()) {
        $this->httpSessionMapper->deleteAllByUser($user);
        $this->httpCookie->destroy('usess');
        return null;
    }

    // EVERYTHING'S FINE, DO THE JOB

    // Generate new http session entity tokens
    $token['raw']  = new Token(CryptoCharGen::numeric(60));
    $token['hash'] = new TokenHash(password_hash((string) $token['raw'], PASSWORD_BCRYPT, ['cost' => 4]));

    // Update and persist http session entity with new token
    $httpSession->setTokenHash($token['hash']);

    $this->httpSessionMapper->update($httpSession);

    // Issue new http cookie usess with updated http session recovery data
    $this->httpCookie->setParameter('usess', [
        'uid'   => (string) $httpSession->getUser()->getId(),
    'sno'   => (string) $httpSession->getSeriesNo(),
    'token' => (string) $token['raw']
        ], 60 * 60 * 24 * 90);

    // User has been found
    return $httpSession->getUser();
}

Context

StackExchange Code Review Q#60203, answer score: 7

Revisions (0)

No revisions yet.