From 678f900d0cbd37a5f304aa46abffcfa2f9db04cb Mon Sep 17 00:00:00 2001 From: Asher Date: Fri, 19 Jan 2024 20:03:11 -0900 Subject: [PATCH 1/5] Add user settings page for revoking OAuth2 apps --- site/src/AppRouter.tsx | 8 ++ site/src/api/api.ts | 16 +++- site/src/api/queries/oauth2.ts | 22 ++++- .../OAuth2ProviderPage/OAuth2ProviderPage.tsx | 69 ++++++++++++++ .../OAuth2ProviderPageView.stories.tsx | 34 +++++++ .../OAuth2ProviderPageView.tsx | 94 +++++++++++++++++++ 6 files changed, 234 insertions(+), 9 deletions(-) create mode 100644 site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx create mode 100644 site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx create mode 100644 site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx diff --git a/site/src/AppRouter.tsx b/site/src/AppRouter.tsx index c051ddacac833..ff4f1afb7d4b1 100644 --- a/site/src/AppRouter.tsx +++ b/site/src/AppRouter.tsx @@ -156,6 +156,10 @@ const ExternalAuthPage = lazy( const UserExternalAuthSettingsPage = lazy( () => import("./pages/UserSettingsPage/ExternalAuthPage/ExternalAuthPage"), ); +const UserOAuth2ProviderSettingsPage = lazy( + () => + import("./pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage"), +); const TemplateVersionPage = lazy( () => import("./pages/TemplateVersionPage/TemplateVersionPage"), ); @@ -362,6 +366,10 @@ export const AppRouter: FC = () => { path="external-auth" element={} /> + } + /> } /> } /> diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 3a454e4389cd3..3b366a0336779 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -974,10 +974,13 @@ export const unlinkExternalAuthProvider = async ( return resp.data; }; -export const getOAuth2ProviderApps = async (): Promise< - TypesGen.OAuth2ProviderApp[] -> => { - const resp = await axios.get(`/api/v2/oauth2-provider/apps`); +export const getOAuth2ProviderApps = async ( + filter?: TypesGen.OAuth2ProviderAppFilter, +): Promise => { + const params = filter?.user_id + ? new URLSearchParams({ user_id: filter.user_id }) + : ""; + const resp = await axios.get(`/api/v2/oauth2-provider/apps?${params}`); return resp.data; }; @@ -1002,6 +1005,7 @@ export const putOAuth2ProviderApp = async ( const response = await axios.put(`/api/v2/oauth2-provider/apps/${id}`, data); return response.data; }; + export const deleteOAuth2ProviderApp = async (id: string): Promise => { await axios.delete(`/api/v2/oauth2-provider/apps/${id}`); }; @@ -1029,6 +1033,10 @@ export const deleteOAuth2ProviderAppSecret = async ( ); }; +export const revokeOAuth2ProviderApp = async (appId: string): Promise => { + await axios.delete(`/api/v2/oauth2-provider/apps/${appId}/tokens`); +}; + export const getAuditLogs = async ( options: TypesGen.AuditLogsRequest, ): Promise => { diff --git a/site/src/api/queries/oauth2.ts b/site/src/api/queries/oauth2.ts index 849cbc5241625..78b31762b2aa5 100644 --- a/site/src/api/queries/oauth2.ts +++ b/site/src/api/queries/oauth2.ts @@ -3,13 +3,14 @@ import * as API from "api/api"; import type * as TypesGen from "api/typesGenerated"; const appsKey = ["oauth2-provider", "apps"]; -const appKey = (id: string) => appsKey.concat(id); -const appSecretsKey = (id: string) => appKey(id).concat("secrets"); +const userAppsKey = (userId: string) => appsKey.concat(userId); +const appKey = (appId: string) => appsKey.concat(appId); +const appSecretsKey = (appId: string) => appKey(appId).concat("secrets"); -export const getApps = () => { +export const getApps = (userId?: string) => { return { - queryKey: appsKey, - queryFn: () => API.getOAuth2ProviderApps(), + queryKey: userId ? appsKey.concat(userId) : appsKey, + queryFn: () => API.getOAuth2ProviderApps({ user_id: userId }), }; }; @@ -91,3 +92,14 @@ export const deleteAppSecret = (queryClient: QueryClient) => { }, }; }; + +export const revokeApp = (queryClient: QueryClient, userId: string) => { + return { + mutationFn: API.revokeOAuth2ProviderApp, + onSuccess: async () => { + await queryClient.invalidateQueries({ + queryKey: userAppsKey(userId), + }); + }, + }; +}; diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx new file mode 100644 index 0000000000000..2195e78688304 --- /dev/null +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx @@ -0,0 +1,69 @@ +import { type FC, useState } from "react"; +import { useMutation, useQuery, useQueryClient } from "react-query"; +import { getErrorMessage } from "api/errors"; +import { getApps, revokeApp } from "api/queries/oauth2"; +import { DeleteDialog } from "components/Dialogs/DeleteDialog/DeleteDialog"; +import { displayError, displaySuccess } from "components/GlobalSnackbar/utils"; +import { useMe } from "contexts/auth/useMe"; +import { Section } from "../Section"; +import OAuth2ProviderPageView from "./OAuth2ProviderPageView"; + +const OAuth2ProviderPage: FC = () => { + const me = useMe(); + const queryClient = useQueryClient(); + const userOAuth2AppsQuery = useQuery(getApps(me.id)); + const revokeAppMutation = useMutation(revokeApp(queryClient, me.id)); + const [appIdToRevoke, setAppIdToRevoke] = useState(); + const appToRevoke = userOAuth2AppsQuery.data?.find( + (app) => app.id === appIdToRevoke, + ); + + // This can happen if the app disappears from the query data but a user has + // already started the revoke flow. It is safe to place this directly in the + // render, because it does not run every single time. + if (appToRevoke === undefined && typeof appIdToRevoke === "string") { + setAppIdToRevoke(undefined); + displayError("Application no longer exists."); + } + + return ( +
+ { + setAppIdToRevoke(app.id); + }} + /> + {appToRevoke !== undefined && ( + setAppIdToRevoke(undefined)} + onConfirm={async () => { + try { + await revokeAppMutation.mutateAsync(appToRevoke.id); + displaySuccess( + `You have successfully revoked the OAuth2 application "${appToRevoke.name}"`, + ); + setAppIdToRevoke(undefined); + } catch (error) { + displayError( + getErrorMessage(error, "Failed to revoke application."), + ); + } + }} + /> + )} +
+ ); +}; + +export default OAuth2ProviderPage; diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx new file mode 100644 index 0000000000000..885c80b2ef0ba --- /dev/null +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx @@ -0,0 +1,34 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import { MockOAuth2ProviderApps } from "testHelpers/entities"; +import OAuth2ProviderPageView from "./OAuth2ProviderPageView"; + +const meta: Meta = { + title: "pages/UserSettingsPage/OAuth2ProviderPageView", + component: OAuth2ProviderPageView, +}; + +export default meta; +type Story = StoryObj; + +export const Loading: Story = { + args: { + isLoading: true, + revoke: () => undefined, + }, +}; + +export const Error: Story = { + args: { + isLoading: false, + error: "some error", + revoke: () => undefined, + }, +}; + +export const Apps: Story = { + args: { + isLoading: false, + apps: MockOAuth2ProviderApps, + revoke: () => undefined, + }, +}; diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx new file mode 100644 index 0000000000000..e50e8e47be96b --- /dev/null +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx @@ -0,0 +1,94 @@ +import Button from "@mui/material/Button"; +import Table from "@mui/material/Table"; +import TableBody from "@mui/material/TableBody"; +import TableCell from "@mui/material/TableCell"; +import TableContainer from "@mui/material/TableContainer"; +import TableHead from "@mui/material/TableHead"; +import TableRow from "@mui/material/TableRow"; +import { type FC } from "react"; +import type * as TypesGen from "api/typesGenerated"; +import { AvatarData } from "components/AvatarData/AvatarData"; +import { Avatar } from "components/Avatar/Avatar"; +import { ErrorAlert } from "components/Alert/ErrorAlert"; +import { TableLoader } from "components/TableLoader/TableLoader"; + +export type OAuth2ProviderPageViewProps = { + isLoading: boolean; + error?: unknown; + apps?: TypesGen.OAuth2ProviderApp[]; + revoke: (app: TypesGen.OAuth2ProviderApp) => void; +}; + +const OAuth2ProviderPageView: FC = ({ + isLoading, + error, + apps, + revoke, +}) => { + return ( + <> + {error && } + + + + + + Name + + + + + {isLoading && } + {apps?.map((app) => ( + + ))} + {apps?.length === 0 && ( + + +
+ No OAuth2 applications have been authorized. +
+ + + )} + +
+ + + ); +}; + +type OAuth2AppRowProps = { + app: TypesGen.OAuth2ProviderApp; + revoke: (app: TypesGen.OAuth2ProviderApp) => void; +}; + +const OAuth2AppRow: FC = ({ app, revoke }) => { + return ( + + + + ) + } + /> + + + + + + + ); +}; + +export default OAuth2ProviderPageView; From 8180890e1dbd362da2260f1b0d69e10f953b9420 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 20 Feb 2024 16:12:27 -0900 Subject: [PATCH 2/5] Update revoke endpoint --- site/src/api/api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 3b366a0336779..f2ce4ce644bef 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1034,7 +1034,7 @@ export const deleteOAuth2ProviderAppSecret = async ( }; export const revokeOAuth2ProviderApp = async (appId: string): Promise => { - await axios.delete(`/api/v2/oauth2-provider/apps/${appId}/tokens`); + await axios.delete(`/login/oauth2/tokens?client_id=${appId}`); }; export const getAuditLogs = async ( From fd1e443da35c6259a1a50747eedc0f5915316853 Mon Sep 17 00:00:00 2001 From: Asher Date: Tue, 20 Feb 2024 16:18:27 -0900 Subject: [PATCH 3/5] Remove deleted app check This keeps running every time you revoke an application. I guess the refresh is happening before the ID is set to undefined? But I do not want to unset it earlier because I want the dialog to stay open. For now I will just remove this block. --- .../OAuth2ProviderPage/OAuth2ProviderPage.tsx | 8 -------- 1 file changed, 8 deletions(-) diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx index 2195e78688304..f2b8f2a41694b 100644 --- a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPage.tsx @@ -18,14 +18,6 @@ const OAuth2ProviderPage: FC = () => { (app) => app.id === appIdToRevoke, ); - // This can happen if the app disappears from the query data but a user has - // already started the revoke flow. It is safe to place this directly in the - // render, because it does not run every single time. - if (appToRevoke === undefined && typeof appIdToRevoke === "string") { - setAppIdToRevoke(undefined); - displayError("Application no longer exists."); - } - return (
Date: Wed, 21 Feb 2024 13:38:00 -0900 Subject: [PATCH 4/5] Update revoke endpoint again Was changed last night. --- site/src/api/api.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index f2ce4ce644bef..886b4d8f4cbc9 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -1034,7 +1034,7 @@ export const deleteOAuth2ProviderAppSecret = async ( }; export const revokeOAuth2ProviderApp = async (appId: string): Promise => { - await axios.delete(`/login/oauth2/tokens?client_id=${appId}`); + await axios.delete(`/oauth2/tokens?client_id=${appId}`); }; export const getAuditLogs = async ( From 43e777a22bedabae699ed60307ff77d55caeec97 Mon Sep 17 00:00:00 2001 From: Asher Date: Wed, 21 Feb 2024 13:38:24 -0900 Subject: [PATCH 5/5] Make error required --- .../OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx | 2 ++ .../OAuth2ProviderPage/OAuth2ProviderPageView.tsx | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx index 885c80b2ef0ba..c8086b444b2d0 100644 --- a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.stories.tsx @@ -13,6 +13,7 @@ type Story = StoryObj; export const Loading: Story = { args: { isLoading: true, + error: undefined, revoke: () => undefined, }, }; @@ -28,6 +29,7 @@ export const Error: Story = { export const Apps: Story = { args: { isLoading: false, + error: undefined, apps: MockOAuth2ProviderApps, revoke: () => undefined, }, diff --git a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx index e50e8e47be96b..efa6d9f362274 100644 --- a/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx +++ b/site/src/pages/UserSettingsPage/OAuth2ProviderPage/OAuth2ProviderPageView.tsx @@ -14,7 +14,7 @@ import { TableLoader } from "components/TableLoader/TableLoader"; export type OAuth2ProviderPageViewProps = { isLoading: boolean; - error?: unknown; + error: unknown; apps?: TypesGen.OAuth2ProviderApp[]; revoke: (app: TypesGen.OAuth2ProviderApp) => void; };