feat: edit an OAuth2 app's redirect URIs as an ordered list (#29603)

Completes PLAT-619, following #29440 and #29502.

- Replace the OAuth2 app’s single callback field with an editable `redirect_uris` list supporting 1–32 entries, with the first labeled “Default callback.”
- Show a `+N` badge for additional redirect URIs in the applications table.
- Remove `omitempty` from request `redirect_uris` so explicit empty lists reach the server.
- Add tests and stories for adding, editing, removing, and requiring at least one URI.
This commit is contained in:
Bobby Ho
2026-09-23 08:39:57 -07:00
committed by GitHub
parent 7555486351
commit 528dd3cc44
19 changed files with 980 additions and 97 deletions
+1
View File
@@ -27479,6 +27479,7 @@ const docTemplate = `{
"type": "string"
},
"icon": {
"description": "Icon replaces the app's stored icon. Omitting it clears the stored\nicon rather than leaving it unchanged.",
"type": "string"
},
"name": {
+1
View File
@@ -25216,6 +25216,7 @@
"type": "string"
},
"icon": {
"description": "Icon replaces the app's stored icon. Omitting it clears the stored\nicon rather than leaving it unchanged.",
"type": "string"
},
"name": {
+3 -2
View File
@@ -80,14 +80,15 @@ func emptyRedirectURIsDetail(fromCallback string) string {
// the scheme, cannot be saved until a request sends a list that passes.
func validateAppRedirectURIFields(uris []string, clientType codersdk.OAuth2ClientType, fromCallback string) []codersdk.ValidationError {
// A failure on the request's callback_url is reported against that field
// and without a list index, since the caller never sent a list.
// and without a row number, since the caller never sent a list. Rows are
// numbered from one to match the labels the web UI shows.
invalid := func(i int, uri, detail string) []codersdk.ValidationError {
if uri != "" && uri == fromCallback {
return []codersdk.ValidationError{{Field: "callback_url", Detail: "callback URL " + detail}}
}
return []codersdk.ValidationError{{
Field: "redirect_uris",
Detail: fmt.Sprintf("redirect URI at index %d %s", i, detail),
Detail: fmt.Sprintf("redirect URI %d %s", i+1, detail),
}}
}
if len(uris) == 0 {
+34 -8
View File
@@ -1038,7 +1038,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
Name: "renamed",
CallbackURL: first,
})
requireRedirectURIsValidationError(t, err, "redirect URI at index 1 must be at most 2048 bytes")
requireRedirectURIsValidationError(t, err, "redirect URI 2 must be at most 2048 bytes")
stored, err := db.GetOAuth2ProviderAppByID(ctx, app.ID)
require.NoError(t, err)
@@ -1356,9 +1356,9 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode())
require.Len(t, sdkErr.Validations, 1)
require.Equal(t, "redirect_uris", sdkErr.Validations[0].Field)
require.Equal(t, "redirect URI at index 0 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
require.Equal(t, "redirect URI 1 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
// The index names the offending entry rather than the first one.
// The row number names the offending entry rather than the first one.
//nolint:gocritic // OAuth2 app management requires owner permission.
_, err = client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
Name: "cleartext-create-second",
@@ -1367,7 +1367,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
sdkErr = nil
require.ErrorAs(t, err, &sdkErr)
require.Len(t, sdkErr.Validations, 1)
require.Equal(t, "redirect URI at index 1 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
require.Equal(t, "redirect URI 2 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
// The deprecated field is reported against its own name.
//nolint:gocritic // OAuth2 app management requires owner permission.
@@ -1406,7 +1406,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
require.ErrorAs(t, err, &sdkErr)
require.Len(t, sdkErr.Validations, 1)
require.Equal(t, "redirect_uris", sdkErr.Validations[0].Field)
require.Equal(t, "redirect URI at index 0 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
require.Equal(t, "redirect URI 1 must use https scheme for non-localhost URLs", sdkErr.Validations[0].Detail)
})
t.Run("LegacyCleartextHTTPCallback", func(t *testing.T) {
@@ -1434,7 +1434,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
_, err := client.PutOAuth2ProviderApp(ctx, app.ID, codersdk.PutOAuth2ProviderAppRequest{
Name: "renamed",
})
requireRedirectURIsValidationError(t, err, "redirect URI at index 0 must use https scheme for non-localhost URLs")
requireRedirectURIsValidationError(t, err, "redirect URI 1 must use https scheme for non-localhost URLs")
stored, err := db.GetOAuth2ProviderAppByID(ctx, app.ID)
require.NoError(t, err)
@@ -1502,6 +1502,32 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
require.Equal(t, "redirect_uris was sent as an empty list, which overrides callback_url; send at least one redirect URI, or omit redirect_uris to use callback_url", sdkErr.Validations[0].Detail)
})
// The web UI only rejects a fragment for public clients, so a confidential
// app's fragment reaches the server. The row number in the message counts
// from one, matching the form labels, rather than the zero-based index.
t.Run("FragmentNamesVisibleRow", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, nil)
_ = coderdtest.CreateFirstUser(t, client)
ctx := testutil.Context(t, testutil.WaitLong)
//nolint:gocritic // OAuth2 app management requires owner permission.
app, err := client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
Name: "fragment-row",
RedirectURIs: []string{first},
})
require.NoError(t, err)
require.Equal(t, codersdk.OAuth2ClientTypeConfidential, app.ClientType)
//nolint:gocritic // OAuth2 app management requires owner permission.
_, err = client.PutOAuth2ProviderApp(ctx, app.ID, codersdk.PutOAuth2ProviderAppRequest{
Name: app.Name,
RedirectURIs: []string{first, "https://example.com/callback#state"},
})
requireRedirectURIsValidationError(t, err, "redirect URI 2 must not contain a fragment component")
})
// A create that sends neither URI field is refused.
t.Run("OmittedFieldsOnCreateRefused", func(t *testing.T) {
t.Parallel()
@@ -1522,7 +1548,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
})
// A malformed entry sent through redirect_uris is attributed to that
// field with its index; the same value sent through callback_url is
// field with its row number; the same value sent through callback_url is
// attributed to callback_url instead.
t.Run("AttributionByOriginField", func(t *testing.T) {
t.Parallel()
@@ -1540,7 +1566,7 @@ func TestOAuth2ProviderAppRedirectURIs(t *testing.T) {
require.ErrorAs(t, err, &sdkErr)
require.Len(t, sdkErr.Validations, 1)
require.Equal(t, "redirect_uris", sdkErr.Validations[0].Field)
require.Equal(t, "redirect URI at index 1 uses the dangerous scheme javascript", sdkErr.Validations[0].Detail)
require.Equal(t, "redirect URI 2 uses the dangerous scheme javascript", sdkErr.Validations[0].Detail)
//nolint:gocritic // OAuth2 app management requires owner permission.
_, err = client.PostOAuth2ProviderApp(ctx, codersdk.PostOAuth2ProviderAppRequest{
+3 -1
View File
@@ -126,7 +126,9 @@ type PutOAuth2ProviderAppRequest struct {
// Deprecated: send redirect_uris instead. If both are sent, callback_url
// must equal the first entry of redirect_uris.
CallbackURL string `json:"callback_url,omitempty" validate:"omitempty"`
Icon string `json:"icon" validate:"omitempty"`
// Icon replaces the app's stored icon. Omitting it clears the stored
// icon rather than leaving it unchanged.
Icon string `json:"icon" validate:"omitempty"`
// Scope replaces the app's current allowlist. Omit to leave the existing
// allowlist untouched. Set to an empty string to clear it, making the app
// unrestricted.
+1 -1
View File
@@ -804,7 +804,7 @@ The current implementation has these limitations:
- No device authorization grant support (RFC 8628)
- Implicit grant (`response_type=token`) is not supported; OAuth 2.1 deprecated this flow due to token leakage risks, and a request for it redirects to the registered callback with `unsupported_response_type`
- Limited to opaque access tokens (no JWT support)
- An application may register at most 32 redirect URIs of at most 2048 bytes each. An application that stored a longer list before this limit existed keeps working, but it cannot be saved again until the list fits. To fix it, send a `PUT` with a `redirect_uris` list that fits, as shown under [Management API](#method-2-management-api). The web UI cannot edit the list: editing **Callback URL** replaces the primary redirect URI and preserves the other registered URIs, and the application page displays only the primary.
- An application may register at most 32 redirect URIs of at most 2048 bytes each. An application that stored a longer list before this limit existed keeps working, but it cannot be saved again until the list fits. To fix it, send a `PUT` with a `redirect_uris` list that fits, as shown under [Management API](#method-2-management-api). In the web UI, open the application and remove entries from **Redirect URIs** until the list fits.
- A cleartext `http://` redirect URI to a host that is not local is rejected. Earlier versions accepted one through the management API for a confidential application, although Dynamic Client Registration always refused it. An application that stored one keeps working, but it cannot be saved again until its list uses `https://` or a local host, as described under [Callback URL schemes](#callback-url-schemes).
- A redirect URI with a private-use scheme must name a path or an authority, as in `com.example.app:/callback` or `com.example.app://auth/callback`. The bare form `com.example.app:callback` is rejected. Dynamic Client Registration accepted it in earlier versions. A client that registered one can re-register with one of the other two forms, or an administrator can correct it with the same `PUT`.
+1 -1
View File
@@ -13114,7 +13114,7 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith
| Name | Type | Required | Restrictions | Description |
|-----------------|-----------------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `callback_url` | string | false | | Deprecated: send redirect_uris instead. If both are sent, callback_url must equal the first entry of redirect_uris. |
| `icon` | string | false | | |
| `icon` | string | false | | Icon replaces the app's stored icon. Omitting it clears the stored icon rather than leaving it unchanged. |
| `name` | string | true | | |
| `redirect_uris` | array of string | false | | Redirect uris is the ordered list of URIs the app may redirect to. The first entry is the primary. Omit both this and callback_url to keep the stored redirect URIs. Other fields are replaced. Sending an empty list is an error, not a way to keep the stored list. |
| `scope` | string | false | | Scope replaces the app's current allowlist. Omit to leave the existing allowlist untouched. Set to an empty string to clear it, making the app unrestricted. |
+4
View File
@@ -8066,6 +8066,10 @@ export interface PutOAuth2ProviderAppRequest {
* must equal the first entry of redirect_uris.
*/
readonly callback_url?: string;
/**
* Icon replaces the app's stored icon. Omitting it clears the stored
* icon rather than leaving it unchanged.
*/
readonly icon: string;
/**
* Scope replaces the app's current allowlist. Omit to leave the existing
@@ -61,7 +61,7 @@ export const WithValidationError: Story = {
message: "Validation failed",
validations: [
{ field: "name", detail: "name error" },
{ field: "callback_url", detail: "url error" },
{ field: "redirect_uris", detail: "url error" },
{ field: "icon", detail: "icon error" },
],
}),
@@ -71,7 +71,7 @@ export const WithValidationError: Story = {
const canvas = within(canvasElement);
await userEvent.type(await canvas.findByLabelText(/^name/i), "test-app");
await userEvent.type(
canvas.getByLabelText(/callback url/i),
canvas.getByLabelText(/default callback/i),
"https://example.com/callback",
);
await userEvent.click(
@@ -87,7 +87,7 @@ export const InvalidCallbackURL: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.type(await canvas.findByLabelText(/^name/i), "test-app");
const callbackInput = canvas.getByLabelText(/callback url/i);
const callbackInput = canvas.getByLabelText(/default callback/i);
// oxlint-disable-next-line eslint/no-script-url -- Deliberately invalid input exercises callback URL rejection.
await userEvent.type(callbackInput, "javascript:alert(1)");
await userEvent.tab();
@@ -102,13 +102,51 @@ export const DynamicallyRegisteredValues: Story = {
"VS Code Coder Extension",
);
await userEvent.type(
canvas.getByLabelText(/callback url/i),
canvas.getByLabelText(/default callback/i),
"vscode://coder.coder-remote/oauth/callback",
);
await userEvent.tab();
},
};
export const MultipleRedirectURIs: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.type(
await canvas.findByLabelText(/^name/i),
"VS Code Coder Extension",
);
await userEvent.type(
canvas.getByLabelText(/default callback/i),
"vscode://coder.coder-remote/oauth/callback",
);
await userEvent.click(
canvas.getByRole("button", { name: /add redirect uri/i }),
);
await userEvent.type(
canvas.getByLabelText(/^redirect uri 2/i),
"https://example.com/callback",
);
},
};
export const InvalidRowState: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.type(await canvas.findByLabelText(/^name/i), "test-app");
await userEvent.click(
canvas.getByRole("button", { name: /add redirect uri/i }),
);
// oxlint-disable-next-line eslint/no-script-url -- Deliberately invalid input exercises redirect URI rejection.
const invalidRedirectURI = "javascript:alert(1)";
await userEvent.type(
canvas.getByLabelText(/^redirect uri 2/i),
invalidRedirectURI,
);
await userEvent.tab();
},
};
export const NoPermissions: Story = {
parameters: {
permissions: {
@@ -21,9 +21,10 @@ export const CreateOAuth2AppPageView: FC = () => {
const queryClient = useQueryClient();
const postAppMutation = useMutation(postApp(queryClient));
const callbackURL = searchParams.get("callback_url");
const defaultValues = {
name: searchParams.get("name") ?? "",
callback_url: searchParams.get("callback_url") ?? "",
redirect_uris: callbackURL ? [callbackURL] : [],
icon: searchParams.get("icon") ?? "",
};
const [icon, setIcon] = useState(defaultValues.icon);
@@ -50,6 +51,9 @@ export const CreateOAuth2AppPageView: FC = () => {
<div className="border border-solid p-6 rounded-lg">
<OAuth2AppForm
// Apps created here are confidential. Public clients only come
// from dynamic client registration.
clientType="confidential"
onSubmit={async (req) => {
try {
const app = await postAppMutation.mutateAsync(req);
@@ -120,7 +120,7 @@ export const WithValidationError: Story = {
message: "Validation failed",
validations: [
{ field: "name", detail: "name error" },
{ field: "callback_url", detail: "url error" },
{ field: "redirect_uris", detail: "url error" },
{ field: "icon", detail: "icon error" },
],
}),
@@ -216,6 +216,56 @@ export const PublicClient: Story = {
},
};
export const MultipleRedirectURIs: Story = {
parameters: {
queries: [
{ key: externalScopesKey, data: MockExternalAPIKeyScopes },
{
key: oauth2ProviderAppKey(appId),
data: {
...mockApp,
redirect_uris: [mockApp.callback_url, "https://example.com/callback"],
},
},
{
key: oauth2ProviderAppSecretsKey(appId),
data: MockOAuth2ProviderAppSecrets,
},
],
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// Waits for the query to resolve so Pixel captures the loaded state.
await canvas.findByLabelText(/^redirect uri 2/i);
},
};
export const InvalidRowState: Story = {
parameters: {
queries: [
{ key: externalScopesKey, data: MockExternalAPIKeyScopes },
{ key: oauth2ProviderAppKey(appId), data: mockApp },
{
key: oauth2ProviderAppSecretsKey(appId),
data: MockOAuth2ProviderAppSecrets,
},
],
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
await userEvent.click(
await canvas.findByRole("button", { name: /add redirect uri/i }),
);
// oxlint-disable-next-line eslint/no-script-url -- Deliberately invalid input exercises redirect URI rejection.
const invalidRedirectURI = "javascript:alert(1)";
await userEvent.type(
canvas.getByLabelText(/^redirect uri 2/i),
invalidRedirectURI,
);
await userEvent.tab();
},
};
export const NoSecretPermissions: Story = {
parameters: {
permissions: {
@@ -202,6 +202,7 @@ export const EditOAuth2AppPageView: FC = () => {
<OAuth2AppForm
key={app.id}
app={app}
clientType={app.client_type}
onSubmit={async (req) => {
try {
const updated = await putAppMutation.mutateAsync({
@@ -14,6 +14,7 @@ const meta = {
title: "pages/DeploymentSettingsPage/OAuth2AppForm",
component: OAuth2AppForm,
args: {
clientType: "confidential",
onSubmit: fn(),
isUpdating: false,
disabled: false,
@@ -2,6 +2,8 @@ import { act, screen, waitFor } from "@testing-library/react";
import userEvent, { type UserEvent } from "@testing-library/user-event";
import { API } from "#/api/api";
import {
OAuth2RedirectURIMaxBytes,
OAuth2RedirectURIsMaxCount,
OAuth2ScopeListMaxBytes,
OAuth2ScopeListMaxNames,
} from "#/api/typesGenerated";
@@ -37,12 +39,17 @@ describe("OAuth2AppForm", () => {
const onSubmit = vi.fn();
render(
<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), name);
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
"vscode://coder.coder-remote/oauth/callback",
);
await user.click(
@@ -52,7 +59,7 @@ describe("OAuth2AppForm", () => {
await waitFor(() => {
expect(onSubmit).toHaveBeenCalledWith({
name: "VS Code Coder Extension",
callback_url: "vscode://coder.coder-remote/oauth/callback",
redirect_uris: ["vscode://coder.coder-remote/oauth/callback"],
icon: "",
scope: "",
});
@@ -72,6 +79,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -87,7 +95,6 @@ describe("OAuth2AppForm", () => {
await waitFor(() => {
expect(onSubmit).toHaveBeenCalledWith({
name: "Cursor MCP Extension",
callback_url: "vscode://coder.coder-remote/oauth/callback",
icon: app.icon,
});
});
@@ -117,11 +124,16 @@ describe("OAuth2AppForm", () => {
const onSubmit = vi.fn();
render(
<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), "test-app");
const callbackURL = screen.getByLabelText(/callback url/i);
const callbackURL = screen.getByLabelText(/default callback/i);
await user.type(callbackURL, callback);
await user.click(
screen.getByRole("button", { name: /create application/i }),
@@ -135,6 +147,11 @@ describe("OAuth2AppForm", () => {
" vscode://coder.coder-remote/oauth/callback",
"vscode://coder.coder-remote/oauth/callback ",
" vscode://coder.coder-remote/oauth/callback ",
" https://example.com/callback",
"https://example.com/callback ",
" https://example.com/callback ",
" http://localhost:3000/callback",
"http://127.0.0.1:3000/callback ",
"URN:ietf:wg:oauth:2.0:oob",
"com.example.app:/oauth2redirect",
"cursor://anysphere.cursor-mcp/oauth/callback",
@@ -142,17 +159,22 @@ describe("OAuth2AppForm", () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
render(
<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), "OAuth App");
await user.type(screen.getByLabelText(/callback url/i), callback);
await user.type(screen.getByLabelText(/default callback/i), callback);
await user.click(
screen.getByRole("button", { name: /create application/i }),
);
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: "OAuth App",
callback_url: callback.trim(),
redirect_uris: [callback.trim()],
icon: "",
scope: "",
}),
@@ -169,11 +191,16 @@ describe("OAuth2AppForm", () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
render(
<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), name);
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
"https://example.com/callback",
);
await user.click(
@@ -184,7 +211,7 @@ describe("OAuth2AppForm", () => {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name,
callback_url: "https://example.com/callback",
redirect_uris: ["https://example.com/callback"],
icon: "",
scope: "",
}),
@@ -194,6 +221,56 @@ describe("OAuth2AppForm", () => {
}
});
// Each "é" is two UTF-8 bytes but one UTF-16 code unit, so these URIs stay
// well under the limit by string length and only cross it by byte count.
const redirectURIPrefix = "https://example.com/";
const redirectURIOfBytes = (bytes: number) =>
redirectURIPrefix +
"é".repeat(Math.floor((bytes - redirectURIPrefix.length) / 2)) +
"a".repeat((bytes - redirectURIPrefix.length) % 2);
it.each([
{ bytes: OAuth2RedirectURIMaxBytes, valid: true },
{ bytes: OAuth2RedirectURIMaxBytes + 1, valid: false },
])(
"enforces the UTF-8 byte limit for a $bytes byte redirect URI",
async ({ bytes, valid }) => {
const uri = redirectURIOfBytes(bytes);
expect(new TextEncoder().encode(uri)).toHaveLength(bytes);
expect(uri.length).toBeLessThan(OAuth2RedirectURIMaxBytes);
const user = userEvent.setup();
const onSubmit = vi.fn();
render(
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), "OAuth App");
await user.click(screen.getByLabelText(/default callback/i));
await user.paste(uri);
await user.click(
screen.getByRole("button", { name: /create application/i }),
);
await act(async () => {});
if (valid) {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: "OAuth App",
redirect_uris: [uri],
icon: "",
scope: "",
}),
);
} else {
expect(onSubmit).not.toHaveBeenCalled();
}
},
);
it.each([
{ callback: "mailto:a@b", valid: false },
{ callback: "mailto://a@b", valid: false },
@@ -218,14 +295,15 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={MockOAuth2ProviderAppPublic}
clientType="public"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.clear(screen.getByLabelText(/callback url/i));
await user.clear(screen.getByLabelText(/default callback/i));
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
callback.replaceAll("[", "[["),
);
await user.type(screen.getByLabelText(/^name/i), " updated");
@@ -237,7 +315,7 @@ describe("OAuth2AppForm", () => {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: `${MockOAuth2ProviderAppPublic.name} updated`,
callback_url: callback,
redirect_uris: [callback],
icon: MockOAuth2ProviderAppPublic.icon,
}),
);
@@ -263,6 +341,7 @@ describe("OAuth2AppForm", () => {
const onSubmit = vi.fn();
render(
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -270,7 +349,7 @@ describe("OAuth2AppForm", () => {
);
await user.type(screen.getByLabelText(/^name/i), "confidential-app");
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
callback.replaceAll("[", "[["),
);
await user.click(
@@ -281,7 +360,7 @@ describe("OAuth2AppForm", () => {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: "confidential-app",
callback_url: callback,
redirect_uris: [callback],
icon: "",
scope: "",
}),
@@ -292,18 +371,301 @@ describe("OAuth2AppForm", () => {
},
);
// Confidential apps could store a non-local http callback before the form
// checked for it. The stored value fails validation on load, so the error
// must show without the field being touched.
it("shows the error for a stored callback that fails validation", async () => {
// The stored list is left out of an unrelated save. If another admin removed
// a URI after this form loaded, resending the loaded list would restore it.
it("omits redirect_uris and callback_url on a rename", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb", "https://b.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.clear(screen.getByLabelText(/^name/i));
await user.type(screen.getByLabelText(/^name/i), "Renamed app");
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await waitFor(() => {
const call = onSubmit.mock.calls[0][0];
expect(call).toStrictEqual({
name: "Renamed app",
icon: app.icon,
});
expect(call).not.toHaveProperty("redirect_uris");
expect(call).not.toHaveProperty("callback_url");
});
});
it("edits an entry in place, keeping its position", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb", "https://b.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.clear(screen.getByLabelText(/default callback/i));
await user.type(
screen.getByLabelText(/default callback/i),
"https://c.example.com/cb",
);
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: app.name,
redirect_uris: ["https://c.example.com/cb", "https://b.example.com/cb"],
icon: app.icon,
}),
);
});
it("removes an entry", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb", "https://b.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.click(
screen.getByRole("button", { name: /remove redirect uri 2/i }),
);
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: app.name,
redirect_uris: ["https://a.example.com/cb"],
icon: app.icon,
}),
);
});
it("adds an entry", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb", "https://b.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.click(screen.getByRole("button", { name: /add redirect uri/i }));
await user.type(
screen.getByLabelText(/^redirect uri 3/i),
"https://d.example.com/cb",
);
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: app.name,
redirect_uris: [
"https://a.example.com/cb",
"https://b.example.com/cb",
"https://d.example.com/cb",
],
icon: app.icon,
}),
);
});
it("disables submit after removing the only entry", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.click(
screen.getByRole("button", { name: /remove redirect uri 1/i }),
);
await waitFor(() =>
expect(
screen.getByRole("button", { name: /update application/i }),
).toBeDisabled(),
);
expect(
screen.getByText(/at least one redirect uri is required/i),
).toBeInTheDocument();
});
it("does not submit when a public app has a disallowed entry in another row", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderAppPublic,
redirect_uris: ["vscode://coder.coder-remote/oauth/callback"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.click(screen.getByRole("button", { name: /add redirect uri/i }));
await user.type(
screen.getByLabelText(/^redirect uri 2/i),
"http://example.com/cb",
);
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await act(async () => {});
expect(onSubmit).not.toHaveBeenCalled();
});
it("does not submit when a row duplicates another row", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: ["https://a.example.com/cb", "https://b.example.com/cb"],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.clear(screen.getByLabelText(/^redirect uri 2/i));
await user.type(
screen.getByLabelText(/^redirect uri 2/i),
"https://a.example.com/cb",
);
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await act(async () => {});
expect(onSubmit).not.toHaveBeenCalled();
expect(
screen.getByText(/already used by another row/i),
).toBeInTheDocument();
});
it("flags every row that duplicates an earlier one", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
const app = {
...MockOAuth2ProviderApps[0],
redirect_uris: [
"https://a.example.com/cb",
"https://b.example.com/cb",
"https://c.example.com/cb",
],
};
render(
<OAuth2AppForm
app={app}
clientType={app.client_type}
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
for (const row of [2, 3]) {
const field = screen.getByLabelText(
new RegExp(`^redirect uri ${row}`, "i"),
);
await user.clear(field);
await user.type(field, "https://a.example.com/cb");
}
await user.click(
screen.getByRole("button", { name: /update application/i }),
);
await act(async () => {});
expect(onSubmit).not.toHaveBeenCalled();
expect(screen.getAllByText(/already used by another row/i)).toHaveLength(2);
});
// Confidential apps could store a non-local http redirect URI before the
// form checked for it. The stored value fails validation on load, so the
// error must show without the field being touched.
it("shows the error for a stored redirect URI that fails validation", async () => {
const user = userEvent.setup();
const onSubmit = vi.fn();
render(
<OAuth2AppForm
app={{
...MockOAuth2ProviderApps[0],
callback_url: "http://intranet.example.com/callback",
redirect_uris: ["http://intranet.example.com/callback"],
}}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -311,21 +673,21 @@ describe("OAuth2AppForm", () => {
);
expect(
await screen.findByText("Please enter a valid callback URL."),
await screen.findByText("Please enter a valid redirect URI."),
).toBeInTheDocument();
await user.type(screen.getByLabelText(/^name/i), " updated");
expect(
screen.getByRole("button", { name: /update application/i }),
).toBeDisabled();
await user.clear(screen.getByLabelText(/callback url/i));
await user.clear(screen.getByLabelText(/default callback/i));
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
"https://intranet.example.com/callback",
);
await waitFor(() =>
expect(
screen.queryByText("Please enter a valid callback URL."),
screen.queryByText("Please enter a valid redirect URI."),
).not.toBeInTheDocument(),
);
await user.click(
@@ -334,17 +696,47 @@ describe("OAuth2AppForm", () => {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: `${MockOAuth2ProviderApps[0].name} updated`,
callback_url: "https://intranet.example.com/callback",
redirect_uris: ["https://intranet.example.com/callback"],
icon: MockOAuth2ProviderApps[0].icon,
}),
);
});
it("keeps the callback error hidden when the stored callback is valid", async () => {
// A list over the count cap has no row to blur, so its message must also
// show on load.
it("shows the error for a stored list over the count cap", async () => {
render(
<OAuth2AppForm
app={{
...MockOAuth2ProviderApps[0],
redirect_uris: Array.from(
{ length: OAuth2RedirectURIsMaxCount + 1 },
(_, i) => `https://alt-${i}.example.com/cb`,
),
}}
clientType="confidential"
onSubmit={vi.fn()}
isUpdating={false}
disabled={false}
/>,
);
expect(
await screen.findByText(
`At most ${OAuth2RedirectURIsMaxCount} redirect URIs are allowed.`,
),
).toBeInTheDocument();
expect(
screen.getByRole("button", { name: /update application/i }),
).toBeDisabled();
});
it("keeps the redirect URI error hidden when the stored list is valid", async () => {
const user = userEvent.setup();
render(
<OAuth2AppForm
app={MockOAuth2ProviderApps[0]}
clientType="confidential"
onSubmit={vi.fn()}
isUpdating={false}
disabled={false}
@@ -353,7 +745,7 @@ describe("OAuth2AppForm", () => {
await user.type(screen.getByLabelText(/^name/i), " updated");
expect(
screen.queryByText("Please enter a valid callback URL."),
screen.queryByText("Please enter a valid redirect URI."),
).not.toBeInTheDocument();
expect(
screen.getByRole("button", { name: /update application/i }),
@@ -364,12 +756,17 @@ describe("OAuth2AppForm", () => {
const onSubmit = vi.fn();
const user = userEvent.setup();
render(
<OAuth2AppForm onSubmit={onSubmit} isUpdating={false} disabled={false} />,
<OAuth2AppForm
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
/>,
);
await user.type(screen.getByLabelText(/^name/i), "test-app");
await user.type(
screen.getByLabelText(/callback url/i),
screen.getByLabelText(/default callback/i),
"https://example.com/callback",
);
await selectScope(user, "workspace:ssh");
@@ -381,7 +778,7 @@ describe("OAuth2AppForm", () => {
await waitFor(() =>
expect(onSubmit).toHaveBeenCalledWith({
name: "test-app",
callback_url: "https://example.com/callback",
redirect_uris: ["https://example.com/callback"],
icon: "",
scope: "workspace:ssh coder:all",
}),
@@ -394,6 +791,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={{ ...MockOAuth2ProviderApps[0], scope: "coder:all" }}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -421,6 +819,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={MockOAuth2ProviderApps[0]}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -470,6 +869,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={{ ...MockOAuth2ProviderApps[0], scope }}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -484,7 +884,6 @@ describe("OAuth2AppForm", () => {
await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
expect(onSubmit.mock.calls[0][0]).toStrictEqual({
name: "foo-updated",
callback_url: MockOAuth2ProviderApps[0].callback_url,
icon: MockOAuth2ProviderApps[0].icon,
});
},
@@ -499,6 +898,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={{ ...MockOAuth2ProviderApps[0], scope: " " }}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -513,7 +913,6 @@ describe("OAuth2AppForm", () => {
await waitFor(() => expect(onSubmit).toHaveBeenCalledTimes(1));
expect(onSubmit.mock.calls[0][0]).toStrictEqual({
name: "foo-updated",
callback_url: MockOAuth2ProviderApps[0].callback_url,
icon: MockOAuth2ProviderApps[0].icon,
});
});
@@ -524,6 +923,7 @@ describe("OAuth2AppForm", () => {
render(
<OAuth2AppForm
app={{ ...MockOAuth2ProviderApps[0], scope: "legacy:scope" }}
clientType="confidential"
onSubmit={onSubmit}
isUpdating={false}
disabled={false}
@@ -7,7 +7,11 @@ import * as Yup from "yup";
import { getErrorMessage } from "#/api/errors";
import { getExternalScopes } from "#/api/queries/oauth2";
import type * as TypesGen from "#/api/typesGenerated";
import { OAuth2AppNameMaxBytes } from "#/api/typesGenerated";
import {
OAuth2AppNameMaxBytes,
OAuth2RedirectURIMaxBytes,
OAuth2RedirectURIsMaxCount,
} from "#/api/typesGenerated";
import { ErrorAlert } from "#/components/Alert/ErrorAlert";
import { Button } from "#/components/Button/Button";
import { ConfirmDialog } from "#/components/Dialog/ConfirmDialog/ConfirmDialog";
@@ -19,23 +23,35 @@ import { MultiSelectCombobox } from "#/components/MultiSelectCombobox/MultiSelec
import { Spinner } from "#/components/Spinner/Spinner";
import { useUnsavedChangesPrompt } from "#/hooks/useUnsavedChangesPrompt";
import { getFormHelpers, iconValidator } from "#/utils/formUtils";
import { RedirectURIsField } from "./components/RedirectURIsField";
type OAuth2AppFormValues = {
name: string;
callback_url: string;
redirect_uris: string[];
icon: string;
scope: string[];
};
// The create page sends this as a POST and the edit page as a PUT, so it has
// to satisfy both request types, and a field added to either is a type error
// here rather than a silently missing field.
type OAuth2AppFormRequest = TypesGen.PostOAuth2ProviderAppRequest &
TypesGen.PutOAuth2ProviderAppRequest;
// A create always sends the redirect URI list. An update may leave it out,
// so the two modes submit different request types.
type OAuth2AppFormSubmit =
| {
app?: undefined;
onSubmit: (
data: TypesGen.PostOAuth2ProviderAppRequest,
) => void | Promise<void>;
}
| {
app: TypesGen.OAuth2ProviderApp;
onSubmit: (
data: TypesGen.PutOAuth2ProviderAppRequest,
) => void | Promise<void>;
};
type OAuth2AppFormProps = {
app?: TypesGen.OAuth2ProviderApp;
onSubmit: (data: OAuth2AppFormRequest) => void | Promise<void>;
type OAuth2AppFormProps = OAuth2AppFormSubmit & {
// Passed on its own because the create page has no app yet. The client
// type decides which redirect URI rules apply.
clientType: TypesGen.OAuth2ClientType;
error?: unknown;
isUpdating: boolean;
defaultValues?: Partial<OAuth2AppFormValues>;
@@ -104,6 +120,50 @@ const isValidCallbackURL = (
}
};
const redirectURIsSchema = (isPublicClient: boolean) =>
Yup.array()
.of(
Yup.string()
.trim()
.test(
"redirect-uri-byte-length",
`A redirect URI cannot be longer than ${OAuth2RedirectURIMaxBytes} UTF-8 bytes.`,
(value) =>
new TextEncoder().encode(value).length <= OAuth2RedirectURIMaxBytes,
)
.test(
"valid-redirect-uri",
"Please enter a valid redirect URI.",
(value) => isValidCallbackURL(value, isPublicClient),
),
)
// The server also deduplicates on save, so without this the form
// would submit a URI it never actually saved and mislead the user.
.test(
"unique-redirect-uri",
"This redirect URI is already used by another row.",
function (uris) {
const seen = new Set<string>();
const errors: Yup.ValidationError[] = [];
for (const [index, uri] of (uris ?? []).entries()) {
const trimmed = uri?.trim();
if (!trimmed) {
continue;
}
if (seen.has(trimmed)) {
errors.push(this.createError({ path: `${this.path}[${index}]` }));
}
seen.add(trimmed);
}
return errors.length === 0 || new Yup.ValidationError(errors);
},
)
.min(1, "At least one redirect URI is required.")
.max(
OAuth2RedirectURIsMaxCount,
`At most ${OAuth2RedirectURIsMaxCount} redirect URIs are allowed.`,
);
const validationSchema = (isPublicClient: boolean) =>
Yup.object({
name: Yup.string()
@@ -115,63 +175,91 @@ const validationSchema = (isPublicClient: boolean) =>
(value) =>
new TextEncoder().encode(value).length <= OAuth2AppNameMaxBytes,
),
callback_url: Yup.string()
.trim()
.required("Please enter a callback URL.")
.test(
"valid-callback-url",
"Please enter a valid callback URL.",
(value) => isValidCallbackURL(value, isPublicClient),
),
redirect_uris: redirectURIsSchema(isPublicClient),
icon: iconValidator,
});
// app and onSubmit stay together in submit so that checking app also
// narrows the type of onSubmit.
export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
app,
onSubmit,
clientType,
error,
isUpdating,
defaultValues,
disabled,
onIconChange,
...submit
}) => {
const { app } = submit;
const didSubmit = useRef(false);
const isPublicClient = app?.client_type === "public";
// A stored callback that no longer passes validation disables Update on
// load. Show its error right away instead of waiting for the field to be
const isPublicClient = clientType === "public";
// A stored list that no longer passes validation disables Update on load.
// Show its errors right away instead of waiting for the field to be
// touched, so the admin can see what to correct.
const storedCallbackInvalid =
app !== undefined && !isValidCallbackURL(app.callback_url, isPublicClient);
const storedRedirectURIsInvalid =
app !== undefined &&
!redirectURIsSchema(isPublicClient).isValidSync(app.redirect_uris);
const form = useFormik<OAuth2AppFormValues>({
initialValues: {
name: app?.name ?? defaultValues?.name ?? "",
callback_url: app?.callback_url ?? defaultValues?.callback_url ?? "",
redirect_uris: app
? [...app.redirect_uris]
: defaultValues?.redirect_uris?.length
? [...defaultValues.redirect_uris]
: [""],
icon: app?.icon ?? defaultValues?.icon ?? "",
scope:
app?.scope.split(" ").filter(Boolean) ?? defaultValues?.scope ?? [],
},
initialTouched: storedCallbackInvalid ? { callback_url: true } : undefined,
initialTouched: storedRedirectURIsInvalid
? { redirect_uris: true }
: undefined,
validationSchema: validationSchema(isPublicClient),
validateOnMount: true,
onSubmit: async ({ scope: selectedScopes, ...values }) => {
onSubmit: async ({
scope: selectedScopes,
redirect_uris: enteredRedirectURIs,
...values
}) => {
didSubmit.current = true;
const redirectURIs = enteredRedirectURIs.map((uri) => uri.trim());
const scope = selectedScopes.join(" ");
// An untouched allowlist is left out of updates rather than echoed
// back. The form cannot round-trip a stored list exactly: a whitespace
// only list grants nothing but would resend as "" and lift the
// restriction, and a legacy list may exceed the current size limits.
const scopeChanged = !app || scope !== form.initialValues.scope.join(" ");
await onSubmit({
const request = {
...values,
name: values.name.trim(),
callback_url: values.callback_url.trim(),
...(scopeChanged ? { scope } : {}),
});
};
if (submit.app === undefined) {
await submit.onSubmit({ ...request, redirect_uris: redirectURIs });
return;
}
// An untouched list is also left out, so a copy loaded before another
// admin changed the list does not overwrite their change.
const stored = submit.app.redirect_uris;
const redirectURIsChanged =
redirectURIs.length !== stored.length ||
redirectURIs.some((uri, index) => uri !== stored[index]);
await submit.onSubmit(
redirectURIsChanged
? { ...request, redirect_uris: redirectURIs }
: request,
);
},
});
const scopesQuery = useQuery(getExternalScopes());
const getFieldHelpers = getFormHelpers(form, error);
const iconField = getFieldHelpers("icon");
const redirectURIsField = getFieldHelpers("redirect_uris");
const redirectURIsErrors = form.errors.redirect_uris;
const redirectURIsRowErrors =
form.touched.redirect_uris && Array.isArray(redirectURIsErrors)
? redirectURIsErrors
: undefined;
const formDisabled = disabled || isUpdating;
const editing = Boolean(app);
const submitDisabled =
@@ -201,6 +289,30 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
onIconChange?.(value);
};
const handleRedirectURIsChange = (values: string[]) => {
void form.setFieldValue("redirect_uris", values);
// Removing the last row disables Update without a blur event to mark
// the field touched, so the required-URI message never shows. Mark it
// touched here (skipping its own validate call, since setFieldValue
// above already triggers one) only for that case, since doing this on
// every keystroke would race with the blur-triggered validateForm below.
if (values.length === 0) {
void form.setFieldTouched("redirect_uris", true, false);
}
};
// Touched is set on blur rather than on every keystroke. Setting it in
// handleRedirectURIsChange as well would fire a second concurrent
// validateForm per keystroke; Formik does not guarantee those resolve in
// order, so the result of an earlier keystroke can overwrite the latest.
const handleRedirectURIsBlur = () => {
void form.setFieldTouched("redirect_uris", true);
};
// The overall-list message (min/max count, or a server validation error)
// is shown once below the rows; a per-row message is shown on its row
// instead, so this excludes the case where Yup reports one message per row.
const showRedirectURIsListError =
redirectURIsField.error && !Array.isArray(redirectURIsErrors);
return (
<Form onSubmit={form.handleSubmit}>
<FormFields>
@@ -213,13 +325,21 @@ export const OAuth2AppForm: FC<OAuth2AppFormProps> = ({
autoFocus
required
/>
<FormField
field={getFieldHelpers("callback_url")}
label="Callback URL"
description="The full URL to redirect to after a user authorizes an installation."
disabled={formDisabled}
required
/>
<div className="flex flex-col gap-2">
<RedirectURIsField
values={form.values.redirect_uris}
onChange={handleRedirectURIsChange}
onBlur={handleRedirectURIsBlur}
disabled={formDisabled}
isPublicClient={isPublicClient}
errors={redirectURIsRowErrors}
/>
{showRedirectURIsListError && (
<span className="text-xs text-content-destructive">
{redirectURIsField.helperText}
</span>
)}
</div>
<div className="flex flex-col gap-2">
<Label htmlFor="icon">Icon</Label>
<div className="text-xs text-content-secondary">
@@ -64,6 +64,21 @@ export const Apps: Story = {
},
};
export const AppWithMultipleRedirectURIs: Story = {
args: {
isLoadingApps: false,
apps: [
{
...MockOAuth2ProviderApps[0],
redirect_uris: [
MockOAuth2ProviderApps[0].callback_url,
"https://example.com/callback",
],
},
],
},
};
export const Empty: Story = {
args: {
isLoadingApps: false,
@@ -5,6 +5,7 @@ import type * as TypesGen from "#/api/typesGenerated";
import { ErrorAlert } from "#/components/Alert/ErrorAlert";
import { Avatar } from "#/components/Avatar/Avatar";
import { AvatarData } from "#/components/Avatar/AvatarData";
import { Badge } from "#/components/Badge/Badge";
import { Button } from "#/components/Button/Button";
import { Loader } from "#/components/Loader/Loader";
import {
@@ -243,12 +244,24 @@ const OAuth2AppRow: FC<OAuth2AppRowProps> = ({ app }) => {
/>
</TableCell>
<TableCell className="min-w-0">
<span
className="block truncate text-content-secondary"
title={app.callback_url}
>
{app.callback_url}
</span>
<div className="flex items-center gap-2 min-w-0">
<span
className="block truncate text-content-secondary"
title={app.redirect_uris[0]}
>
{app.redirect_uris[0]}
</span>
{app.redirect_uris.length > 1 && (
<Badge
size="sm"
className="shrink-0"
title={app.redirect_uris.slice(1).join("\n")}
aria-label={`${app.redirect_uris.length - 1} more redirect URI${app.redirect_uris.length > 2 ? "s" : ""}`}
>
+{app.redirect_uris.length - 1}
</Badge>
)}
</div>
</TableCell>
<TableCell className="w-10 text-center">
<div className="flex justify-end items-center pr-4">
@@ -0,0 +1,109 @@
import { screen } from "@testing-library/react";
import userEvent from "@testing-library/user-event";
import { useState } from "react";
import { render } from "#/testHelpers/renderHelpers";
import { RedirectURIsField } from "./RedirectURIsField";
// onChange is a controlled callback, so exercising edit/remove/add needs a
// stateful wrapper to feed each call's result back in as the next `values`.
const ControlledField = ({
initialValues,
onChange,
isPublicClient = false,
}: {
initialValues: string[];
onChange: (values: string[]) => void;
isPublicClient?: boolean;
}) => {
const [values, setValues] = useState(initialValues);
return (
<RedirectURIsField
values={values}
disabled={false}
isPublicClient={isPublicClient}
onChange={(next) => {
setValues(next);
onChange(next);
}}
onBlur={() => {}}
/>
);
};
describe("RedirectURIsField", () => {
it("calls onChange with the edited entry", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(
<ControlledField
initialValues={["https://a.example.com/cb"]}
onChange={onChange}
/>,
);
await user.type(screen.getByLabelText(/default callback/i), "2");
expect(onChange).toHaveBeenLastCalledWith(["https://a.example.com/cb2"]);
});
it("calls onChange with the entry removed", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(
<ControlledField
initialValues={["https://a.example.com/cb", "https://b.example.com/cb"]}
onChange={onChange}
/>,
);
await user.click(
screen.getByRole("button", { name: /remove redirect uri 2/i }),
);
expect(onChange).toHaveBeenCalledWith(["https://a.example.com/cb"]);
});
it("calls onChange with a new empty entry appended", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(
<ControlledField
initialValues={["https://a.example.com/cb"]}
onChange={onChange}
/>,
);
await user.click(screen.getByRole("button", { name: /add redirect uri/i }));
expect(onChange).toHaveBeenCalledWith(["https://a.example.com/cb", ""]);
});
it("allows removing the only entry, leaving the list empty", async () => {
const user = userEvent.setup();
const onChange = vi.fn();
render(
<ControlledField
initialValues={["https://a.example.com/cb"]}
onChange={onChange}
/>,
);
await user.click(
screen.getByRole("button", { name: /remove redirect uri 1/i }),
);
expect(onChange).toHaveBeenCalledWith([]);
});
it("disables Add redirect URI once at the max count", () => {
const values = Array.from(
{ length: 32 },
(_, i) => `https://${i}.example.com/cb`,
);
render(<ControlledField initialValues={values} onChange={vi.fn()} />);
expect(
screen.getByRole("button", { name: /add redirect uri/i }),
).toBeDisabled();
});
});
@@ -0,0 +1,97 @@
import { PlusIcon, XIcon } from "lucide-react";
import { type FC, useId } from "react";
import { OAuth2RedirectURIsMaxCount } from "#/api/typesGenerated";
import { Button } from "#/components/Button/Button";
import { FormField } from "#/components/FormField/FormField";
import type { FormHelpers } from "#/utils/formUtils";
type RedirectURIsFieldProps = {
values: string[];
onChange: (values: string[]) => void;
onBlur: () => void;
disabled: boolean;
isPublicClient: boolean;
errors?: string[];
};
export const RedirectURIsField: FC<RedirectURIsFieldProps> = ({
values,
onChange,
onBlur,
disabled,
isPublicClient,
errors,
}) => {
const baseId = useId();
const atMax = values.length >= OAuth2RedirectURIsMaxCount;
const setEntry = (index: number, value: string) => {
onChange(values.map((v, i) => (i === index ? value : v)));
};
const removeEntry = (index: number) => {
onChange(values.filter((_, i) => i !== index));
};
return (
<div className="flex flex-col gap-3">
<div className="flex flex-col gap-1">
<span className="text-sm font-medium">Redirect URIs</span>
<span className="text-xs text-content-secondary">
{isPublicClient
? "URIs this application may redirect to. This app has no client secret, so http is limited to a loopback address; https, a custom scheme, and the out-of-band URN are also accepted."
: "URIs this application may redirect to after a user authorizes it."}
</span>
</div>
{values.map((value, index) => {
const fieldHelpers: FormHelpers = {
name: `redirect_uris.${index}`,
id: `${baseId}-${index}`,
value,
error: Boolean(errors?.[index]),
helperText: errors?.[index],
onChange: (event) => setEntry(index, event.target.value),
onBlur,
};
return (
<div key={index} className="flex items-start gap-2">
<div className="flex-1">
<FormField
field={fieldHelpers}
label={
index === 0 ? "Default callback" : `Redirect URI ${index + 1}`
}
disabled={disabled}
required={index === 0}
/>
</div>
<Button
type="button"
variant="outline"
size="icon"
disabled={disabled}
aria-label={`Remove redirect URI ${index + 1}`}
className="mt-8"
onClick={() => removeEntry(index)}
>
<XIcon />
</Button>
</div>
);
})}
<Button
type="button"
variant="outline"
disabled={disabled || atMax}
className="self-start"
onClick={() => onChange([...values, ""])}
>
<PlusIcon />
Add redirect URI
</Button>
</div>
);
};