Commit Graph
7 Commits
Author SHA1 Message Date
kevinveenbirkenbachandClaude Opus 5 093c84c620 fix(suitecrm): block the personas SAML cannot serve, cover their ground natively
Both shared flows gate performKeycloakLogin on 'openid-connect/auth' (personas/biber.js:85, personas/admin.js:64). SuiteCRM's SSO is SAML - d15716643 moved it off the oauth2-proxy onto nbgrp/onelogin-saml-bundle - so its auth URL is /realms/<realm>/login-actions/authenticate for client <base>/saml/metadata. The gate never opens, performKeycloakLogin never runs, and the personas fail at 'did NOT reach an authenticated surface'.

The trace of job 95883075126 carries no fill() at all: goto, clearCookies x2, waitForSelector x5, click x2, waitForFunction x2, fetch. Nothing was ever typed. Keycloak's 24 LOGIN_ERROR entries are the consequence - requests to login-actions/authenticate without credentials, which is also why they carry no username attribute.

The block alone would drop coverage, so saml.spec.js takes it over where the session already exists: assertCspInjections on the authenticated surface, inAppLogout, session-status must report active:false, then assertUnauthenticatedLanding. That is what runBiberFlow asserted, minus runRoleInteraction, which SuiteCRM never passed. The SAML round trip itself stays green in the two existing tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-19 16:41:09 +02:00
kevinveenbirkenbachandClaude Opus 5 7125cb9d2c test(suitecrm): cover both login paths with role-local specs
The role's only login coverage was the shared biber flow, and that flow can
only drive OIDC: personas/biber.js authenticates solely when the URL contains
`openid-connect/auth`. SAML redirects to /protocol/saml and the ldap variant
uses SuiteCRM's own form, so neither path could be exercised — across every
persona helper there are 15 hits for `openid-connect` and none for `saml`.

Both specs assert through /session-status rather than by hunting a logout
control in the DOM. That is what the whole SSO change is about — whether
SuiteCRM owns a session or only the edge does — and it does not race an
asynchronously rendering Angular surface, which is exactly where the 2026-08-08
run tore.

Three timing decisions are load-bearing, each from a defect found while
reviewing the first draft:

- The ACS response is awaited explicitly instead of polling the URL.
  performKeycloakLogin only checks that page.url() CONTAINS the canonical
  domain, and php-saml puts the SP's own URL in RelayState, so Keycloak's login
  URL already contains it: a URL wait returns before the assertion is posted.
- The `POST /login` response is awaited explicitly in the ldap spec. SuiteCRM 8
  logs in by XHR with no document navigation, so waitForLoadState resolves while
  the request is still in flight.
- /session-status is requested exactly once, after the login completed.
  SecurityController's unauthenticated branch calls $session->invalidate(),
  so a probe fired mid-login destroys the session it reports on. The first draft
  did all three wrongly and would have gone red against a WORKING deployment.

The ldap spec clicks #login-button because the login form declares no
(ngSubmit); Enter only worked through HTML implicit submission.

Verified in the production Playwright image against a protocol stub that
replays the choreography (fake IdP with a Keycloak-shaped form, auto-posted
assertion, XHR login): saml.spec.js 2 passed, ldap.spec.js 2 passed. The
counter-runs matter more — against an app that does not redirect to Keycloak the
SAML spec fails with its own message, and against an app with no login form the
ldap spec times out on #login-button, so neither is vacuously green. Also
verified: collection of all 9 tests, the three service-gate combinations, and a
missing credential failing loudly instead of skipping.

playwright.env.j2's persona rationale said biber has no account when SSO is off.
He does — LDAP_AUTO_CREATE mints one on first bind. He is blocked because the
shared flow cannot drive that login, which is what ldap.spec.js now covers.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 18:02:30 +02:00
kevinveenbirkenbachandClaude Opus 5 d15716643a feat(suitecrm): authenticate against Keycloak via SAML instead of a proxy
The role sat behind the oauth2-proxy, which authenticates at the edge and hands
the app nothing: SuiteCRM kept serving its own Angular login after the Keycloak
round trip, so no session ever existed and the app had no logout control. CI run
31927586060 is the only run in which the role's Playwright spec ever executed,
and it failed exactly there - "no in-app logout control reachable on the current
authenticated surface" at personas/utils/logout.js.

SuiteCRM 8.9.1 ships nbgrp/onelogin-saml-bundle and treats AUTH_TYPE=saml as a
first-class firewall in config/packages/security.php:249, so the app can own its
session without a line of PHP from us. flavor: saml turns the proxy off by
itself - is_proxy_gated is `enabled AND flavor == 'oauth2'` - which also drops
the redis session store and the auth_request clamp in the vhost. The role
declares no allowed_groups, so no edge ACL is lost.

Four settings are not cosmetic; each is a failure reproduced against a live
Keycloak 26.7.1 until the login completed:

- SAML_COMPRESS_REQUESTS: the HTTP-Redirect binding requires DEFLATE, and
  Keycloak rejects an uncompressed AuthnRequest with a ZipException.
- SAML_USE_ATTRIBUTE_FRIENDLY_NAME=false: the bundle defaults to matching
  attributes by friendlyName, which Keycloak's mappers do not set, so not a
  single attribute arrives.
- APP_DEBUG is forced to 0 whenever SAML is on: public/legacy/modules/Users/
  User.php:944 reads $_POST['ut'] unguarded while creating the user, and with
  debug on Symfony turns that warning into an exception the authenticator
  reports as a failed login - the account is created and the FIRST login of
  every new user fails anyway.
- SAML_SP_ENTITY_ID must equal the Keycloak client's clientId.

The IdP signing certificate is read from the realm descriptor at boot rather
than templated in, so a key rotation needs no redeploy. It uses the same public
issuer URL the browser is redirected to - one address, not two - with the SOCKS
proxy espocrm established for onion issuers. An unreachable IdP or a descriptor
without a certificate aborts the start; both paths are verified, as is the skip
when the variable is absent on LDAP/native deployments.

Verified end to end locally against Keycloak 26.7.1, MariaDB 12.3 and SuiteCRM
8.9.1 from the upstream tarball: session-status reports active:true for the
Keycloak user, auto-created on the first attempt against an empty users table.

web-app-keycloak/TODO.md listed this role for the trusted-header bridge. Nothing
of it was implemented, and SuiteCRM exposes no header entry point at all
(REMOTE_USER / X-Forwarded-User / X-Auth-Request appear nowhere in core/ or
public/legacy), so it moves to the same exclusion as jitsi: use the native
mechanism the app already ships.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-17 18:02:30 +02:00
kevinveenbirkenbachandClaude Opus 5 bb6ec837de fix(suitecrm): stop handing the persona a password LDAP never reads
With AUTH_TYPE=ldap the main firewall gets json_login_ldap and nothing else -
the native branch returns early, so the two are mutually exclusive and no admin
special case survives. The seeded admin keeps its rights but its local password
is never consulted, while the persona kept typing exactly that. Omit the key
when LDAP is on so admin.js falls through to the directory password, matching
the espocrm gate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-14 16:37:20 +02:00
kevinveenbirkenbachandClaude Opus 5 f713ba8350 refactor(credentials): address them at secrets.credentials, like every other meta file
meta/schema.yml became meta/secrets.yml, but its content still landed at
applications.<app>.credentials — the one meta file whose payload did not sit
under a root named after the file. Every other topic obeys that rule:
meta/volumes.yml goes to volumes, meta/rbac.yml to rbac. Credentials now go to
secrets.credentials.

The path exists exactly once, in utils/manager/credential_key.py, and
OVERRIDE_SECTION is derived from its two segments rather than spelled beside
them, so the dotted form used by --set and the nested form used by the
inventory cannot drift. Six consumers read it: the loader, the generator, the
CLI override parser, the two emit helpers, and the four lookup validators.

--set now accepts exactly one key shape,
applications.<app>.secrets.credentials.<key>. The three short forms are
rejected instead of guessed at: the same short key names different credentials
on different apps. That also retired primary_app_id, which only existed to
disambiguate them.

Three defects surfaced while wiring this up. _apply_one_role_schema still wrote
to the old root, so an app's own credentials would have moved while its
providers' stayed behind. web-app-pgadmin reads through
lookup('applications', ...).credentials.<key>, an access path no regex over
lookup('config', ...) could see, and would have deployed with an empty postgres
password. And _META_NON_GENERIC_TOPICS still listed 'schema' — a bare topic
name, invisible to any path search — so the generic file-root loop assigned the
raw schema under secrets and the follow-up write landed inside the cached
document, truncating keycloak's four credentials to one for every later
reader.

tests/integration/ansible/lookups/config/test_credentials_roundtrip.py pins both
directions of the seam and the cache invariant that last defect broke.

Existing inventories still carry the pre-rename root; migrating them is
deliberately out of scope here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-12 16:51:56 +02:00
kevinveenbirkenbachandClaude Opus 5 b1d9b56d42 fix(playwright): block the personas the sso-off variants cannot serve, and stop three claims that were false
The 21 env templates added two commits ago left the persona flags off entirely
in most roles. biber.js has no native-login branch at all -- with
SSO_SERVICE_ENABLED false it never authenticates and then hard-fails at :201
rather than skipping -- and admin.js gates its native fallback on !oidcEnabled.
CI deploys every matrix variant, and variant 1 of these roles pins
services.sso.enabled false, so each of them was a guaranteed red job that had
never been observed because the specs had never run.

Nine flags are static true: chess, confluence and jira pin sso.enabled false in
meta/services.yml with no variant override and provision no native admin, so
there is no login surface in any variant. Eleven follow services.sso.enabled in
the ternary web-app-baserow already uses, for the roles that do have an
SSO-enabled variant where the personas should run. pgadmin and phpldapadmin
lose the credential keys that a permanently blocked persona can never read.

Eleven nocheck rationales cited "the sys-svc-mail integration tests" as the
coverage that justifies not gating on EMAIL_SERVICE_ENABLED. No such tests
exist: roles/sys-svc-mail* contain no tests at all, and
tests/integration/infrastructure/services/test_mailu_dependency.py is a static
YAML parity check. The real delivery coverage is web-app-mailu's own spec,
playwright.spec.js:154. Each is now a fact checkable inside its own role.

Eight more claimed the CSS injector's origin is asserted by the shared CSP
baseline. personas/utils/csp.js:96 bails with `if (!host) continue`, and
injectorBaseUrl("css") reads CSS_BASE_URL, which no consumer role renders --
so the assertion is a guaranteed no-op everywhere. Rewritten to the
mechanically true reason: assertCspInjections resolves the injector through a
computed loop variable, never a literal gate argument, so test_spec_env_gates'
literal-only regex cannot see it.

The CSP hole itself is left standing: rendering CSS_BASE_URL would arm the
injection assertion for every consumer role at once, which is a change of a
different size than correcting a comment.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-06 20:41:19 +02:00
kevinveenbirkenbachandClaude Opus 5 92faba5d2d feat(playwright): make the env template mandatory so specs actually run
test_has_spec.py forces every roles/web-app-* to ship a Playwright spec and
calls a missing one a hard error, because the persona contract must never be
silently absent from the deploy capstone. Nothing forced the other half. The
runner discovers roles by rglob('templates/playwright.env.j2')
(discover_playwright_roles.py:53), so a spec without that file is staged
nowhere and executed never.

21 of 76 web-app roles were in exactly that state. Verified rather than
inferred: running the discovery filter directly returns 61 roles and none of
the 21 is among them, `make compose-playwright` refuses without a rendered
.env from a prior deploy, and `git log --all` on those paths shows the file
never existed on any branch -- the single hit is web-app-gitlab on a fork
branch. Their specs have never run once. 15 of the 21 deploy in CI today,
reporting green while their only functional verification never fires.

The new lint mirrors test_has_spec and carries a per-role opt-out that is
used nowhere: uniform beats exceptions.

Every one of the 21 now has a template written against its own
meta/services.yml rather than a shared skeleton -- espocrm splits
ADMIN_PASSWORD from ADMIN_NATIVE_PASSWORD because OIDC_ALLOW_ADMIN_USER
leaves both login paths open, the four permanently auth-less roles declare
their personas blocked instead of shipping a knowingly-red spec, and
web-app-mastodon gained a real test-seaweedfs.js because no honest
suppression existed for a role that does route Paperclip to S3.

web-app-magento moves pre-alpha -> alpha here: it now has a spec that runs,
which is what the lifecycle gate is about.

Six meta/services.yml files change only where adding a template falsified an
existing suppression -- gitlab and shopware both claimed the role 'ships no
templates/playwright.env.j2', which stopped being true in this commit.

Not resolved here, and the adversarial review says so plainly: the specs are
enabled, not proven. None has executed against a live stack. The review found
three classes of defect that land in follow-up commits rather than being
smuggled in silently -- persona blocks that are missing or should be tied to
sso.enabled, eleven suppressions naming a sys-svc-mail test suite that does
not exist, and eight claiming a CSP assertion that cannot fire because no
consumer role renders CSS_BASE_URL.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-06 14:17:55 +02:00