Skip to content

Commit

Permalink
rename seed_table to secret_table
Browse files Browse the repository at this point in the history
  • Loading branch information
@dantownsend
dantownsend committed Aug 9, 2024
1 parent 7424132 commit a2d7719
Showing 1 changed file with 4 additions and 4 deletions.
8 changes: 4 additions & 4 deletions piccolo_api/mfa/authenticator/provider.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
class AuthenticatorProvider(MFAProvider):

def __init__(
self, seed_table: t.Type[AuthenticatorSecret] = AuthenticatorSecret
self, secret_table: t.Type[AuthenticatorSecret] = AuthenticatorSecret
):
"""
:param seed_table:
Expand All @@ -18,13 +18,13 @@ def __init__(
certain functionality.
"""
self.seed_table = seed_table
self.secret_table = secret_table

async def authenticate_user(self, user: BaseUser, code: str) -> bool:
return await self.seed_table.authenticate(user_id=user.id, code=code)
return await self.secret_table.authenticate(user_id=user.id, code=code)

async def is_user_enrolled(self, user: BaseUser) -> bool:
return await self.seed_table.is_user_enrolled(user_id=user.id)
return await self.secret_table.is_user_enrolled(user_id=user.id)

async def send_code(self, user: BaseUser):
"""
Expand Down

2 comments on commit a2d7719

@sinisaos
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dantownsend I know this is the beginning of the implementation, but I already like it because it has some solutions that will help us later (eg if I understood correctly, is_user_enrolled will let us know if the user is using some MFAProvider or not and so we know which workflow the login page should use, changes to session_login() etc.).
When I was doing server side two-factor authentication, my workflow was: user registration -> redirect to qr code template and scan qr code -> redirect to login page and pass valid user credential -> redirect to page where we enter TOTP code from app authenticator (if the code is correct, it redirects to a protected page, if not, it returns an error).
I like that you changed the session_login() so that we can change workflow since we don't have registration in Piccolo Admin: if user has MFAProvider enabled login page and pass valid user credential -> redirect to page where we render QR code for scaning in authenticator app -> redirect to login page and pass valid user credential again -> redirect to page to enter TOTP code from app authenticator (if the code is correct, it redirects to a protected page, if not, it returns an error). If user has MFAProvider disabled we use standard user/password session auth. Sorry to bother you with a long comment, but I like your clever solution and look forward to your progress with this.

@dantownsend
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sinisaos Thanks! I've been considering different options for MFA, and I think just modifying the session_login endpoint is the simplest approach.

The session_login endpoint can be used via API (which is what Piccolo Admin does), or will render a HTML form if you do a GET request to it. I'm trying to modify it now, so if MFA is enabled, the HTML form will show the input for the TOTP code.

Once that is working and tested, we can integrate it with Piccolo Admin.

Please sign in to comment.