mirror of
https://github.com/outline/outline.git
synced 2026-06-13 11:25:03 +03:00
feat: Implement RFC 9700 hardening against refresh token reuse (#10960)
* feat: Implement RFC 9700 hardening against refresh token reuse * tests * Update tests with less mocking, hit actual endpoints
This commit is contained in:
@@ -0,0 +1,61 @@
|
||||
"use strict";
|
||||
|
||||
/** @type {import("sequelize-cli").Migration} */
|
||||
module.exports = {
|
||||
async up(queryInterface, Sequelize) {
|
||||
await queryInterface.sequelize.transaction(async (transaction) => {
|
||||
await queryInterface.addColumn(
|
||||
"oauth_authentications",
|
||||
"grantId",
|
||||
{
|
||||
type: Sequelize.UUID,
|
||||
allowNull: true,
|
||||
},
|
||||
{ transaction }
|
||||
);
|
||||
|
||||
await queryInterface.addColumn(
|
||||
"oauth_authorization_codes",
|
||||
"grantId",
|
||||
{
|
||||
type: Sequelize.UUID,
|
||||
allowNull: true,
|
||||
},
|
||||
{ transaction }
|
||||
);
|
||||
|
||||
await queryInterface.addIndex("oauth_authentications", ["grantId"], {
|
||||
transaction,
|
||||
});
|
||||
|
||||
await queryInterface.addIndex("oauth_authorization_codes", ["grantId"], {
|
||||
transaction,
|
||||
});
|
||||
});
|
||||
},
|
||||
|
||||
async down(queryInterface, Sequelize) {
|
||||
await queryInterface.sequelize.transaction(async (transaction) => {
|
||||
await queryInterface.removeIndex("oauth_authentications", ["grantId"], {
|
||||
transaction,
|
||||
});
|
||||
await queryInterface.removeIndex(
|
||||
"oauth_authorization_codes",
|
||||
["grantId"],
|
||||
{
|
||||
transaction,
|
||||
}
|
||||
);
|
||||
await queryInterface.removeColumn("oauth_authentications", "grantId", {
|
||||
transaction,
|
||||
});
|
||||
await queryInterface.removeColumn(
|
||||
"oauth_authorization_codes",
|
||||
"grantId",
|
||||
{
|
||||
transaction,
|
||||
}
|
||||
);
|
||||
});
|
||||
},
|
||||
};
|
||||
@@ -74,6 +74,14 @@ class OAuthAuthentication extends ParanoidModel<
|
||||
@Column
|
||||
refreshTokenExpiresAt: Date;
|
||||
|
||||
/**
|
||||
* The ID of the grant that this authentication belongs to. Used for
|
||||
* refresh token rotation and revocation of all tokens in a grant.
|
||||
*/
|
||||
@Column(DataType.UUID)
|
||||
@SkipChangeset
|
||||
grantId: string | null;
|
||||
|
||||
/** A list of scopes that this authentication has access to */
|
||||
@Matches(/[\/\.\w\s]*/, {
|
||||
each: true,
|
||||
|
||||
@@ -48,6 +48,14 @@ class OAuthAuthorizationCode extends IdModel<
|
||||
@SkipChangeset
|
||||
codeChallengeMethod?: string;
|
||||
|
||||
/**
|
||||
* The ID of the grant that this authorization code belongs to. Used for
|
||||
* refresh token rotation and revocation of all tokens in a grant.
|
||||
*/
|
||||
@Column(DataType.UUID)
|
||||
@SkipChangeset
|
||||
grantId: string | null;
|
||||
|
||||
/** A list of scopes that this authorization code has access to */
|
||||
@Matches(/[\/\.\w\s]*/, {
|
||||
each: true,
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import crypto from "crypto";
|
||||
import { Scope } from "@shared/types";
|
||||
import { OAuthAuthentication } from "@server/models";
|
||||
import { OAuthAuthentication, OAuthAuthorizationCode } from "@server/models";
|
||||
import {
|
||||
buildOAuthAuthentication,
|
||||
buildOAuthClient,
|
||||
@@ -230,5 +231,125 @@ describe("#oauth.token", () => {
|
||||
"Missing client_secret for confidential client"
|
||||
);
|
||||
});
|
||||
|
||||
it("should revoke all tokens in a grant when a refresh token is reused", async () => {
|
||||
const user = await buildUser();
|
||||
const client = await buildOAuthClient({
|
||||
teamId: user.teamId,
|
||||
clientType: "confidential",
|
||||
});
|
||||
const grantId = crypto.randomUUID();
|
||||
|
||||
// Create initial authentication
|
||||
const auth1 = await buildOAuthAuthentication({
|
||||
user,
|
||||
scope: [Scope.Read],
|
||||
oauthClientId: client.id,
|
||||
grantId,
|
||||
});
|
||||
|
||||
// Use the refresh token once (rotation)
|
||||
const res1 = await server.post("/oauth/token", {
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: toFormData({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: auth1.refreshToken,
|
||||
client_id: client.clientId,
|
||||
client_secret: client.clientSecret,
|
||||
}),
|
||||
});
|
||||
expect(res1.status).toEqual(200);
|
||||
const body1 = await res1.json();
|
||||
const auth2RefreshToken = body1.refresh_token;
|
||||
|
||||
// Create an unrelated authentication
|
||||
const otherAuth = await buildOAuthAuthentication({
|
||||
user,
|
||||
scope: [Scope.Read],
|
||||
});
|
||||
|
||||
// Use the OLD refresh token again (reuse detection)
|
||||
const res2 = await server.post("/oauth/token", {
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: toFormData({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: auth1.refreshToken,
|
||||
client_id: client.clientId,
|
||||
client_secret: client.clientSecret,
|
||||
}),
|
||||
});
|
||||
|
||||
// The request should fail
|
||||
expect(res2.status).toEqual(400);
|
||||
|
||||
// All tokens in the grant should be revoked
|
||||
const foundAuth1 = await OAuthAuthentication.findByPk(auth1.id, {
|
||||
paranoid: false,
|
||||
});
|
||||
const foundAuth2 =
|
||||
await OAuthAuthentication.findByRefreshToken(auth2RefreshToken);
|
||||
const foundOtherAuth = await OAuthAuthentication.findByPk(otherAuth.id);
|
||||
|
||||
expect(foundAuth1?.deletedAt).toBeTruthy();
|
||||
expect(foundAuth2).toBeNull();
|
||||
expect(foundOtherAuth).not.toBeNull();
|
||||
});
|
||||
|
||||
it("should revoke associated authorization codes when reuse is detected", async () => {
|
||||
const user = await buildUser();
|
||||
const client = await buildOAuthClient({ teamId: user.teamId });
|
||||
const grantId = crypto.randomUUID();
|
||||
|
||||
const auth = await buildOAuthAuthentication({
|
||||
user,
|
||||
scope: [Scope.Read],
|
||||
oauthClientId: client.id,
|
||||
grantId,
|
||||
});
|
||||
|
||||
const code = await OAuthAuthorizationCode.create({
|
||||
authorizationCodeHash: "hash",
|
||||
scope: [Scope.Read],
|
||||
redirectUri: client.redirectUris[0],
|
||||
oauthClientId: client.id,
|
||||
userId: user.id,
|
||||
expiresAt: new Date(Date.now() + 10000),
|
||||
grantId,
|
||||
});
|
||||
|
||||
// Use the refresh token once (rotation)
|
||||
await server.post("/oauth/token", {
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: toFormData({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: auth.refreshToken,
|
||||
client_id: client.clientId,
|
||||
client_secret: client.clientSecret,
|
||||
}),
|
||||
});
|
||||
|
||||
// Use the OLD refresh token again (reuse detection)
|
||||
await server.post("/oauth/token", {
|
||||
headers: {
|
||||
"Content-Type": "application/x-www-form-urlencoded",
|
||||
},
|
||||
body: toFormData({
|
||||
grant_type: "refresh_token",
|
||||
refresh_token: auth.refreshToken,
|
||||
client_id: client.clientId,
|
||||
client_secret: client.clientSecret,
|
||||
}),
|
||||
});
|
||||
|
||||
// The authorization code should be gone
|
||||
const foundCode = await OAuthAuthorizationCode.findByPk(code.id);
|
||||
expect(foundCode).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -797,10 +797,12 @@ export async function buildOAuthAuthentication({
|
||||
oauthClientId,
|
||||
user,
|
||||
scope,
|
||||
grantId,
|
||||
}: {
|
||||
oauthClientId?: string;
|
||||
user: User;
|
||||
scope: string[];
|
||||
grantId?: string;
|
||||
}) {
|
||||
const oauthClient = oauthClientId
|
||||
? await OAuthClient.findByPk(oauthClientId, { rejectOnEmpty: true })
|
||||
@@ -836,6 +838,7 @@ export async function buildOAuthAuthentication({
|
||||
refreshTokenHash: hash(refreshToken),
|
||||
refreshTokenExpiresAt: new Date(Date.now() + 1000 * 60 * 60 * 24 * 30),
|
||||
scope,
|
||||
grantId,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import crypto from "crypto";
|
||||
import {
|
||||
RefreshTokenModel,
|
||||
AuthorizationCodeModel,
|
||||
User as OAuthUser,
|
||||
} from "@node-oauth/oauth2-server";
|
||||
import { Required } from "utility-types";
|
||||
import { Scope } from "@shared/types";
|
||||
@@ -21,6 +22,14 @@ interface Config {
|
||||
grants: string[];
|
||||
}
|
||||
|
||||
/**
|
||||
* An extension of the OAuth2Server User type that includes a grantId for
|
||||
* session tracking.
|
||||
*/
|
||||
interface GrantUser extends OAuthUser {
|
||||
grantId?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* This interface is used by the OAuth2Server library to handle OAuth2
|
||||
* authentication and authorization flows. See the library's documentation:
|
||||
@@ -40,24 +49,45 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
/** Supported grant types */
|
||||
grants: ["authorization_code", "refresh_token"],
|
||||
|
||||
/**
|
||||
* Generates a new access token.
|
||||
*
|
||||
* @returns The generated access token.
|
||||
*/
|
||||
async generateAccessToken() {
|
||||
return `${OAuthAuthentication.accessTokenPrefix}${crypto
|
||||
.randomBytes(32)
|
||||
.toString("hex")}`;
|
||||
},
|
||||
|
||||
/**
|
||||
* Generates a new refresh token.
|
||||
*
|
||||
* @returns The generated refresh token.
|
||||
*/
|
||||
async generateRefreshToken() {
|
||||
return `${OAuthAuthentication.refreshTokenPrefix}${crypto
|
||||
.randomBytes(32)
|
||||
.toString("hex")}`;
|
||||
},
|
||||
|
||||
/**
|
||||
* Generates a new authorization code.
|
||||
*
|
||||
* @returns The generated authorization code.
|
||||
*/
|
||||
async generateAuthorizationCode() {
|
||||
return `${OAuthAuthorizationCode.authorizationCodePrefix}${crypto
|
||||
.randomBytes(32)
|
||||
.toString("hex")}`;
|
||||
},
|
||||
|
||||
/**
|
||||
* Retrieves an access token by its value.
|
||||
*
|
||||
* @param accessToken The access token to retrieve.
|
||||
* @returns The access token if found, false otherwise.
|
||||
*/
|
||||
async getAccessToken(accessToken: string) {
|
||||
const authentication =
|
||||
await OAuthAuthentication.findByAccessToken(accessToken);
|
||||
@@ -77,13 +107,47 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Retrieves a refresh token by its value, with reuse detection.
|
||||
*
|
||||
* @param refreshToken The refresh token to retrieve.
|
||||
* @returns The refresh token if found, false otherwise.
|
||||
*/
|
||||
async getRefreshToken(refreshToken: string) {
|
||||
const authentication =
|
||||
let authentication =
|
||||
await OAuthAuthentication.findByRefreshToken(refreshToken);
|
||||
|
||||
if (!authentication) {
|
||||
// If the refresh token is not found, it may have already been used or
|
||||
// revoked. In this case we perform reuse detection as recommended by RFC 9700.
|
||||
authentication = await OAuthAuthentication.findOne({
|
||||
where: {
|
||||
refreshTokenHash: hash(refreshToken),
|
||||
},
|
||||
paranoid: false,
|
||||
});
|
||||
|
||||
if (authentication?.grantId) {
|
||||
await Promise.all([
|
||||
OAuthAuthentication.destroy({
|
||||
where: {
|
||||
grantId: authentication.grantId,
|
||||
},
|
||||
}),
|
||||
OAuthAuthorizationCode.destroy({
|
||||
where: {
|
||||
grantId: authentication.grantId,
|
||||
},
|
||||
}),
|
||||
]);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
const user = authentication.user;
|
||||
Object.assign(user, { grantId: authentication.grantId });
|
||||
|
||||
return {
|
||||
refreshToken,
|
||||
refreshTokenExpiresAt: authentication.refreshTokenExpiresAt,
|
||||
@@ -92,10 +156,16 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
id: authentication.oauthClient.clientId,
|
||||
grants: this.grants,
|
||||
},
|
||||
user: authentication.user,
|
||||
user,
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Retrieves an authorization code by its value.
|
||||
*
|
||||
* @param authorizationCode The authorization code to retrieve.
|
||||
* @returns The authorization code if found, false otherwise.
|
||||
*/
|
||||
async getAuthorizationCode(authorizationCode) {
|
||||
const code = await OAuthAuthorizationCode.findByCode(authorizationCode);
|
||||
if (!code) {
|
||||
@@ -107,6 +177,9 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
return false;
|
||||
}
|
||||
|
||||
const user = code.user;
|
||||
Object.assign(user, { grantId: code.grantId });
|
||||
|
||||
return {
|
||||
authorizationCode,
|
||||
expiresAt: code.expiresAt,
|
||||
@@ -118,10 +191,17 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
id: oauthClient.clientId,
|
||||
grants: this.grants,
|
||||
},
|
||||
user: code.user,
|
||||
user,
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Retrieves a client by its ID and secret.
|
||||
*
|
||||
* @param clientId The client ID.
|
||||
* @param clientSecret The client secret.
|
||||
* @returns The client if found and valid, false otherwise.
|
||||
*/
|
||||
async getClient(clientId: string, clientSecret?: string) {
|
||||
const client = await OAuthClient.findByClientId(clientId);
|
||||
if (!client) {
|
||||
@@ -141,6 +221,14 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Saves an access and refresh token.
|
||||
*
|
||||
* @param token The token object to save.
|
||||
* @param client The client that requested the token.
|
||||
* @param user The user that authorized the token.
|
||||
* @returns The saved token.
|
||||
*/
|
||||
async saveToken(token, client, user) {
|
||||
const {
|
||||
accessToken,
|
||||
@@ -159,6 +247,7 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
scope: token.scope,
|
||||
oauthClientId: client.databaseId,
|
||||
userId: user.id,
|
||||
grantId: (user as GrantUser).grantId || crypto.randomUUID(),
|
||||
});
|
||||
|
||||
return {
|
||||
@@ -175,6 +264,14 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Saves an authorization code.
|
||||
*
|
||||
* @param code The authorization code object to save.
|
||||
* @param client The client that requested the code.
|
||||
* @param user The user that authorized the code.
|
||||
* @returns The saved authorization code.
|
||||
*/
|
||||
async saveAuthorizationCode(code, client, user) {
|
||||
const {
|
||||
authorizationCode,
|
||||
@@ -194,6 +291,7 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
codeChallengeMethod,
|
||||
oauthClientId: client.databaseId,
|
||||
userId: user.id,
|
||||
grantId: (user as GrantUser).grantId || crypto.randomUUID(),
|
||||
});
|
||||
|
||||
return {
|
||||
@@ -209,6 +307,12 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
};
|
||||
},
|
||||
|
||||
/**
|
||||
* Revokes a refresh token.
|
||||
*
|
||||
* @param token The token object containing the refresh token to revoke.
|
||||
* @returns True if the token was revoked, false otherwise.
|
||||
*/
|
||||
async revokeToken(token) {
|
||||
const auth = await OAuthAuthentication.findByRefreshToken(
|
||||
token.refreshToken
|
||||
@@ -220,6 +324,12 @@ export const OAuthInterface: RefreshTokenModel &
|
||||
return false;
|
||||
},
|
||||
|
||||
/**
|
||||
* Revokes an authorization code.
|
||||
*
|
||||
* @param code The authorization code object to revoke.
|
||||
* @returns True if the code was revoked, false otherwise.
|
||||
*/
|
||||
async revokeAuthorizationCode(code) {
|
||||
const authCode = await OAuthAuthorizationCode.findByCode(
|
||||
code.authorizationCode
|
||||
|
||||
Reference in New Issue
Block a user