justinmayer / kagi

WebAuthn security keys and TOTP multi-factor authentication for Django
BSD 2-Clause "Simplified" License
91 stars 10 forks source link

Library design going forward - high and low level API expected? #60

Open sergei-maertens opened 1 year ago

sergei-maertens commented 1 year ago

First of all - thanks for this library, the demo project is really cool to test out the various features!

We're looking to integrate kagi in some serious projects, and one of the things I'm noticing with (most) MFA-related libraries is that they're either quite invasive or make it too easy to shoot yourself in the foot.

So, I'm hoping that kagi takes an approach where you have some drop-in "recipes"/functionality (which would be the current situation, like the kagi LoginView and then updating settings to point to the kagi login URL), which would serve the majority of package users probably, and then some lower-level tooling to wire things up yourself if need be.

For example, we need a way to be able to enforce MFA only in the admin (middleware checking the MFA auth status if request.path startswith reverse("admin:index")?) and redirect people to the setup/configuration page if that's not the case. BUT, since we also support OIDC login which has their own MFA setup, it should also be possible to disable this admin-mfa-enforcement.

Doing these checks whether MFA is required or not, whether the user has that enabled or not (I see some request.session stuff happening in your LoginView) seem to me as viable candidates for public utilities used in your recipes and exposed/documented for people using low-level interfaces.

Is this something you would be open to? I'm fairly hopeful we could contribute this from Maykin Media.

MarkusH commented 1 year ago

Hi @sergei-maertens, there are a few general changes we'd like to make to this library. One of them is adding support for django-rest-framework with serializers and views. As you can imagine, that would duplicate some of the logic that currently exists in forms and views.

So, yeah, if we can find a way to extract "validate this token" into something that's reusable and could possibly be used by people outside of any of Kagi's high-level features (views/forms/serialiezrs), that'd be great.

As for the MFA requirements that you are talking about, I see the following feature requests:

Does that list make sense to you, @sergei-maertens? Did I miss anything? Is there anything I misunderstood?

sergei-maertens commented 1 year ago

Sounds on point!

MarkusH commented 1 year ago

Sounds on point!

I've turned those feature requests into dedicated tickets. For those, the discussion can happen there. This ticket is only about an architectural re-design, e.g. to support django-rest-framework.