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

Adding screen parsing for OTP QR code #2597

Merged
merged 3 commits into from
Jun 10, 2023
Merged

Adding screen parsing for OTP QR code #2597

merged 3 commits into from
Jun 10, 2023

Conversation

AnomalRoil
Copy link
Member

This adds the capability to parse the content of the (multiple) display of the user to detect and add an OTP QR code to a given entry.

Usage:

$ gopass otp -snip testentry
⚠ Scanning screen n°0
✅ Area scanned on screen n°0: (0,0)-(3440,1440)
⚠ Found a qrcode, checking.
⚠ Not an otpauth:// QR code, please make sure to only have your OTP qrcode displayed.
⚠ Scanning screen n°1
✅ Area scanned on screen n°1: (0,0)-(2560,1440)
⚠ Found a qrcode, checking.
✅ Found an otpauth:// QR code on screen n°1 ((0,0)-(2560,1440))
(Over)writing otpauth URL in key 'otpauth'? [Y/n/q]:
Value written, carrying on to display OTP value from it.
824573

I also updated our dependencies, while I was at it, by running go get -u -t ./... and go mod tidy.

@AnomalRoil AnomalRoil force-pushed the feat/qrsnip branch 2 times, most recently from 59a2477 to 4a7234b Compare May 31, 2023 13:23
[CLEANUP] Fixing issues from linter

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
@AnomalRoil
Copy link
Member Author

What should we do about the license @dominikschulz ? To me it seems okay since we already have deps using the BSD license, but not sure.

The generator part with the WTFPL license is fine too.

@dominikschulz
Copy link
Member

BSD is definitely fine with me. WTFPL should be ok as well.

@AnomalRoil
Copy link
Member Author

The problem is it's slightly different, it has an extra clause:
https://github.com/jezek/xgb/blob/master/LICENSE

// * Neither the name of Google Inc. nor the names of its
// contributors may be used to endorse or promote products derived from
// this software without specific prior written permission.

@dominikschulz
Copy link
Member

This is fine. We won't do that 🙂

@AnomalRoil
Copy link
Member Author

AnomalRoil commented Jun 1, 2023

Should I add WTFPL to the .license-lint.yml and add xgb to our license allowlisted_modules in there too?

Edit: I went ahead and I've done it now it passes on CI.

PS: note the big point here is that it requires CGO to work on Darwin (which our Goreleaser based process and cross-compilation pipeline doesn't support. A way to solve it might be to run Goreleaser from a darwin runner, or just to leave that feature to people willing to compile it themselves, which is the case for now.) It works fine without CGO on other Unix systems and on Windows.

Signed-off-by: Yolan Romailler <anomalroil@users.noreply.github.com>
@dominikschulz
Copy link
Member

Thank you. I will review the rest of the PR later (soon).

Copy link
Member

@dominikschulz dominikschulz left a comment

Choose a reason for hiding this comment

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

LGTM

nit: Usually I prefer to split updating dependencies and code changes, but this time we can leave it as is.

@dominikschulz dominikschulz merged commit eb6c101 into master Jun 10, 2023
@dominikschulz dominikschulz deleted the feat/qrsnip branch June 10, 2023 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants