Working towards SSO support

[hubert-springbok]

Hi @dantownsend - kudos for a great repository!

My team is looking to escape Django as we move to a new project and we've been pleasantly surprised by piccolo after initial experimentation. However, one thing that is a showstopper for us is ability to set up exclusively SSO login - no password auth is allowed.

We had a look at AdminRouter and we believe that:

• allowing custom auth middleware instead of the default one should be easy and work just like our main FastAPI app
• current implementation of __init__ means that we need to copy out and maintain almost 500 lines of code

We'd like to offer a PR where we maintain the exact same functionality and implementation of AdminRouter, but break down the init into several helper functions, which can then be re-used when we subclass it.
This improved encapsulation step should allow us to get going and help other people here, including discussion 412 and MR 269

Would you accept such a PR?

[Skelmis]

I'd be keen to see SSO support added and happy to hop in at the review stages to help get it moving along.

[dantownsend]

That's a good point with the AdminRouter.__init__ method - it is pretty massive, which makes subclassing harder.

I think breaking that up into smaller methods is a good idea.

I am interested to see ways of decoupling the auth from Piccolo Admin.

[sinisaos]

It would be great if the default session auth (I think it's great, but obviously not appropriate in every situation) could be somehow easily replaced, but also work well with the admin UI. So the user could use custom authentication (as in this case) or could use Piccolo JWTAuth or unauthenticated user etc. I think it would also be good if we did a small refactor and split the large endpoints.py into several smaller files (for TableConfig, FormConfig, etc.) as it would make everything more readable and maintainable. Something like this

[sinisaos]

@dantownsend Would it be worth splitting endpoints.py into multiple files like in the branch from the previous comment?

[hubert-springbok]

I'd say it is, but you need to be careful with backwards compatibility: people import and customise the create_ method and the AdminRouter classes, so they expect to import them from this file...

[sinisaos]

@hubert-springbok Yes, you're right. I hadn't thought about backwards compatibility. Thanks.

[hubert-springbok]

@Skelmis , please have a look at PR 442.

With this setup, it becomes much easier to:

• avoid creation of certain password-centric endpoints: mfa, password change etc.
• change or drop the middleware on the API sub-app
• get rid of the auth_table and session_table entirely, if needed

Once you approve these changes, we can look at:

• extracting a base class out of this
• providing an implementation focusing on OAuth