FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Ensure that an OTP's issuer and label values are escaped correctly by josh- · Pull Request #391 · Shane32/QRCoder · GitHub

Ensure that an OTP's issuer and label values are escaped correctly - #391

Merged
codebude merged 3 commits into
Shane32:masterfrom
josh-:fix-otp-space-escaping
Apr 7, 2024
Merged

Ensure that an OTP's issuer and label values are escaped correctly#391
codebude merged 3 commits into
Shane32:masterfrom
josh-:fix-otp-space-escaping

Conversation

josh- commented Mar 8, 2022
edited
Loading

Copy link
Copy Markdown
Contributor

Summary

This PR fixes/implements the following bugs/features:

  • Creating an OTP with an issuer/label that contains a space or other restricted characters does not properly escape the issuer/label

What existing problem does the pull request solve?**

Currently, if you create an OTP with an issuer that contains a space, for example:

var oneTimePassword = new OneTimePassword {
	Issuer = "Google Google",
	Label = "test@google.com",
	Secret = "pwq6 5q55"
};

and then call ToString on that oneTimePassword, that will return:

otpauth://totp/Google Google:test@google.com?secret=pwq65q55&issuer=Google%20Google

(note the first "Google Google" is not escaped, like the second one in the issuer parameter)

which then looks like this in Google Authenticator after scanning the resulting QR code:

This PR resolves this issue by correctly escaping both instances of the issuer in the URL, which then appears correctly in Google Authenticator:

otpauth://totp/Google%20Google:test@google.com?secret=pwq65q55&issuer=Google%20Google

Test plan

Test cases have been added.

Closing issues

N/A

prvacy commented Mar 9, 2022
edited
Loading

Copy link
Copy Markdown

I discovered the same problem yesterday, thank you for the fix! Also, MS Authenticator does not support unescaped URLs, so that fix is really important.

Copy link
Copy Markdown

Thanks for this fix, it addresses issue #375.

I suggest also escaping the Label value, since an email address can contain / and ? characters.

josh- commented Mar 15, 2022

Copy link
Copy Markdown
Contributor Author

Thanks for this fix, it addresses issue #375.

I suggest also escaping the Label value, since an email address can contain / and ? characters.

Thanks @tom-156842, done 👍

josh- changed the title Ensure that an OTP's issuer is correctly escaped when it contains a space Ensure that an OTP's issuer and label values are escaped correctly Mar 15, 2022

hdocsek commented Oct 31, 2022
edited
Loading

Copy link
Copy Markdown

@codebude Do you have any update on when this fix will be released?

doggy8088 left a comment
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

@codebude

Shane32 commented Apr 6, 2024

Copy link
Copy Markdown
Owner

All changes here look good. Generated QR codes scan properly with Google Authenticator. Tests have already been added 👍

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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.

7 participants


Back | FazBrowse Home | New Git URL