$client->getPublicId() != $token->getClientId() about fosoauthserverbundle HOT 13 CLOSED

+1

from fosoauthserverbundle.

You're right, this method has a wrong behavior. Would you mind write a pull request to fix it?

For the OAuth2 lib, the bug is really tricky. Actually, there won't be any problem to use the real public id, and to fix the method you pointed. It will work, and the test $client->getPublicId() != $authCode->getClientId() will return true.

There is an issue I'm trying to fix on this bundle: the random id. It should not follow the pattern: {client_id}_{random_id}. The bug I'm describing shows us why it's bad.

• getPublicId() will return something like: 4_AEZFEDZD...
• getClientId() will return 4

Or, without a strict equality, we have an integer equals to a integer converted from a string. As the client id is always the first character of the public id, and the second one is an underscore, by chance we have a true equality.

Then, two ways to fix this issue:

• first, to patch the lib;
• second, to change the public id pattern, it will be BC break, but necessary.

from fosoauthserverbundle.

Actually, the lib just describe a getter for a client id in the IClientInterface, so the method is right.

That means forget my previous comment. There is a patch to fix the strict equality in the lib to do, but nothing else. Btw, Propel doesn't fit the spec... There is a bug in the Propel implementation.

from fosoauthserverbundle.

I'm sorry willdurand but I do not follow you. In the Oauth2-php lib, the 2 lines are contradictory:

Line 814 : $client->getId() != $token->getClientId()
Line 748 : $client->getPublicId() != $authCode->getClientId()

Why would you have both a getId and a getPublicId, when the IOAuth2Client and OAuth2Client mention only a getPublicId?

from fosoauthserverbundle.

Oups sorry, just understood that in fact we agree. :)

So we just need a patch to change the line 814, I'm I right?

from fosoauthserverbundle.

Yep. And a deep inspection of the lib. We also have to put === on lines you mentioned.

from fosoauthserverbundle.

Done. Here is the pull request : FriendsOfSymfony/oauth2-php#6

from fosoauthserverbundle.

I'm closing this issue as it's fixed now.

from fosoauthserverbundle.

It isn't fix in Propel, is it ?

from fosoauthserverbundle.

@vbardales there is no issue with the model layer.

from fosoauthserverbundle.

Ok, getClientId is not overriden in FOSOAuthServerBundle / Propel / AuthCode.php in 1.1.x branch.

from fosoauthserverbundle.

@vbardales ah, you are probably using the 1.1.x branch, so you should upgrade or backport commits from the master branch. I don't really know how to upgrade the 1.1.x from master.
It should be possible because the BC break is just due to the AuthorizeFormType (getDefaultOptions() signature to be more precise). If you want to try to backport features from master into 1.1x, I will be glad. Your best option is to cherry-pick commits I think.

from fosoauthserverbundle.

@vbardales yep

from fosoauthserverbundle.