speakeasy icon indicating copy to clipboard operation
speakeasy copied to clipboard

Issuer option is not passed to otpauthURL in generateSecret

Open Cheprer opened this issue 7 years ago • 33 comments

In function generateSecret, 'issuer' option is not passed to the otpauthURL function as an option.

Cheprer avatar Apr 18 '17 10:04 Cheprer

The git repo includes issuer option in generateSecret function.

Can you specify "speakeasy": "git://github.com/speakeasyjs/speakeasy.git#e63b936d562fa16c5970d0821a221cf1a59f559b" in package.json to pull the latest source code and pass issuer in generateSecret function?

The wiki https://github.com/speakeasyjs/speakeasy/wiki/General-Usage-for-Time-Based-Token includes an example to pass issuer value to generateSecret.

railsstudent avatar Apr 18 '17 23:04 railsstudent

hey @railsstudent thanks for the answer I've tried the code version you've sent and it adds the issuer but there is another problem. Generated OTP auth is wrong or not correctly handled at least by Google Authenticator:

Version on npm generates this and it works: otpauth://totp/jan.beseda%40test.com?secret=MFHDCTK6NM2WEWRON5RGK2K6OVZDK4DS

Version you've sent git://github.com/speakeasyjs/speakeasy.git#e63b936d562fa16c5970d0821a221cf1a59f559b generates this: otpauth://totp/jan.beseda%40test.com?secret=INOXSVDHM44FQNR2KRCSSZKLEQ2F46TOORBEOZLBGRPCIJLDG4SA&issuer=TEST%20%20http%3A%2F%2Flocalhost%3A3000&algorithm=SHA1&digits=6&period=30'

Cheprer avatar May 23 '17 07:05 Cheprer

What do you mean by wrong? The verify function does not return correct value when code is passed in?

railsstudent avatar May 23 '17 10:05 railsstudent

Verify function returns correct value but scanned QR code with Google Authenticator generates the wrong value.

Cheprer avatar May 23 '17 10:05 Cheprer

The secret values are different. Regression?

jakelee8 avatar Jun 04 '17 14:06 jakelee8

Here's the problem: when you run the code "generateSecret()" and pass in an "issuer" as part of the options, the issuer gets dropped due to the following lines:

Line 474 in index.js

// add in the Google Authenticator-compatible otpauth URL
  if (otpauth_url) {
    SecretKey.otpauth_url = exports.otpauthURL({
      secret: SecretKey.ascii,
      label: name
    });
  }

As you can see, even if you pass in "issuer" the issuer won't be passed to the otpauthurl api that generates the google authenticator compatible otpauth URL.

BrandonCopley avatar Jun 30 '17 22:06 BrandonCopley

Ahh, this fix appears to already exist in the current branch - what can we do to get this pushed up to NPM?

BrandonCopley avatar Jun 30 '17 23:06 BrandonCopley

@Cheprer @BrandonCopley Have time to try https://github.com/speakeasyjs/libotp ?

jakelee8 avatar Jul 01 '17 03:07 jakelee8

@jakelee8 as @BrandonCopley mentioned the problem is that the issuer is not passed to otpauthURL constructor so when I've modified code to something like this then it works (of course it needs some checks and defaults in the beginning):

if (otpauth_url) {
    SecretKey.otpauth_url = exports.otpauthURL({
      secret: SecretKey.ascii,
      label: name,
      issuer: options.issuer
    });
  }

works for https://github.com/speakeasyjs/speakeasy npm version 2.0.0

Cheprer avatar Jul 03 '17 09:07 Cheprer

@railsstudent @jakelee8 hey guys I can see this code correctly fixed on the repository on master branch but the NPM package doesn't have it. Can you please update NPM package please and close this issue? Much appreciated, Cheprer

Cheprer avatar Jul 12 '17 11:07 Cheprer

@markbao

jakelee8 avatar Jul 12 '17 17:07 jakelee8

+1

max-arias avatar Jul 13 '17 17:07 max-arias

+1

guillaumev avatar Aug 25 '17 14:08 guillaumev

+1 it's not on NPM yet

BarryCarlyon avatar Sep 23 '17 12:09 BarryCarlyon

Also:

if you take the response to generateSecret and then make your own URL you have to pre-encode the label your self as the library isn't. To answer @Cheprer

Which as he reports results in:

image

The correct function call is:

var secret = speakeasy.generateSecret({
    length: 20,
    name: 'Barry Carlyon',
    issuer: 'Some Issuer'
});

secret.otpauth_url = speakeasy.otpauthURL({
    secret: secret.ascii,
    label: encodeURIComponent('Barry Carlyon'),
    issuer: 'Some Issuer'
});

The fault when scanning the QR code only failing as the URL contains unencoded spaces

For example:

node create_user.js otpauth://totp/Barry%20Carlyon?secret=SOMESCRET otpauth://totp/Barry Carlyon?secret=SOMESCRET&issuer=Some%20Issuer

BarryCarlyon avatar Sep 23 '17 13:09 BarryCarlyon

@markbao controls the NPM package

jakelee8 avatar Sep 23 '17 15:09 jakelee8

@BarryCarlyon thanks for wrapping it up. I hope guys will make it stable and put on NPM soon.

Cheprer avatar Sep 25 '17 07:09 Cheprer

The fault when scanning the QR code only failing as the URL contains unencoded spaces

I'm having the same issue. A string with a space in breaks the url from otpauthURL. How can I make the issuer name have spaces (I tried %20 and it appears in the actual name)?

LukeXF avatar Oct 04 '17 23:10 LukeXF

@LukeXF I answered the problem in my solution code:

secret.otpauth_url = speakeasy.otpauthURL({
    secret: secret.ascii,
    label: encodeURIComponent('Barry Carlyon'),
    issuer: 'Some Issuer'
});

BarryCarlyon avatar Oct 05 '17 18:10 BarryCarlyon

It still adds in %20 with encodeURIComponent()

LukeXF avatar Oct 05 '17 22:10 LukeXF

@LukeXF That what encodeURIComponent() does, it replaces space with %20

railsstudent avatar Oct 06 '17 02:10 railsstudent

@railsstudent Yes, but the %20 is showing up on Google Authenticator. Project%20Name for example instead of Project Name

LukeXF avatar Oct 06 '17 17:10 LukeXF

Sounds like double escaping

On Oct 6, 2017, at 10:11 AM, Luke Brown [email protected] wrote:

@railsstudent Yes, but the %20 is showing up on Google Authenticator. Project%20Name for example instead of Project Name

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

jakelee8 avatar Oct 06 '17 17:10 jakelee8

@LukeXF Are you using the npm 2.0.0 version of the package, or the latest master from git?

In the npm version, label is not URL-encoded: https://github.com/speakeasyjs/speakeasy/blob/4e0aa91dee166306e07ea11e371d5a5df95f4391/index.js#L644

In the latest master from git, it is: https://github.com/speakeasyjs/speakeasy/blob/e63b936d562fa16c5970d0821a221cf1a59f559b/index.js#L704

In both versions, issuer should be already URL-encoded, since we pass in an object for the querystring to url.format and that runs querystring's stringify. Let me know about the above question and we can figure out what's going on with issuer.

markbao avatar Oct 06 '17 17:10 markbao

@markbao I'm using NPM 2.0.0. But even with using encodeURIComponent() myself, the %20 still outputs as %20 and not a space. Any suggestions?

LukeXF avatar Oct 06 '17 23:10 LukeXF

Problem was with base32 encoding. Because I'm using a base32 secret, I needed to specify an encoding so that it didn't load ASCII be default.

See https://github.com/speakeasyjs/speakeasy/issues/95#issuecomment-334955321 for reference.

var url = speakeasy.otpauthURL({
	secret: secret.base32,
	label: req.user.local.email,
	issuer: 'The Spaces Work',
	encoding: "base32"
});
screen shot 2017-10-07 at 19 15 46

LukeXF avatar Oct 07 '17 18:10 LukeXF

I've used @LukeXF 's way and it works. Big thanks but even though the function generateSecret doesn't work properly still because it generates ASCII secret and then from it it generates the base32. Maybe there is a glitch. If I'll have time. I'll try to make a pull request with fix but not sure when. So I would keep this issue still open.

Cheprer avatar Dec 14 '17 10:12 Cheprer

@markbao any plans on releasing a new version where the issuer is passed to the otpauthURL function on generateSecret?

joaonice avatar Aug 24 '18 15:08 joaonice

Hi all, old issue but the issuer still misses in NPM on the generateSecret() method. Any plans to implement it?

mikgross avatar Oct 21 '19 12:10 mikgross

This is still in the NPM package. Please update!

nrail-joerg-walter avatar May 28 '20 15:05 nrail-joerg-walter