Skip to content
Open
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
43 changes: 20 additions & 23 deletions src/dialogs/SecretCreateDialog.vue
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,14 @@
:disabled="saving"
@update:members="additionalFields = $event" />

<NcSelect
<!-- The shared destination picker (no "Vault root" option, by
design): a secret always lives in a vault, so creating one
at the root cannot be offered. Replaces a local NcSelect
that still listed the root. -->
<DestinationSelect
v-model="selectedFolderId"
:options="folderOptions"
:inputLabel="t('keepiq', 'Folder')"
:reduce="(opt) => opt.value"
:clearable="false" />
mode="folders"
:label="t('keepiq', 'Folder')" />

<NcNoteCard
v-if="!policyVerdict.compliant"
Expand Down Expand Up @@ -167,6 +169,7 @@ import {
import Dice5 from 'vue-material-design-icons/Dice5.vue'
import Plus from 'vue-material-design-icons/Plus.vue'
import AdditionalFieldsEditor from '../components/AdditionalFieldsEditor.vue'
import DestinationSelect from '../components/DestinationSelect.vue'
import KeyGeneratorModal from './KeyGeneratorModal.vue'
import {
CARD_TYPE_NAME,
Expand All @@ -193,6 +196,7 @@ export default {

components: {
AdditionalFieldsEditor,
DestinationSelect,
Dice5,
KeyGeneratorModal,
NcButton,
Expand Down Expand Up @@ -264,24 +268,6 @@ export default {
}))
},

/**
* The folder picker options: the vault root plus every folder the
* user owns.
*
* @return {Array<{value: string|null, label: string}>}
* @spec openspec/specs/secrets-write-ui/spec.md#requirement-create-a-secret-from-the-ui
* @spec openspec/specs/secrets/spec.md#requirement-folder-management
*/
folderOptions() {
const roots = [{ value: null, label: t('keepiq', 'Vault root') }]
return roots.concat(
useFolderStore().folders.map((folder) => ({
value: folder.id,
label: folder.name,
})),
)
},

/**
* The label for the secret-value field, which reads "Note" for the
* `note` system type and "Secret value" otherwise.
Expand Down Expand Up @@ -340,10 +326,21 @@ export default {
return evaluateScore(this.policy, this.selectedTypeName, this.value)
},

/**
* Whether Create may run: not busy, named, a folder chosen (secrets
* cannot live at the vault root), and the type's own required value
* present and policy-compliant.
*
* @return {boolean}
* @spec exclude Form-enablement guard; no domain behaviour.
*/
canSubmit() {
if (this.saving || this.locked || this.name.trim() === '') {
return false
}
if (!this.selectedFolderId) {
return false
}
if (this.isCard) {
return this.card.number !== ''
}
Expand Down
15 changes: 8 additions & 7 deletions src/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -327,25 +327,26 @@
{
"id": "quick-action-new-secret",
"type": "tile",
"_note": "Union of the bump-ncvue branch's real quick-action tiles with development's ADR-049 placeholder text; wired here in place of the quick-actions-text placeholder since these already point at live deep links. Tile fields sit at the def's top level because CnDashboardPage.getTileConfig() reads them from there, not from content. LINK SHAPE: CnTileWidget resolves linkType 'app' as generateUrl('/apps/' + linkValue), so linkValue must NOT start with /apps/ (a full path doubled into /apps//apps/… and 404'd). The SPA router is hash-based (createWebHashHistory on /apps/keepiq), so an in-app page needs the hash in the value: 'keepiq#/secrets' → {webroot}/apps/keepiq#/secrets, correct on index.php-prefixed instances too.",
"_note": "Union of the bump-ncvue branch's real quick-action tiles with development's ADR-049 placeholder text; wired here in place of the quick-actions-text placeholder since these already point at live deep links. Tile fields sit at the def's top level because CnDashboardPage.getTileConfig() reads them from there, not from content. LINK SHAPE: both in-app tiles use linkType 'route', which the tile widgets push through the host vue-router. linkType 'app' resolved to a FULL page load, and the vault master key is memory-only, so every tile click re-locked the vault and landed on /lock — and the old 'keepiq#/secrets' values were dead weight on top, because main.js routes with createWebHistory and the hash fragment is never read (an earlier revision of this note claimed createWebHashHistory; that stale claim made the hash links look correct on review). The values carry ?action=create / ?action=register, which SecretList and ApplicationRegisterView consume (open the create/register dialog) and then strip from the URL. Requires @conduction/nextcloud-vue with route-type tile support (CnTileWidget/CnDashTileWidget router navigation).",
"title": "New secret",
"icon": "M19,13H13V19H11V13H5V11H11V5H13V11H19V13Z",
"icon": "M7.5 3C9.5 3 11.1 4.2 11.7 6H21V9H18V12H15V9H11.7C11.1 10.8 9.4 12 7.5 12C5 12 3 10 3 7.5S5 3 7.5 3M7.5 6C6.7 6 6 6.7 6 7.5S6.7 9 7.5 9 9 8.3 9 7.5 8.3 6 7.5 6M8 17H11V14H13V17H16V19H13V22H11V19H8V17Z",
"iconType": "svg",
"_iconNote": "mdi key-plus, verbatim (vue-material-design-icons/KeyPlus.vue) — the plus badge sits UNDER the key; that is the published glyph, kept unmodified on purpose. A hand-recomposed side-badge variant was tried and dropped: stock MDI beats a custom remix.",
"backgroundColor": "#21468B",
"textColor": "#ffffff",
"linkType": "app",
"linkValue": "keepiq#/secrets"
"linkType": "route",
"linkValue": "/secrets?action=create"
},
{
"id": "quick-action-register-application",
"type": "tile",
"title": "Register application",
"icon": "M15,14C12.33,14 7,15.33 7,18V20H23V18C23,15.33 17.67,14 15,14M15,12A4,4 0 0,0 19,8A4,4 0 0,0 15,4A4,4 0 0,0 11,8A4,4 0 0,0 15,12M5,15V12H8V10H5V7H3V10H0V12H3V15H5Z",
"icon": "M1 12H10.8L8.3 9.5L9.7 8.1L14.6 13L9.7 17.9L8.3 16.5L10.8 14H1V12M21 2H3C1.9 2 1 2.9 1 4V10.1H3V6H21V20H3V16H1V20C1 21.1 1.9 22 3 22H21C22.1 22 23 21.1 23 20V4C23 2.9 22.1 2 21 2",
"iconType": "svg",
"backgroundColor": "#3b82f6",
"textColor": "#ffffff",
"linkType": "app",
"linkValue": "keepiq#/applications"
"linkType": "route",
"linkValue": "/applications?action=register"
},
{
"id": "quick-action-documentation",
Expand Down
35 changes: 35 additions & 0 deletions src/views/ApplicationRegisterView.vue
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,41 @@ export default {
},
},

watch: {
/**
* Dashboard quick-action deep link (`/applications?action=register`):
* open the register dialog and strip the marker from the URL, so a
* refresh (which re-locks the vault and round-trips the query through
* the lock screen's `returnUrl`) does not re-open the dialog. A
* watcher rather than a mounted() check because CnPageRenderer keeps
* the view mounted when only the query changes.
*
* @param {string|undefined} action The `action` query value.
* @spec openspec/specs/application-mgmt/spec.md#requirement-register-application
*/
'$route.query.action': {
immediate: true,
/**
* Open the dialog and strip the marker; anything else is ignored.
* The dialog is local view state (not a registry modal), so the
* query replace cannot close it the way it would a cnOpenModal
* dialog.
*
* @param {string|undefined} action The `action` query value.
* @return {void}
* @spec openspec/specs/application-mgmt/spec.md#requirement-register-application
*/
handler(action) {
if (action === 'register') {
this.dialogOpen = true

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.

🟢 Minor — Dialog opens before router.replace(); asymmetric with SecretList but safe here

This watcher does this.dialogOpen = true (line 152) before this.$router.replace({ query }) (line 155). The corresponding logic in SecretList.consumeCreateAction does the opposite: await router.replace() → await $nextTick() → openCreateSecret().

Why the asymmetry is safe here: dialogOpen is local view state on <NcDialog>. CnAppRoot's $route watcher only nullifies activeModalKey — the registry-modal state used by cnOpenModal. A local dialogOpen: true isn't touched by that watcher, so the ordering doesn't cause an open-then-immediately-close race.

Why it's still worth calling out: the strip-then-open ordering in SecretList is documented in a comment there because it's load-bearing for that (registry-modal) case. A future author copying the ApplicationRegisterView pattern to a view that DOES use a registry modal would carry the wrong ordering forward and hit the race. A one-line comment in this watcher noting "local-state dialogs aren't subject to CnAppRoot's registry-modal close" would prevent the pattern-transfer mistake.

Verification: grep -n "dialogOpen\|replace\|cnOpenModal" src/views/ApplicationRegisterView.vue | head — line 152 sets dialogOpen = true, line 155 replaces without await. grep -n "activeModalKey" /tmp/pr-936-worktree/src/components/CnAppRoot/CnAppRoot.vue — the $route watcher only touches registry-modal state.

const query = { ...this.$route.query }
delete query.action
this.$router.replace({ query })
}
},
},
},

mounted() {
this.store.fetchApplications().catch(() => {})
},
Expand Down
59 changes: 59 additions & 0 deletions src/views/SecretList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -67,10 +67,17 @@
@close="closeBulkDialog"
@done="onBulkDone" />

<!-- showFormDialog OFF: CnIndexPage also watches ?action=create
and would open its own generic schema-form dialog OVER the
registry's SecretCreateDialog (the consumeCreateAction
watcher below). The prop is the library's documented
"consumer manages its own dialog" opt-out; the @add path is
unaffected because this view listens to it. -->
<CnIndexPage
viewMode="list"
:availableViewModes="['list', 'cards', 'table']"
listLabel="List"
:showFormDialog="false"
:selectedIds="bulkStore.selectedIds"
rowClickToView
:objects="listObjects"
Expand Down Expand Up @@ -968,6 +975,32 @@ export default {
this.bulkStore.setSelection(pruned)
}
},

/**
* Dashboard quick-action deep link (`/secrets?action=create`): open
* the create-secret dialog. A watcher rather than a mounted() check,
* because CnPageRenderer keeps the list mounted when only the query
* changes (its render key is the page id), so a later navigation to
* the same page with the marker must still be seen.
*
* @param {string|undefined} action The `action` query value.
* @spec openspec/specs/secrets-write-ui/spec.md#requirement-create-a-secret-from-the-ui
*/
'$route.query.action': {
immediate: true,
/**
* Dispatch the marker to the consumer; anything else is ignored.
*
* @param {string|undefined} action The `action` query value.
* @return {void}
* @spec openspec/specs/secrets-write-ui/spec.md#requirement-create-a-secret-from-the-ui
*/
handler(action) {
if (action === 'create') {
this.consumeCreateAction()
}
},
},
},

/**
Expand Down Expand Up @@ -1434,6 +1467,7 @@ export default {
* view, and reload the list on success.
*
* @return {void}
* @spec openspec/specs/secrets-write-ui/spec.md#requirement-create-a-secret-from-the-ui
*/
openCreateSecret() {
this.cnOpenModal('secret-create', {
Expand All @@ -1442,6 +1476,31 @@ export default {
})
},

/**
* Consume the dashboard tile's `?action=create` marker: strip it from
* the URL, then open the create-secret dialog for the current folder.
*
* Stripping matters because a full page load re-locks the vault and
* the lock screen's `returnUrl` carries the query along — without it,
* every unlock after a refresh would re-open the dialog.
*
* STRICTLY strip-first: CnAppRoot closes the active registry modal on
* EVERY route change (so dialogs cannot follow a navigation to another
* page), and the query replace IS a route change — opening first got
* the dialog closed in the same tick it opened. The nextTick lets that
* route watcher run before the modal opens.
*
* @return {Promise<void>}
* @spec openspec/specs/secrets-write-ui/spec.md#requirement-create-a-secret-from-the-ui
*/
async consumeCreateAction() {
const query = { ...this.$route.query }
delete query.action
await this.$router.replace({ query })
await this.$nextTick()
this.openCreateSecret()
},

/**
* Open the create-folder dialog and reload the folder tree on success.
*
Expand Down
8 changes: 7 additions & 1 deletion tests/dialogs/SecretCreateDialog.additionalFields.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,13 @@ const stubs = {
}

async function mountDialog() {
const wrapper = mount(SecretCreateDialog, { propsData: {}, global: { stubs } })
const wrapper = mount(SecretCreateDialog, {
// A folder is required since the picker lost its "Vault root" option
// (secrets cannot live at the root). These scenarios are about the
// additional fields, so any folder will do.
propsData: { folderId: 'folder-1' },
global: { stubs },
})
await wrapper.vm.$nextTick()

return wrapper
Expand Down
16 changes: 13 additions & 3 deletions tests/dialogs/SecretCreateDialog.requiredFields.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,12 @@ describe('SecretCreateDialog — required fields and folder default', () => {
await wrapper.vm.submit()
expect(create).not.toHaveBeenCalled()

// Name and value alone are no longer enough: the picker lost its
// "Vault root" option, so a folder must be chosen too.
wrapper.vm.name = 'Both present'
expect(wrapper.vm.canSubmit).toBe(false)

wrapper.vm.selectedFolderId = 'folder-1'
expect(wrapper.vm.canSubmit).toBe(true)
})

Expand Down Expand Up @@ -143,16 +148,21 @@ describe('SecretCreateDialog — required fields and folder default', () => {
expect(create.mock.calls[0][0].folderId).toBe('folder-42')
})

it('sends a null folder when created at the vault root', async () => {
// This used to assert the opposite — "sends a null folder when created at
// the vault root". The root is not a place a secret can live (top-level
// folders are Vaults; a rootless secret has nowhere to be shown), so the
// picker no longer offers it and a folderless form must stay blocked.
it('refuses to create at the vault root — a folder must be chosen', async () => {
const create = vi
.spyOn(useSecretStore(), 'createSecret')
.mockResolvedValue({ id: 's1' })
const wrapper = await mountDialog()

wrapper.vm.name = 'At the root'
wrapper.vm.value = STRONG
await wrapper.vm.submit()

expect(create.mock.calls[0][0].folderId ?? null).toBeNull()
expect(wrapper.vm.canSubmit).toBe(false)
await wrapper.vm.submit()
expect(create).not.toHaveBeenCalled()
})
})
44 changes: 44 additions & 0 deletions tests/views/ApplicationRegisterView.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,4 +91,48 @@ describe('ApplicationRegisterView', () => {
await flush()
expect(dialog.attributes('data-open')).toBe('true')
})

// The dashboard's "Register application" tile deep-links to
// `/applications?action=register` through the router (a full page load
// would drop the in-memory vault key). The view consumes the marker:
// dialog open, marker stripped so a refresh cannot re-open it.
it('opens the register dialog from the dashboard quick action and strips the marker', async () => {
vi.spyOn(axios, 'get').mockResolvedValue({ data: [] })
const replace = vi.fn()
const wrapper = mount(ApplicationRegisterView, {
global: {
mocks: {
$route: { query: { action: 'register', view: 'cards' } },
$router: { replace, push: vi.fn() },
},
},
})
await flush()
expect(
wrapper
.find('[data-testid="application-register-dialog"]')
.attributes('data-open'),
).toBe('true')
expect(replace).toHaveBeenCalledWith({ query: { view: 'cards' } })
})

it('ignores an action marker that is not register', async () => {
vi.spyOn(axios, 'get').mockResolvedValue({ data: [] })
const replace = vi.fn()
const wrapper = mount(ApplicationRegisterView, {
global: {
mocks: {
$route: { query: { action: 'create' } },
$router: { replace, push: vi.fn() },
},
},
})
await flush()
expect(
wrapper
.find('[data-testid="application-register-dialog"]')
.attributes('data-open'),
).toBe('false')
expect(replace).not.toHaveBeenCalled()
})
})
Loading
Loading