Make framework integration easier

[dwt]

Is your feature request related to a problem? Please describe.

I'm currently building framework integration for Zope (and will build Pyramid next). What's frustrating me is that the integration layer with these frameworks only very loosely specifies what is required of these frameworks to actually interoperate with Authlib.

Describe the solution you'd like

I've started the Zope layer from a refactoring of the flask layer, to get started, and in the process pulled out an FramworkIntegration object, that only has the job to describe what authlib needs from the framework. That way it is much simpler to know what has to be implemented to make things work, without having to understand so much about how authlib works. Have a look at it here. https://github.com/zms-publishing/zope.openid-connect/tree/master/zope/openid_connect/authlib_integration

Of course it is still very rough, but the Idea here is to have a delegate object, that subclasses the Framework-Integration Interface, that has methods to access session and cache values and documents what kind of requirements these sessions / caches have to be viable (i.e. these values can not be user visible, but these can...). Also how to get at form or query arguments from a request, etc.

There will maybe be different interfaces to support sync and async operation, and probably some optional methods to support stuff like Flasks partially configured state (app factory pattern). See the repo for some rough sketches.

Describe alternatives you've considered

I've tried to adapt the code from the flask example directly and found it very hard as I had to understand quite a bit about authlib to make that happen. Maybe now I do, but having a simple delegate object that just encapsulates how the specific framework accomplishes things was in the end much more viable.

Additional context

I'd like to add, that having such a delegate pattern, should also make unit testing much simpler, as it is quite easy to fake such an interface without having to bring up / in the actual frameworks.

What I'm currently having is a simple delegate object, but this could also become a factory and holder of the Authlib objects, allowing it to make callbacks - not sure what is the best way here yet.

Also, this is very much still a work in progress, but I wanted to a) get feedback and b) see how you react, and if maybe that is something that can be extracted into a project in the authlib organisation.

[dwt]

Also related to #54 probably.

[dwt]

@lepture ping?

[lepture]

I don't understand the problem. As I looked at your code, I can't see the improvement. Currently, all framework client integrations are based on code in _client. It is started with _ for a reason, Authlib has changed its structure in 0.13, and I'm not sure if the structure is good enough, I'm still improving it.

And also, there are some private methods which can be public, but they are written as private, because I'm not sure if it is the right time to public them.

I've tried to adapt the code from the flask example directly and found it very hard as I had to understand quite a bit about authlib to make that happen.

First, zope doesn't look like Flask. Most frameworks will pass a request instance. Maybe you should take a look at the Django client as an example.

And yes, it is better to understand Authlib before writing the framework integrations. Just like I have to understand each framework to make integrations for framework too.

The real problem here is that we don't have a documentation for developers to write framework integrations. That documentation may happen in the future and that documentation will be available before v1.0.

[dwt]

@lepture Thanks for taking the time to look at the code. I appreciate that.

Maybe it helps if I formulate the Goal I had in the refactoring more clearly. I noticed that in the different framework adapters there is a lot of code in common, but at the same time, even the generic implementations where assuming some things about the actual implementation of the frameworks. So for example base_app.py assumes that request.session exists, and has a dict-like interface.

At the same time, those interfaces and interface names don't specify what the code actually expects from the interface (i.e. is it public, i.e. can the browser see those values, does it matter if they are visible, does it matter if they are changeable by the browser). Of course you could write that down as documentation, and probably you should, but I think that would be much better documented on an interface that a specific adapter object for that framework has to provide. Much like for example the Flask and Zope interfaces in my code try (though very rough).

That being said - I don't hang on to the code, extracting that interface was the fastest way for me to adapt something to get Zope to work - so if you have a better version of the integration planned - do go ahead.

I still thin that it will be very worthwhile to isolate your code completely from assumptions like that a request has a .session that has dict like behaviour.

Also I think that such a clear border between authib and framework code would make testing much easier, as it gives a more clear boundary where (simple) mocks can be used.

To move this forward: Do you see what interface for the framework integration I'm trying to go towards? What do you think about that?

[lepture]

@dwt I got it. Our current _client is designed to be used by Django, Flask and Starlette, and it works well. But to make it better for other frameworks, we need to improve the interface to make no assumptions. And also, we can rename _client into base_client, so that other frameworks can inherit from it without fear.

We can make it happen in v0.14.

[dwt]

@lepture as I've now got some more to work on this - how do we continue with that? Do you want to change the API of _client? If so how?

I see two ways moving the API forward, there is

1. Adding more methods to be overridden by subclasses e.g. at least {get,set}_session_value(...), to allow shared code to access framework specific functionality without knowing how that actually works.
2. Looking at the code of the different (synchronous) *Cleint classes, I still really like the idea, of pulling out a FrameworkInteraction class (and assorted subclasses) that just wrap the framework differences (how to get/set session values, create redirects, and getting config values.

Regardless of the approach chosen, the generic code cannot assume request has a dict-like session interface.

So how can I help move this forward? I can provide a pull request that delegates {get,set}_session_value(...) to the subclass to override as a start - though of course I would really like the (slightly) bigger refactoring to pull out the FramworkIntegration Interface as a common mock point.

[lepture]

@dwt oh, let's try FrameworkInteraction at first.

[lepture]

It is available in v0.14

[dwt]

Wow, this looks really good, will give it a go as soon as I have time to work on this again (sadly, currently completely occupied with completely boring and urgent email administration stuff)