feat: Allow PKCE clients to refresh tokens (#10769)

* Add clientType concept

* Add clientType mutations

* tsc

* i18n

* fix: Invalid input handling

* tsc
This commit is contained in:
Tom Moor
2025-12-03 18:09:43 -05:00
committed by GitHub
parent a8048962f6
commit 94252672f8
17 changed files with 409 additions and 31 deletions
+1 -1
View File
@@ -50,7 +50,7 @@ export type Item = {
export type Option = Item | Separator;
type Props = {
type Props = Omit<React.HTMLAttributes<HTMLButtonElement>, "onChange"> & {
/* Options to display in the select menu. */
options: Option[];
/* Current chosen value. */
@@ -0,0 +1,37 @@
import * as React from "react";
import { useTranslation } from "react-i18next";
import { InputSelect } from "../InputSelect";
/**
* An input that allows a choice of OAuth client type.
*/
export const InputClientType = React.forwardRef(
(
props: Omit<React.ComponentProps<typeof InputSelect>, "options" | "label">,
ref: React.Ref<HTMLButtonElement>
) => {
const { t } = useTranslation();
return (
<InputSelect
{...props}
label={t("Client type")}
ref={ref}
style={{ marginBottom: "1em" }}
options={[
{
type: "item",
label: t("Confidential"),
value: "confidential",
description: t("Suitable for server-side applications"),
},
{
type: "item",
label: t("Public"),
value: "public",
description: t("Suitable for client-side or mobile applications"),
},
]}
/>
);
}
);
@@ -11,6 +11,7 @@ import Input, { LabelText } from "~/components/Input";
import isCloudHosted from "~/utils/isCloudHosted";
import Switch from "../Switch";
import EventBoundary from "@shared/components/EventBoundary";
import { InputClientType } from "./InputClientType";
export interface FormData {
name: string;
@@ -20,6 +21,7 @@ export interface FormData {
avatarUrl: string;
redirectUris: string[];
published: boolean;
clientType: "confidential" | "public";
}
export const OAuthClientForm = observer(function OAuthClientForm_({
@@ -47,6 +49,7 @@ export const OAuthClientForm = observer(function OAuthClientForm_({
avatarUrl: oauthClient?.avatarUrl ?? "",
redirectUris: oauthClient?.redirectUris ?? [],
published: oauthClient?.published ?? false,
clientType: oauthClient?.clientType ?? "confidential",
},
});
@@ -79,6 +82,17 @@ export const OAuthClientForm = observer(function OAuthClientForm_({
)}
/>
</label>
<Controller
control={control}
name="clientType"
render={({ field }) => (
<InputClientType
value={field.value}
onChange={field.onChange}
ref={field.ref}
/>
)}
/>
<Input
type="text"
label={t("Name")}
+5
View File
@@ -45,6 +45,11 @@ class OAuthClient extends ParanoidModel {
@observable
clientSecret: string;
/** The type of this client, confidential or public */
@Field
@observable
clientType: "confidential" | "public";
/** Whether this app is published (available to other workspaces) */
@Field
@observable
+48 -25
View File
@@ -31,6 +31,7 @@ import ImageInput from "./components/ImageInput";
import SettingRow from "./components/SettingRow";
import { createInternalLinkActionV2 } from "~/actions";
import { NavigationSection } from "~/actions/sections";
import { InputClientType } from "~/components/OAuthClient/InputClientType";
type Props = {
oauthClient: OAuthClient;
@@ -76,6 +77,7 @@ const Application = observer(function Application({ oauthClient }: Props) {
avatarUrl: oauthClient.avatarUrl ?? "",
redirectUris: oauthClient.redirectUris ?? [],
published: oauthClient.published ?? false,
clientType: oauthClient.clientType,
},
});
@@ -175,6 +177,25 @@ const Application = observer(function Application({ oauthClient }: Props) {
/>
</SettingRow>
<SettingRow
name="clientType"
label={t("Client type")}
description={t("Confidential clients can securely store a secret")}
>
<Controller
control={control}
name="clientType"
render={({ field }) => (
<InputClientType
hideLabel
value={field.value}
onChange={field.onChange}
ref={field.ref}
/>
)}
/>
</SettingRow>
<SettingRow
name="description"
label={t("Tagline")}
@@ -252,33 +273,35 @@ const Application = observer(function Application({ oauthClient }: Props) {
/>
</Input>
</SettingRow>
<SettingRow
name="clientSecret"
label={t("OAuth client secret")}
description={t(
"Store this value securely, do not expose it publicly"
)}
>
<Input
id="clientSecret"
type="password"
value={oauthClient.clientSecret}
readOnly
{oauthClient.clientType === "confidential" && (
<SettingRow
name="clientSecret"
label={t("OAuth client secret")}
description={t(
"Store this value securely, do not expose it publicly"
)}
>
<Tooltip content={t("Rotate secret")} placement="top">
<NudeButton type="button" onClick={handleRotateSecret}>
<ReplaceIcon size={20} />
</NudeButton>
</Tooltip>
<CopyButton
<Input
id="clientSecret"
type="password"
value={oauthClient.clientSecret}
success={t("Copied to clipboard")}
tooltip={t("Copy")}
icon={<CopyIcon size={20} />}
/>
</Input>
</SettingRow>
readOnly
>
<Tooltip content={t("Rotate secret")} placement="top">
<NudeButton type="button" onClick={handleRotateSecret}>
<ReplaceIcon size={20} />
</NudeButton>
</Tooltip>
<CopyButton
value={oauthClient.clientSecret}
success={t("Copied to clipboard")}
tooltip={t("Copy")}
icon={<CopyIcon size={20} />}
/>
</Input>
</SettingRow>
)}
<SettingRow
name="redirectUris"
label={t("Callback URLs")}
@@ -0,0 +1,16 @@
"use strict";
/** @type {import('sequelize-cli').Migration} */
module.exports = {
async up(queryInterface, Sequelize) {
await queryInterface.addColumn("oauth_clients", "clientType", {
type: Sequelize.STRING,
allowNull: false,
defaultValue: "confidential",
});
},
async down(queryInterface, Sequelize) {
await queryInterface.removeColumn("oauth_clients", "clientType");
},
};
+5
View File
@@ -15,6 +15,7 @@ import {
Length,
BeforeCreate,
AllowNull,
IsIn,
} from "sequelize-typescript";
import { randomString } from "@shared/random";
import { OAuthClientValidation } from "@shared/validations";
@@ -71,6 +72,10 @@ class OAuthClient extends ParanoidModel<
@Column
clientId: string;
@IsIn([Array.from(OAuthClientValidation.clientTypes)])
@Column(DataType.STRING)
clientType: (typeof OAuthClientValidation.clientTypes)[number];
@Column(DataType.BLOB)
@Encrypted
clientSecret: string;
+1
View File
@@ -15,6 +15,7 @@ export default function presentOAuthClient(oauthClient: OAuthClient) {
avatarUrl: oauthClient.avatarUrl,
clientId: oauthClient.clientId,
clientSecret: oauthClient.clientSecret,
clientType: oauthClient.clientType,
redirectUris: oauthClient.redirectUris,
published: oauthClient.published,
createdAt: oauthClient.createdAt,
+8
View File
@@ -22,6 +22,11 @@ export type OAuthClientsInfoReq = z.infer<typeof OAuthClientsInfoSchema>;
export const OAuthClientsCreateSchema = BaseSchema.extend({
body: z.object({
/** OAuth client type */
clientType: z
.enum(OAuthClientValidation.clientTypes)
.default("confidential"),
/** OAuth client name */
name: z.string(),
@@ -63,6 +68,9 @@ export const OAuthClientsUpdateSchema = BaseSchema.extend({
body: z.object({
id: z.string().uuid(),
/** OAuth client type */
clientType: z.enum(OAuthClientValidation.clientTypes).optional(),
/** OAuth client name */
name: z.string().optional(),
+189 -2
View File
@@ -1,7 +1,11 @@
import { Scope } from "@shared/types";
import { OAuthAuthentication } from "@server/models";
import { buildOAuthAuthentication, buildUser } from "@server/test/factories";
import { getTestServer } from "@server/test/support";
import {
buildOAuthAuthentication,
buildOAuthClient,
buildUser,
} from "@server/test/factories";
import { getTestServer, toFormData } from "@server/test/support";
const server = getTestServer();
@@ -45,3 +49,186 @@ describe("#oauth.revoke", () => {
expect(res.status).toEqual(200);
});
});
describe("#oauth.token", () => {
describe("refresh_token grant", () => {
it("should successfully refresh token for confidential client with client_secret", async () => {
const user = await buildUser();
const client = await buildOAuthClient({
teamId: user.teamId,
clientType: "confidential",
});
const auth = await buildOAuthAuthentication({
user,
scope: [Scope.Read],
oauthClientId: client.id,
});
const refreshToken = auth.refreshToken;
// Reload with oauthClient included
await auth.reload({ include: ["oauthClient"] });
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: auth.oauthClient.clientId,
client_secret: auth.oauthClient.clientSecret,
}),
});
expect(res.status).toEqual(200);
const body = await res.json();
expect(body.access_token).toBeTruthy();
expect(body.refresh_token).toBeTruthy();
expect(body.token_type).toBe("Bearer");
expect(body.expires_in).toBeGreaterThan(0);
});
it("should successfully refresh token for public client without client_secret", async () => {
const user = await buildUser();
const client = await buildOAuthClient({
teamId: user.teamId,
clientType: "public",
});
const auth = await buildOAuthAuthentication({
user,
scope: [Scope.Read],
oauthClientId: client.id,
});
const refreshToken = auth.refreshToken;
// Reload with oauthClient included
await auth.reload({ include: ["oauthClient"] });
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: auth.oauthClient.clientId,
}),
});
expect(res.status).toEqual(200);
const body = await res.json();
expect(body.access_token).toBeTruthy();
expect(body.refresh_token).toBeTruthy();
expect(body.token_type).toBe("Bearer");
expect(body.expires_in).toBeGreaterThan(0);
});
it("should successfully refresh token for public client with client_secret", async () => {
const user = await buildUser();
const client = await buildOAuthClient({
teamId: user.teamId,
clientType: "public",
});
const auth = await buildOAuthAuthentication({
user,
scope: [Scope.Read],
oauthClientId: client.id,
});
const refreshToken = auth.refreshToken;
// Reload with oauthClient included
await auth.reload({ include: ["oauthClient"] });
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: auth.oauthClient.clientId,
client_secret: auth.oauthClient.clientSecret,
}),
});
expect(res.status).toEqual(200);
const body = await res.json();
expect(body.access_token).toBeTruthy();
expect(body.refresh_token).toBeTruthy();
expect(body.token_type).toBe("Bearer");
expect(body.expires_in).toBeGreaterThan(0);
});
it("should error when refresh_token is missing for refresh_token grant", async () => {
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
client_id: "test-client-id",
}),
});
expect(res.status).toEqual(400);
const body = await res.json();
expect(body.error).toBeDefined();
expect(body.error_description).toContain(
"Missing refresh_token for refresh_token grant type"
);
});
it("should error when client_id is invalid", async () => {
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
refresh_token: "invalid-refresh-token",
client_id: "test-client-id",
}),
});
expect(res.status).toEqual(400);
const body = await res.json();
expect(body.error).toBeDefined();
expect(body.error_description).toContain("Invalid client_id");
});
it("should error when confidential client tries to refresh without client_secret", async () => {
const user = await buildUser();
const client = await buildOAuthClient({
teamId: user.teamId,
clientType: "confidential",
});
const auth = await buildOAuthAuthentication({
user,
scope: [Scope.Read],
oauthClientId: client.id,
});
const refreshToken = auth.refreshToken;
// Reload with oauthClient included
await auth.reload({ include: ["oauthClient"] });
const res = await server.post("/oauth/token", {
headers: {
"Content-Type": "application/x-www-form-urlencoded",
},
body: toFormData({
grant_type: "refresh_token",
refresh_token: refreshToken,
client_id: auth.oauthClient.clientId,
}),
});
expect(res.status).toEqual(400);
const body = await res.json();
expect(body.error).toBeDefined();
expect(body.error_description).toContain(
"Missing client_secret for confidential client"
);
});
});
});
+34 -1
View File
@@ -22,6 +22,13 @@ const app = new Koa();
const router = new Router();
const oauth = new OAuth2Server({
model: OAuthInterface,
requireClientAuthentication: {
// Allow public clients (those without a client secret) to refresh without a client secret.
refresh_token: false,
},
// Always revoke the used refresh token and issue a new one, see:
// https://www.rfc-editor.org/rfc/rfc6819#section-5.2.2.3
alwaysIssueNewRefreshToken: true,
});
router.post(
@@ -70,8 +77,34 @@ router.post(
router.post(
"/token",
validate(T.TokenSchema),
rateLimiter(RateLimiterStrategy.OneHundredPerHour),
async (ctx) => {
async (ctx: APIContext<T.TokenReq>) => {
const grantType = ctx.input.body.grant_type;
const refreshToken = ctx.input.body.refresh_token;
const clientId = ctx.input.body.client_id;
const clientSecret = ctx.input.body.client_secret;
// Because we disabled client authentication for refresh_token grant type at the library
// initialization, we need to manually enforce it here for confidential clients.
if (grantType === "refresh_token" && !clientSecret) {
if (!refreshToken) {
throw ValidationError(
"Missing refresh_token for refresh_token grant type"
);
}
if (!clientId) {
throw ValidationError("Missing client_id for refresh_token grant type");
}
const client = await OAuthClient.findByClientId(clientId);
if (!client) {
throw ValidationError("Invalid client_id");
}
if (client.clientType === "confidential") {
throw ValidationError("Missing client_secret for confidential client");
}
}
// Note: These objects are mutated by the OAuth2Server library
const request = new OAuth2Server.Request(ctx.request);
const response = new OAuth2Server.Response(ctx.response);
@@ -31,9 +31,17 @@ export default function oauthErrorHandler() {
return;
}
ctx.status = err.code || 500;
ctx.status = err.status || err.statusCode || err.code || 500;
// Map common HTTP status codes to OAuth error types
let errorType = "server_error";
if (ctx.status === 400) {
errorType = "invalid_request";
} else if (ctx.status === 401) {
errorType = "invalid_client";
}
ctx.body = {
error: err.name,
error: errorType,
error_description: err.message,
};
}
+14
View File
@@ -1,6 +1,20 @@
import z from "zod";
import { BaseSchema } from "../api/schema";
export const TokenSchema = BaseSchema.extend({
body: z.object({
grant_type: z.string(),
code: z.string().optional(),
redirect_uri: z.string().optional(),
client_id: z.string().optional(),
client_secret: z.string().optional(),
refresh_token: z.string().optional(),
scope: z.string().optional(),
}),
});
export type TokenReq = z.infer<typeof TokenSchema>;
export const TokenRevokeSchema = BaseSchema.extend({
body: z.object({
token: z.string(),
+17
View File
@@ -54,3 +54,20 @@ export function withAPIContext<T>(
} as APIContext);
});
}
/**
* Helper function to convert an object to form-urlencoded string.
* Useful for testing OAuth endpoints that expect application/x-www-form-urlencoded content type.
*
* @param obj Object to convert to form-urlencoded string
* @returns Form-urlencoded string representation of the object
*/
export function toFormData(obj: Record<string, any>): string {
return Object.entries(obj)
.filter(([_, value]) => value !== undefined)
.map(
([key, value]) =>
`${encodeURIComponent(key)}=${encodeURIComponent(value)}`
)
.join("&");
}
+1
View File
@@ -135,6 +135,7 @@ export const OAuthInterface: RefreshTokenModel &
return {
id: client.clientId,
redirectUris: client.redirectUris,
clientType: client.clientType,
databaseId: client.id,
grants: this.grants,
};
@@ -353,6 +353,11 @@
"Unknown": "Unknown",
"Mark all as read": "Mark all as read",
"You're all caught up": "You're all caught up",
"Client type": "Client type",
"Confidential": "Confidential",
"Suitable for server-side applications": "Suitable for server-side applications",
"Public": "Public",
"Suitable for client-side or mobile applications": "Suitable for client-side or mobile applications",
"Icon": "Icon",
"OAuth client icon": "OAuth client icon",
"My App": "My App",
@@ -975,6 +980,7 @@
"Rotating the client secret will invalidate the current secret. Make sure to update any applications using these credentials.": "Rotating the client secret will invalidate the current secret. Make sure to update any applications using these credentials.",
"Displayed to users when authorizing": "Displayed to users when authorizing",
"Application icon": "Application icon",
"Confidential clients can securely store a secret": "Confidential clients can securely store a secret",
"Developer information shown to users when authorizing": "Developer information shown to users when authorizing",
"Developer name": "Developer name",
"Developer URL": "Developer URL",
+3
View File
@@ -94,6 +94,9 @@ export const OAuthClientValidation = {
/** The maximum length of an OAuth client redirect URI */
maxRedirectUriLength: 1000,
/** The allowed OAuth client types */
clientTypes: ["confidential", "public"] as const,
};
export const RevisionValidation = {