Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MFA #405

Merged
merged 18 commits into from
Sep 9, 2024
Merged

MFA #405

merged 18 commits into from
Sep 9, 2024

Conversation

dantownsend
Copy link
Member

@dantownsend dantownsend commented Aug 16, 2024

Related to piccolo-orm/piccolo_api#292

The workflow for MFA is pretty complex, so rather than reimplementing all of the UI in Vue, we'll just mount the MFA endpoint, and send the user there.

So the only bit of Vue code which needs changing is the login form, which needs to render an MFA code input.

@dantownsend dantownsend added the enhancement New feature or request label Aug 16, 2024
piccolo_admin/endpoints.py Outdated Show resolved Hide resolved
piccolo_admin/endpoints.py Outdated Show resolved Hide resolved
piccolo_admin/endpoints.py Fixed Show fixed Hide fixed
@sinisaos
Copy link
Member

@dantownsend I tried your changes and they are great, I really like them. Everything works and I really like your solution for the login form.

@dantownsend
Copy link
Member Author

@sinisaos Thanks! I think it's almost there now.

@dantownsend dantownsend marked this pull request as ready for review September 9, 2024 12:55
@dantownsend
Copy link
Member Author

I'm going to release this. There are some things which need improving - I added issues to cover these.

I'll try it out in some of my apps.

Thanks again both for all the input. 🙇‍♂️

@dantownsend dantownsend merged commit dd10b39 into master Sep 9, 2024
11 checks passed
@sinisaos
Copy link
Member

sinisaos commented Sep 9, 2024

@dantownsend I don't know if that's a good idea, but can you add MFA related packages to requirements extras, just like in the Piccolo API. In the fresh env, an error is raised because packages related to MFA (qrcode, pyotp, cryptography, etc.) are missing. Thanks

@dantownsend
Copy link
Member Author

@sinisaos Yeah, I can see that being annoying.

The documentation needs to be quite a bit better too - will try and do that later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants