bshaffer / oauth2-server-php

A library for implementing an OAuth2 Server in php
http://bshaffer.github.io/oauth2-server-php-docs
MIT License
3.26k stars 950 forks source link

Duplicate access tokens generated #854

Open twarkie opened 7 years ago

twarkie commented 7 years ago

Greetings!

I have investigated an issue with our implementation for a couple of weeks. The issue is that existing tokens are overwritten by new sessions that get the same generated token.

Looking at createAccessToken() and generateAccessToken() I notice that it does not check for existing tokens. It simply generates a new token and treats it as truly unique. The PDO storage that I'm using has this piece of code:

if ($this->getAccessToken($access_token)) {
    $stmt = $this->db->prepare(sprintf('UPDATE %s SET client_id=:client_id, expires=:expires, user_id=:user_id, scope=:scope where access_token=:access_token', $this->config['access_token_table']));
} else {
    $stmt = $this->db->prepare(sprintf('INSERT INTO %s (access_token, client_id, expires, user_id, scope) VALUES (:access_token, :client_id, :expires, :user_id, :scope)', $this->config['access_token_table']));
}

I have added logging for the update part which triggers a couple of times a day, meaning that an existing token gets overwritten with new info, and the existing login is now incorrect.

What are the odds for that? :-) Seems pretty low so that's why I'm here to check if maybe I'm doing something wrong? What info can I provide you to be able to help me?

Thanks!

twarkie commented 7 years ago

Oh, my temporary fix for this is simply:

do {
    $accessToken = $this->generateAccessToken();
} while($this->tokenStorage->getAccessToken($accessToken));

And some numbers: ~500 tokens generated a day, ~100k tokens in total.

ssanders commented 7 years ago

This may also relate to Duplicate PRIMARY key in setRefreshToken() when using PDO. #616

twarkie commented 7 years ago

Yep, the column size for access_token is the issue here as well. The proposed schema here: https://bshaffer.github.io/oauth2-server-php-docs/cookbook/ needs to be updated with a larger field for access_token.

bluebaroncanada commented 7 years ago

There's a method at the bottom of Pdo.php that has all the tables. Don't use the ones from the docs. I've already submitted a couple of issues with his docs.

twarkie commented 7 years ago

Actually, the column size is probably not the issue here. The generated key length is 40 characters which fits. Still using my workaround.

ssanders commented 7 years ago
do {$accessToken = $this->generateAccessToken();}
while ($this->tokenStorage->getAccessToken($accessToken));

$token = array(
    "access_token" => $accessToken, //$this->generateAccessToken(),
//$token["refresh_token"] = $this->generateRefreshToken();
do {$token["refresh_token"] = $this->generateRefreshToken();}
while ($this->refreshStorage->getRefreshToken($token["refresh_token"]));