Comments (13)
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.
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.
Related Issues (20)
- Koa Wrapper for this version? HOT 5
- TypeScript rewrite HOT 6
- `validateRedirectUri` is not in the TypeScript types HOT 1
- An option to require PKCE parameters HOT 6
- Does this library support user approval dialog during authorization code grant? HOT 28
- State of this project? HOT 21
- Is implementation of `verifyScope` required? HOT 17
- generateAuthorizationCode not being awaited HOT 3
- TypeScript: Remove callback from types in 5.x HOT 4
- Move all ES5 style classes into ES6+ style class HOT 2
- getClient called with non-decoded secret/client HOT 3
- [Documentation] revokeAuthorizationCode argument should be named `code.authorizationCode`, not `code.code` HOT 4
- Client Credentials broken in 5.0.0-rc.1 HOT 12
- Insufficient integration tests HOT 3
- Contribution guidelines do not cover how to PR fixes for docs HOT 2
- wrong typing for revokeToken argument HOT 26
- PR #197 fix removed after merge HOT 3
- Typings for `validateScope` don't correctly reflect that `scope` arg can be undefined
- `authenticate` endpoint still expects `scope` as a `string` instead of `string[]` HOT 4
- PKCE Refresh token HOT 12
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from node-oauth2-server.