Comments (12)
Also the current implementation enforces that if the client credential flow is used, that there is a technical user assigned to the Client. Thats why we have the getUserFromClient-model-function. This is imho wrong, as the client has an id and secret, which equivalent to the Resource Owner Password Flow. The client id is so to say the userid/username.
So there should be no getUserFromClient method at all, as it makes no sense.
from node-oauth2-server.
@jankapunkt I think we should not remove the validateScope function at this moment.
- Everyone has already implemented this function
- It gives the implementer a lot of freedom, we should only implement the spec & provide the tools.
I'll close this issue for now, we can reopen it later if needed.
from node-oauth2-server.
So do I understand it correctly, that you want to have an additional function that supports the user to validate their integrity before runtime?
from node-oauth2-server.
No. Currently the implementor has to implement e.g. validateScope Method
https://oauth2-server.readthedocs.io/en/latest/model/spec.html#model-validatescope
// list of valid scopes
const VALID_SCOPES = ['read', 'write'];
function validateScope(user, client, scope) {
if (!scope.split(' ').every(s => VALID_SCOPES.indexOf(s) >= 0)) {
return false;
}
return scope;
}
But I say: Why should you implement this function to validateScope?
It should be actually:
// list of valid scopes
const VALID_SCOPES = ['read', 'write'];
function getValidScopesOfUser(user, client, scope) {
return VALID_SCOPES;
}
The actual validating should be done in the Framework, reducing the risk, that the implementor fucks somehow up.
from node-oauth2-server.
So we want to implement how we get scope? I don't think we should, because scope can be passed a number of different ways. By us implementing it, we kinda force the user to pass scope to us a certain way.
If we are to assume a query parameter, scope can be...
'scope=test,test2,test3'
Or
'scope=[test,test2,test]'
Depending on the API design that a person chooses, we can expect either of these things. Not to mention that frameworks like express with automatically concerns these to an array.
Unless the RFC says to pass it a certain way? Then we can provide it but implementing the method can override our default behavior
from node-oauth2-server.
Hmm. Actually frameworks use url package of node to parse the query. But I agree that it could be an issue.
But I also think this argument does not count, because validateScope does not get the request object. It gets user client and scope (derived by oauth2-server from the request) as parameters. So even now the implementor does not have the ability to parse the query string from the original request.
from node-oauth2-server.
I think we can get rid of validateScope completely.
We would need to add scopes to the client. This would be conform with RFC 6749 as it states
https://datatracker.ietf.org/doc/html/rfc6749#section-1.3.4
The client credentials (or other forms of client authentication) can
be used as an authorization grant when the authorization scope is
limited to the protected resources under the control of the client,
or to protected resources previously arranged with the authorization
server.
"under the control of the client" => client specific scopes (?)
And then we have an internal method for filterScopes ( name pending) which takes the userScopes, clientScopes and requestedScopes as parameters.
At the moment we call validateScope we have already loaded the client and the user. So why do we need to pass it to validateScope of the model to run standardized business logic? Model also indicates, that it should be just an abstraction layer for databases and should imho not contain business logic.
- First generate an intersection set of all allowed scopes of the user and all the allowed scopes of the client.
- Check if all the requested scopes are in the intersection or not.
Example Valid:
Step 1:
userScopes: "read write train eat sleep repeat"
clientScopes: "read train sleep repeat"
=> intersection: "read train sleep repeat"
Step 2:
intersection: "read train sleep repeat"
requestedScope: "read"
result: "read"
Example Invalid:
Step 1:
userScopes: "read write train eat sleep repeat"
clientScopes: "read train sleep repeat"
=> intersection: "read train sleep repeat"
Step 2:
intersection: "read train sleep repeat"
requestedScope: "write"
result: ""
You can now either return an access token with an empty scope or just throw an error as we can do nothing we the access token. I think we should throw an InvalidScopeError, as the requested scope is obviously not allowed. And in implementations, where the scope is not utilized we would get empty strings.
In the client credential flow you would just pass the clientScopes as the userScopes, to avoid to have a check which flow we currently use.
I think verifyScope could do the same.
from node-oauth2-server.
I would expect something like this. No need for the intersection.
export function filterScopes(user: Pick<IUser, "scopes">, client: Pick<IClient, "scopes">, requestedScopes: string[]) {
const result: string[] = [];
const userScopesSet = new Set(user.scopes);
const clientScopesSet = new Set(client.scopes);
for (let i = 0, il = requestedScopes.length; i < il; i++) {
if (
userScopesSet.has(requestedScopes[i]) &&
clientScopesSet.has(requestedScopes[i])
) {
result.push(requestedScopes[i]);
} else {
throw new InvalidScopeError()
}
}
return result;
}
from node-oauth2-server.
Or better: Just validate and if an invalid scope is there, throw
export function validateScopes(user: Pick<IUser, "scopes">, client: Pick<IClient, "scopes">, requestedScopes: string[]) {
const userScopesSet = new Set(user.scopes);
const clientScopesSet = new Set(client.scopes);
for (let i = 0, il = requestedScopes.length; i < il; i++) {
if (
!userScopesSet.has(requestedScopes[i]) ||
!clientScopesSet.has(requestedScopes[i])
)
{
throw new InvalidScopeError(
"The requested scope is invalid, unknown, malformed, or exceeds the scope granted by the resource owner."
);
}
}
}
from node-oauth2-server.
To avoid backwards breaking, the method can contain a deprecation warning of some sort but we still run it if it's implemented by some people (aka me lol)
from node-oauth2-server.
Also the current implementation enforces that if the client credential flow is used, that there is a technical user assigned to the Client. Thats why we have the getUserFromClient-model-function. This is imho wrong, as the client has an id and secret, which equivalent to the Resource Owner Password Flow. The client id is so to say the userid/username.
So there should be no getUserFromClient method at all, as it makes no sense.
sorry for offtopic, but is there any issue or PR to make User not required relationship with Client for client_credentials
grant? i also noticed another complaint in the old lib - oauthjs/node-oauth2-server#729
from node-oauth2-server.
In the Client Credential Flow you expect that the Client itself has right to access the resources. So you would actually have a clientId, which would be the actual "username".
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 11
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.