Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* @vitest-environment jsdom
*/
import { act, type ComponentProps } from 'react'
import { createRoot, type Root } from 'react-dom/client'
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { SecretValueField } from './secret-value-field'

vi.mock('@sim/emcn', () => ({
ChipInput: (props: ComponentProps<'input'>) => <input {...props} />,
}))

describe('SecretValueField', () => {
let container: HTMLDivElement
let root: Root

beforeEach(() => {
container = document.createElement('div')
document.body.appendChild(container)
root = createRoot(container)
})

afterEach(() => {
act(() => root.unmount())
container.remove()
})

it('keeps editable plaintext out of the DOM until focus', () => {
act(() => root.render(<SecretValueField value='private-value' onChange={() => undefined} />))
const input = container.querySelector('input')
expect(input).not.toBeNull()
expect(input?.type).toBe('text')
expect(input?.value).toBe('••••••••••')

act(() => input?.focus())
expect(input?.type).toBe('text')
expect(input?.value).toBe('private-value')

act(() => input?.blur())
expect(input?.type).toBe('text')
expect(input?.value).toBe('••••••••••')
})

it('uses a fixed mask for viewers regardless of secret length', () => {
act(() => root.render(<SecretValueField value='short' canEdit={false} />))
const input = container.querySelector('input')
expect(input?.value).toBe('••••••••••')

act(() => root.render(<SecretValueField value='a-much-longer-private-value' canEdit={false} />))
expect(input?.value).toBe('••••••••••')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,11 @@ export function SecretValueField({
const [focused, setFocused] = useState(false)
const editable = canEdit && !readOnly
const maskActive = canEdit && !unmasked && !focused
const displayValue = canEdit ? value : BULLET.repeat(VIEWER_MASK_LENGTH)
const displayValue = canEdit
? maskActive && value
? BULLET.repeat(VIEWER_MASK_LENGTH)
: value
: BULLET.repeat(VIEWER_MASK_LENGTH)

const mergedStyle: CSSProperties | undefined = maskActive
? ({ ...style, WebkitTextSecurity: 'disc' } as CSSProperties)
Expand All @@ -69,13 +73,12 @@ export function SecretValueField({
className={className}
type='text'
value={displayValue}
readOnly
readOnly={!editable || !focused}
style={mergedStyle}
onChange={(event) => {
if (editable) onChange?.(event.target.value)
}}
onFocus={(event) => {
if (editable) event.currentTarget.removeAttribute('readOnly')
event.currentTarget.scrollLeft = 0
setFocused(true)
onFocus?.(event)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,8 @@ function copyName(key: string) {
}

interface SecretRowMenuProps {
/** Accessible name that identifies the row owning this menu. */
label: string
/** Copies the secret's name. */
onCopyName: () => void
/** Opens credential details; omit when the row has no backing credential. */
Expand All @@ -58,10 +60,10 @@ interface SecretRowMenuProps {
* Trailing `...` actions menu for a secret row. Mirrors the Teammates /
* Organization member menu so the settings experience is consistent.
*/
function SecretRowMenu({ onCopyName, onViewDetails, onDelete }: SecretRowMenuProps) {
function SecretRowMenu({ label, onCopyName, onViewDetails, onDelete }: SecretRowMenuProps) {
return (
<RowActionsMenu
label='Secret actions'
label={label}
triggerClassName='ml-2'
actions={[
...(onViewDetails ? [{ label: 'View details', onSelect: onViewDetails }] : []),
Expand Down Expand Up @@ -220,6 +222,7 @@ function WorkspaceVariableRow({
return (
<div className='contents'>
<ChipInput
aria-label={`Workspace secret name ${envKey}`}
className={cn(!canRename && 'cursor-text')}
value={renamingKey === envKey ? pendingKeyValue : envKey}
onChange={(e) => {
Expand All @@ -238,12 +241,14 @@ function WorkspaceVariableRow({
/>
<div />
<SecretValueField
aria-label={`Workspace secret value ${envKey}`}
value={value}
onChange={(next) => onValueChange(envKey, next)}
canEdit={canEdit}
name={`workspace_env_value_${envKey}_${generateShortId()}`}
/>
<SecretRowMenu
label={`Secret actions for ${envKey}`}
onCopyName={() => copyName(envKey)}
onViewDetails={hasCredential && onViewDetails ? () => onViewDetails(envKey) : undefined}
onDelete={canEdit ? () => onDelete(envKey) : undefined}
Expand Down Expand Up @@ -298,6 +303,7 @@ function NewWorkspaceVariableRow({
/>
{hasContent ? (
<SecretRowMenu
label={`Secret actions for ${envVar.key || 'new workspace secret'}`}
onCopyName={() => copyName(envVar.key)}
onDelete={() => {
onUpdate(index, 'key', '')
Expand Down Expand Up @@ -861,6 +867,9 @@ export function SecretsManager() {
return (
<div className='contents'>
<ChipInput
aria-label={
envVar.key ? `Personal secret name ${envVar.key}` : 'New personal secret name'
}
data-input-type='key'
error={Boolean(isConflicted || keyError)}
value={envVar.key}
Expand All @@ -876,6 +885,9 @@ export function SecretsManager() {
/>
<div />
<SecretValueField
aria-label={
envVar.key ? `Personal secret value ${envVar.key}` : 'New personal secret value'
}
data-input-type='value'
value={envVar.value}
onChange={(next) => updateEnvVar(originalIndex, 'value', next)}
Expand All @@ -888,6 +900,7 @@ export function SecretsManager() {
/>
{hasContent ? (
<SecretRowMenu
label={`Secret actions for ${envVar.key || 'new personal secret'}`}
onCopyName={() => copyName(envVar.key)}
onDelete={() => removeEnvVar(originalIndex)}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import type { Metadata } from 'next'
import { notFound, redirect } from 'next/navigation'
import { getSession } from '@/lib/auth'
import { getCredentialActorContext } from '@/lib/credentials/access'
import { getWorkspaceHostContextForViewer } from '@/lib/workspaces/host-context'
import { SecretDetail } from '@/app/workspace/[workspaceId]/settings/secrets/[credentialId]/secret-detail'
import { resolveWorkspaceGroup } from '@/ee/access-control/utils/permission-check'
import { canOpenSecretDetail } from './secret-detail-access'

export const metadata: Metadata = {
title: 'Secret',
Expand All @@ -10,6 +16,34 @@ export default async function SecretDetailPage({
}: {
params: Promise<{ workspaceId: string; credentialId: string }>
}) {
const session = await getSession()
if (!session?.user) redirect('/login')

const { workspaceId, credentialId } = await params
const hostContext = await getWorkspaceHostContextForViewer(workspaceId, session.user.id)
if (!hostContext) notFound()

const [permissionGroup, access] = await Promise.all([
hostContext.hostOrganizationId && hostContext.ownerBilling.isEnterprise
? resolveWorkspaceGroup(session.user.id, hostContext.hostOrganizationId, workspaceId)
: null,
getCredentialActorContext(credentialId, session.user.id),
])

if (
!canOpenSecretDetail({
workspaceId,
secretsHidden: permissionGroup?.config.hideSecretsTab === true,
access: {
credential: access.credential,
hasWorkspaceAccess: access.hasWorkspaceAccess,
hasActiveMembership: access.member?.status === 'active',
isAdmin: access.isAdmin,
},
})
) {
notFound()
}

return <SecretDetail workspaceId={workspaceId} credentialId={credentialId} />
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe, expect, it } from 'vitest'
import { canOpenSecretDetail } from './secret-detail-access'

const allowedAccess = {
credential: { workspaceId: 'workspace-a', type: 'env_workspace' },
hasWorkspaceAccess: true,
hasActiveMembership: true,
isAdmin: false,
}

describe('canOpenSecretDetail', () => {
it('allows active members and derived admins for matching environment credentials', () => {
expect(
canOpenSecretDetail({
workspaceId: 'workspace-a',
secretsHidden: false,
access: allowedAccess,
})
).toBe(true)
expect(
canOpenSecretDetail({
workspaceId: 'workspace-a',
secretsHidden: false,
access: {
...allowedAccess,
credential: { workspaceId: 'workspace-a', type: 'env_personal' },
hasActiveMembership: false,
isAdmin: true,
},
})
).toBe(true)
})

it.each([
['missing credential', { ...allowedAccess, credential: null }, false],
[
'cross-workspace credential',
{
...allowedAccess,
credential: { workspaceId: 'workspace-b', type: 'env_workspace' },
},
false,
],
[
'wrong credential type',
{ ...allowedAccess, credential: { workspaceId: 'workspace-a', type: 'oauth' } },
false,
],
['missing workspace access', { ...allowedAccess, hasWorkspaceAccess: false }, false],
[
'missing credential membership',
{ ...allowedAccess, hasActiveMembership: false, isAdmin: false },
false,
],
])('rejects %s', (_label, access, expected) => {
expect(
canOpenSecretDetail({
workspaceId: 'workspace-a',
secretsHidden: false,
access,
})
).toBe(expected)
})

it('rejects permission-group hidden Secrets', () => {
expect(
canOpenSecretDetail({
workspaceId: 'workspace-a',
secretsHidden: true,
access: allowedAccess,
})
).toBe(false)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
interface SecretDetailCredential {
workspaceId: string
type: string
}

interface SecretDetailAccess {
credential: SecretDetailCredential | null
hasWorkspaceAccess: boolean
hasActiveMembership: boolean
isAdmin: boolean
}

export function canOpenSecretDetail(options: {
workspaceId: string
secretsHidden: boolean
access: SecretDetailAccess
}): boolean {
const { access } = options
return Boolean(
!options.secretsHidden &&
access.credential &&
access.credential.workspaceId === options.workspaceId &&
(access.credential.type === 'env_personal' || access.credential.type === 'env_workspace') &&
access.hasWorkspaceAccess &&
(access.hasActiveMembership || access.isAdmin)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Personal Owners Require Membership Rows

The new route gate requires an active credentialMember row even for env_personal credentials. If personal credential ownership is represented only by envOwnerUserId, the owner is neither an active member nor a derived admin and now receives a 404 when opening their own secret detail page. Include personal ownership in this authorization decision or ensure that every personal credential owner has an active membership row.

)
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export function SecretDetail({ workspaceId, credentialId }: SecretDetailProps) {

<DetailSection title='Value'>
<SecretValueField
aria-label='Secret value'
value={valueField.value}
onChange={valueField.setValue}
canEdit={valueField.canEdit}
Expand Down
Loading
Loading