Skip to content

feat: policy signature flow#7518

Draft
vitormattos wants to merge 2340 commits into
mainfrom
feat/policy-signature-flow-phase1-groundwork
Draft

feat: policy signature flow#7518
vitormattos wants to merge 2340 commits into
mainfrom
feat/policy-signature-flow-phase1-groundwork

Conversation

@vitormattos

Copy link
Copy Markdown
Member

No description provided.

@vitormattos vitormattos added this to the Next Major (34) milestone Apr 14, 2026
@vitormattos vitormattos self-assigned this Apr 14, 2026
@github-project-automation github-project-automation Bot moved this to 0. Needs triage in Roadmap Apr 14, 2026
@vitormattos vitormattos marked this pull request as draft April 14, 2026 17:54
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 2 times, most recently from 955dec8 to 2b5c509 Compare April 14, 2026 20:24
@vitormattos vitormattos marked this pull request as ready for review April 14, 2026 20:34
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from e0949a0 to e843d15 Compare April 23, 2026 14:49
@vitormattos vitormattos marked this pull request as draft April 23, 2026 14:50
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 5 times, most recently from a34a632 to 6aa5f3b Compare April 28, 2026 13:46
vitormattos added a commit that referenced this pull request Apr 28, 2026
FIXES:
- Register all 6 signature_text individual policy keys in SignatureTextPolicy
- Each key now has proper normalizers and defaults matching backend
- Fix mock object type errors by converting willReturnMap to willReturnCallback
- Mock callbacks now always return string (never null) to match type hints
- Update mock expectations from once()/exactly(2) to atLeastOnce() for deleteKey()
- Align test expectations with actual migration call counts

DETAILS:
* SignatureTextPolicy: Now exposes 6 individual keys via keys() and get()
* Each key (template, template_font_size, signature_width, etc.) has proper specs
* Render mode key includes allowed values: 'default', 'graphic', 'text'
* Migration tests: Fixed mock return types to prevent TypeError
* All getValueString() calls now guaranteed to return string via callback
* Adjusted deleteKey() and setValueString() expectations to handle migration flow

Test Results Expected:
- 0 Unknown policy key errors (all 6 now registered)
- 0 Mock type errors (callbacks always return string)
- 0 Mock expectation violations (atLeastOnce accounts for cleanup calls)
vitormattos added a commit that referenced this pull request Apr 28, 2026
…olicy

- Fix useSignatureTextPolicy.ts accessing .effectiveValue instead of non-existent .value
- Simplify useSignatureTextPolicy return type annotation using ComputedRef
- Fix model.ts serializeSignatureTextPolicyConfig to return JSON string
- Fix model.ts normalizeSignatureTextPolicyConfig to handle JSON string inputs
- Fix SignatureTextRuleEditor.vue Emits type to use EffectivePolicyValue
- Remove trailing newline from SignatureTextPolicy.php

Fixes: TypeScript type checking and PHP-CS formatting failures in PR #7518
Tests: npm run ts:check passes, php-cs formatting validated
vitormattos added a commit that referenced this pull request Apr 28, 2026
FIXES:
- Register all 6 signature_text individual policy keys in SignatureTextPolicy
- Each key now has proper normalizers and defaults matching backend
- Fix mock object type errors by converting willReturnMap to willReturnCallback
- Mock callbacks now always return string (never null) to match type hints
- Update mock expectations from once()/exactly(2) to atLeastOnce() for deleteKey()
- Align test expectations with actual migration call counts

DETAILS:
* SignatureTextPolicy: Now exposes 6 individual keys via keys() and get()
* Each key (template, template_font_size, signature_width, etc.) has proper specs
* Render mode key includes allowed values: 'default', 'graphic', 'text'
* Migration tests: Fixed mock return types to prevent TypeError
* All getValueString() calls now guaranteed to return string via callback
* Adjusted deleteKey() and setValueString() expectations to handle migration flow

Test Results Expected:
- 0 Unknown policy key errors (all 6 now registered)
- 0 Mock type errors (callbacks always return string)
- 0 Mock expectation violations (atLeastOnce accounts for cleanup calls)

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
vitormattos added a commit that referenced this pull request Apr 28, 2026
…olicy

- Fix useSignatureTextPolicy.ts accessing .effectiveValue instead of non-existent .value
- Simplify useSignatureTextPolicy return type annotation using ComputedRef
- Fix model.ts serializeSignatureTextPolicyConfig to return JSON string
- Fix model.ts normalizeSignatureTextPolicyConfig to handle JSON string inputs
- Fix SignatureTextRuleEditor.vue Emits type to use EffectivePolicyValue
- Remove trailing newline from SignatureTextPolicy.php

Fixes: TypeScript type checking and PHP-CS formatting failures in PR #7518
Tests: npm run ts:check passes, php-cs formatting validated
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from 1c39cdb to 892fbd0 Compare April 28, 2026 17:08
@vitormattos vitormattos marked this pull request as ready for review April 28, 2026 20:48
@vitormattos vitormattos marked this pull request as draft April 28, 2026 20:52
@vitormattos vitormattos force-pushed the feat/policy-signature-flow-phase1-groundwork branch 3 times, most recently from c6cc732 to 0e665a6 Compare May 4, 2026 14:31
Comment thread playwright/support/nc-provisioning.ts Fixed
Comment thread playwright/support/nc-provisioning.ts Fixed
Comment thread playwright/support/nc-provisioning.ts Fixed
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…s.vue

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…E2E specs

- TemplateLoader: wrap resolveKnownPolicyStates() in try-catch to prevent
  500 responses when groups_request_sign contains an invalid value (e.g.
  empty string from unconfigured state). Also broaden canRequestSign()
  catch to \Throwable.
- LoadAdditionalListener: inject certificate_ok initial state before
  adding libresign-init script so the Files app New menu shows the
  'New signature request' entry reliably.
- Add useRequestSignPolicyGuard() to 6 Playwright specs that hit
  /apps/files during setup and were failing with 500 due to contaminated
  groups_request_sign system config left by prior tests:
  footer-reset-persistence, sign-herself-with-click-to-sign,
  sign-herself-with-drawn-signature, sign-herself-with-pkcs12-certificate,
  sign-password-non-retriable-error, signature-flow-policy-request-sidebar.
- policy-request-sign-hidden-seed-overlay: improve
  clickVisibleRuleMenuAction to waitFor({ state: 'visible' }) before
  clicking (NcActions popover needs time to open), add
  scrollIntoViewIfNeeded and force-click fallback. Also add an aria-busy
  idle wait before the Remove action sequence to let in-flight saves
  settle.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
…f catching broadly

TemplateLoader and LoadAdditionalListener are reverted to their original
form — the broad \Throwable catch-all and the redundant certificate_ok
injection introduced in the previous commit were regressions.

Root fix: DefaultPolicyResolver.applyLayer() now catches
\InvalidArgumentException from validateValue() when reading a stored
layer value. If the stored value is invalid (e.g. groups_request_sign=""
left from an unconfigured or migrated state) the layer is skipped and the
current/default value is kept instead of propagating a 500 error to the
Files page render.

Write-time validation is unchanged — the exception still surfaces
normally when a user actively tries to save an invalid value.

test: add 4 regression cases to DefaultPolicyResolverTest covering empty
string, empty object/array, and garbage stored values for a policy that
requires at least one authorised group.

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
When a policy config key exists in appconfig with an empty string
value (e.g. set directly via occ or legacy migration), PolicySource
was treating it as an explicit non-default system rule.  This caused
two issues:

1. loadSystemPolicy() returned scope='global' with an empty/invalid
   value instead of scope='system' with the built-in default.
2. hydrateExplicitSystemRuleCounts() incremented everyoneCount to 1
   for the empty key, causing getSystemPolicySnapshot() in Playwright
   tests to set exists=true.  A later restoreSystemPolicySnapshot()
   then tried to POST the empty value back to the policy API, which
   the validator correctly rejected with 400.

Fix loadSystemPolicy() to treat a stored empty string as having no
explicit system value, and add a configvalue != '' guard to the rule-
count SQL query so everyoneCount stays 0 for such keys.

Also fix useRequestSignPolicyGuard.afterEach() to call deleteAppConfig
instead of setAppConfig('') when the original value was an empty string,
preventing the broken value from being written back to the database at
all.

Regression test added: testLoadSystemPolicyTreatsEmptyStoredValueAsNoExplicitSystemValue

Signed-off-by: Vitor Mattos <1079143+vitormattos@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: 0. Needs triage

Development

Successfully merging this pull request may close these issues.

2 participants