Comments (7)
@jorenvandeweyer great you found this!
from node-oauth2-server.
I thought about this issue yesterday.
We have to be careful how we implement this. With the current ignored scope parameter there is no potential security issue. You always get the scope of the original requested scope (?).
But Imagine following case:
Authorization Code Grant: User can select three scopes: "read write delete" but selects only read scope. Now if refresh token would contains scope value, it could contain "read write delete" scope. What you would expect is that you have again only "read"-scope as the user specified in the authorization code flow specified that he only wants to grant "read" permission to the resource server.
If we would just check if the user has the scope allowed by using the usual validateScope-method, an attacker could get with a refresh token more privileges than actually allowed by the user in the first password or authorization- grant. So we actually have to persist the original requested scope from the user then always check against that original scope when a scope is specified in the refresh token flow.
from node-oauth2-server.
@jorenvandeweyer is this still an issue?
from node-oauth2-server.
Yes this is still an issue and it is not that easy to fix.
- We should check our scope input better
scope = scope-token *( SP scope-token )
scope-token = 1*( %x21 / %x23-5B / %x5D-7E )
regex: ^[\x21\x23-\x5B\x5D-\x7E]+(?:\s+[\x21\x23-\x5B\x5D-\x7E]+)*$
-
We should always parse the scope into an array of strings, this would be breaking.
-
Then we can validate the scope input against the original scope with something like this
const previousScopes = ['user:read', 'user:write']
const newScopes = ['user:read']
const valid = newScopes.every(newScope => previousScopes.includes(newScope))
from node-oauth2-server.
We should always parse the scope into an array of strings, this would be breaking
If it's entirely internal (transforming any incoming scope to an array before furhter) I think it's at least non-breaking for "the outside", right?
I think this should be implemented in the formats package, what do you think?
from node-oauth2-server.
I think we should also change it for the outside.
Line 385 in 48baa8b
would become:
validateScope?(user: User, client: Client, scope: string[]): Promise<string[] | Falsey>;
which means that all implementations of validateScope functions should return a string instead of an array of strings.
from node-oauth2-server.
Then let's target this for 5.x and mark it breaking
from node-oauth2-server.
Related Issues (20)
- 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
- Poor example code for verifyScope in docs
- `AuthenticateOptions.scope` type is declared incorrectly
- v5.1`AuthorizationOptions.scope`Breaking Change HOT 3
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.