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

Issuer option is not passed to otpauthURL in generateSecret #86

Open
Cheprer opened this issue Apr 18, 2017 · 33 comments
Open

Issuer option is not passed to otpauthURL in generateSecret #86

Cheprer opened this issue Apr 18, 2017 · 33 comments
Assignees

Comments

@Cheprer
Copy link

Cheprer commented Apr 18, 2017

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

@Cheprer Cheprer changed the title Missing issuer option on NPM, outdated package Issuer option is not passed to otpauthURL in generateSecret Apr 18, 2017
@railsstudent
Copy link
Collaborator

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.

@Cheprer
Copy link
Author

Cheprer commented May 23, 2017

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'

@railsstudent
Copy link
Collaborator

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

@Cheprer
Copy link
Author

Cheprer commented May 23, 2017

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

@jakelee8
Copy link
Collaborator

jakelee8 commented Jun 4, 2017

The secret values are different. Regression?

@BrandonCopley
Copy link

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
Copy link

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

@jakelee8
Copy link
Collaborator

jakelee8 commented Jul 1, 2017

@Cheprer
Copy link
Author

Cheprer commented Jul 3, 2017

@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
Copy link
Author

Cheprer commented Jul 12, 2017

@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

@jakelee8
Copy link
Collaborator

@markbao

@max-arias
Copy link

+1

1 similar comment
@guillaumev
Copy link

+1

@BarryCarlyon
Copy link

+1 it's not on NPM yet

@BarryCarlyon
Copy link

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

@jakelee8
Copy link
Collaborator

@markbao controls the NPM package

@Cheprer
Copy link
Author

Cheprer commented Sep 25, 2017

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

@LukeXF
Copy link

LukeXF commented Oct 4, 2017

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)?

@BarryCarlyon
Copy link

@LukeXF I answered the problem in my solution code:

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

@LukeXF
Copy link

LukeXF commented Oct 5, 2017

It still adds in %20 with encodeURIComponent()

@railsstudent
Copy link
Collaborator

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

@LukeXF
Copy link

LukeXF commented Oct 6, 2017

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

@jakelee8
Copy link
Collaborator

jakelee8 commented Oct 6, 2017 via email

@markbao
Copy link
Collaborator

markbao commented Oct 6, 2017

@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:

pathname: label,

In the latest master from git, it is:

pathname: encodeURIComponent(label),

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.

@LukeXF
Copy link

LukeXF commented Oct 6, 2017

@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
Copy link

LukeXF commented Oct 7, 2017

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 #95 (comment) 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

@Cheprer
Copy link
Author

Cheprer commented Dec 14, 2017

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.

@joaonice
Copy link

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

@mikgross
Copy link

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

@nrail-joerg-walter
Copy link

This is still in the NPM package. Please update!

@jlous
Copy link

jlous commented Oct 21, 2020

No releases i 4 years, no progress on v.3 i 3 years. Is project dead?

@carvalholeo
Copy link

I think yes, @jlous. @mlogan did a fork, but I think that project is dead too. I tried to contact @markbao on Twitter, but he didn't answer me, so I guess that could be great if someone fork again and implement these features/bug fixes.

@jlous
Copy link

jlous commented Oct 26, 2020

Alternatively, https://www.npmjs.com/package/otplib seems to be alive and well.
Will consider migrating my code.

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

No branches or pull requests