Password Grant - allow multiple sessions per user

[michaelhogguk]

Firstly I wanted to thank you for your awesome library @alexbilbie :)

For the Password Grant, why are existing sessions deleted when the new session is created?

public function completeFlow($inputParams = null)
{
    ...
    // Delete any existing sessions just to be sure
    $this->authServer->getStorage('session')->deleteSession($authParams['client_id'], 'user', $userId);

    // Create a new session
    $sessionId = $this->authServer->getStorage('session')->createSession(
    ...
}

The Password Grant is often used for first-party "official" mobile apps. A user may wish to install such an app on two devices (eg: an iPhone and an iPod touch) and stay signed in on both of them.

With the existing functionality in the Password Grant, when the user signs in on their second device, deleteSession() will delete their first session from the oauth_sessions table, effectively signing them out from their first device.

If the call to deleteSession() was removed, then oauth_sessions could contain an unlimited number of sessions per user, enabling a user to stay signed in on an unlimited number of devices. Sessions can be deleted from oauth_sessions when:

• The session doesn't have a refresh token, and the current time passes access_token_expires, or
• The session does have a refresh token, and a very large amount of time (eg: a year) has passed since access_token_expires (it's very unlikely that the refresh token will be used a year after the access token has expired).

This is similar to Issue #25. The difference is:

• Issue Refresh Token Grant - scope limitation and additional access tokens #25 concerns the Refresh Token Grant, creating multiple access tokens using one refresh token.
• This issue concerns the Password Grant, creating multiple access tokens by signing in with username and password on multiple devices.

[cziegenberg]

In #25 I proposed a similar change for the deleteSession method: "differ between the owner type to delete only expired sessions when the Client Credentials Grant is used."

Perhaps this needs to be done for all grant types? I couldn't find anything in the RFC that doesn't allow this behavior. This is already possible and depends on your implementation of the Session Storage, but should be default I think.

[jacobweber]

Is there a reason not to do this in the Auth Code and Implicit grants as well?

[jfse]

I think @jacobweber is correct here, this should be removed for all grant types.

Basic example: you have an android application, it uses a certain client id. If a user has a mobile phone and a tablet he would be unable to login to both devices unless they use different client ids. This is a common thing I would think, even having 2 android mobile phones is not entierly unheard of.

As it is today, if you login to one device your tokens for the other would be removed.

Some way to clear old sessions are necessary though, but it would need to make sure there are no valid tokens associated with them first as stated by @michaelhogg above. For myself I would write a cronjob to clear these, but it might be a good idea to have this functionality available in the repository in some way.

[stefanocutello]

Followup on @jacobweber and @jfse comment.

Re: skip deleteSession in AuthCode
@alexbilbie I need to allow multiple access_token for all credentials in v2.1.1: as a quick patch I'm considering to remove the call to deleteSession() from the AuthCode grant (like you did for ClientCredential and Password) could you please explain the reasons why you have not done it on v2.1.1? Is there any risks/downside in doing so there (before upgrading to v3 where I see "All grants no longer remove old sessions by default")?

Re: cleanup old session
@jfse I'm also considering a cronjob to cleanup old expired session but I agree there should be a garbage collect module builtin in the library (that can be used in a cronjob). Is there anything like that?