Skip to content
This repository was archived by the owner on Oct 15, 2024. It is now read-only.

Allow importing TOTP configuration through QR codes #903

Merged
merged 11 commits into from
Jun 30, 2020
Merged

Allow importing TOTP configuration through QR codes #903

merged 11 commits into from
Jun 30, 2020

Conversation

@msfjarvis
Copy link
Member

@msfjarvis msfjarvis commented Jun 30, 2020 •
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring
  • Dependency updates

📜 Description

Adds a button to password creation/edit screen that starts a camera view to import QR codes. The implementation
is very rudimentary and does no error checking, which we should change. The view also starts in landscape which
I will probably need to rectify in the source library.

💡 Motivation and Context

Let's just say that I am better at Kotlin than at Bash and leave it at that.

💚 How did you test it?

Manually imported by 20+ TOTP sites into files with and without extra content and validated the generated OTPs.

📝 Checklist

  • I formatted the code with the IDE's reformat action (Ctrl + Shift + L/Cmd + Shift + L)
  • I reviewed submitted code
  • I added a CHANGELOG entry if applicable

🔮 Next steps

📸 Screenshots / GIFs

¯\(ツ)/¯

@msfjarvis
Allow importing TOTP configuration through QR codes
5041c0e 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis added this to the 1.10.0 milestone Jun 30, 2020
msfjarvis added 4 commits June 30, 2020 17:03
@msfjarvis
Hide OTP import button if TOTP is already present
e413b84 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Convoluted newline nonsense
4af3617 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Silence warning
5c14274 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Try simplifying some things
7acfce0 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis msfjarvis marked this pull request as ready for review June 30, 2020 12:14
@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020

This works for me, I've imported ~30 TOTP configs without a hitch so far.

@msfjarvis msfjarvis self-assigned this Jun 30, 2020
fmeum
fmeum reviewed Jun 30, 2020
View reviewed changes
Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

If we keep pumping out OTP features at this speed, andOTP will soon be deprecated in favor of Password Store. ;-)

msfjarvis added 4 commits June 30, 2020 18:03
@msfjarvis
Reuse existing logic
6349328 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Improve OTP import button UI
b88dc90 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
fix changelog brainfart
ef1bd05 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Support directly importing secrets
975efd1 
Is this a thing?

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020

I'm not very sure about 975efd1 so just let me know if putting only the secret in the QR code is something that doesn't actually happen and I'll revert it.

/cc @erayd

@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020 •
edited
Loading

I'll go check up on why this opens in landscape...

@fmeum
Copy link
Member

fmeum commented Jun 30, 2020

I'll go check up on why this opens in landscape...

I think that's because of https://github.com/journeyapps/zxing-android-embedded/blob/13440ad8751dcaba4177fa50dfee639dac52a53d/zxing-android-embedded/AndroidManifest.xml#L35

@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020

I'll go check up on why this opens in landscape...

I think that's because of https://github.com/journeyapps/zxing-android-embedded/blob/13440ad8751dcaba4177fa50dfee639dac52a53d/zxing-android-embedded/AndroidManifest.xml#L35

image

I'll go make this portrait and push a new tag out.

@msfjarvis
Force CaptureActivity to respect device orientation
85560e7 
Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020

Or well, this works too. Gotta love ManifestMerger.

fmeum
fmeum approved these changes Jun 30, 2020
View reviewed changes
Copy link
Member

@fmeum fmeum left a comment

Choose a reason for hiding this comment

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

Works great!

@msfjarvis msfjarvis merged commit 5e74507 into android-password-store:develop Jun 30, 2020
@msfjarvis msfjarvis deleted the feature/otp-auth-importer branch June 30, 2020 13:51
msfjarvis added a commit that referenced this pull request Jun 30, 2020
@msfjarvis
Merge branch 'develop' into feature/autofill_sms_otp
e63eb57 
* develop:
  Allow importing TOTP configuration through QR codes (#903)
  Workaround to prevent crash on first run (#898)
@erayd
Copy link

erayd commented Jun 30, 2020

I'm not very sure about 975efd1 so just let me know if putting only the secret in the QR code is something that doesn't actually happen and I'll revert it. /cc @erayd

I don't know the answer to this one sorry. My guess is that nobody does this though, because a context-less OTP code doesn't make much sense most of the time.

That said, it could still be handy to have for getting seeds out of e.g. a shell by piping it to e.g. qrencode. It's a little bit feature creep, but useful.

@msfjarvis
Copy link
Member Author

msfjarvis commented Jun 30, 2020

I'm not very sure about 975efd1 so just let me know if putting only the secret in the QR code is something that doesn't actually happen and I'll revert it. /cc @erayd

I don't know the answer to this one sorry. My guess is that nobody does this though, because a context-less OTP code doesn't make much sense most of the time.

That said, it could still be handy to have for getting seeds out of e.g. a shell by piping it to e.g. qrencode. It's a little bit feature creep, but useful.

If there's no precedent in the wild I'm just gonna revert it and stay on the safer side. Thanks for the info!

msfjarvis added a commit to fmeum/Android-Password-Store that referenced this pull request Jul 1, 2020
@msfjarvis
Merge branch 'develop' into fhenneke_multiple_ssh
41f92b1 
* develop: (62 commits)
  Scroll to files and enter folders when created (android-password-store#909)
  Run a treewide reformat (android-password-store#908)
  Improve how secrets and stored and used (android-password-store#907)
  Improve and refactor Autofill heuristics (android-password-store#905)
  Use PreferenceKeys file to manage SharedPreferences keys. (android-password-store#891)
  Revert "Support directly importing secrets" (android-password-store#904)
  Allow importing TOTP configuration through QR codes (android-password-store#903)
  Bump version
  Prepare release 1.9.2
  update changelog
  Workaround to prevent crash on first run (android-password-store#898)
  Workaround to prevent crash on first run (android-password-store#898)
  Offer TOTP Autofill for OTP fields (android-password-store#899)
  Merge SshKeyGenFragment into its activity (android-password-store#897)
  Reintroduce TOTP support (android-password-store#890)
  Sync with release branch (android-password-store#896)
  build: bump version
  Prepare release 1.9.1
  Backport Actions fixes (android-password-store#894)
  Rework GitHub Actions (android-password-store#893)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@Skrilltrax Skrilltrax Awaiting requested review from Skrilltrax

1 more reviewer

@fmeum fmeum fmeum approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@msfjarvis msfjarvis

Labels

None yet

Projects

None yet

Milestone

1.10.0

Development

Successfully merging this pull request may close these issues.

3 participants

@msfjarvis @fmeum @erayd