Shared sessiondb

[tpazderka]

• Any changes relevant to users are recorded in the CHANGELOG.md.
• The documentation has been updated, if necessary.

---

[tpazderka]

@schlenk Could you please have a quick look? If it looks reasonable to you, I will add tests and will fox the FIXME things I have noticed.

[schlenk]

Parameter should be sub not uid.

[schlenk]

Looks mostly reasonable as an API.

One small point that might be problematic is the fact, that the API assumes the authn event is always stored serialized as json, when iterating to find the sids for an uid. Maybe it would be better to have a two layer api, a facade that takes AuthnEvent objects and handles serialization/deserialization and the string based set/get interface for implementers to override.

[tpazderka]

The AuthnEvent stuff is already handled in SessionDB directly: create_authz_session serializes the AuthnEvent to json, so nothing else shoudl be possible.

This is already two levels deep API (which I don't like...) and adding another layer doesn't look like a good idea to me.

[schlenk]

I see your point. Agreed, we do not want even more layers there. The responsibilities are not really well factored between the TokenFactories, SessionDB and the persistence layer. Your other change to cleanup the RefreshDB/Token stuff goes in the same direction of cleaning this up.

This new API works as a better persistence option.

I think a few more of the get_ methods taking a uid parameter and a few other methods of the current SessionDB could be moved to the new class as well, as those look a lot like DB access patterns one might like to optimize e.g. get_client_ids_for_uid(). Making the persistance part less dumb about what it stores.

[tpazderka]

Yes, that is a good point. We should probably move that over as well.

The question is if we want to force users to implement that methods, or if we implement a dumb lookup and let them optimize it if they find the need to?

I am leaning towards the second option.

[schlenk]

I think the second option (provide some dumb/clear to read default implementations) is best. Then users with a smart backend can optimize and people with a dumb backend just use it as is.

[codecov-io]

Codecov Report

Merging #660 into master will increase coverage by 0.03%.
The diff coverage is 84.05%.

@@            Coverage Diff             @@
##           master     #660      +/-   ##
==========================================
+ Coverage   62.02%   62.06%   +0.03%     
==========================================
  Files          62       62              
  Lines       11331    11364      +33     
  Branches     2010     2014       +4     
==========================================
+ Hits         7028     7053      +25     
- Misses       3722     3730       +8     
  Partials      581      581

Impacted Files	Coverage Δ
src/oic/oic/provider.py	67.84% <100%> (-0.35%)	⬇️
src/oic/utils/sdb.py	84.74% <83.82%> (+1.71%)	⬆️
src/oic/utils/keyio.py	70.39% <0%> (-0.96%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 75275e9...f027a0a. Read the comment docs.

[tpazderka]

OK, I have moved more stuff, I think that should be all. I will work on tests of the new code.

[tpazderka]

OK, tests are finished. For some reason, Codecov is not picking up the new coverage but it should be mostly covered now, so please review.

[schlenk]

LGTM.