Skip to content
This repository has been archived by the owner on Nov 2, 2020. It is now read-only.

Commit

Permalink
feat(Secret): Check session and user_id match or not in jwt payload
Browse files Browse the repository at this point in the history
In commit `36f49a0`, we verity $jti is force expired or not by call redis->sIsMember(Constant::invalidUserSessionSet, $payload['jti']), However redis cache is not reliable. When user has a force-expired jwt-token and the $payload['jti'] in not record in Constant::invalidUserSessionSet (for example, the key is delete or flushdb ), the jwt-token will pass our force-expired check.

Since both `sIsMember` and `zScore` will be called and both cost o(1), we revert to use zSet Constant::mapUserSessionToId to store the relationship of session to user_id.
if zScore return false , it means the cache through, check this in database and restore the user_id or 0 (it means the session is not exist or is expired) in redis cache. if zScore return other float , for example, (double) 0 means this session is marked as valid, so we compare with $payload['user_id'] to (loose)check if the relationship of session to user_id is matched or not.
  • Loading branch information
Rhilip committed Aug 12, 2019
1 parent dfa67da commit 358ba5d
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

### Refactor
- **Auth/JWT:** Better for auth by JWT (36f49a0)
- **Auth/Middleware:** merge Old Auth{ByCookies, ByPasskey}Middleware (71cd7d7)
- **Config:** Remove params `$throw` in Config()->get() (706cc9a)
- **RateLimit:** Change last param of isRateLimitHit and rate limit store Namespace (4dd571d)
- **Site:** Simple Category Detail get function (ffa6855)
Expand Down
11 changes: 9 additions & 2 deletions apps/components/Site.php
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,15 @@ protected function loadCurUserIdFromCookies()
if (strcasecmp($payload['secure_login_ip'], $now_ip_crc) !== 0) return false;
}

// Verity $jti is force expired or not by check invalidUserSessionSet
if (app()->redis->sIsMember(Constant::invalidUserSessionSet, $payload['jti'])) return false;
// Verity $jti is force expired or not by checking mapUserSessionToId
$expired_check = app()->redis->zScore(Constant::mapUserSessionToId, $payload['jti']);
if ($expired_check === false) { // session is not see in Zset Cache (may lost or first time init), load from database ( Lazy load... )
$uid = app()->pdo->createCommand('SELECT `uid` FROM `user_session_log` WHERE sid = :sid AND `expired` != 1 LIMIT 1')->bindParams([
'sid' => $payload['jti']
])->queryScalar();
app()->redis->zAdd(Constant::mapUserSessionToId, $uid ?: 0, $payload['jti']); // Store 0 if session -> uid is invalid
if ($uid === false) return false; // this session is not exist or marked as expired
} elseif ($expired_check != $payload['user_id']) return false; // may return (double) 0 , which means already make invalid ; or it check if user obtain this session (may Overdesign)

// Check if user want secure access but his environment is not secure
if (!app()->request->isSecure() && // if User requests is not secure , then
Expand Down
6 changes: 2 additions & 4 deletions apps/libraries/Constant.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ class Constant
const cookie_name = 'rid';

const mapUsernameToId = 'Map:user_username_to_user_id:hash';
const mapUserPasskeyToId = 'Map:user_passkey_to_user_id:zset';
const mapUserPasskeyToId = 'Map:user_passkey_to_user_id:zset'; // (double) 0 means invalid
const mapUserSessionToId = 'Map:user_session_to_user_id:zset'; // (double) 0 means invalid

// --- invalid Zset ---
const invalidUserIdZset = 'Site:invalid_user_id:zset';

// --- invalid Set ---
const invalidUserSessionSet = 'Site:invalid_user_session:set'; // Store the force invalid session data

// Tracker Use
const trackerInvalidInfoHashZset = 'Tracker:invalid_torrent_info_hash:zset'; // FIXME use set instead
const trackerAllowedClientList = 'Tracker:allowed_client_list:string';
Expand Down
5 changes: 3 additions & 2 deletions apps/models/form/Auth/UserLoginForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ protected function isMaxUserSessionsReached()
}
}

public function LoginFail()
public function LoginFail() // FIXME
{
app()->redis->zAdd('SITE:fail_login_ip_zset', time(), app()->request->getClientIp());
app()->redis->hIncrBy('SITE:fail_login_ip_count', app()->request->getClientIp(), 1);
Expand Down Expand Up @@ -164,11 +164,12 @@ private function createUserSession()
// Official Payload key
$payload = [
'iss' => config('base.site_url'),
'sub' => config('base.site_generator'),
'iat' => $timenow,
'jti' => $jti,
];

$cookieExpire = 0x7fffffff; // for never
$cookieExpire = 0x7fffffff; // Tuesday, January 19, 2038 3:14:07 AM (though it is not security)
if ($this->logout === 'yes' || config('security.auto_logout') > 1) {
$cookieExpire = $timenow + 15 * 60; // for 15 minutes
}
Expand Down
11 changes: 4 additions & 7 deletions apps/models/form/Auth/UserLogoutForm.php
Original file line number Diff line number Diff line change
Expand Up @@ -66,15 +66,12 @@ public function flush()

private function invalidSession()
{
// Set this session is expired
app()->cookie->delete(Constant::cookie_name); // Clean cookie
app()->redis->zAdd(Constant::mapUserSessionToId, 0, $this->sid); // Quick Mark this invalid in cache

// Set this session expired
app()->pdo->createCommand('UPDATE `user_session_log` SET `expired` = 1 WHERE sid = :sid')->bindParams([
'sid' => $this->sid
])->execute();

// Clean cookie
app()->cookie->delete(Constant::cookie_name);

// Mark this invalid
app()->redis->sAdd(Constant::invalidUserSessionSet, $this->sid);
}
}
14 changes: 7 additions & 7 deletions apps/views/auth/login.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,25 +65,25 @@
<legend><a href="#adv_option" data-toggle="collapse" class="btn btn-link">Advanced Options</a></legend>
<div id="adv_option" class="collapse">
<div class="form-group">
<?php // 0 - disable -> 'disabled' ; 1 - option -> '' ; 2 - force -> 'checked disabled' ?>
<?php // -1 - disable -> 'disabled' ; 0 - option -> '' ; 1 - option but default checked -> 'checked' ; 2 - force -> 'checked disabled' ?>
<div class="switch">
<input type="checkbox" name="logout" id="logout" value="yes"
<?php if (config('security.auto_logout') > 1): ?>checked<?php endif; ?>
<?php if (config('security.auto_logout') != 1): ?>disabled<?php endif; ?>
<?php if (config('security.auto_logout') > 0): ?>checked<?php endif; ?>
<?php if (in_array(config('security.auto_logout'), [-1, 2])): ?>disabled<?php endif; ?>
>
<label for="logout">Automatically Log me out after 15 minutes</label>
</div>
<div class="switch">
<input type="checkbox" name="securelogin" id="securelogin" value="yes"
<?php if (config('security.secure_login') > 1): ?>checked<?php endif; ?>
<?php if (config('security.secure_login') != 1): ?>disabled<?php endif; ?>
<?php if (config('security.secure_login') > 0): ?>checked<?php endif; ?>
<?php if(in_array(config('security.secure_login'), [-1, 2])): ?>disabled<?php endif; ?>
>
<label for="securelogin">Restrict session to my login IP</label>
</div>
<div class="switch">
<input type="checkbox" name="ssl" id="ssl" value="yes"
<?php if (app()->request->isSecure() || config('security.ssl_login') > 1): ?>checked<?php endif; ?>
<?php if (app()->request->isSecure() || config('security.ssl_login') != 1): ?>disabled<?php endif; ?>
<?php if (app()->request->isSecure() || config('security.ssl_login') > 0): ?>checked<?php endif; ?>
<?php if (app()->request->isSecure() || in_array(config('security.ssl_login'), [-1, 2])): ?>disabled<?php endif; ?>
>
<label for="ssl">Enable SSL (HTTPS)</label>
</div>
Expand Down

0 comments on commit 358ba5d

Please sign in to comment.