Skip to content

Commit 2c4bf1c

Browse files
committed
fix(ui): refine OAuth consent and app revocation DX
1 parent 357a1ac commit 2c4bf1c

4 files changed

Lines changed: 24 additions & 47 deletions

File tree

apps/sim/app/(auth)/oauth/consent/consent-view.tsx

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { Chip, cn } from '@sim/emcn'
3+
import { Chip } from '@sim/emcn'
44
import { Check } from '@sim/emcn/icons'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { signOut } from '@/lib/auth/auth-client'
@@ -19,7 +19,6 @@ import { AUTH_BUTTON_CLASS } from '@/app/(auth)/components/constants'
1919
import { OAuthConsentLoading } from '@/app/(auth)/oauth/consent/loading'
2020
import { useOAuthConsent, useOAuthPublicClient } from '@/hooks/queries/oauth-provider'
2121

22-
/** Why the page refuses to render a grant, when it does. */
2322
export type OAuthConsentRefusal = 'expired' | 'missing' | 'tampered' | 'unsigned'
2423

2524
const REFUSAL_MESSAGES: Record<OAuthConsentRefusal, string> = {
@@ -81,15 +80,7 @@ export function OAuthConsentView({
8180
const client = useOAuthPublicClient(clientId ?? undefined, authorizationRequestKey ?? undefined)
8281
const consent = useOAuthConsent()
8382

84-
/**
85-
* No grant card without a client the server itself named.
86-
*
87-
* A failed lookup used to fall through to the fallback name and leave Allow
88-
* enabled, so an unknown or deleted client still rendered a complete,
89-
* clickable authorization — with everything on it read from the URL rather
90-
* than from Sim. Naming the app is this page's whole job, so not being able
91-
* to name it is a refusal, not a degraded heading.
92-
*/
83+
/** Refuses clients Sim cannot name because URL metadata alone is untrusted. */
9384
const reason: OAuthConsentRefusal | null = refusal ?? (clientId ? null : 'missing')
9485
if (reason || client.isError) {
9586
return (
@@ -110,11 +101,7 @@ export function OAuthConsentView({
110101
if (client.isPending) return <OAuthConsentLoading />
111102

112103
const isCli = clientId === SIM_CLI_CLIENT_ID
113-
/**
114-
* The registered name the lookup returned, never the raw client id and never
115-
* a name asserted by the URL: the id is what an impostor controls, so a
116-
* client the server declines to name is one this card must not vouch for.
117-
*/
104+
/** Uses only the server-registered name; the client ID comes from the URL. */
118105
const appName = client.data?.name?.trim()
119106
if (!appName) {
120107
return (
@@ -183,9 +170,10 @@ export function OAuthConsentView({
183170
</AuthSubmitButton>
184171
<Chip
185172
type='button'
173+
variant='border'
186174
fullWidth
187175
disabled={consent.isPending}
188-
className={cn(AUTH_BUTTON_CLASS, 'border border-[var(--border)]')}
176+
className={AUTH_BUTTON_CLASS}
189177
onClick={() => decide(false)}
190178
>
191179
Deny

apps/sim/app/(auth)/oauth/consent/page.tsx

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -42,20 +42,8 @@ export default async function OAuthConsentPage({
4242
}
4343

4444
/**
45-
* Two ways a request can reach this page without Sim having authorized it,
46-
* both of which must refuse before anything is rendered.
47-
*
48-
* A repeated parameter is tampering, not a client quirk: the card names the
49-
* app and lists what it may do, and it reads those from the URL, where
50-
* `URLSearchParams.get` answers with the FIRST occurrence — so a link that
51-
* prepends its own `client_id` and `scope` would show a trustworthy app and
52-
* one harmless permission over somebody else's request.
53-
*
54-
* A missing `sig` means the query was never signed by the authorize
55-
* endpoint, so the whole thing is hand-written. Consent still fails at Allow
56-
* (the plugin refuses an unsigned `oauth_query`), but it fails with a
57-
* generic error, after the person has already read an authorization request
58-
* that Sim never issued. Both are caught here so the lie is never rendered.
45+
* Repeated fields can make displayed consent diverge from the signed request;
46+
* unsigned requests never passed through the authorization endpoint.
5947
*/
6048
const tampered = Object.entries(raw).some(
6149
([key, value]) => key !== 'ba_param' && Array.isArray(value)

apps/sim/app/workspace/[workspaceId]/settings/components/authorized-apps/authorized-apps.tsx

Lines changed: 17 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useMemo, useState } from 'react'
3+
import { useState } from 'react'
44
import { ChipConfirmModal, toast } from '@sim/emcn'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { formatDate } from '@sim/utils/formatting'
@@ -27,23 +27,21 @@ export function AuthorizedApps() {
2727
const apps = useAuthorizedApps()
2828
const revoke = useRevokeAuthorizedApp()
2929
const [searchTerm, setSearchTerm] = useSettingsSearch()
30-
const [pendingRevoke, setPendingRevoke] = useState<AuthorizedApp | null>(null)
30+
const [pendingRevokeClientId, setPendingRevokeClientId] = useState<string | null>(null)
3131

3232
const list = apps.data ?? EMPTY_APPS
33-
const filtered = useMemo(() => {
34-
const term = searchTerm.trim().toLowerCase()
35-
if (!term) return list
36-
return list.filter((app) => app.name.toLowerCase().includes(term))
37-
}, [list, searchTerm])
33+
const pendingRevoke = list.find((app) => app.clientId === pendingRevokeClientId) ?? null
34+
const term = searchTerm.trim().toLowerCase()
35+
const filtered = term ? list.filter((app) => app.name.toLowerCase().includes(term)) : list
3836

3937
const confirmRevoke = () => {
40-
const app = pendingRevoke
41-
if (!app) return
42-
revoke.mutate(app.clientId, {
43-
onSuccess: () => toast.success(`Revoked ${app.name}`),
38+
if (!pendingRevokeClientId) return
39+
const appName = pendingRevoke?.name ?? 'app'
40+
revoke.mutate(pendingRevokeClientId, {
41+
onSuccess: () => toast.success(`Revoked ${appName}`),
4442
onError: (error) => toast.error(getErrorMessage(error, 'Failed to revoke access')),
4543
/** Keep the modal open so its pending state remains visible through the mutation. */
46-
onSettled: () => setPendingRevoke(null),
44+
onSettled: () => setPendingRevokeClientId(null),
4745
})
4846
}
4947

@@ -82,7 +80,11 @@ export function AuthorizedApps() {
8280
<RowActionsMenu
8381
label='Authorized app actions'
8482
actions={[
85-
{ label: 'Revoke', destructive: true, onSelect: () => setPendingRevoke(app) },
83+
{
84+
label: 'Revoke',
85+
destructive: true,
86+
onSelect: () => setPendingRevokeClientId(app.clientId),
87+
},
8688
]}
8789
/>
8890
}
@@ -93,9 +95,9 @@ export function AuthorizedApps() {
9395
</SettingsPanel>
9496

9597
<ChipConfirmModal
96-
open={pendingRevoke !== null}
98+
open={pendingRevokeClientId !== null}
9799
onOpenChange={(open) => {
98-
if (!open) setPendingRevoke(null)
100+
if (!open) setPendingRevokeClientId(null)
99101
}}
100102
srTitle='Revoke access'
101103
title='Revoke access'

apps/sim/hooks/queries/oauth-provider.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ async function fetchAuthorizedApps(signal?: AbortSignal): Promise<AuthorizedApp[
2222
return data.apps
2323
}
2424

25-
/** The apps the signed-in account has authorized, for the settings list. */
2625
export function useAuthorizedApps() {
2726
return useQuery({
2827
queryKey: oauthProviderKeys.authorizedApps(),

0 commit comments

Comments
 (0)