openid session storage should be deleted when the authcode is exchanged

[cfryanr]

openid session storage should be deleted when the authcode is exchanged

[cfryanr]

According to the comments on #538 (comment) it was decided to deprecate the DeleteOpenIDConnectSession function because:

Not sure if it makes sense to delete the rows though as it contains info required for refreshing the oidc session

Note that discussion about "rows" is specific to how Hydra chooses to implement the storage interface.

In general, the above is not a true statement about how the fosite storage interfaces work. Note that GetOpenIDConnectSession is only ever called by PopulateTokenEndpointResponse in handler/openid/flow_explicit_token.go. Also note that the PopulateTokenEndpointResponse function only works for authorization code exchanges because the first line of the function returns an error unless the grant type is authorization_code. Therefore, the openid session storage is only ever needed during authorization code exchange, and has no impact on refresh grants.

If Hydra's storage implementation does not want to delete openid session storage during authorization code exchange, it could choose to make its implementation of DeleteOpenIDConnectSession a no-op, rather than deprecating DeleteOpenIDConnectSession.

[mitar]

What about DeleteAccessTokenSession? That also seems it is never called?

See also #798.

[cfryanr]

@mitar, fosite will call RevokeAccessToken from the TokenRevocationStorage interface instead of calling DeleteAccessTokenSession. I'm not sure of the history or reason behind that, but it has been that way for years.

[mitar]

But that is done only in memory storage implementation. This is not done necessary in all storage implementations?

[cfryanr]

For example, RevokeAccessToken is called by the token endpoint in several cases where the access token should be revoked based upon client actions. It is up to the implementor of the storage interface to cause that function to delete the access token session from storage.

However, I think you are correct that when the access token has expired, nothing in fosite will call the storage interface to tell it that the token has expired. It seems to be up to the implementor of the storage interface to handle garbage collecting old expired sessions without any help from fosite, if I understand correctly.

[mitar]

Conceptually, why it is safe to delete OIDC session after authcode is exchanged, but not the regular session? Is this because for regular session it is still needed to be able to support refresh tokens?

[cfryanr]

There's not a "regular session" in the fosite storage interfaces. There are CreateAuthorizeCodeSession, CreatePKCERequestSession, CreateOpenIDConnectSession, CreateAccessTokenSession, CreateRefreshTokenSession (and maybe others?).

Each of those session storage types serve a different purpose and have a different lifecycle based on their purpose.

The data stored by CreateOpenIDConnectSession can only be read by calling GetOpenIDConnectSession. The only caller of GetOpenIDConnectSession is during authorization code exchange in flow_explicit_token.go. So if the authorization code exchange is successful, then nobody will ever call GetOpenIDConnectSession for that session ever again, so it is safe to call DeleteOpenIDConnectSession for it.

This lifecycle is approximately the same for the PKCE request session storage. That is also only needed during authcode exchange, and will never be needed again, so it is deleted.

However, this lifecycle is not the same for the other storage types.

Authorize code session storage lives beyond authcode exchange because fosite uses that as a place to store a true/false state that the authcode has already been exchanged. If another client later tries to exchange the same authcode again, fosite wants to treat that as a special case for security reasons.

Access token and refresh token session storage are both used during the refresh grant, so they need to live longer than the OpenID and PKCE session storage.

Is that what you were asking? Hope that helps.

[mitar]

Thanks!

[mitar]

(I am asking those questions in the context of #798.)