diff --git a/server/migrations/20251218223224-add-grant-id-to-oauth-models.js b/server/migrations/20251218223224-add-grant-id-to-oauth-models.js new file mode 100644 index 0000000000..332a29b871 --- /dev/null +++ b/server/migrations/20251218223224-add-grant-id-to-oauth-models.js @@ -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, + } + ); + }); + }, +}; diff --git a/server/models/oauth/OAuthAuthentication.ts b/server/models/oauth/OAuthAuthentication.ts index 8d81d8b03a..049112d962 100644 --- a/server/models/oauth/OAuthAuthentication.ts +++ b/server/models/oauth/OAuthAuthentication.ts @@ -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, diff --git a/server/models/oauth/OAuthAuthorizationCode.ts b/server/models/oauth/OAuthAuthorizationCode.ts index d6c742759a..890b979fb3 100644 --- a/server/models/oauth/OAuthAuthorizationCode.ts +++ b/server/models/oauth/OAuthAuthorizationCode.ts @@ -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, diff --git a/server/routes/oauth/index.test.ts b/server/routes/oauth/index.test.ts index d8d9427f64..fbde594d14 100644 --- a/server/routes/oauth/index.test.ts +++ b/server/routes/oauth/index.test.ts @@ -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(); + }); }); }); diff --git a/server/test/factories.ts b/server/test/factories.ts index e17a22233c..2f5ce5e1ff 100644 --- a/server/test/factories.ts +++ b/server/test/factories.ts @@ -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, }); } diff --git a/server/utils/oauth/OAuthInterface.ts b/server/utils/oauth/OAuthInterface.ts index 7f9e3830af..549c765982 100644 --- a/server/utils/oauth/OAuthInterface.ts +++ b/server/utils/oauth/OAuthInterface.ts @@ -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