Repository navigation
feat: group personal API key scopes by product area #110525
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
91e137b
cdbfc17
3e739fd
04d01ff
a31ddd6
f81f7c0
1f1851a
e01beb5
8150e6f
8e571e9
6880f56
e9d4324
c3c1106
e301b99
71c9c89
9c3e9b9
bc56746
77b721b
c45f573
4467452
63419c6
708432d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,122 @@ | ||
| import clsx from 'clsx' | ||
| import { type ReactNode, useId, useState } from 'react' | ||
|
|
||
| import { IconChevronRight } from '@posthog/icons' | ||
| import { LemonSegmentedButton, LemonTag } from '@posthog/lemon-ui' | ||
|
|
||
| import type { ScopeAccessLevel } from 'lib/scopes' | ||
|
|
||
| interface ScopeAccessGroupProps { | ||
| /** Group heading, such as a product area. */ | ||
| label: string | ||
| /** How many rows in the group sit at each level, for the tags in the header. */ | ||
| counts: Record<ScopeAccessLevel, number> | ||
| /** The level the group control shows as selected. Undefined when the rows disagree. */ | ||
| value: ScopeAccessLevel | undefined | ||
| /** Called with the level the user picks for the whole group. */ | ||
| onChange: (level: ScopeAccessLevel) => void | ||
| /** Reason the No access option should be disabled. Set to a non-empty string to disable. */ | ||
| noneDisabledReason?: string | ||
| /** Reason the Read option should be disabled. Set to a non-empty string to disable. */ | ||
| readDisabledReason?: string | ||
| /** Reason the Write option should be disabled. Set to a non-empty string to disable. */ | ||
| writeDisabledReason?: string | ||
| /** Tooltip on the selected level, for when some rows sit at another level. */ | ||
| valueTooltip?: string | ||
| /** Whether the rows show on first render. */ | ||
| defaultOpen?: boolean | ||
| /** Prefix of the `data-attr`s: `<prefix>-toggle-<slug>` on the toggle, `<prefix>-<slug>-<level>` on an option. */ | ||
| dataAttrPrefix: string | ||
| /** The rows of the group, rendered when the group is open. */ | ||
| children: ReactNode | ||
| } | ||
|
|
||
| export function ScopeAccessGroup({ | ||
| label, | ||
| counts, | ||
| value, | ||
| onChange, | ||
| noneDisabledReason, | ||
| readDisabledReason, | ||
| writeDisabledReason, | ||
| valueTooltip, | ||
| defaultOpen = false, | ||
| dataAttrPrefix, | ||
| children, | ||
| }: ScopeAccessGroupProps): JSX.Element { | ||
| const [open, setOpen] = useState(defaultOpen) | ||
| const panelId = useId() | ||
| const total = counts.none + counts.read + counts.write | ||
| const groupSlug = label.toLowerCase().replace(/[^a-z0-9]+/g, '-') | ||
|
|
||
| return ( | ||
| <div className="border-t border-border first:border-t-0 @container/scope-group"> | ||
| {/* The header stays on one line. In a narrow container, such as an OAuth popup window, | ||
| the label truncates and the counts hide, so the control keeps its place. */} | ||
| <div className="flex items-center gap-2 py-2 min-h-10"> | ||
| <button | ||
| type="button" | ||
| className="flex items-center gap-2 flex-1 min-w-0 text-left font-semibold cursor-pointer bg-transparent border-0 p-0 text-inherit" | ||
| onClick={() => setOpen(!open)} | ||
| aria-expanded={open} | ||
| aria-controls={panelId} | ||
| data-attr={`${dataAttrPrefix}-toggle-${groupSlug}`} | ||
| > | ||
| <IconChevronRight | ||
| className={clsx( | ||
| 'shrink-0 text-muted transition-transform motion-reduce:transition-none', | ||
| open && 'rotate-90' | ||
| )} | ||
| /> | ||
| <span className="truncate">{label}</span> | ||
| </button> | ||
| <div className="flex items-center gap-2 ml-auto shrink-0"> | ||
| <span className="hidden @min-[36rem]/scope-group:flex items-center gap-2"> | ||
| <span className="text-xs text-muted whitespace-nowrap"> | ||
| <span translate="no">{total}</span> {total === 1 ? 'permission' : 'permissions'} | ||
| </span> | ||
| <span className="flex items-center gap-1"> | ||
| {counts.none > 0 && ( | ||
| <LemonTag size="small" type="muted"> | ||
| <span translate="no">{counts.none}</span> none | ||
| </LemonTag> | ||
| )} | ||
| {counts.read > 0 && ( | ||
| <LemonTag size="small" type="success"> | ||
| <span translate="no">{counts.read}</span> read | ||
| </LemonTag> | ||
| )} | ||
| {counts.write > 0 && ( | ||
| <LemonTag size="small" type="warning"> | ||
| <span translate="no">{counts.write}</span> write | ||
| </LemonTag> | ||
| )} | ||
| </span> | ||
| </span> | ||
| <div role="group" aria-label={`${label} access`}> | ||
| <LemonSegmentedButton | ||
| size="xsmall" | ||
| value={value} | ||
| onChange={(level) => onChange(level as ScopeAccessLevel)} | ||
| options={[ | ||
| { label: 'No access', value: 'none', disabledReason: noneDisabledReason }, | ||
| { label: 'Read', value: 'read', disabledReason: readDisabledReason }, | ||
| { label: 'Write', value: 'write', disabledReason: writeDisabledReason }, | ||
| ].map((option) => ({ | ||
| ...option, | ||
| // Tells a group change apart from a row change in autocapture. | ||
| 'data-attr': `${dataAttrPrefix}-${groupSlug}-${option.value}`, | ||
| tooltip: option.value === value ? valueTooltip : undefined, | ||
| }))} | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {open && ( | ||
| <div id={panelId} className="flex flex-col pb-2 pl-7"> | ||
| {children} | ||
| </div> | ||
| )} | ||
| </div> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| import { AGENT_USE_CASE_SCOPES } from 'lib/agentScopes.generated' | ||
| import { OAUTH_SCOPES_HIDDEN } from 'lib/oauthScopes.generated' | ||
| import { | ||
| AGENT_CLI_API_KEY_SCOPES, | ||
|
|
@@ -140,9 +139,12 @@ describe('API_KEY_SCOPE_PRESETS', () => { | |
| expect(preset.label).toBe('Read-only access') | ||
| }) | ||
|
|
||
| it('contains :read for every entry in API_SCOPES except unprivileged-excluded scopes', () => { | ||
| it('contains :read for every readable entry in API_SCOPES except unprivileged-excluded scopes', () => { | ||
| const preset = findPreset('read_only_access') | ||
| const expected = API_SCOPES.filter(({ unprivilegedExcluded }) => !unprivilegedExcluded) | ||
| const expected = API_SCOPES.filter( | ||
| ({ unprivilegedExcluded, disabledActions }) => | ||
| !unprivilegedExcluded && !disabledActions?.includes('read') | ||
| ) | ||
| .map(({ key }) => `${key}:read`) | ||
| .sort() | ||
| expect([...preset.scopes].sort()).toEqual(expected) | ||
|
|
@@ -176,14 +178,14 @@ describe('API_KEY_SCOPE_PRESETS', () => { | |
| expect(preset.scopes).not.toContain('integration:write') | ||
| expect(preset.scopes).not.toContain('user:write') | ||
| }) | ||
| }) | ||
|
|
||
| it('only includes scopes the key creation UI can render', () => { | ||
| it.each(API_KEY_SCOPE_PRESETS.filter(({ value }) => value !== 'all_access').map(({ value }) => value))( | ||
| 'preset %s only sets levels the key creation UI can render', | ||
| (value) => { | ||
| const renderableScopes = getRenderableKeyCreationScopes() | ||
|
|
||
| expect(AGENT_CLI_API_KEY_SCOPES).toEqual( | ||
| (AGENT_USE_CASE_SCOPES as readonly string[]).filter((scope) => renderableScopes.has(scope)) | ||
| ) | ||
| expect(AGENT_CLI_API_KEY_SCOPES.every((scope) => renderableScopes.has(scope))).toBe(true) | ||
| }) | ||
| }) | ||
| const unrenderable = findPreset(value).scopes.filter((scope) => !renderableScopes.has(scope)) | ||
| expect(unrenderable).toEqual([]) | ||
|
Comment on lines
+183
to
+188
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The replacement test only checks that preset scopes are selectable. It would still pass if the clamp dropped a generated Agent CLI scope instead of falling back from write to read. Keep an assertion that every generated scope is retained at its nearest selectable level; otherwise, a missing permission could go unnoticed. Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src/lib/scopes.test.ts
Line: 183-188
Comment:
**Agent CLI completeness goes untested**
The replacement test only checks that preset scopes are selectable. It would still pass if the clamp dropped a generated Agent CLI scope instead of falling back from write to read. Keep an assertion that every generated scope is retained at its nearest selectable level; otherwise, a missing permission could go unnoticed.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
| } | ||
| ) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,16 @@ import type { APIScopeAction, APIScopeObject } from '~/types' | |
|
|
||
| export const MAX_API_KEYS_PER_USER = 10 // Same as in posthog/api/personal_api_key.py | ||
|
|
||
| export type ScopeAccessLevel = 'none' | 'read' | 'write' | ||
|
|
||
| export const countScopeRowsByLevel = (rows: { value: ScopeAccessLevel }[]): Record<ScopeAccessLevel, number> => { | ||
| const counts: Record<ScopeAccessLevel, number> = { none: 0, read: 0, write: 0 } | ||
| for (const row of rows) { | ||
| counts[row.value] += 1 | ||
| } | ||
| return counts | ||
| } | ||
|
|
||
| export type APIScope = { | ||
| key: APIScopeObject | ||
| objectName: string | ||
|
|
@@ -315,9 +325,23 @@ export const API_KEY_CREATION_DISABLED_SCOPES = new Set( | |
| API_SCOPES.flatMap(({ key, disabledActions }) => (disabledActions ?? []).map((action) => `${key}:${action}`)) | ||
| ) | ||
|
|
||
| export const AGENT_CLI_API_KEY_SCOPES = AGENT_USE_CASE_SCOPES.filter((scope) => | ||
| API_KEY_CREATION_RENDERABLE_SCOPES.has(scope) | ||
| ) | ||
| // A preset must not set a level the key picker cannot show, or the picker shows the level on a | ||
| // disabled segment. A write on a write-disabled object falls back to read, which write implies on the | ||
| // server anyway. A scope with no allowed level is dropped. | ||
| export const clampToKeyCreationScopes = (scopes: readonly string[]): string[] => [ | ||
| ...new Set( | ||
| scopes.flatMap((scope) => { | ||
| if (API_KEY_CREATION_RENDERABLE_SCOPES.has(scope)) { | ||
| return [scope] | ||
| } | ||
| const [object, action] = scope.split(':') | ||
| const read = `${object}:read` | ||
| return action === 'write' && API_KEY_CREATION_RENDERABLE_SCOPES.has(read) ? [read] : [] | ||
| }) | ||
| ), | ||
| ] | ||
|
|
||
| export const AGENT_CLI_API_KEY_SCOPES = clampToKeyCreationScopes(AGENT_USE_CASE_SCOPES) | ||
|
|
||
| export const API_KEY_SCOPE_PRESETS: { | ||
| value: string | ||
|
|
@@ -353,9 +377,11 @@ export const API_KEY_SCOPE_PRESETS: { | |
| value: 'mcp_server', | ||
| label: 'MCP Server', | ||
| // file_system is excluded because the MCP server doesn't request it, not because it's privileged. | ||
| scopes: API_SCOPES.filter( | ||
| ({ key, unprivilegedExcluded }) => !unprivilegedExcluded && key !== 'file_system' | ||
| ).map(({ key }) => `${key}:write`), | ||
| scopes: clampToKeyCreationScopes( | ||
|
a-lider marked this conversation as resolved.
Resolved
|
||
| API_SCOPES.filter(({ key, unprivilegedExcluded }) => !unprivilegedExcluded && key !== 'file_system').map( | ||
| ({ key }) => `${key}:write` | ||
| ) | ||
| ), | ||
|
Comment on lines
+380
to
+384
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When someone creates a personal API key with the MCP Server preset, this clamp changes Prompt To Fix With AIThis is a comment left during a code review.
Path: frontend/src/lib/scopes.tsx
Line: 380-384
Comment:
**MCP preset loses write access**
When someone creates a personal API key with the MCP Server preset, this clamp changes `user:write` to `user:read`. The MCP catalog requires `user:write` for `user-settings-update`, so the tool disappears for that key. Preserve the permission the preset needs, or change the preset’s stated capabilities.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is intended. A key made from the MCP Server preset now ends up with the same scopes as one where you pick every scope by hand: the picker doesn't offer Why write is disabled on some scopes:
The MCP preset had
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense — thanks for clarifying the picker’s intentional limits and the MCP preset’s previous mismatch. Since |
||
| access_type: 'all', | ||
| }, | ||
| { | ||
|
|
@@ -367,7 +393,9 @@ export const API_KEY_SCOPE_PRESETS: { | |
| { | ||
| value: 'read_only_access', | ||
| label: 'Read-only access', | ||
| scopes: API_SCOPES.filter(({ unprivilegedExcluded }) => !unprivilegedExcluded).map(({ key }) => `${key}:read`), | ||
| scopes: clampToKeyCreationScopes( | ||
| API_SCOPES.filter(({ unprivilegedExcluded }) => !unprivilegedExcluded).map(({ key }) => `${key}:read`) | ||
| ), | ||
| }, | ||
| { value: 'all_access', label: 'All access', scopes: ['*'] }, | ||
| ] | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.