Fix for situations with multiple workers and persistent storage

[davidkhess]

This is a fix for situations where there are multiple frontend worker processes (different address spaces) and there is a working persistent storage on the services layer.

This code was assuming that a client object was already existent in memory if a finalize call is made referring to it by issuer. But this may not be the case if begin() happened in one worker and finalize() happens in a different one.

[peppelinux]

Thank you @davidkhess
I'd merge this now but I'd like to involve you in the following finding:

1. We should have a review on this commits, it looks very good to me but still no time to doing an accurate test
2. this is more important than the previous finding... We should enable a decent CI on this repository.

Regarding the 2nd finding.
If you would like to enjoy in this I can suggest you to take a look to some Tox/travis CI w have already configured in the jwtconnect stack, eg:

• https://github.com/IdentityPython/JWTConnect-Python-OidcService/blob/master/tox.ini
• https://github.com/IdentityPython/JWTConnect-Python-OidcMsg/blob/master/tox.ini
• https://github.com/IdentityPython/JWTConnect-Python-OidcMsg/blob/master/.travis.yml

hope that you'll enjoy with this task, this would help us to evaluate and merge the PR in less time.
That's not mandatory, I just want to tell you that I don't know when I'll be able to activate CI and then merge your PR

[davidkhess]

@peppelinux I would love to help but I unfortunately don't have the time or resources to contribute at that level at the moment.

Based on the error message though, seems like it's just a version conflict with the installed OpenSSL in your CI host?

Anyway, best of luck.

[davidkhess]

@rohe I can't tell by looking at the git history, but it seems like this merge request somehow got reverted in some refactoring that was done? At least, the change no longer appears to be on master.

[rohe]

We rewrote how to handle persistence a while ago.
There where many reasons for not having a persistent layer in OidcRP but instead have it outside.
OidcRP now only does load/dump/clear on dynamic information.
If you want to have multiple worker you have figure out how to share state between them.

If you look at example/satosa/example.py you can see a starting point.

[davidkhess]

Yes, I saw to my surprise that in what looked like a minor version release of OidcMSG the SQLAlchemy storage was removed. That seems like a breaking change, no?

Regardless, this bug exists on master and will present itself even if I provide my own storage implementation. If begin() starts in one worker and and finalize() occurs in another this code on master will fail even in the presence of an external storage solution without this fix.

In other words, this fix isn't related to storage directly but is required for a deployment with external storage to work at all.

P.S. I don't find the path example/satosa/example.py in this repo....?

[davidkhess]

@rohe I've been searching through the entire IdentityPython organization for over an hour now and can't find any documentation about nor examples of how to integrate an external storage into OidcRP (since my previous solution using OidcMsg no longer works due to it dropping that support).

Can you point me to something that explains what I need to do to migrate?

P.S. I really appreciate the work you all have been doing on these packages, but the cost of using them is turning out to be surprisingly high for me....

[peppelinux]

hi @davidkhess

here an implementation of a satosa oidc frontend based on flush/load/dump methods of oidcop session manager
https://github.com/UniversitaDellaCalabria/SATOSA-oidcop/blob/6731989dad5e95d862f0312b6ef2dad92544788b/satosa_oidcop/idpy_oidcop.py#L155

[davidkhess]

@peppelinux thank you. However, I'm just trying to use OidcRP and the link you gave me doesn't even have an import from this package into it. So, I'm left with more questions than answers as to how that code and the references in it relates to migrating my external storage from the previous solution (via OidcMsg) to what OidcRP requires now.

[rohe]

I broke my arm yesterday (the right one and I right-handed ) so my typing is very slow.
I'll try to write some documentation in the next couple of days.

[davidkhess]

Oh my! No need to rush. I’ll make do.

I hope you are feeling better soon.