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

feat: Allow for non-standard provisioning URI params, eg. image/icon #91

Merged
merged 6 commits into from
Aug 30, 2023

Conversation

mdp
Copy link
Owner

@mdp mdp commented Jul 31, 2019

No description provided.

@TSMMark
Copy link

TSMMark commented Jul 25, 2022

Looks great, nice! Can we expect a gem release?

@mdp mdp changed the base branch from master to main August 1, 2022 20:54
@mdp
Copy link
Owner Author

mdp commented Aug 1, 2022

Yeah, so the main branch has moved along a bit and this MR is not up to date. I'll try and get it out this week after bringing it back up to date.

@TSMMark
Copy link

TSMMark commented Aug 2, 2022

Awesome, looking forward to it!

@mdp mdp self-assigned this Aug 16, 2022
@mdp
Copy link
Owner Author

mdp commented Aug 16, 2022

@TSMMark Can you give this a quick review and I'll publish it ASAP. Sorry for the delay on this. Thanks

Copy link

@TSMMark TSMMark left a comment

Choose a reason for hiding this comment

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

LGTM. I might be able to test this in the next few days.

@@ -108,14 +110,29 @@
end

describe '#provisioning_uri' do
it 'accepts the account name' do
let(:hotp) { ROTP::HOTP.new('a' * 32, name: "[email protected]", issuer: "Example.com") }
Copy link

Choose a reason for hiding this comment

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

intentional whitespace?

Suggested change
let(:hotp) { ROTP::HOTP.new('a' * 32, name: "[email protected]", issuer: "Example.com") }
let(:hotp) { ROTP::HOTP.new('a' * 32, name: "[email protected]", issuer: "Example.com") }

@TSMMark
Copy link

TSMMark commented Aug 17, 2022

During local testing I couldn't get any image to show up my the authenticator app, even though the image param was present in the otpauth:// url...

Not sure if there's some specification for the image for it to show up in an authenticator app. I'm using this OTP Auth app: https://apps.apple.com/us/app/otp-auth/id659877384

@TSMMark
Copy link

TSMMark commented Aug 17, 2022

i'm not very familiar with the protocol, is this the proper encoding of the url attributes?

Screen Shot 2022-08-17 at 12 11 16 PM

@mdp
Copy link
Owner Author

mdp commented Aug 18, 2022

That last one definitely looks odd. Here's what I'm getting from the rotp with the image param:

ROTP::TOTP.new(TEST_SECRET, name: "[email protected]", issuer: "Example.com",
          provisioning_params: { image: 'https://assets.stickpng.com/thumbs/580b57fcd9996e24bc43c53e.png' }
)

otpauth://totp/Example.com:m%40mdp.im?secret=JBSWY3DPEHPK3PXP&issuer=Example.com&image=https%3A%2F%2Fassets.stickpng.com%2Fthumbs%2F580b57fcd9996e24bc43c53e.png

Which I encoded to:
image

And here's what I get in FreeOTP:

3f19fb0d-ae8d-48ee-be81-58ca9356d340

From what I can tell, the image doesn't get used in Authy, and Google Authenticator doesn't do logo/images at all.

@mdp
Copy link
Owner Author

mdp commented Aug 23, 2022

Regarding the iOS OTP app - https://apps.apple.com/us/app/otp-auth/id659877384 - I'm guessing they've not implemented the image param feature. They also mention "OTP Auth does not connect to the internet and will not send your accounts to someone else!" which, if entirely true, would prevent fetching the image URI param.

FreeOTP is the only one I've found that does it.

@TSMMark
Copy link

TSMMark commented Aug 24, 2022

I'm using OTP Auth app https://apps.apple.com/us/app/otp-auth/id659877384 and many of the accounts have images / logos.

But maybe it's possible that there are some "presets" in the app, where they have some static images pre-defined based on well-known providers like 1password, binance, discord, github, google, amazon, etc

@mdp
Copy link
Owner Author

mdp commented Aug 24, 2022

Yeah, that would be my guess. The only references I can find to the image param are for FreeOTP, which I got to work with the image param.

pyauth#96
https://stackoverflow.com/questions/41232168/token-image-in-google-authenticator-or-freeotp
https://stackoverflow.com/questions/62303077/mfa-one-time-password-image

@mdp mdp changed the title Allow for non-standard provisioning URI params, eg. image/icon Feat: Allow for non-standard provisioning URI params, eg. image/icon Aug 30, 2023
@mdp mdp changed the title Feat: Allow for non-standard provisioning URI params, eg. image/icon feat: Allow for non-standard provisioning URI params, eg. image/icon Aug 30, 2023
@mdp mdp merged commit 45d8aac into main Aug 30, 2023
5 checks passed
@mdp mdp deleted the image_param branch January 9, 2024 14:41
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