Skip to content

Commit 285c1dd

Browse files
jasonraimondijasonraimondi
committed
fix: add client ownership validation for token revocation per RFC 7009
 - Add client_id to ParsedRefreshToken interface for validation - Enforce client ownership validation during token revocation - Return silent 200 responses for invalid tokens instead of errors - Handle decode errors gracefully per RFC 7009 section 2.2
1 parent a85f808 commit 285c1dd

File tree

6 files changed

+466
-22
lines changed

6 files changed

+466
-22
lines changed

‎src/grants/abstract/abstract.grant.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export interface ParsedAccessToken extends JwtPayload {
4242
}
4343
export interface ITokenData extends ParsedAccessToken {}
4444
export interface ParsedRefreshToken extends JwtPayload {
45+
client_id: string;
4546
access_token_id: string;
4647
refresh_token_id: string;
4748
}

‎src/grants/auth_code.grant.ts‎

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -314,26 +314,53 @@ export class AuthCodeGrant extends AbstractAuthorizedGrant {
314314
async respondToRevokeRequest(req: RequestInterface): Promise<ResponseInterface> {
315315
req.body["grant_type"] = this.identifier;
316316

317-
if (this.options.authenticateRevoke) await this.validateClient(req);
317+
// Silently ignore - per RFC 7009, invalid tokens should not cause error responses
318+
// @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.2
319+
const errorResponse = new OAuthResponse();
320+
321+
let authenticatedClient;
322+
if (this.options.authenticateRevoke) {
323+
try {
324+
authenticatedClient = await this.validateClient(req);
325+
} catch (err) {
326+
this.options.logger?.log(err);
327+
return errorResponse;
328+
}
329+
}
318330

319331
const token = this.getRequestParameter("token", req);
320332

321333
if (!token) {
322-
throw OAuthException.invalidParameter("token", "Missing `token` parameter in request body");
334+
return errorResponse;
323335
}
324336

325-
const parsedCode: unknown = this.jwt.decode(token);
337+
let parsedCode: unknown;
338+
try {
339+
parsedCode = this.jwt.decode(token);
340+
} catch (err) {
341+
this.options.logger?.log(err);
342+
return errorResponse;
343+
}
326344

327345
if (!this.isAuthCodePayload(parsedCode)) {
328-
throw OAuthException.invalidParameter("token", "Token does not contain valid auth code payload");
346+
return errorResponse;
347+
}
348+
349+
if (this.options.authenticateRevoke && authenticatedClient && parsedCode) {
350+
if (parsedCode.client_id !== authenticatedClient.id) {
351+
this.options.logger?.log("Token client ID does not match authenticated client");
352+
return errorResponse;
353+
}
329354
}
330355

331356
try {
332357
await this.authCodeRepository.revoke(parsedCode.auth_code_id);
333-
return new OAuthResponse();
334-
} catch (e) {
335-
throw OAuthException.invalidParameter("token", "Error during token revocation");
358+
} catch (err) {
359+
this.options.logger?.log(err);
360+
// Silently ignore - per RFC 7009, invalid tokens should not cause error responses
336361
}
362+
363+
return errorResponse;
337364
}
338365

339366
private isAuthCodePayload(code: unknown): code is PayloadAuthCode {

‎src/grants/client_credentials.grant.ts‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,15 +64,53 @@ export class ClientCredentialsGrant extends AbstractGrant {
6464
async respondToRevokeRequest(req: RequestInterface): Promise<ResponseInterface> {
6565
req.body["grant_type"] = this.identifier;
6666

67-
if (this.options.authenticateRevoke) await this.validateClient(req);
67+
// Silently ignore - per RFC 7009, invalid tokens should not cause error responses
68+
// @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.2
69+
const errorResponse = new OAuthResponse();
70+
71+
let authenticatedClient;
72+
if (this.options.authenticateRevoke) {
73+
try {
74+
authenticatedClient = await this.validateClient(req);
75+
} catch (err) {
76+
this.options.logger?.log(err);
77+
return errorResponse;
78+
}
79+
}
6880

69-
let { oauthToken } = await this.tokenFromRequest(req);
81+
let parsedToken;
82+
let oauthToken;
83+
try {
84+
const tokenData = await this.tokenFromRequest(req);
85+
parsedToken = tokenData.parsedToken;
86+
oauthToken = tokenData.oauthToken;
87+
} catch (err) {
88+
this.options.logger?.log(err);
89+
return errorResponse;
90+
}
7091

71-
// Invalid tokens do not cause an error response since the client cannot handle such an error.
72-
// @see https://datatracker.ietf.org/doc/html/rfc7009#section-2.2
73-
if (oauthToken) await this.tokenRepository.revoke(oauthToken).catch();
92+
if (this.options.authenticateRevoke && authenticatedClient && parsedToken) {
93+
const tokenClientId = this.options.tokenCID === "id" ? authenticatedClient.id : authenticatedClient.name;
94+
95+
if (this.isAccessTokenPayload(parsedToken) && parsedToken.cid !== tokenClientId) {
96+
this.options.logger?.log("Token client ID does not match authenticated client");
97+
return errorResponse;
98+
}
99+
100+
if (this.isRefreshTokenPayload(parsedToken) && parsedToken.client_id !== authenticatedClient.id) {
101+
this.options.logger?.log("Token client ID does not match authenticated client");
102+
return errorResponse;
103+
}
104+
}
105+
106+
try {
107+
if (oauthToken) await this.tokenRepository.revoke(oauthToken);
108+
} catch (err) {
109+
this.options.logger?.log(err);
110+
// Silently ignore - per RFC 7009, invalid tokens should not cause error responses
111+
}
74112

75-
return new OAuthResponse();
113+
return errorResponse;
76114
}
77115

78116
private readonly revokeTokenTypeHintRegExp = /^(access_token|refresh_token|auth_code)$/;

‎test/e2e/authorization_server.spec.ts‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,7 @@ describe("authorization_server", () => {
525525

526526
describe("#revoke", () => {
527527
let client: OAuthClient = {
528-
id: "1",
528+
id: "16c11812-89da-4d68-9e9c-7715323e34f5",
529529
name: "test client",
530530
secret: "super-secret-secret",
531531
redirectUris: ["http://localhost"],
@@ -591,10 +591,12 @@ describe("authorization_server", () => {
591591
});
592592
});
593593

594-
it("throws when missing client id and secret", async () => {
594+
it("returns 200 when missing client id and secret (silent failure per RFC 7009)", async () => {
595595
request.body = {};
596596

597-
await expect(authorizationServer.revoke(request)).rejects.toThrowError(/Check the `client_id` parameter/i);
597+
const response = await authorizationServer.revoke(request);
598+
expect(response.status).toBe(200);
599+
expect(response.body).toEqual({});
598600
});
599601
});
600602

@@ -607,12 +609,12 @@ describe("authorization_server", () => {
607609
});
608610
});
609611

610-
it("throws when missing token param", async () => {
612+
it("returns 200 when missing token param (silent failure per RFC 7009)", async () => {
611613
request.body = { grant_type: "client_credentials" };
612614

613-
await expect(authorizationServer.revoke(request)).rejects.toThrowError(
614-
/Missing `token` parameter in request body/i,
615-
);
615+
const response = await authorizationServer.revoke(request);
616+
expect(response.status).toBe(200);
617+
expect(response.body).toEqual({});
616618
});
617619

618620
it("succeeds by access token", async () => {
@@ -686,12 +688,16 @@ describe("authorization_server", () => {
686688
};
687689
inMemoryDatabase.authCodes[authCode.code] = authCode;
688690

691+
const token = await testingJwtService.sign({
692+
auth_code_id: authCode.code,
693+
client_id: client.id
694+
});
695+
689696
request.body = {
690-
token: await testingJwtService.sign({ auth_code_id: authCode.code }),
697+
token,
691698
token_type_hint: "auth_code",
692699
};
693700
const response = await authorizationServer.revoke(request);
694-
695701
expect(response.status).toBe(200);
696702
expect(inMemoryDatabase.authCodes[authCode.code].expiresAt).toEqual(new Date(0));
697703
});

‎test/e2e/grants/auth_code.grant.spec.ts‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,4 +586,134 @@ describe("authorization_code grant", () => {
586586
await expect(accessTokenResponse).rejects.toThrow(OAuthException);
587587
});
588588
});
589+
590+
describe("respond to revoke request", () => {
591+
let authorizationRequest: AuthorizationRequest;
592+
let authorizationCode: string;
593+
594+
beforeEach(async () => {
595+
authorizationRequest = new AuthorizationRequest("authorization_code", client, "http://example.com");
596+
authorizationRequest.isAuthorizationApproved = true;
597+
authorizationRequest.codeChallengeMethod = "S256";
598+
authorizationRequest.codeChallenge = codeChallenge;
599+
authorizationRequest.user = user;
600+
const redirectResponse = await grant.completeAuthorizationRequest(authorizationRequest);
601+
const authorizeResponseQuery = new URLSearchParams(redirectResponse.headers.location.split("?")[1]);
602+
authorizationCode = String(authorizeResponseQuery.get("code"));
603+
});
604+
605+
it("successfully revokes valid auth code", async () => {
606+
request = new OAuthRequest({
607+
body: {
608+
token: authorizationCode,
609+
},
610+
});
611+
612+
const response = await grant.respondToRevokeRequest(request);
613+
614+
expect(response.status).toBe(200);
615+
expect(response.body).toEqual({});
616+
});
617+
618+
it("returns 200 for invalid token (silent failure per RFC 7009)", async () => {
619+
request = new OAuthRequest({
620+
body: {
621+
token: "invalid-token",
622+
},
623+
});
624+
625+
const response = await grant.respondToRevokeRequest(request);
626+
627+
expect(response.status).toBe(200);
628+
expect(response.body).toEqual({});
629+
});
630+
631+
it("returns 200 for missing token parameter (silent failure per RFC 7009)", async () => {
632+
request = new OAuthRequest({
633+
body: {},
634+
});
635+
636+
const response = await grant.respondToRevokeRequest(request);
637+
638+
expect(response.status).toBe(200);
639+
expect(response.body).toEqual({});
640+
});
641+
642+
it("returns 200 for malformed JWT token (silent failure per RFC 7009)", async () => {
643+
request = new OAuthRequest({
644+
body: {
645+
token: "not.a.jwt",
646+
},
647+
});
648+
649+
const response = await grant.respondToRevokeRequest(request);
650+
651+
expect(response.status).toBe(200);
652+
expect(response.body).toEqual({});
653+
});
654+
655+
describe("with client authentication", () => {
656+
beforeEach(() => {
657+
grant = createGrant({ authenticateRevoke: true });
658+
});
659+
660+
it("successfully revokes when client owns the token", async () => {
661+
client.secret = "secret123";
662+
inMemoryDatabase.clients[client.id] = client;
663+
664+
request = new OAuthRequest({
665+
headers: {
666+
authorization: `Basic ${Buffer.from(`${client.id}:${client.secret}`).toString("base64")}`,
667+
},
668+
body: {
669+
token: authorizationCode,
670+
},
671+
});
672+
673+
const response = await grant.respondToRevokeRequest(request);
674+
675+
expect(response.status).toBe(200);
676+
expect(response.body).toEqual({});
677+
});
678+
679+
it("returns 200 when client does not own the token (silent failure per RFC 7009)", async () => {
680+
const otherClient = {
681+
id: "other-client",
682+
name: "other client",
683+
secret: "other-secret",
684+
redirectUris: ["http://example.com"],
685+
allowedGrants: ["authorization_code"],
686+
scopes: [],
687+
};
688+
inMemoryDatabase.clients[otherClient.id] = otherClient;
689+
690+
request = new OAuthRequest({
691+
headers: {
692+
authorization: `Basic ${Buffer.from(`${otherClient.id}:${otherClient.secret}`).toString("base64")}`,
693+
},
694+
body: {
695+
token: authorizationCode,
696+
},
697+
});
698+
699+
const response = await grant.respondToRevokeRequest(request);
700+
701+
expect(response.status).toBe(200);
702+
expect(response.body).toEqual({});
703+
});
704+
705+
it("returns 200 when authentication required but not provided (silent failure)", async () => {
706+
request = new OAuthRequest({
707+
body: {
708+
token: authorizationCode,
709+
},
710+
});
711+
712+
const response = await grant.respondToRevokeRequest(request);
713+
714+
expect(response.status).toBe(200);
715+
expect(response.body).toEqual({});
716+
});
717+
});
718+
});
589719
});

0 commit comments

Comments
 (0)