search

PHPGangsta / GoogleAuthenticator

PHP class to generate and verify Google Authenticator 2-factor authentication
http://phpgangsta.de/4376
BSD 2-Clause "Simplified" License
2.25k stars 693 forks source link

urlencode title #15

Closed yanniks closed 9 years ago

yanniks commented 9 years ago

Otherwise we might occur problems when using things like whitespaces.

PHPGangsta commented 9 years ago

Hi Yannik,

the parameter is already urlencoded. Could you please explain why your extra call is needed?

Thanks Michael

yanniks commented 9 years ago

For some reason it was not working when I scanned the QR code with Google Authenticator (iOS). Urlencoding the string fixed that problem.

JN-Jones commented 9 years ago

The encoding should be '&issuer='.urlencode($title);. Otherwise the & and = get encoded too.

yanniks commented 9 years ago

Yeah, makes sense. Shall I change my PR?

PHPGangsta commented 9 years ago

JN-Jones: The & and = definitly HAVE to be encoded, and I think Yannik is right, the title even has to be encoded twice.

Please check the resulting URL... It is a "https://" URL to google charts, and that URL has a parameter "chl=".. This parameter is an "otpauth://" URL. So the & before the "issuer" definitly has to be encoded, because otherwise it would be a parameter to the google charts URL, and not the "otpauth" URL. And that's also why the "title" parameter has to be encoded twice...

So I would say $urlencoded .= urlencode('&issuer='. urlencode($title)); is correct.

What do you think?

JN-Jones commented 9 years ago

Ah, yes, missed that it's a parameter itself. Yeah, title needs to be encoded twice in that case.

yanniks commented 9 years ago

Yes, this is at least logical.

RobThree commented 9 years ago

Also see: https://github.com/PHPGangsta/GoogleAuthenticator/issues/11