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

"decode" function is not used for v9 in strict mode #866

Closed
kento-machida opened this issue Dec 28, 2022 · 14 comments
Closed

"decode" function is not used for v9 in strict mode #866

kento-machida opened this issue Dec 28, 2022 · 14 comments

Comments

@kento-machida
Copy link

Description

We upgraded from v8.5.1 to v9.0.0, but happend TypeError that "decode" is not a function in strict mode.
I think this issue cause is below change, define non-enum in this function.
#741

This change point is not included in Migration note, so we did not notice.
I'm sure that many other users also have used this function.
How about mentioning this matter in the Migration note and Readme?

Reproductiona

In strict mode, use "decode" function.

Environment

jsonwebtoken : ^9.0.0

node : v16.16.0 (use strict mode)
Typescript : ^4.7.4
OS : windows10

@aperona-hai
Copy link

We are also hitting this

@arnoBruynseels
Copy link

arnoBruynseels commented Jan 3, 2023

We are hitting the following issue when we test and mock our code with jest.

import jwt from 'jsonwebtoken';

jest.spyOn(jwt, 'decode').mockImplementation(() => 'some token');

Results into the following error:

Cannot assign to read only property 'decode' of object '[object Object]'

Which has the same root problem, meaning that the decode function now is read only due to it being defined in the defineProperty function which results it in being non-writable beacause the writable property value defaults to false of the defineProperty function.
(https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty)

You can find the source code here:

https://github.com/auth0/node-jsonwebtoken/blob/v9.0.0/index.js

Proposed solution:

Add writable: true in the definition

@nippysaurus
Copy link

It looks like decode might have been removed quite a while ago.
#741

@psyvision
Copy link

It looks like decode might have been removed quite a while ago. #741

It's still there -> https://github.com/auth0/node-jsonwebtoken/blob/v9.0.0/decode.js

#741 was never merged.

We've encountered this too when upgrading to mitigate GHSA-8cf7-32gw-wr33 as we use sinon to stub it for testing.

@blakejoy
Copy link

blakejoy commented Jan 4, 2023

Also running into this issue when using jest. Trying to remediate the same vuln as well @psyvision

@jirihofman
Copy link

It looks like decode might have been removed quite a while ago. #741

It's still there -> https://github.com/auth0/node-jsonwebtoken/blob/v9.0.0/decode.js

#741 was never merged.

We've encountered this too when upgrading to mitigate GHSA-8cf7-32gw-wr33 as we use sinon to stub it for testing.

It seems this is the root cause: 15a1bc4. We are also struggling with sinon stubbing after upgrade from 8.5.1 to 9.0.0.

const jwt = require('jsonwebtoken');
sinon.stub(jwt, 'decode')
TypeError: Cannot redefine property: decode

@robertfines
Copy link

I am running into the same issue when trying to stub decode using sinon after upgrading from 8.5.1 to v9.0.0 to resolve the same vulnerability.
Versions prior to 9.0.0 worked fine doing:

import jwt from 'jsonwebtoken';
import * as sinon from 'sinon';


chai.use(chaiAsPromised);

describe('getPrincipalIdentifier', () => {
  let sandbox: sinon.SinonSandbox;
  let decode: sinon.SinonStub;
  let token: string;
  
  beforeEach(() => {
    sandbox = sinon.createSandbox();
    decode = sandbox.stub(jwt, 'decode');
    token = faker.random.uuid();
  });

  afterEach(() => {
    sandbox.restore();
  });

  it(`should return the principal id from the decoded token`, async () => {
    const pid = faker.random.uuid();
    decode.returns({ payload: { principal_identifier: pid } });
    return chai.expect(getPrincipalIdentifier(token)).to.be.equal(pid);
  });
...
});

But I now receive:
TypeError: Cannot redefine property: decode

@frankyhun
Copy link

frankyhun commented Jan 9, 2023

As a workaround until it gets fixed, instead of

jest.spyOn(jwt, 'decode').mockImplementation(() => 'some token');

you can use

Object.defineProperty(jwt, 'decode', {
  writeable: true,  // necessary to for jest.restoreAllMocks()
  value: () => {return 'some token';},
});

@Doyler123
Copy link

Doyler123 commented Jan 11, 2023

Using Object.defineProperty causes an error

Error:
Cannot redefine property: decode TypeError: Cannot redefine property: decode

@Drew-Kimberly
Copy link

Drew-Kimberly commented Jan 17, 2023

We have legitimate use-cases for decode in production. Bumping to 9.x caused issues for us as well. I don't mind if decode is removed in favor of a more explicitly named method like decodeUnsafe (#741) as long as the functionality is available.

This is particularly pertinent since the 8->9 version bump mitigates a number of vulnerabilities in the package (meaning some consumers are somewhat forced to upgrade ASAP).

@jromero-pg
Copy link

Workaround:

To continue using jest.spyOn(jwt, 'decode'), you could simply mock the module in your test file:

import jwt from 'jsonwebtoken';
//...

jest.mock('jsonwebtoken');
//...

jest.spyOn(jwt, 'decode')
//...

@kento-machida
Copy link
Author

kento-machida commented Jan 23, 2023

I think it should once revert back to defineProperty (delete settings) due to run decode function normally and upgrade to v9 for all users.
Could you please consider the above ?

Object.defineProperty(module.exports, 'decode', {
enumerable: false,
value: require('./decode'),
});

@BenWildeman
Copy link

A better way of making this work in jest without mocking the whole lib is as follows:

import * as jwt from 'jsonwebtoken';

jest.mock('jsonwebtoken', () => {
    return {
        __esModule: true,
        ...jest.requireActual('jsonwebtoken'),
        decode: require('jsonwebtoken/decode')
    };
});

that way you're not mocking every function as suggested in #866 (comment), and still allows you stub decode

@kento-machida
Copy link
Author

This issue already done to fix.
Close it,

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