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

Clarify Error message when bitwarden vault not unlocked #5811

Merged
merged 6 commits into from
Jan 22, 2023

Conversation

Chr1s70ph
Copy link
Contributor

SUMMARY

The Error message of the Bitwarden lookup-plugin is not conclusive of what actually is the Error.
This PR adresses the issue, and tells the user to unlock the vault, instead of logging in. (Just logging in, still results in the error, because the vault is not unlocked)

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

bitwarden lookup plugin

ADDITIONAL INFORMATION

You can be logged into the Bitwarden-CLI, but it will still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in. If you run 'bw unlock' without being logged in, you are prompted to log in. This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.

You can be logged into the Bitwarden-CLI, but it can still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in.
If you run 'bw unlock' without being logged in, you are prompted to log in. 
This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.
@Chr1s70ph Chr1s70ph changed the title Clarify Error message when vault not unlocked Clarify Error message when bitwarden vault not unlocked Jan 10, 2023
Nobody needs negation
@ansibullbot
Copy link
Collaborator

cc @lungj
click here for bot help

@ansibullbot ansibullbot added docs lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) small_patch Hopefully easy to review labels Jan 10, 2023
@ansibullbot ansibullbot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR and removed small_patch Hopefully easy to review labels Jan 10, 2023
@russoz
Copy link
Collaborator

russoz commented Jan 11, 2023

Hi @Chr1s70ph
Thank you for the contribution! Please add a changelog fragment.

@russoz
Copy link
Collaborator

russoz commented Jan 11, 2023

I believe you need to rename the attribute logged_in in the mocked up objects in tests/unit/plugins/lookup/test_bitwarden.py

@ansibullbot ansibullbot added tests tests unit tests/unit and removed needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR labels Jan 11, 2023
@Chr1s70ph
Copy link
Contributor Author

@russoz I have added the changes :)

@felixfontein felixfontein added check-before-release PR will be looked at again shortly before release and merged if possible. backport-5 labels Jan 12, 2023
@github-actions
Copy link

github-actions bot commented Jan 13, 2023

Docs Build 📝

Thank you for contribution!✨

This PR has been merged and your docs changes will be incorporated when they are next published.

@felixfontein
Copy link
Collaborator

If nobody objects, I'll merge this in ~a week.

@ansibullbot ansibullbot added the stale_ci CI is older than 7 days, rerun before merging label Jan 21, 2023
@felixfontein felixfontein removed the check-before-release PR will be looked at again shortly before release and merged if possible. label Jan 22, 2023
@felixfontein felixfontein merged commit bf117c8 into ansible-collections:main Jan 22, 2023
@patchback
Copy link

patchback bot commented Jan 22, 2023

Backport to stable-5: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-5/bf117c839cdfbf5d6fd398df6750b53c71f8e16b/pr-5811

Backported as #5877

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

patchback bot pushed a commit that referenced this pull request Jan 22, 2023
* Clarify Error message when vault not unlocked

You can be logged into the Bitwarden-CLI, but it can still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in.
If you run 'bw unlock' without being logged in, you are prompted to log in.
This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.

* RM: negation

Nobody needs negation

* Update function name

* FIX: tests

* ADD: changelog

* Update changelogs/fragments/5811-clarify-bitwarden-error.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit bf117c8)
@patchback
Copy link

patchback bot commented Jan 22, 2023

Backport to stable-6: 💚 backport PR created

✅ Backport PR branch: patchback/backports/stable-6/bf117c839cdfbf5d6fd398df6750b53c71f8e16b/pr-5811

Backported as #5878

🤖 @patchback
I'm built with octomachinery and
my source is open — https://github.com/sanitizers/patchback-github-app.

@felixfontein
Copy link
Collaborator

@Chr1s70ph thanks for your contribution!
@russoz thanks for reviewing!

patchback bot pushed a commit that referenced this pull request Jan 22, 2023
* Clarify Error message when vault not unlocked

You can be logged into the Bitwarden-CLI, but it can still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in.
If you run 'bw unlock' without being logged in, you are prompted to log in.
This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.

* RM: negation

Nobody needs negation

* Update function name

* FIX: tests

* ADD: changelog

* Update changelogs/fragments/5811-clarify-bitwarden-error.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit bf117c8)
felixfontein pushed a commit that referenced this pull request Jan 22, 2023
…warden vault not unlocked (#5877)

Clarify Error message when bitwarden vault not unlocked (#5811)

* Clarify Error message when vault not unlocked

You can be logged into the Bitwarden-CLI, but it can still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in.
If you run 'bw unlock' without being logged in, you are prompted to log in.
This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.

* RM: negation

Nobody needs negation

* Update function name

* FIX: tests

* ADD: changelog

* Update changelogs/fragments/5811-clarify-bitwarden-error.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit bf117c8)

Co-authored-by: Christoph <[email protected]>
felixfontein pushed a commit that referenced this pull request Jan 22, 2023
…warden vault not unlocked (#5878)

Clarify Error message when bitwarden vault not unlocked (#5811)

* Clarify Error message when vault not unlocked

You can be logged into the Bitwarden-CLI, but it can still be locked. This took me several hours to debug, since every time I ran 'bw login' it told me, that I am already logged in.
If you run 'bw unlock' without being logged in, you are prompted to log in.
This clarifies the Error occurring and can drastically reduce debugging time, since you don't have to look into the source code to get an understanding of whats wrong.

* RM: negation

Nobody needs negation

* Update function name

* FIX: tests

* ADD: changelog

* Update changelogs/fragments/5811-clarify-bitwarden-error.yml

Co-authored-by: Felix Fontein <[email protected]>

Co-authored-by: Felix Fontein <[email protected]>
(cherry picked from commit bf117c8)

Co-authored-by: Christoph <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs lookup lookup plugin new_contributor Help guide this first time contributor plugins plugin (any type) stale_ci CI is older than 7 days, rerun before merging tests tests unit tests/unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants