Get rid of dependencies? about node-oauth2-server HOT 13 CLOSED

I actually think that your suggested code is much better than taking that old stuff from basic-auth package.
I mean, the low complexity of the original package is not worth the potential supply chain attack vector for this package.

from node-oauth2-server.

@jorenvandeweyer I think we keep the basic-auth and the type-is dependencies. However, we should keep this issue open in order to mark this topic as important. There may be the time we may have to fork or drop these deps.

from node-oauth2-server.

I'm going to look more into this today. At first glance think we should definitely remove basic-auth but I agree type-is would be more difficult.

from node-oauth2-server.

It seems that the reason for basic-auth was for safe buffers support so it could support older versions of Node. What would we simply parse the basic auth headers ourselves then?

from node-oauth2-server.

Removing basic-auth would be a major version change since it throws errors. These errors may be being caught in peoples apis.

from node-oauth2-server.

I would prefer that we remove those dependencies. Just simply by the fact, that it reduces the surface for potential attack vectors.

from node-oauth2-server.

@jwerre

You mentioned that integrating or resolving those dependencies would be a major version. I kind of disagree. It would be more of a minor change, as the interfaces would not be changed.

from node-oauth2-server.

@Uzlopak I think what you did would be fine for a minor change but I don't think we should throw the errors. They should be returned.

from node-oauth2-server.

Hmm, we could return Errors, but would this not result in a deoptimization by V8 as the function gets polymorphic?

from node-oauth2-server.

These decisions (throw vs return) should also be consulted by the OAuth 2.0 standard RFC(s) in order to see what it states. We need to be compliant before being performant.

from node-oauth2-server.

First off @Uzlopak, thank you for taking the time to do this. I'm fine with throwing errors but I'm not a fan of throwing frivolous errors. Let's not crash the process when ultimately this is all that's true:

if (typeof string !== 'string') {
    return undefined;
}

getClientCredentials should only throw an invalid_client which it does here.

The errors thrown on lines 53, 57, and 83 should also be invalid_client errors. It is my option that it's better to remove them entirely and return undefined or {} and allow the invalid_client to throw.

from node-oauth2-server.

Can you also add the grant type check after line 176 similar to the one on line 202

from node-oauth2-server.

@jankapunkt any update on you want to approach these dependencies?

from node-oauth2-server.