Urlencode account name in otpauth URL #23
Conversation
HumHub username can contain UTF-8. However qrcode.js cannot handle that and won't render the qr code at all. This Commit wraps the account name with urlencode to fix that issue.
| @@ -143,9 +143,9 @@ public function getQrCodeSecretKeyFile($requirePinCode = false) | |||
| } | |||
|
|
|||
| return $this->renderFile([ | |||
| 'qrCodeText' => 'otpauth://totp/' . Yii::$app->request->hostName . ':' . TwofaHelper::getAccountName() . '?secret=' . $secret . '&issuer=' . Yii::$app->request->hostName, | |||
| 'qrCodeText' => 'otpauth://totp/' . Yii::$app->request->hostName . ':' . urlencode(TwofaHelper::getAccountName()) . '?secret=' . $secret . '&issuer=' . Yii::$app->request->hostName, | |||
There was a problem hiding this comment.
Not completely sure why you'd want to use urlencode, personally I'd use Html::encode(TwofaHelper::getAccountName()) method due to special characters
https://www.yiiframework.com/doc/guide/2.0/en/helper-html#encoding-and-decoding-content
The other option would be wrapping Yii::$app->user->getIdentity()->username shown below with the same method
Lines 254 to 263 in 07955b3
There was a problem hiding this comment.
Html:encode() uses a different encoded that is used for HTML content. The letter ä would then be ä. Unfortunately that is not the correct encoding for URLs. Here an ä would be %C3%A4 as it is specified by RFC 3986.
However thanks to your comment I realised, that urlencode()isn't the correct function either as this incorrectly encodes the whitespace in the output of getAccountname() using + instead of %20.
There was a problem hiding this comment.
@weyhmueller Hello, I have tested the vars Yii::$app->name and Yii::$app->user->getIdentity()->username with the char ä, yes we cannot use Html::encode() there.
If we need %20 instead of + then we should use rawurlencode(), also we should use this function instead the urlencode() because Google Authenticator application displays the + instead of expected normal space . For example, if TwofaHelper::getAccountName() === 'HumHub ä - loginä' then in Google Authenticator application I see account name as:
urlencode()=>humhub.com (HumHub+ä+-+loginä)rawurlencode()=>humhub.com (HumHub ä - loginä)
So just use rawurlencode() and it will works properly.
There was a problem hiding this comment.
@weyhmueller Ah I see you already fixed that in next commit a481e09, thank you!
Fix of the first fix. PHP's urlencode uses '+' to encode spaces which is not valid in the url path section. The rawurlencode function is the PHP function that does it right.
The HumHub username can contain UTF-8 characters, and in our case it does. However qrcode.js cannot handle that
and won't render the qr code at all. Only output would be an exception in the JS-Console.
This Commit wraps the account name with urlencode to fix that issue.