search

roddhjav / pass-import

A pass extension for importing data from most existing password managers
https://www.passwordstore.org/
GNU General Public License v3.0
809 stars 89 forks source link

Fix: Support 1PIF file format for 1Password v7.9.11 (macOS) #207

Closed davidandreoletti closed 9 months ago

davidandreoletti commented 10 months ago

Rule out CSV file format when a data line looks like json formatted line

With 1Password 7.9.11, pass-import erroneous detects a 1PIF file as a CSV file. This PR improves the CSV detection capability by checking if the CSV file look like a valid JSON file. IFF it does, then the file is not considered a CSV file.

Removed empty line when reading 1PIF file

1PIF exported from 1Password 7.9.11 contains an extra empty line at the end of the file. This empty line is now removed.

Encoded special characters properly when reading JSON strings

1PIF exported from 1Password 7.9.11 does not encode special character. These characters are now properly encoded.

PS: thank you for starting this project and help reduce vendor lock-in. 1Password :-(

roddhjav commented 10 months ago

Thanks for your PR.

Don't worry about the failed CI, this is unrelated to you PR. However, when I run the test locally, test_main_decrypt_without_manager fail with:

Error in tests.main.test_pass.TestMainPass.test_main_decrypt_without_manager
  File "/usr/lib/python3.11/unittest/case.py", line 57, in testPartExecutor
    yield
  File "/usr/lib/python3.11/unittest/case.py", line 623, in run
    self._callTestMethod(testMethod)
  File "/usr/lib/python3.11/unittest/case.py", line 579, in _callTestMethod
    if method() is not None:
       ^^^^^^^^
  File "/home/***/pass-import/tests/main/test_pass.py", line 124, in test_main_decrypt_without_manager
    self.main(cmd)
  File "/home/***/pass-import/tests/__init__.py", line 213, in main
    pass_import.__main__.main()
  File "/home/***/pass-import/pass_import/__main__.py", line 456, in main
    cls_import = detectmanager(conf)
                 ^^^^^^^^^^^^^^^^^^^
  File "/home/***/pass-import/pass_import/__main__.py", line 307, in detectmanager
    pm = detect.manager(to_detect)
         ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/***/pass-import/pass_import/auto.py", line 147, in manager
    if file.is_format():
       ^^^^^^^^^^^^^^^^
  File "/home/***/pass-import/pass_import/formats/csv.py", line 91, in is_format
    _ = next(self.reader)
        ^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3.11/csv.py", line 111, in __next__
    row = next(self.reader)
          ^^^^^^^^^^^^^^^^^

Can you have a look a it. You can run the test suite locally or simply test it manually with pass import tests/assets/db/andotp.gpg

davidandreoletti commented 10 months ago

File "/usr/lib/python3.11/csv.py", line 111, in next row = next(self.reader)

@roddhjav Running the whole test suites locally depends on a lot of password manager binaries available on PATH. As a result, many test suite failures unrelated to this PR show up. Not a criticism, just a remark ;-)

This issue got buried in the noise. I pushed a fix.

Hopefully, CI will confirm its the last one.

roddhjav commented 9 months ago

Thanks a lot. Merged!

I have added so exception to not run all tests if some deps are not meet.