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

Use domain while getting totp instead of url #901

Merged
merged 1 commit into from
Oct 3, 2024

Conversation

ykeremy
Copy link
Contributor

@ykeremy ykeremy commented Oct 3, 2024

Important

Change _get_secret_value_from_url() in bitwarden.py to use domain instead of url for TOTP retrieval.

  • Behavior:
    • Change in _get_secret_value_from_url() in bitwarden.py to use domain instead of url for TOTP retrieval.
    • This change aims to improve the specificity of TOTP code retrieval by focusing on domain rather than full URL.

This description was created by Ellipsis for 6ffd306. It will automatically update as commits are pushed.

@ykeremy ykeremy added the sync label Oct 3, 2024
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 6ffd306 in 16 seconds

More details
  • Looked at 13 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 0 drafted comments based on config settings.

Workflow ID: wflow_M4cduQ5J1NwyBhzN


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -193,7 +193,7 @@ async def _get_secret_value_from_url(

# TODO (kerem): To make this more robust, we need to store the item id of the totp login item
# and use it here to get the TOTP code for that specific item
totp_command = ["bw", "get", "totp", url, "--session", session_key]
totp_command = ["bw", "get", "totp", domain, "--session", session_key]
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider using tldextract.extract(url).registered_domain instead of tldextract.extract(url).domain to ensure the domain includes the suffix (e.g., 'example.co.uk' instead of just 'example'). This will help avoid potential conflicts with similar domain names.

@ykeremy ykeremy merged commit 9e50456 into main Oct 3, 2024
2 checks passed
@ykeremy ykeremy deleted the ykeremy/fix-bitwarden-totp-use-domain-name branch October 3, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant