Add Proton Pass importer#1438
Conversation
alexbakker
left a comment
alexbakker
left a comment
There was a problem hiding this comment.
Thanks for submitting this! I'd like to see a couple of touch-ups before we merge this.
Also, a couple of general style notes:
{goes at the end of the line, not on a new line.- Because we prefix private fields with an underscore, there's almost never a need to access them through
this. - Please leave out trivial comments like
// Unzipand// Read file. - Insert a space where appropriate. So, not
if(true){, butif (true) {.
| _importers.add(new Definition("Google Authenticator", GoogleAuthImporter.class, R.string.importer_help_google_authenticator, true)); | ||
| _importers.add(new Definition("Microsoft Authenticator", MicrosoftAuthImporter.class, R.string.importer_help_microsoft_authenticator, true)); | ||
| _importers.add(new Definition("Plain text", GoogleAuthUriImporter.class, R.string.importer_help_plain_text, false)); | ||
| _importers.add(new Definition("Proton Pass", ProtonPassImporter.class, R.string.importer_help_proton_pass, true)); |
There was a problem hiding this comment.
Your importer does not support reading directly from the Proton Pass' internal storage, so the boolean at the end should be false.
| @Override | ||
| protected State read(InputStream stream, boolean isInternal) throws DatabaseImporterException { | ||
| // Unzip | ||
| ZipInputStream zis = new ZipInputStream(stream); |
There was a problem hiding this comment.
Wrap this in a try-with-resources statement.
| BufferedReader br = new BufferedReader(new InputStreamReader(zis)); | ||
| StringBuilder json = new StringBuilder(); | ||
| String line; | ||
| while((line = br.readLine()) != null){ | ||
| json.append(line); | ||
| } | ||
| br.close(); |
There was a problem hiding this comment.
Replace this block with IOUtils.readAll.
| br.close(); | ||
|
|
||
| // Parse JSON | ||
| JSONTokener tokener = new JSONTokener(json.toString()); |
There was a problem hiding this comment.
You can instantiate a JSONObject from the string directly, no need for a JSONTokener.
| <string name="importer_help_authy">Supply a copy of <b>/data/data/com.authy.authy/shared_prefs/com.authy.storage.tokens.authenticator.xml</b>, located in the internal storage directory of Authy.</string> | ||
| <string name="importer_help_andotp">Supply an andOTP export/backup file.</string> | ||
| <string name="importer_help_bitwarden">Supply a Bitwarden export/backup file. Encrypted files are not supported.</string> | ||
| <string name="importer_help_proton_pass">Supply a Proton pass export/backup zip. Encrypted files are not supported.</string> |
There was a problem hiding this comment.
"Supply a Proton Pass export ZIP containing a JSON file. Encrypted files are not supported."
| } | ||
|
|
||
| //Json not found | ||
| throw new DatabaseImporterException("Invalid proton zip file"); |
There was a problem hiding this comment.
"Unable to find \"Proton Pass/data.json\" in the ZIP file."
| return null; | ||
| } | ||
|
|
||
| Uri toptURI = Uri.parse(content.getString("totpUri")); |
There was a problem hiding this comment.
GoogleAuthInfo.parseUri has an overload that takes a string.
| JSONObject item = items.getJSONObject(j); | ||
|
|
||
| try{ | ||
| VaultEntry entry = this.fromItem(item); |
There was a problem hiding this comment.
To stay a bit more consistent with the other importers, please rename fromItem to convertItem.
Check whether it's a TOTP item here instead of returning null from fromItem. Catch all possible exceptions inside convertItem and wrap them in a DatabaseImporterEntryException there.
|
Since this pull request has been inactive for a while, I'm going to close this for now. |
2 participants
No description provided.