box-node-sdk icon indicating copy to clipboard operation
box-node-sdk copied to clipboard

Replace request with the native https module, or a more secure library with no dependencies

Open coolaj86 opened this issue 3 years ago • 1 comments

Is your feature request related to a problem? Please describe.

Security assurance. request has hundreds of transitive dependencies, none of which are needed for what the Box SDK does.

Literally 0.

Describe the solution you'd like

Wrap node's native https with about 50-100 LoC, as seen here: https://github.com/box/box-node-sdk/issues/537#issuecomment-918659303

Or, use a smaller, safer library, such as @root/request or perhaps even axios.

However, for security, and because the core business value of this library is to wrap an http request, I'd recommend just using the straight https module in a wrapper function like you do presently.

Describe alternatives you've considered

I could almost use shrinkwrap.json to replace request with @root/request, but it would also require a small code change (adding an await - re: https://github.com/therootcompany/request.js#super-simple-to-use "streaming").

Additional context

https://github.com/box/box-node-sdk/blob/main/src/api-request.ts#L233

coolaj86 avatar Sep 13 '21 23:09 coolaj86

Hi @coolaj86 ,

Thanks for submitting this request! It's already on our roadmap (SDK-1381), but we don't currently have it prioritized right now. We'll take a look and get back to you when we have a better idea of how we're going to address it.

Thanks so much!

@PJSimon

PJSimon avatar Sep 14 '21 23:09 PJSimon

Hello,

Could we have an update on this issue ? THX

euidzero avatar May 25 '23 08:05 euidzero

Hi @euidzero ,

Recent release has a bunch of security fixes and updates, but not replacement of requests library. Team is currently working on a refreshed version of Javascript/Typescript SDK that uses node-fetch. Refreshed SDK will be available in June-July timeframe and will come with significant security bump and higher API coverage.

mgrytsai avatar May 29 '23 08:05 mgrytsai

Hi @mgrytsai Do we have any updates on the refreshed SDK as mentioned in the above comment. We are awaiting for this release.

manukm-ibm avatar Aug 01 '23 03:08 manukm-ibm

Hi @manukm-ibm @euidzero @coolaj86

We already released the beta version of the new SDK with the name box-typescript-sdk-gen which is already available on npm.

Please take a look. We are looking forward to any feedback from you all.

Thanks, Minh

congminh1254 avatar Aug 23 '23 08:08 congminh1254

Hi, Correct me if I'm wrong but this new SDK typescript only, meaning we'll have to rewrite all of our javascript code ?

euidzero avatar Aug 29 '23 12:08 euidzero

Hi @euidzero,

This SDK can work with javascript code as usual without any problem, you just need migrate function / class name to match with this new version of SDK.

Regards, Minh

congminh1254 avatar Aug 29 '23 12:08 congminh1254

Is there an update for when the nodejs package will fix this vulnerability? It has been outstanding for quite some time now. It is understood that there is a beta version for typescript, but this only seems to accommodate a developer token connection (tokens are only valid 1hr) - how can we connect with OAuth 2.0 with JSON Web Tokens (Server Authentication)?

emilydoran1 avatar Sep 07 '23 14:09 emilydoran1

how can we connect with OAuth 2.0 with JSON Web Tokens (Server Authentication)?

box-typescript-sdk-gen also supports OAuth 2.0, JWT and CCG authentication. You can find guides and tutorials in the docs directory.

Here is guide how to authenticate with OAuth 2.0 an JSON Web Token (JWT)

We've evaluated the possibility of replacing requests library in box-node-sdk. However, doing so would result in a significant breaking change for many users. Instead, we recommend exploring the advantages offered by the new box-typescript-sdk-gen.

mgrytsai avatar Sep 08 '23 09:09 mgrytsai

Hello, I've tried replacing the authentication config with those outlined in the docs, however, that breaks all of my API calls. It really doesn't seem practical to have everyone switch to this typescript package, refactor the connection, AND have to refactor the code with zero guidance. I cannot make a simple call such as this.client.files.uploadFile(folderID, filename, stream).

I have 4+ enterprise-grade application calling many Box APIs in series, it is unacceptable to require all clients to refactor each API call to accommodate this. Responses are unable to be parsed to get simple fields such as statusCode, and Box API reference doesn't contain any samples for this. This vulnerability has been outstanding for ~6 months now, and developers have been told the update would be in place by July (which is 2 months past now) and now we're being told that we must refactor all existing code to a new package.

emilydoran1 avatar Sep 08 '23 17:09 emilydoran1

We've made a numerous attempts to replace deprecated request package. However, it was not a feasible option.

Since we are in beta phase with box-typescript-sdk-gen, I'd like to understand better what are the biggest challenges you have. Would you be open to have a short zoom call with SDK team to help troubleshoot the migration? If so, please send me a note at [email protected] and I'll schedule a call.

Alternatively, could you also share sample code that you are trying to migrate and we can troubleshoot together?

mgrytsai avatar Sep 11 '23 13:09 mgrytsai

                var stream = fs.createReadStream(filepath)

                let attrs = { name: filename, parent: { id: folderID } }
                let body = {
                    attributes: attrs,
                    file: stream
                }
                await this.client.uploads.uploadFile(body)
                this.log.info('File uploaded to Box.')
                resolve('File uploaded to Box.')
            } catch (e) {
                let parsedError = JSON.parse(e.message)
                // check if error caused because file already exists
                if(e.statusCode == 409){
                    this.log.info('File cannot be uploaded because it already exists')
                    resolve('File already exists')
                    return
                }

I am currently trying to parse the error message to get the status code (to see if file already exists) -- the error.message is not parsable it appears. For this app, I also need to call the Box search item API, upload file version (not sure if typescript package has this, I couldn't find in the docs). I was able to get the status via a temporary workaround to parse the text between the brackets. However, searching content without an example in the docs is quite a headache to debug.

emilydoran1 avatar Sep 11 '23 15:09 emilydoran1

Hello @emilydoran1, thank you for your comment. It will contribute to the improvement of the box-typescript-sdk-gen.

As for the errors, we are working on their reorganization, so that, among other things, the statusCode you mentioned will be directly available in the object without the need for message parsing.

We also know that currently, the documentation is incomplete, so we are still working on expanding it. Additionally, we are also working on a migration guide that should be added to our SDK soon, which will facilitate the migration from box-node-sdk to box-typescript-sdk-gen.

Below, I present code snippets for searching and uploading file versions:

 const items = await client.search.getSearch({query: 'Mobile', type: 'file', fields: 'name,modified_at,size,extension,permissions,sync_state', file_extensions: 'pdf,doc', limit: 2});
  items.entries.forEach((item) =>
  console.log(`${item.type} ${item.id} is named "${item.name}"`)
);

Uploading a file version is similar to uploading a file, with the additional requirement of the fileId argument.

const attrs = { name: 'filename.txt', parent: { id: '0' } };
const body = {
  attributes: attrs,
  file: fs.createReadStream('filename.txt')
};

const fileId = '1234'
const files = await client.uploads.uploadFileVersion(fileId, body)
const file = files.entries[0];
console.log(file)
console.log(`File uploaded with id ${file.id}, name ${file.name});

An advantage of our generated SDKs is that they completely cover the public Box API at https://developer.box.com/reference/. If a method is located in a specific section on the page at https://developer.box.com/reference/, you can also find that method in the manager with the same (or similar) name in our SDK. For example, the "UPLOADS" section on the website has methods like "Upload file," "Upload file version," and "Preflight check before upload." In our SDK, to refer to these methods, you simply need to call:

await client.uploads.uploadFile(body);
client.uploads.uploadFileVersion(fileId, body)
await client.uploads.preflightFileUpload(body)

As for the parameter convention, we have prepared the generated API so that path parameters (if available for the API) are placed first, followed by the requestBody object, then an object containing all query parameters, and finally an object that accepts headers.

We apologize for any shortcomings. We are currently working intensively to ensure that box-typescript-sdk-gen meets yours expectations. If there is anything we can assist with or if you have any further questions, please don't hesitate to let us know, and we will do our best to help you.

Regards, Artur

arjankowski avatar Sep 21 '23 16:09 arjankowski

Hi @coolaj86

We have just replace request library with @cypress/request and it fixes all the security issues.

Please check if it's working on your side now.

Best, Minh

congminh1254 avatar Oct 26 '23 12:10 congminh1254