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

Bitwarden updates #1547

Merged
merged 1 commit into from
Jan 14, 2025
Merged

Bitwarden updates #1547

merged 1 commit into from
Jan 14, 2025

Conversation

wintonzheng
Copy link
Contributor

@wintonzheng wintonzheng commented Jan 14, 2025

Important

Improved error handling in Bitwarden operations by ensuring organization and collection IDs are required, with appropriate error logging and redundant checks removed.

  • Error Handling:
    • In bitwarden.py, added checks in _get_sensitive_information_from_identity() and _get_credit_card_data() to raise BitwardenAccessDeniedError if both bw_organization_id and collection_id are not provided.
    • Removed redundant check for bw_organization_id and bw_collection_ids in get_credit_card_data().
  • Logging:
    • Added error logging in _get_credit_card_data() in bitwarden.py when no collection_id or organization_id is provided.
  • Workflow Service:
    • Removed redundant error raising in create_workflow_from_request() in service.py for missing bw_organization_id and bw_collection_ids.

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

<!-- ELLIPSIS_HIDDEN -->

> [!IMPORTANT]
> Adds `bitwarden_helper.py` for Bitwarden session management, updates error handling in `bitwarden.py`, and modifies command examples in `restart_task.py` and `restart_workflow.py`.
>
>   - **New Script**:
>     - Adds `bitwarden_helper.py` to `dev_scripts` for generating Bitwarden session keys and running custom commands.
>   - **Error Handling**:
>     - In `bitwarden.py`, adds error handling for missing organization or collection IDs in `_get_sensitive_information_from_identity()` and `_get_credit_card_data()`.
>   - **Scripts Update**:
>     - Removes hardcoded '7' from command examples in `restart_task.py` and `restart_workflow.py`.
>
> <sup>This description was created by </sup>[<img alt="Ellipsis" src="https://img.shields.io/badge/Ellipsis-blue?color=175173">](https://www.ellipsis.dev?ref=Skyvern-AI%2Fskyvern-cloud&utm_source=github&utm_medium=referral)<sup> for 1dd86e3142b63ae73e56869b7cb377f4f08f4c90. It will automatically update as commits are pushed.</sup>

<!-- ELLIPSIS_HIDDEN -->
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.

👍 Looks good to me! Incremental review on b3f815a in 23 seconds

More details
  • Looked at 51 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/services/bitwarden.py:580
  • Draft comment:
    Consider re-adding the check for bw_organization_id and bw_collection_ids here to maintain consistency with other functions like _get_credit_card_data and _get_sensitive_information_from_identity.
  • Reason this comment was not posted:
    Comment was not on a valid diff hunk.

Workflow ID: wflow_186RuMxxRj9YMcEF


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

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.

👍 Looks good to me! Reviewed everything up to b3f815a in 33 seconds

More details
  • Looked at 51 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 1 drafted comments based on config settings.
1. skyvern/forge/sdk/services/bitwarden.py:580
  • Draft comment:
    The check for bw_organization_id and bw_collection_ids was removed here, but similar checks were added in other functions. Consider adding a similar check here for consistency and to prevent potential access issues.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.

Workflow ID: wflow_oEuRKSVCU17up12X


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@suchintan suchintan merged commit 111d1e3 into main Jan 14, 2025
6 checks passed
@suchintan suchintan deleted the suchintan.bitwarden-updates branch January 14, 2025 06:28
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.

3 participants