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

[Feature request] Is it possible to add URI string format support for both import and export #296

Closed
PM2p5 opened this issue Aug 19, 2023 · 13 comments · Fixed by #317
Closed
Assignees
Labels
enhancement New feature or request

Comments

@PM2p5
Copy link

PM2p5 commented Aug 19, 2023

Hello community,

The otpauth:// URI scheme was originally formalized by Google. It's very common to see sites all around world use this format for the 2FA setup process. Is it possible to add a feature to import an utf-8 encoded file which contains a list of otpauth:// URI?

The file content is like this:

otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example
otpauth://totp/Example:bob@doodle.com?secret=JBSWY3DPEHPK3PXP&issuer=Example

the command switch could be something like:
cotp import --otpauth_uri --path "./utf8encoded.txt"

Thanks in advance!

@replydev
Copy link
Owner

Yeah it could be implemented easily. I will do that when i can.

Thank you!

@replydev replydev self-assigned this Aug 19, 2023
@replydev replydev added the enhancement New feature or request label Aug 19, 2023
@replydev
Copy link
Owner

replydev commented Sep 7, 2023

@PM2p5 I forgot that import from OTP Uri should be already possible, by running cotp add -u. Let me know if that works.

@PM2p5
Copy link
Author

PM2p5 commented Sep 12, 2023

@PM2p5 I forgot that import from OTP Uri should be already possible, by running cotp add -u. Let me know if that works.

Hello, replydev!

I've tested the 'add -u' switch with input as following,

cotp add -u
Password: the_password
Insert the otp uri: otpauth://totp/Example:bob@doodle.com?secret=JBSWY3DPEHPK3PXP&issuer=Example

Yeah, it works well!

The point is not on adding one OTP URI, but on the lacking of import/export a batch of OTP URIs.
So it's not something 'cotp add -u', but something cotp import --otpauth_uri --path "./utf8encoded_uris.txt" and cotp export --otp_uri --path "utf8encoded_plain_uris.txt"

@replydev
Copy link
Owner

Ok, I misunderstood the requirement, to import and export as batch could be good.

@replydev
Copy link
Owner

replydev commented Sep 15, 2023

Hi @PM2p5, I found some time to implement an import / export draft from OTP Uris. Let me know your impressions.

This is the PR, you can take a debug artifact from Github Actions: https://github.com/replydev/cotp/actions/runs/6201005097?pr=317

@PM2p5
Copy link
Author

PM2p5 commented Sep 16, 2023

Hi @PM2p5, I found some time to implement an import / export draft from OTP Uris. Let me know your impressions.

This is the PR, you can take a debug artifact from Github Actions: https://github.com/replydev/cotp/actions/runs/6201005097?pr=317

Hi, replydev!

I've tested both export and import features on Windows,

Tested the export with following switches:

cotp export -u
cotp export -u -p ./
cotp export -u -p ./1.txt

They all working! The default output path is ./, and the default output file name is exported.cotp!

The exported file is of json? or ron? format with contents like this:

{"items":[
"otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example",
"otpauth://totp/Example:bob@doodle.com?secret=JBSWY3DPEHPK3PXP&issuer=Example"
]
}

Although this isn't the newline separated OTP URIs format I was after, the current export feature is working nicely!

The import feature is something different!

first, try to import what it has exported:(the json? ron? file!)

ct import --otp-uri -p ./exported.cotp
Password: the_password
An error occurred: "Issuer not found in OTP Uri"

This means the import not working with the export feature!

2nd, try to import an hand-made utf-8 BOM, newline separated OTP URIs formatted file:

ct import --otp-uri -p utf8bom_OTPURIs_newline_separeted.txt
Password: the_password
An error occurred: expected value at line 1 column 1

the file content of utf8bom_OTPURIs_newline_separeted.txt:

otpauth://totp/Example:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example
otpauth://totp/Example:bob@doodle.com?secret=JBSWY3DPEHPK3PXP&issuer=Example

3rd, try to import an hand-made utf-8 without BOM, newline separated OTP URIs formatted file,
with same file content as the BOM one:

ct import --otp-uri -p utf8_OTPURIs_newline_separeted.txt
Password: the_password
An error occurred: expected value at line 1 column 1

The import feature is not working at all!

Here is a small detail, the export switch is a bit of different from the import switch:
export -u, --otpuri VS import --otp-uri, is this by intention?

@replydev
Copy link
Owner

  1. The exported format is JSON. It has to be JSON serializable by design. You could extract items with jq.
  2. As the format is JSON, import from new-line separated uris would not work.
  3. I tried to import your example (your exported JSON file) and it works correctly to me. Could you provide me a valid test case?
  4. Regarding import / export arguments, you are right, it is better to align them.

Thanks a lot for your contribution.

@PM2p5
Copy link
Author

PM2p5 commented Sep 18, 2023

3. I tried to import your example (your exported JSON file) and it works correctly to me. Could you provide me a valid test case?

Hi, replydev!

I'm sorry that I've forgotten to put a valid test case to my bug report.

The whole process I've done is as following:
I've import an Aegis DB to the cotp App, enter cotp and confirmed the codes are working!
Then export codes from cotp with the switch of cotp export -u, what I've got is this:

{"items":[
"otpauth://totp/12345%40Github%20?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false",
"otpauth://totp/google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false"
]}

Import this file with:

cotp import --otp-uri -p ./exported.cotp
Password: the_password
An error occurred: "Issuer not found in OTP Uri"

The issue is that the export feature exported a file with items without the issuer, which should be fine, but the import feature doesn't allow that!

And here is another issue on the import feature, the current version of Google Authenticator App's internal has changed a lot, you can't import it's codes with it's DB now, the decryption key is in the Android key store! So the import feature for Google is broken! The only and valid way to import the codes from it is to support the format of newline-separated OTP URIs, which is already mentioned.

For a working way to do this, please refer to:
https://github.com/jamie-mh/AuthenticatorPro/wiki/Importing-from-Google-Authenticator
https://github.com/dim13/otpauth
TLDR: Google-Authenticator -> Export to QR picture -> take the picture to a PC ->
decode the picture to the special Google-Authenticator's format like this otpauth-migration://offline?data=... ->
convert the special format to plain OTP URIs with otpauth ->
import newline-separated OTP URIs with AuthenticatorPro/Aegis

One more thing, it's my honor for the participation on the best TUI OTP App ever!
Thank you for the hard and great works!

@replydev
Copy link
Owner

Hi again @PM2p5. Sorry for delay but these times i do not have so much time.

I pushed a fix for the import problem you reported.
Regarding the Google Authenticator changed i would open another issue.

If you can please confirm that it's working for you now.
Thank you.

@PM2p5
Copy link
Author

PM2p5 commented Sep 30, 2023

Hi again @PM2p5. Sorry for delay but these times i do not have so much time.

I pushed a fix for the import problem you reported. Regarding the Google Authenticator changed i would open another issue.

If you can please confirm that it's working for you now. Thank you.

Good day, @replydev!

Don't say sorry, we all had the experience of busying on something! Who ever dare to say you are lazy? lol!

I've running following tests on the debug artifacts of pull https://github.com/replydev/cotp/actions/runs/6343919848?pr=317,
Debug artifacts version v1.3.0-DEBUG-da9a56c2b60b

Test 1, create a blank db, then export it with export -u switch

cotp
Choose a password: the_password
Retype the same password: the_password
No codes, type "cotp -h" to get help

cotp export -o
Password:
Exported to path: .\exported.cotp
Success

the file I got is:

{"items":[]}

Nice!

Test 2, let's add some codes with the add -u switch:

cotp add -u
Password:
Insert the otp uri: otpauth://totp/Wonderland:alice@google.com?secret=JBSWY3DPEHPK3PXP&issuer=Example
Success

cotp add -u
Password:
Insert the otp uri: otpauth://totp/2Ponies%40Github%20No.1?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false
An error occurred: Issuer not found in OTP Uri

cotp add -u
Password:
Insert the otp uri: otpauth://totp/google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false
An error occurred: Issuer not found in OTP Uri

So the add -u feature needs some tweaks.

Test 3, checking the export -o feature:

cotp export -o
Password: the_password
Exported to path: .\exported.cotp

what I've got is:

{"items":[
"otpauth://totp/Wonderland:alice%40google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false"
]}

It seems that the issuer=Example part was lost!

Test 4, import this file.

cotp import -o -p exported.cotp
Success

then check the codes, the screen shot will look like this:
Capture-%40 was not convert to the @ symbol
So the %40 was not convert to the @ symbol.

Test 5, export the db with cotp export -o -p exported2.cotp and check the content of the exported file:

{"items":[
"otpauth://totp/Wonderland:alice%40google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false",
"otpauth://totp/Wonderland:alice%2540google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false"
]}

The %40 after both import and export has changed to %2540, this might indicates some logical errors related to the serialize/de-serialize!

Test 6, import an hand crafted file exported3.cotp with following contents:

{"items":[
"otpauth://totp/2Ponies%40Github%20No.1?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false",
"otpauth://totp/google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false"
]}
cotp import -o -p exported3.cotp
An error occurred: "Issuer not found in OTP Uri"

The import feature still not supporting import OTP URIs with no issuer!

The features around OTP URI scheme might need more overhaul to achieve a usable status.
Contributing to open source and the community is great, but don't let it to be a burden!
Take care and have a nice weekend!

@replydev
Copy link
Owner

replydev commented Oct 9, 2023

Hi @PM2p5, thank you for your clear instruction. The fact was that, reading the specification, i didn't knew that the issuer is optional.

So the latest changes:

  • Put a default Account issuer when it's missing.
  • Take the issuer from the URl query parameters if it's missing from the label.
  • Correctly handle URL encoded URIs.

Feel free to download the latest binary from CI: https://github.com/replydev/cotp/actions/runs/6459034054

@PM2p5
Copy link
Author

PM2p5 commented Oct 10, 2023

Hi, @replydev

  • Put a default Account issuer when it's missing.

  • Take the issuer from the URl query parameters if it's missing from the label.

I'm not sure if the behavior of generate a default Account issuer is correct, can't it be blank? I took more peaking on the format specification, the yubico's one is very clear on the topic.
https://docs.yubico.com/yesdk/users-manual/application-oath/uri-string-format.html

It mentioned the format is something like:

otpauth://TYPE/LABEL?PARAMETERS

Let's ignore the the TYPE might be totp/hotp for now,
The LABEL is made with two parts, one Account Name, one optional Issuer, so a label should like this:

otpauth://TYPE/Optional_Issuer:Alice@Google.com?PARAMETERS

The Parameters could made with 6 parts, the Issuer, Period and Algorithm parts are optional.

So, both issuers are optional, if an URI is missing both issuer parts in LABEL and PARAMETERS, it's fine.

Short facts from the yubico's article:

Label

The label is used to identify which account a credential is associated with. It also serves as the unique identifier for the credential itself.

The label is created from:
Name 	Description
Issuer 	An optional string value indicating the provider or service this account is associated with.
Parameters
Issuer

The issuer parameter is an optional string value indicating the provider or service the credential is associated with. It is URL-encoded according to RFC 3986.

Valid values corresponding to the label examples above would be:

issuer=Example

issuer=ACME%20Co

The issuer parameter is recommended, but it can be absent. Also, the issuer parameter and issuer string in label should be equal.

Feel free to download the latest binary from CI: https://github.com/replydev/cotp/actions/runs/6459034054

Yeah, it passed all the previous tests, what I got is:

{"items":[
"otpauth://totp/Account:2Ponies%40Github%20No.1?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false",
"otpauth://totp/Account:google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false",
"otpauth://totp/Wonderland:alice%40google.com?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30&lock=false"
]}

The good thing is, I think the current status of the App is quite charming and promising! Thanks for the hard work(again!) :-)
The only down side is, I think adding an Account to the optional issuer in the LABEL is a bad idea!

Have a great day and on!

@replydev
Copy link
Owner

You are right. Yesterday i've read the exact same doc, but I misunderstood the "Account", part.

The fact is that, currently, the issuer is a required field, but the label should be instead.

I will handle this last case and the finally merge :).

replydev added a commit that referenced this issue Oct 10, 2023
This PR implements import / export logic from a bulk of OTP Uris.
Closes #296
Closes #263
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants