feat: add oauth config url in the ui #6
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR adds OAuth server endpoint configuration display in the admin UI. When creating a new OAuth client, administrators can now see and copy the OAuth server endpoints (issuer, authorization, token, and discovery URLs) needed to configure their OAuth client applications.
Changes:
- Added a new
OAuthEndpointsCardcomponent that displays OAuth endpoints with copy-to-clipboard functionality - Integrated the endpoints card into the new client creation page above the client form
- Added E2E tests to verify the endpoints are displayed correctly
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
components/admin/oauth-endpoints-card.tsx |
New component that displays OAuth server endpoints in a card with copy buttons for each endpoint |
app/admin/dashboard/clients/new/page.tsx |
Updated to fetch OAuth configuration and display the endpoints card above the client form |
e2e/admin/clients.spec.ts |
Added new test suite to verify OAuth endpoints display correctly on the new client page |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test("should display OAuth endpoints on new client page", async ({ page }) => { | ||
| await page.goto("/admin/dashboard/clients/new"); | ||
|
|
||
| // Verify OAuth endpoints card is visible | ||
| await expect(page.getByText("OAuth Server Endpoints")).toBeVisible(); | ||
|
|
||
| // Verify key endpoints are displayed | ||
| await expect(page.getByTestId("oauth-endpoint-issuer")).toBeVisible(); | ||
| await expect(page.getByTestId("oauth-endpoint-authorization")).toBeVisible(); | ||
| await expect(page.getByTestId("oauth-endpoint-token")).toBeVisible(); | ||
| await expect(page.getByTestId("oauth-endpoint-discovery")).toBeVisible(); | ||
|
|
||
| // Verify endpoints have values | ||
| const issuerValue = await page.getByTestId("oauth-endpoint-issuer").inputValue(); | ||
| expect(issuerValue).toBeTruthy(); | ||
|
|
||
| const authValue = await page.getByTestId("oauth-endpoint-authorization").inputValue(); | ||
| expect(authValue).toContain("/api/oauth/authorize"); | ||
|
|
||
| const tokenValue = await page.getByTestId("oauth-endpoint-token").inputValue(); | ||
| expect(tokenValue).toContain("/api/oauth/token"); | ||
|
|
||
| const discoveryValue = await page.getByTestId("oauth-endpoint-discovery").inputValue(); | ||
| expect(discoveryValue).toContain("/.well-known/openid-configuration"); | ||
| }); |
There was a problem hiding this comment.
The E2E test verifies that endpoints are displayed but doesn't test the copy functionality. Consider adding a test case that clicks the copy buttons and verifies the copied state (Check icon) appears, to ensure the interactive features work correctly.
| const handleCopy = (text: string, key: string) => { | ||
| navigator.clipboard.writeText(text); | ||
| setCopied(key); | ||
| setTimeout(() => setCopied(null), 2000); |
There was a problem hiding this comment.
The handleCopy function uses navigator.clipboard.writeText without error handling. This can fail in non-secure contexts (non-HTTPS) or when clipboard permissions are denied. Consider wrapping the clipboard operation in a try-catch block and providing user feedback if the operation fails.
| const handleCopy = (text: string, key: string) => { | |
| navigator.clipboard.writeText(text); | |
| setCopied(key); | |
| setTimeout(() => setCopied(null), 2000); | |
| const handleCopy = async (text: string, key: string) => { | |
| if (!navigator?.clipboard?.writeText) { | |
| window.alert("Copy to clipboard is not supported in this browser."); | |
| return; | |
| } | |
| try { | |
| await navigator.clipboard.writeText(text); | |
| setCopied(key); | |
| setTimeout(() => setCopied(null), 2000); | |
| } catch (error) { | |
| console.error("Failed to copy to clipboard:", error); | |
| window.alert("Failed to copy to clipboard. Please copy the text manually."); | |
| } |
| <Button | ||
| type="button" | ||
| variant="outline" | ||
| size="icon" | ||
| data-testid={`copy-${key}`} | ||
| onClick={() => handleCopy(value, key)} | ||
| > | ||
| {copied === key ? ( | ||
| <Check className="size-4" /> | ||
| ) : ( | ||
| <Copy className="size-4" /> | ||
| )} | ||
| </Button> |
There was a problem hiding this comment.
The copy buttons lack accessible labels. Screen reader users won't know what these buttons do. Add an aria-label attribute to the Button components that describes what will be copied, for example: aria-label={Copy ${label}}.
| const config = getOpenIDConfiguration(); | ||
|
|
There was a problem hiding this comment.
The getOpenIDConfiguration function is called without any error handling. If the OAUTH_ISSUER_URL environment variable is not set, this will result in an undefined issuer value that could cause runtime errors. While the page is a Server Component and should fail at build/startup time, consider adding validation to provide a clearer error message for misconfiguration.
| const config = getOpenIDConfiguration(); | |
| let config; | |
| try { | |
| config = getOpenIDConfiguration(); | |
| } catch (error) { | |
| const message = | |
| error instanceof Error ? error.message : String(error); | |
| throw new Error( | |
| `Failed to load OpenID configuration. Ensure the OAUTH_ISSUER_URL environment variable is set correctly. Original error: ${message}`, | |
| ); | |
| } | |
| if (!config || !config.issuer) { | |
| throw new Error( | |
| "Invalid OpenID configuration: missing issuer. Please ensure the OAUTH_ISSUER_URL environment variable is set.", | |
| ); | |
| } |
No description provided.