Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing Implementation in Storage Classes #20

Closed
yankeeinlondon opened this issue Jul 3, 2013 · 4 comments
Closed

Missing Implementation in Storage Classes #20

yankeeinlondon opened this issue Jul 3, 2013 · 4 comments

Comments

@yankeeinlondon
Copy link

I have noticed that all of the demonstration Storage classes leave out the implementation of 4 methods which are required by the various interfaces that they implement:

  1. AuthorizationCodeInterface::enforceRedirect
  2. AuthorizationCodeInterface::createAuthorizationCode
  3. AccessTokenInterface::createAccessToken
  4. ResponseTypeInterface::getAuthorizeResponse (also redefined in AuthorizationCodeInterface)

Am I missing something? There's no mention of this in the otherwise very verbose and high quality documentation.

@bshaffer
Copy link
Owner

bshaffer commented Jul 3, 2013

I believe you are getting the Storage interfaces mixed up with the GrantType interfaces

Brent Shaffer

On Wed, Jul 3, 2013 at 1:54 AM, Ken Snyder notifications@github.com
wrote:

I have noticed that all of the demonstration Storage classes leave out the implementation of 5 methods which are required by the various interfaces that they implement:

  1. AuthorizationCodeInterface::enforceRedirect
  2. AuthorizationCodeInterface::createAuthorizationCode
  3. AuthorizationCodeInterface::getAuthorizeResponse
  4. AccessTokenInterface::createAccessToken
  5. ResponseTypeInterface::getAuthorizeResponse

Am I missing something? There's no mention of this in the otherwise very verbose and high quality documentation.

Reply to this email directly or view it on GitHub:
#20

@yankeeinlondon
Copy link
Author

Am I? Let me give you a concrete example:

  1. The mongo storage class implements the following interfaces:

    class Mongo implements AuthorizationCodeInterface,
    AccessTokenInterface,
    ClientCredentialsInterface,
    UserCredentialsInterface,
    RefreshTokenInterface,
    JwtBearerInterface
    { ... }

  2. These interfaces define a set of interfaces that must be implemented ... 98% of which are implemented by the Mongo storage class but the four listed above are not.

I have glanced at the PDO class as well and it appears to have the same deficiency. I hope I'm not being too thick headed but when I say storage interfaces all i mean are those interfaces that your example storage classes implement (or in this case ... partially implement).

@bshaffer
Copy link
Owner

bshaffer commented Jul 3, 2013

Yes, you are, but it's an easy mistake to make. The methods you are referencing miss the entire namespace. Their namespaces are:

  • OAuth2\ResponseType\AuthorizationCodeInterface::enforceRedirect
  • OAuth2\ResponseType\AuthorizationCodeInterface::createAuthorizationCode
  • OAuth2\ResponseType\AccessTokenInterface::createAccessToken
  • OAuth2\ResponseType\ResponseTypeInterface::getAuthorizeResponse

These are ResponseType objects, not Storage objects. Their interfaces are different, and not implemented by the Mongo storage engine.

So the responsibilities of these classes do not really coincide with a Storage object.... it's not up to Mongo to determine if the redirect is enforced. It's up to you when configuring your ResponseType objects. Why would anyone need to use interfaces instead of just using the objects I've added? I can't think of a good reason, but the interfaces are there to promote loose coupling of code, in the event someone's implementation does require their own method of determining this.

I hope this makes sense.

@yankeeinlondon
Copy link
Author

Thanks, makes sense now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants