Minor fixes

[nsklikas]

This PR contains some minor fixes.

• User grant.get_token to reduce db queries.
• Fixes a bug that was created when we started storing id_tokens in grant.issued_tokens
• Fixes pkce for the refresh_token and token_exchange grants (it should do nothing)

[peppelinux]

Do we have test if the _session_info doesn't have a grant key?

anyway ILGTM!

[nsklikas]

hmm, this wouldn't fail, but I think that https://github.com/IdentityPython/oidc-op/pull/31/files#diff-315b01569b3a7bd24a653670e83e37c463e9e86cce66153b84931bccb96ed963R83 could raise an error in that case.

Good catch, I'll look into it and try to write some tests.

[rohe]

It should not be possible to have a session without a grant.
As the code is right now when a session is created a grant is added to the session.
This is before user consent or application of service policies so the grant will not really allow anything but it's there.

[rohe]

An item in issued_tokens should always be an instance of a SessionToken or a sub class of SessionToken
never a simple string. That is an assumption that is built into the code.
I'm not sure I want to step back on the requirement.

You obviously creates ID Tokens 'outside' session management.
Probably because minting ID Tokens is handled differently from issuing access/refresh tokens.
If anything maybe we should change that instead of allowing strings in issued_tokens.
I've sort of contemplated this for a while.

[nsklikas]

You are right.

I'm not sure whether we should store ID tokens, it's something that I have thought about too. And I don't see a reason other than for the introspection endpoint (some providers allow introspection of ID tokens). The minting of ID tokens is done in https://github.com/IdentityPython/oidc-op/blob/develop/src/oidcop/oauth2/authorization.py#L764.

I thought that that behavior was intentional, that's why I allowed strings in issued_tokens.
I will undo the changes to get_token and (if you agree) I will remove the line that appends the ID token in the issued_tokens (https://github.com/IdentityPython/oidc-op/blob/develop/src/oidcop/oauth2/authorization.py#L775).

Whether we want ID tokens to inherit from SessionToken or not we should do it in another PR.

[peppelinux]

Probably I miss the storytelling about the session tokens and the id token but I'm wondering on the meaning of "issued tokens".

If a SSO session issued some tokens well these token should be related to that session.
So, I'm wondering, that even the id token is a session's issued_token.

Actually the session carries as issued tokens only access and authz token

@roland that's ok, I understand that we could have many token of many nature and these will not belong to the session ... But it's a fact THAT the session issued all those tokens.

Finally, OK, where to store the issued id token and how to have them in a dump?
I can live without them, sure, but here my curiosity still looks hungry :)

[rohe]

I'm just putting the last touches on making ID Tokens session tokens.
Going through all the tests to make sure I haven't missed something somewhere.

This means that in the future if we want an ID Token you used the mint_token method of Grant.

[peppelinux]

I'm just putting the last touches on making ID Tokens session tokens.
Going through all the tests to make sure I haven't missed something somewhere.

This means that in the future if we want an ID Token you used the mint_token method of Grant.

Ok, great.
We could consider this chore in the 0.6.0 release, together with this #33
Generally, we're facing that the session dump should be enriched, regardless of the underlying relationship of the elements, which is what you are actually and substantially addressing

[rohe]

@nsklikas you should probably go back and see what of this is still needed to change.
I think some changes you propose has been overtaken by events like making an ID Token a session token.

[rohe]

You didn't have to go back. :-)