Do not re-validate an unchanged host/url against secure_context_uri

[jtauschl]

https://community.openproject.org/wp/OP-19970

Summary

SecureContextUriValidator, applied to Storages::NextcloudStorage#host and Wikis::XWikiProvider#url, does not guard on whether the field actually changed — unlike its sibling NextcloudCompatibleHostValidator on the same host attribute, which already checks contract.model.changed_attributes.include?(attribute). As a result, every contract validation run re-checks the field's current value, even when an update leaves it untouched.

Once a storage's host (or an XWiki provider's url) predates this check or otherwise fails it, every subsequent update that doesn't even touch that field — a rename, or any other unrelated-field change — is permanently rejected with the same "Secure Context" error.

Fixes the bug reported at OP-19970.

Changes

• Splits the combined validates :host, secure_context_uri: true, nextcloud_compatible_host: true declaration in Storages::Adapters::Providers::Nextcloud::Contracts::GeneralInformationContract, adding a model.host_changed? guard to secure_context_uri specifically. Order is preserved (secure_context_uri still runs before the network-probing nextcloud_compatible_host), so a newly-set unsafe host is still rejected before any live request is made to it.
• Adds the same model.url_changed? guard to Wikis::XWikiProviders::BaseContract's secure_context_uri validation.
• Adds RSpec regression coverage in both contracts' specs for: (a) an unrelated field change with an already-persisted insecure value → valid, no network probe; (b) the same value being changed to a new insecure value → still invalid, no network probe attempted before rejection.

Verification

Verified live against a running OpenProject 17.7.1 instance (could not run the RSpec suite locally — no working Ruby/bundler setup in this environment; the new spec files were only syntax-checked with ruby -c, please verify on CI):

• Renaming a Nextcloud storage whose host was http://nc.openproject.com (unchanged, insecure) — before: HTTP 422 "Secure Context" error; after: succeeds, host unchanged.
• Updating that same storage's host to a new insecure value (http://another-unsafe-host.example) — still correctly rejected, confirmed no network probe was attempted before rejection (rejection returned in ~0.07s).
• The analogous XWiki provider case (rename with unchanged insecure url; newly-set insecure url) verified the same way via a direct contract call.
• A storage whose host genuinely needs live-reachability validation (new host, or an existing host actually being changed) still triggers nextcloud_compatible_host's probe exactly as before.

Test plan

• CI passes the new/existing specs in modules/storages/spec/contracts/storages/storages/nextcloud_update_contract_spec.rb and modules/wikis/spec/contracts/wikis/xwiki_providers/base_contract_spec.rb
• Reviewer confirms the validator ordering (secure_context_uri before nextcloud_compatible_host) is preserved as intended

[Copilot]

Pull request overview

This pull request fixes a validation bug where SecureContextUriValidator would re-run on every contract validation even when host/url was unchanged, blocking unrelated updates (e.g., renames) for records that already contained an insecure value before the validator existed.

Changes:

• Guard secure_context_uri validation behind model.host_changed? for Nextcloud storages to avoid re-validating unchanged hosts while keeping validator ordering (secure-context check still runs before any network-probing validation).
• Guard secure_context_uri validation behind model.url_changed? for XWiki providers to allow unrelated updates when the URL remains unchanged.
• Add regression specs covering “unchanged insecure value + unrelated change is valid” and “changed to a new insecure value remains invalid (and does not trigger network probing for Nextcloud).”

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File	Description
modules/storages/app/common/storages/adapters/providers/nextcloud/contracts/general_information_contract.rb	Splits validations and adds a host_changed? guard so secure-context checks only run when the host changes, preserving validation order before network probing.
modules/storages/spec/contracts/storages/storages/nextcloud_update_contract_spec.rb	Adds regression coverage ensuring unchanged insecure hosts don’t block unrelated updates and don’t trigger metadata discovery requests.
modules/wikis/app/contracts/wikis/xwiki_providers/base_contract.rb	Adds a url_changed? guard so secure-context validation only runs when the URL is being changed.
modules/wikis/spec/contracts/wikis/xwiki_providers/base_contract_spec.rb	Adds regression coverage for unchanged insecure URLs being allowed on unrelated updates, while still rejecting newly-set insecure URLs.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.