Skip to content

Commit

Permalink
Use get(..) instead of [..] for safe lookup of value (Fixes ansible…
Browse files Browse the repository at this point in the history
…-collections#7240) (ansible-collections#7241)

A OnePassword field item might not have a value (property) when the user has omitted it (on purpose).
  • Loading branch information
wkleinheerenbrink authored and Eric Trombly committed Oct 25, 2023
1 parent 17f1477 commit 7bbfc69
Show file tree
Hide file tree
Showing 5 changed files with 118 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
bugfixes:
- onepassword - fix KeyError exception when trying to access value of a field that is not filled out in OnePassword item (https://github.com/ansible-collections/community.general/pull/7241).
8 changes: 4 additions & 4 deletions plugins/lookup/onepassword.py
Original file line number Diff line number Diff line change
Expand Up @@ -462,10 +462,10 @@ def _parse_field(self, data_json, field_name, section_title=None):
# If the field name doesn't exist in the section, match on the value of "label"
# then "id" and return "value"
if field.get("label") == field_name:
return field["value"]
return field.get("value", "")

if field.get("id") == field_name:
return field["value"]
return field.get("value", "")

# Look at the section data and get an indentifier. The value of 'id' is either a unique ID
# or a human-readable string. If a 'label' field exists, prefer that since
Expand All @@ -475,10 +475,10 @@ def _parse_field(self, data_json, field_name, section_title=None):
if section_title == current_section_title:
# In the correct section. Check "label" then "id" for the desired field_name
if field.get("label") == field_name:
return field["value"]
return field.get("value", "")

if field.get("id") == field_name:
return field["value"]
return field.get("value", "")

return ""

Expand Down
42 changes: 42 additions & 0 deletions tests/unit/plugins/lookup/onepassword_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,47 @@ def load_file(file):
"expected": ["first value"],
"output": load_file("v2_out_03.json")
},
{
# Request data from an omitted value (label lookup, no section)
"vault_name": "Test Vault",
"queries": ["Omitted values"],
"kwargs": {
"field": "label-without-value",
},
"expected": [""],
"output": load_file("v2_out_04.json")
},
{
# Request data from an omitted value (id lookup, no section)
"vault_name": "Test Vault",
"queries": ["Omitted values"],
"kwargs": {
"field": "67890q7mspf4x6zrlw3qejn7m",
},
"expected": [""],
"output": load_file("v2_out_04.json")
},
{
# Request data from an omitted value (label lookup, with section)
"vault_name": "Test Vault",
"queries": ["Omitted values"],
"kwargs": {
"field": "section-label-without-value",
"section": "section-without-values"
},
"expected": [""],
"output": load_file("v2_out_04.json")
},
{
# Request data from an omitted value (id lookup, with section)
"vault_name": "Test Vault",
"queries": ["Omitted values"],
"kwargs": {
"field": "123345q7mspf4x6zrlw3qejn7m",
"section": "section-without-values",
},
"expected": [""],
"output": load_file("v2_out_04.json")
},
],
}
67 changes: 67 additions & 0 deletions tests/unit/plugins/lookup/onepassword_fixtures/v2_out_04.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
{
"id": "bgqegp3xcxnpfkb45olwigpkpi",
"title": "OmittedValues",
"version": 1,
"vault": {
"id": "stpebbaccrq72xulgouxsk4p7y",
"name": "Private"
},
"category": "LOGIN",
"last_edited_by": "WOUTERRUYBH7BFPHMZ2KKGL6AU",
"created_at": "2023-09-12T08:30:07Z",
"updated_at": "2023-09-12T08:30:07Z",
"additional_information": "fluxility",
"sections": [
{
"id": "7osqcvd43i75teocdzbb6d7mie",
"label": "section-without-values"
}
],
"fields": [
{
"id": "username",
"type": "STRING",
"purpose": "USERNAME",
"label": "username",
"value": "fluxility",
"reference": "op://Testcase/OmittedValues/username"
},
{
"id": "password",
"type": "CONCEALED",
"purpose": "PASSWORD",
"label": "password",
"value": "j89Dyb7psat*[email protected]_V_xtMhrn37rQ_2uRYoRiozj6TjWVLy2pbfEvjnse",
"entropy": 427.01202392578125,
"reference": "op://Testcase/OmittedValues/password",
"password_details": {
"entropy": 427,
"generated": true,
"strength": "FANTASTIC"
}
},
{
"id": "notesPlain",
"type": "STRING",
"purpose": "NOTES",
"label": "notesPlain",
"reference": "op://Testcase/OmittedValues/notesPlain"
},
{
"id": "67890q7mspf4x6zrlw3qejn7m",
"type": "URL",
"label": "label-without-value",
"reference": "op://01202392578125/OmittedValues/section-without-values/section-without-value"
},
{
"id": "123345q7mspf4x6zrlw3qejn7m",
"section": {
"id": "6hbtca5yrlmoptgy3nw74222",
"label": "section-without-values"
},
"type": "URL",
"label": "section-label-without-value",
"reference": "op://01202392578125/OmittedValues/section-without-values/section-without-value"
}
]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
SPDX-License-Identifier: GPL-3.0-or-later
SPDX-FileCopyrightText: 2022, Ansible Project

0 comments on commit 7bbfc69

Please sign in to comment.