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

SM-874: Fix Python Integration #229

Merged
merged 24 commits into from
Nov 27, 2023
Merged

SM-874: Fix Python Integration #229

merged 24 commits into from
Nov 27, 2023

Conversation

coltonhurst
Copy link
Member

@coltonhurst coltonhurst commented Sep 12, 2023

Type of change

- [x] Bug fix
- [ ] New feature development
- [ ] Tech debt (refactoring, code cleanup, dependency upgrades, etc)
- [ ] Build/deploy pipeline (DevOps)
- [ ] Other

Objective

The Python integration is out of date and is not currently working. This PR fixes the Python integration by adding the ability to log in via access token, which is currently the supported way to interact with the Bitwarden Secrets Manager SDK. This PR also adds a ProjectsClient to bitwarden_client.py for easy use, and updates the error handling on Project or Secret deletes in the SDK.

Code changes

  • crates/sdk-schemas/src/main.rs: We need the AccessTokenLoginResponse struct to be generated so access token auth is available to integrations
  • languages/python/BitwardenClient/bitwarden_client.py:
    • Import the following from .schemas:
      • AccessTokenLoginRequest
      • AccessTokenLoginResponse
      • ResponseForAccessTokenLoginResponse
    • Define a new function access_token_login to support authenticating with an access token
    • Add the project_ids parameter to the create and update functions for SecretsClient
    • Add a ProjectsClient class and projects() to the BitwardenClient
    • Removed the password login methods, as they are not supported
    • Removed the imports that are not needed, and add one that are (like sys, etc.)
  • languages/python/README.md: Update the readme instructions, give more examples
  • languages/python/login.py: Renamed login.py to example.py
  • languages/python/example.py: Update the example to showcase auth with an access token

Before you submit

  • Please add unit tests where it makes sense to do so (encouraged but not required)

@coltonhurst coltonhurst self-assigned this Sep 12, 2023
@bitwarden-bot
Copy link

bitwarden-bot commented Sep 12, 2023

Logo
Checkmarx One – Scan Summary & Details242e5ab7-ec19-4266-9a1a-1bcb4f805fd1

No New Or Fixed Issues Found

Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

Looking good. Couple of minor suggestions.

languages/python/login.py Outdated Show resolved Hide resolved
languages/python/login.py Outdated Show resolved Hide resolved
@coltonhurst coltonhurst marked this pull request as ready for review September 15, 2023 14:49
Thomas-Avery
Thomas-Avery previously approved these changes Sep 18, 2023
Copy link
Contributor

@Thomas-Avery Thomas-Avery left a comment

Choose a reason for hiding this comment

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

LGTM

languages/python/login.py Outdated Show resolved Hide resolved
Thomas-Avery
Thomas-Avery previously approved these changes Sep 19, 2023
Hinton
Hinton previously approved these changes Sep 21, 2023
@coltonhurst coltonhurst dismissed stale reviews from Thomas-Avery and Hinton via 8f0885e October 18, 2023 13:27
@coltonhurst coltonhurst requested a review from Hinton October 23, 2023 19:53
Thomas-Avery
Thomas-Avery previously approved these changes Oct 23, 2023
Copy link
Member

@dani-garcia dani-garcia left a comment

Choose a reason for hiding this comment

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

Two small comments, otherwise I'd say it looks good

languages/python/BitwardenClient/bitwarden_client.py Outdated Show resolved Hide resolved
languages/python/example.py Outdated Show resolved Hide resolved
dani-garcia
dani-garcia previously approved these changes Nov 16, 2023
@coltonhurst coltonhurst merged commit 92a67b1 into master Nov 27, 2023
46 checks passed
@coltonhurst coltonhurst deleted the sm/sm-874 branch November 27, 2023 16:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants