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

[IMP] added option for TOTP/2FA bypass for admin passkey #550

Closed
wants to merge 1 commit into from

Conversation

codeagencybe
Copy link
Member

I have added a new option to bypass TOTP/2FA for admin passkey.
In the odoo.conf one needs to add the new parameter as below.
By default, this new option is always false, so it will not bypass TOTP/2FA if it has been set on a user login.

New odoo.conf parameter:
auth_admin_passkey_ignore_totp = True

I have tested this new feature in several production setups and it's working perfectly.

…otp configuration option

🐛 fix(res_users.py): set ignore_totp session variable based on the value of auth_admin_passkey_ignore_totp configuration option
The README.rst file has been updated to include documentation for the new configuration option `auth_admin_passkey_ignore_totp`. When enabled, this option allows the 2FA (Two-Factor Authentication) feature to be ignored.

In the res_users.py file, the `ignore_totp` session variable is now set based on the value of the `auth_admin_passkey_ignore_totp` configuration option. If the option is enabled, the `ignore_totp` session variable is set to True. This ensures that the `_mfa_url` method returns None when `ignore_totp` is True, effectively bypassing the 2FA check.
@legalsylvain
Copy link
Contributor

Hi @codeagencybe could you take a look on red CI ?

@bofiltd
Copy link

bofiltd commented Dec 17, 2023

@codeagencybe Thanks for this contribution. Makes sense.
Can you please re-trigger tests?

@bosd
Copy link
Contributor

bosd commented Feb 25, 2024

@codeagencybe Can you fix pre-commit?

@astirpe
Copy link
Member

astirpe commented Mar 12, 2024

@codeagencybe will you attend the comments above, to make pre-commit and tests green?

self._send_email_passkey(users[0])
else:
raise

def _mfa_url(self):

Choose a reason for hiding this comment

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

Module should inherit auth_totp_mail_enforce to bypass.

Copy link
Member

Choose a reason for hiding this comment

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

As an alternative to inheriting auth_totp_mail_enforce, we could make use of a glue module to combine this PR and auth_totp_mail_enforce. Here is a proposal of a glue module for V17: #625

@codeagencybe
Copy link
Member Author

Hello all.

Apologies for late reply, seems like I don't receive any notifications from GitHub except from the last message from Cas.

I will review everything shortly and update.
Thanks!

@@ -74,6 +75,12 @@ def _check_credentials(self, password, env):
password = hashlib.sha512(password.encode()).hexdigest()

if password and file_password == password:
request.session['ignore_totp'] = config.get("auth_admin_passkey_ignore_totp", False)
Copy link
Member

Choose a reason for hiding this comment

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

@codeagencybe

I ported your PR to V17, here: #624

To make the existing tests passing with success, I added this change f138da2
This way we avoid that the session is being written if it's not existing.

I think you need to implement the same here as well.

Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jul 14, 2024
@github-actions github-actions bot closed this Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants