Remove dependency: social-auth-storage-sqlalchemy

[Theodlz]

As you might know, social-auth-storage-sqlalchemy is not maintained anymore, and the last release was in 2017... Since we migrated to sqlalchemy 2.0, we've been experiencing some issues with authentication, hard for us to debug, as these issues happen in social_core, when it would call methods from the Models coming from social-auth-storage-sqlalchemy.

This first PR (of probably a few) aims to redefine the classes existing in social-auth-storage-sqlalchemy, as well as the init_storage method from social-tornado, in an effort to get more visibility and control on the auth issues, and to avoid any future breaking changes that with social-auth-storage-sqlalchemy that hasn't been maintained in years.

EDIT: Similarly, social-tornado isn't maintained. The package is only few hundreds of lines, and it would make sense to have it copied here as well for the same reasons.

[stefanv]

@Theodlz This makes sense, but why not try to contribute this upstream first?

[Theodlz]

Hi @stefanv, that is also an option of course. So the reason is that we can't reproduce the errors locally, meaning that I can't develop my own version of the package and test what changes improve the auth instability in Fritz. So if I contributed to upstream, I will probably do: 1. PR, 2. release, 3. pin in baselayer, 4. pin in SP, 5. pin in Fritz, 6. deploy, and redo all of that if the bugs are still there. Whereas if this is in baselayer directly, I avoid the PRs + releases (which could also just not happen, or take a long time to go in).

The changes in this PR aren't making any critical changes that would fix any auth bugs yet, so for now I don't have anything to contribute to upstream anyway. The goal here is just to have it in baselayer so that we can easily add bugfixes. I will keep working on it and flag (with comments) which methods of social-auth-storage-sqlalchemy have been identified as problematic in prod.

Let me know if that makes sense.

[stefanv]

Makes sense, and happy for you to experiment here, if you are also committing to taking changes upstream afterwards. Let me know when you are ready for merge.

[Theodlz]

I agree that once we have a stable version of our own, contributing to upstream is best.

I'll keep monitoring the issues on prod and I will probably add a few commits to this PR before it can go in.

Thanks!

[mcoughlin]

@Theodlz @stefanv This might be a good thing to try to get in this week?

[Theodlz]

@mcoughlin I tried running the lastest Skyportal commit with this, and it works just fine with real Google authentication. If @stefanv has reviewed it and is fine merging it, we can proceed with this.

EDIT: Making one last verification before we merge, please wait

EDIT2: When pinning the new version of baselayer in SkyPortal, there will be 2 imports to update @mcoughlin. Otherwise works just fine.

[mcoughlin]

@stefanv Can you review / merge?

[stefanv]

This looks fine with me, but @Theodlz we already have psa.py, so let's add those classes in there, and then we can configure them with clearer names: baselayer.psa.TornadoStorage and baselayer.psa.TornadoStrategy.

Once you've made that change, you can go ahead and merge.

[Theodlz]

Great @stefanv! I'll tag you here once I'm done moving it to psa.py (I'll probably do that later this afternoon)

[Theodlz]

@stefanv just to double check, you want me to move everything I added (auth.py, social.py, strategy,py) in PSA, or only specific classes?

[Theodlz]

@stefanv just to double check, you want me to move everything I added (auth.py, social.py, strategy,py) in PSA, or only specific classes?

The auth.py handler is probably in the right place already

[Theodlz]

So I ended up moving strategy.py and social.py into psa.py

[Theodlz]

@stefanv any chance you can have a quick look? We are having more and more issues in production, and getting this in would make it easier to figure it out (and even might fix it).

[stefanv]

Looks good, thanks!