Comments (13)
RFC 6750 makes it mandatory to use specific characters, which base64 uses, for the Bearer Token.
https://datatracker.ietf.org/doc/html/rfc6750#section-2.1
So they always had in mind, that you could use the Bearer Token for transporting information.
https://www.oauth.com/oauth2-servers/access-tokens/self-encoded-access-tokens/
And this specific "JSON Web Token (JWT) Profile for OAuth 2.0 Client Authentication and Authorization Grants"
https://datatracker.ietf.org/doc/html/rfc7523
https://datatracker.ietf.org/doc/html/draft-ietf-oauth-access-token-jwt
Also this:
https://oauth.net/2/jwt/
from node-oauth2-server.
Also this
oauthjs/node-oauth2-server#629 (comment)
from node-oauth2-server.
This is definitely something that has to be covered in tests as well. @Uzlopak what is the second comment actually saying?
from node-oauth2-server.
I think the issue is still existing. Did not tested though... But only by reading the code it seems trivial to hack such an oauth2-server-implemenation.
In generateAccessToken of the model you get the scope passed. As an implementor you expect that the scope is validated and stripped from invalid or not allowed scopes.
This bug will not occur if you check the access token as simple identifier to look up in the database on every call or dont use scopes. But in cases like JWT and stateless resource server you have a big security issue as you could login with "admin"-scope and till the refreshtoken is used to create a new access token you have the admin scope.
So instead of making a Promise all, first you have to sanitize the scope and then you could run the following generateAccessToken, generateRefreshToken in parallel.
from node-oauth2-server.
Reported it to snyk
I wrote here what I wrote to snyk
oauthjs/node-oauth2-server#720
from node-oauth2-server.
Awesome, but not much hope in them doing anything. In the meantime we can address the fix here.
from node-oauth2-server.
I thought that the RFC implementation is NOT for JWTs, this only handles returning access tokens. No where in the RFC does it mention how to handle JWTs, in which case, how are you using JWTs @Uzlopak ?
from node-oauth2-server.
This seems to be high priority to me. Am I correct, that this is related to #71 ?
from node-oauth2-server.
Well #71 would be a breaking change as we would modify the model. There could be implementations, were it the implementer wants to have partial valid scopes without throwing errors... for what reason...
This issue can be patched in the saveToken functions of ClientCredentialsGrant, PasswordGrant and AuthorizationCodeGrant by running the validateScope of the model separately and use the validated scope in the following code.
E.g.
AuthorizationCodeGrantType.prototype.saveToken = function(user, client, authorizationCode, requestedScope) {
const scope = this.validateScope(user, client, requestedScope);
var fns = [
this.generateAccessToken(client, user, scope),
this.generateRefreshToken(client, user, scope),
this.getAccessTokenExpiresAt(),
this.getRefreshTokenExpiresAt()
];
return Promise.all(fns)
.bind(this)
.spread(function(accessToken, refreshToken, accessTokenExpiresAt, refreshTokenExpiresAt) {
var token = {
accessToken: accessToken,
authorizationCode: authorizationCode,
accessTokenExpiresAt: accessTokenExpiresAt,
refreshToken: refreshToken,
refreshTokenExpiresAt: refreshTokenExpiresAt,
scope: scope
};
return promisify(this.model.saveToken, 3).call(this.model, token, client, user);
});
};
Actually sonarqube was kind of hinting to this bug as it was classifying the "reuse" of the scope variable as code smell. Thats why I renamed the parameter from "scope" to "requestedScope".
from node-oauth2-server.
@Uzlopak can we approach this issue from a different perspective? I first like to create a few tests (which could also be part of the new security test suite, see #65) before implementing the fixes. Can you help me out with a few ones? Maybe creating a new branch fix-scope-bug
and add one or two?
from node-oauth2-server.
I am currently too far in my implementation and tbh I have some deadlines to meet. Last week I printed out RFC 6749 and I implement currently an OAuth2-Authorization Server based on fastify. Everytime I implement something according to spec, i mark the corresponding part in the RFC as done. I use oauth2-server just as inspiration on how to implement it and thats why I am finding those issues, when the code does not fit the corresponding code in oauth2-server. I try to write for everything a unit test. So I am half way through and have 100% test coverage. So the chance for a fuck up shrinks...
But still I would actually prefer if we would first write an InMemoryModel. This is what I wrote for my code (but not used it for unit tests, yet). It would make sense to implement this according your needs and then write some unit tests. You could initialize the model with addUser and addClient in the mocha beforeEach-hook and run clear on the afterEach-hook.
But like I said: I did not use it yet. So maybe it does not work as expected.
import { IClient } from "../interfaces/IClient";
import { IUser } from "../interfaces/IUser";
import generateRandomToken from "../utils/generateRandomToken";
import { IToken } from "../interfaces/IToken";
import { IAuthorizationCode, IModel } from "../interfaces";
const authorizationCodeMap: Map<string, IAuthorizationCode> = new Map();
const clientMap: Map<string, IClient> = new Map();
const accessTokenMap: Map<string, IToken> = new Map();
const refreshTokenMap: Map<string, IToken> = new Map();
const userMap: Map<string, IUser> = new Map();
if (process.env.NODE_ENV === "production") {
console.error("This model is not designed to be used in production. Terminating for security.");
process.exit(1);
}
export const InMemoryModel: IModel = {
getAccessToken: async (accessToken) => {
return accessTokenMap.get(accessToken)!;
},
getAuthorizationCode: async (authorizationCode) => {
return authorizationCodeMap.get(authorizationCode);
},
getClient: async (clientId, clientSecret) => {
const client = clientMap.get(clientId);
if (!client) {
return;
}
if (client.secret && client.secret === clientSecret) {
return client;
}
if (client) {
return client;
}
return;
},
revokeAuthorizationCode: async (code) => {
return authorizationCodeMap.delete(code.authorizationCode);
},
saveAuthorizationCode: async (code, client, user) => {
authorizationCodeMap.set(code.authorizationCode, code);
if (code.expiresAt) {
setTimeout(function () { authorizationCodeMap.delete(code.authorizationCode!); }, (code.expiresAt.getTime() - Date.now()));
}
return code;
},
revokeRefreshToken: async (refreshToken) => {
return refreshTokenMap.delete(refreshToken.refreshToken!);
},
generateAccessToken: async (client, user, scope) => {
return generateRandomToken(32);
},
generateRefreshToken: async (client, user, scope) => {
return generateRandomToken(32);
},
generateAuthorizationCode: async (client, user, scope) => {
return generateRandomToken(32);
},
getRefreshToken: async (refreshToken) => {
return refreshTokenMap.get(refreshToken);
},
getUser: async (username, password) => {
const user = userMap.get(username);
return (
(user && user.password === password)
? user
: undefined
);
},
saveToken: async (token, client, user) => {
const tokenData = {
accessToken : token.accessToken,
accessTokenExpiresAt : token.accessTokenExpiresAt,
refreshToken : token.refreshToken,
refreshTokenExpiresAt: token.refreshTokenExpiresAt,
scope : token.scope,
client : client,
user : user,
};
if (token.accessToken) {
accessTokenMap.set(token.accessToken, tokenData);
setTimeout(function () { accessTokenMap.delete(token.accessToken); }, (token.accessTokenExpiresAt.getTime() - Date.now()));
}
if (token.refreshToken) {
refreshTokenMap.set(token.refreshToken, token);
setTimeout(function () { refreshTokenMap.delete(token.refreshToken!); }, (token.refreshTokenExpiresAt!.getTime() - Date.now()));
}
return tokenData;
},
};
export function addClient(client: IClient) {
clientMap.set(client.id, client);
}
export function addUser(user: IUser) {
userMap.set(user.id, user);
}
export function clear() {
userMap.clear();
clientMap.clear();
authorizationCodeMap.clear();
accessTokenMap.clear();
refreshTokenMap.clear();
}
export default InMemoryModel;
from node-oauth2-server.
@Uzlopak - can you please check on #103 if this fixes the issue?
from node-oauth2-server.
Fixed in #229 for 4.x & #228 for development
from node-oauth2-server.
Related Issues (20)
- Update GitHub core actions HOT 1
- 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
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.