(DO NOT MERGE) EST Branch to setup AI#2
Open
gqcorneby wants to merge 61 commits into
Open
Conversation
Tighten the naming convention from "kebab-case or camelCase" to camelCase-only for all custom/<feature> paths (backend and frontend). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Adds a new rule for schema-level upstream isolation (side tables vs column injection) that complements the existing custom-package-isolation file rule. Documents the openboxes-domain-model skill in CLAUDE.md. Ignores .claude/scheduled_tasks.lock runtime artifact.
Documents the load-bearing details for safely landing custom Liquibase
migrations on the EST fork:
- Placement: include line goes after the upstream release loop and
before the views rebuild so upstream migrations apply first, ours
second, and view rebuilds last.
- Aggregator clarification: adding includes inside custom/changelog.groovy
is never an upstream touch.
- FK ordering: include order in custom/changelog.groovy must put FK-target
files above FK-holder files; rollback runs in reverse-application order.
- Mandatory rollback {} block on every changeset.
- Liquibase id-collision protection via (id, author, filename) tuple —
use author 'eyeseetea' and the custom/ folder for namespace isolation.
Adds the one-time upstream-file touch to grails-app/migrations/changelog.groovy so every client and feature branch inherits the custom-migration entrypoint. New custom migrations now only need to: 1. Drop a file under grails-app/migrations/custom/<date>-<feature>.groovy 2. Append a one-line include to custom/changelog.groovy (which is ours) No more per-feature edits to the upstream master changelog. The aggregator ships empty — Liquibase loads the empty databaseChangeLog block as a no-op until real changesets are added. Updated .claude/rules/custom-package-isolation.md to reflect that the wiring is pre-installed; the rule no longer asks future authors to edit the upstream master changelog.
gqcorneby
changed the title
…ges.properties Grails 3.3's PluginAwareResourceBundleMessageSource only globs grails-app/i18n/messages*.properties at the root, so custom bundles under grails-app/i18n/custom/ never load at runtime — every messageSource.getMessage(...) call silently falls back to the defaultMessage parameter, defeating translation. Update custom-package-isolation.md and groovy/patterns.md to point custom-feature i18n keys at the upstream root bundle (with a "# <feature> (custom)" block, documented as an upstream touch point in design.md) and explicitly forbid the i18n/custom/ subfolder, the messageSource basename override, and the orphan crowdin.yml glob.
Replaces docker/openboxes-config.client-template.properties with the YAML- formatted docker/openboxes.client-template.yml. The new template mirrors application.yml's nesting, makes locale/currency/identifier lists readable, and documents the load-order intent inline. docker-compose.yml mount line is updated to mount openboxes.yml at /app/.grails/openboxes.yml — the YAML loader entry already in application.yml:32 picks it up automatically. Sysadmins can keep secrets (DB/SMTP creds) out of VCS by injecting via env vars in docker-compose.yml or by templating a deploy-time file separately.
- Add literal block scalar (|) to 'Custom code isolation' and 'Configuration' sections so trailing prose lines don't break YAML parsing - Replace stale 'docker/openboxes-config.properties' reference with 'docker/openboxes.yml' (+ client-template.yml) to match current docker/ layout
Custom changesets use id `<yyyy-MM-dd>-<NN>-<desc>` with author `eyeseetea`, not a global `custom-NNNN` counter (which needs cross-branch coordination and clashes between parallel feature branches). Maintainability rule; Liquibase runs any unique tuple regardless. Standardize going forward.
Adds OpenSpec proposals for DHIS2 OAuth2 SSO (dhis2-oauth-core) and iframe embedding (dhis2-iframe-embedding), plus the archived investigation spike (dhis2-oauth-spike) that gated them. The spike confirmed DHIS2 OAuth endpoints, /api/me shape, embedded Tomcat 8.5.88, the User.active=false rejection path in AuthController, HTTP client availability, and *.localtest.me resolution. Follow-up design confidence re-scored to 9/10. Adds DHIS2 OAuth + iframe config blocks to docker/openboxes.client-template.yml and a spike-only docker-compose for local DHIS2.
Cleanup of the previous commit, which accidentally committed both the pre-archive and post-archive spike directories due to stale index state.
Adds DHIS2 Authorization Code login alongside the existing username/password flow. First-login auto-registers users with active=false; admins activate via the existing user-admin screen (new "Pending DHIS2 access" filter). - Side-table custom_dhis2_user_link matches by stable DHIS2 UID - Hand-rolled client (no Spring Security plugin); hooks into AuthService - SecurityInterceptor gates inactive users to a pending-access page - Feature disabled by default; opt-in via per-deployment openboxes.yml
Rename to follow Grails service naming convention so the bean is auto-registered, removing the manual resources.groovy wiring. Reuse a single pooled HttpClient across calls, consolidate the request/response handling, and add a search filter + roles fetch to the pending-users admin query.
- Move pending.gsp to grails-app/views/custom/dhis2auth/ to comply with custom-package isolation; controller renders explicit view path. - Drop fetchMode JOIN on roles in Dhis2AdminService — Hibernate applies LIMIT before deduplication on collection joins, breaking pagination. Pending users are a small set so lazy fetch is fine. - Remove redundant @transactional on Dhis2RegistrationService (Grails services are transactional by default; the annotation narrows scope silently if future methods are added unannotated). - Add `static transactional = false` and `volatile` to Dhis2OAuthService fields backing the pooled HttpClient; drop now-redundant @NotTransactional annotations. Same treatment on Dhis2SessionService. - Rewrite Dhis2UserLinkSpec — beforeInsert/beforeUpdate tests called methods that don't exist; replaced with concrete unique-constraint saves and a GORM auto-timestamp assertion. - Date-prefix the Liquibase migration filename to match the custom-package-isolation convention and update the changelog include. - Tighten weak Spock assertions: assert full encoded URL in Dhis2OAuthServiceSpec with edge-case where: rows; assert the linked user id in Dhis2RegistrationServiceSpec. - design.md: fix the schema diagram (UUID CHAR(38), not BIGINT), the config key path (openboxes.dhis2.oauth.*), list the UrlMappings touch point, and replace the application.yml note with the external-config approach actually used.
grails.gorm.transactions.Transactional uses a compile-time AST transform that reliably binds the Hibernate session; without it the default Grails transaction wrapping does not cover flush() calls in private methods, causing TransactionRequiredException at runtime.
- Dhis2OAuthServiceSpec: stub GrailsApplication.getConfig() was failing a Spock cast (returns grails.config.Config, not ConfigObject); switch to grailsApplication.config property assignment instead. - Dhis2UserLinkSpec: User/Person not mocked caused validate() failures; add DataTest + mockDomains. Unique constraint tests and auto-timestamp tests removed — neither is enforced by in-memory GORM unit tests; those guarantees live at the DB schema level (SQL UNIQUE, Hibernate events). - Dhis2UserLink: add blank: false to dhis2Uid constraint so empty strings correctly fail validation.
Restore trailing whitespace on upstream docker-compose.yml lines so the only diff is the functional app port mapping. Add docker-compose.yml and docker/openboxes.yml to design.md upstream touch points.
Covers task 3.5: controller session/redirect/400 paths, SecurityInterceptor pending-access gate, and an @Integration spec exercising the real HTTP client against an embedded DHIS2 stub with real GORM persistence. Promote dhis2-auth spec to openspec/specs; sync tasks.md and design.md (Deploy status).
Implementation complete and tests green. Moved to archive/2026-06-04-dhis2-oauth-core.
Remove the 9090:8080 compose override (app falls back to base 8080:8080) so docker-compose.yml is upstream-pristine, and delete the spike-only DHIS2 stack docker/dhis2-sso/docker-compose.spike.yml. Neither is needed by the feature — both were local-dev/validation scaffolding. Drop the compose touch point from the archived design.md.
Add resolveIdentity branching on profile: v42 reads the username from the id_token sub claim (back-channel TLS, no signature verification), v40/v41 keep /api/me. Add PKCE (S256) on the authorize/token exchange and an optional client_secret_post auth method. Preserve the original cause when wrapping a malformed id_token in Dhis2OAuthException.
findOrRegister branches on the presence of a UID: v40/v41 keep the UID path, v42 looks up (or creates) an inactive user keyed by username with placeholder names an admin fills at approval. A tombstoned link forces re-approval rather than silently reusing the prior account. Username collisions now escalate the suffix (-dhis2, -dhis2-2, ...) and throw once exhausted instead of failing on a duplicate-key save.
The callback now stores the PKCE verifier on initiate, passes it to the token exchange, and resolves identity via resolveIdentity. Restrict the post-login targetUri redirect to local paths only, rejecting protocol- relative (//) and backslash forms that browsers normalise to off-domain.
Describe the profile: v40 | v42 switch, the v42 endpoint/scope set, the required DHIS2-side client PATCHes (scopes, auth method, bcrypt secret), and that v42 needs no userUrl and never requests the email scope.
Proposal, design, delta spec, and tasks for the v42 username-keyed identity pivot. Not yet archived (live e2e pending).
Sync the MODIFIED 'DHIS2 OAuth login option' requirement (v40/v42 profiles, PKCE, id_token sub identity) into the main spec, then move the completed change to openspec/changes/archive/2026-06-11-dhis2-oauth-v42.
- Guard active flag before re-writing User on tombstone re-approval to avoid a redundant UPDATE on every recycled-username login. - Drop no-op URLEncoder on the base64url PKCE code_challenge. - Assert lastLoginAt advances in the v42 returning-user spec.
Extend the dhis2-iframe-embedding change with a silent-SSO phase, grounded in a source-level investigation of DHIS2 prompt=none support: - validation/prompt-none.md: v42 (Spring Authorization Server 1.5.x) honors OIDC prompt=none; legacy v40/UAA does not. - proposal/design D5: in-frame prompt=none flow with break-out on login_required/consent_required; v40 login-once fallback; third-party cookie risk; consent prerequisite. - specs/dhis2-auth: ADDED 'Embedded silent authentication' requirement. - tasks Phase 5: implementation + Spock tests + live prompt=none validation.
Tested against DHIS2 2.42.4.1: no-session -> login_required; live-session -> silent authorization code, zero UI. Silent SSO viable.
Custom servlet filter emits Content-Security-Policy: frame-ancestors scoped to openboxes.custom.iframe.frameAncestors (default 'self'), and a response wrapper suppresses any X-Frame-Options so CSP is the single framing source. Registered in resources.groovy after the Sentry filter. No in-repo X-Frame-Options source exists, so no upstream neutralization needed.
…(Phase 5) Embedded entry point (/oauth/dhis2/initiate?embedded=true) runs the v42 OAuth flow with prompt=none when iframe embedding is enabled; v40 ignores it (no silent support). On login_required/consent_required the callback renders a break-out page that navigates window.top to interactive login, so DHIS2's unframeable login renders top-level. A one-shot session flag prevents loops. All custom/dhis2auth + custom/iframe specs green.
- CSP filter now no-ops when frameAncestors is empty, so non-embedding deployments stay byte-for-byte upstream (no new header by default). - IframeCookieCustomizer installs Tomcat Rfc6265CookieProcessor with SameSite=None on OB's embedded Tomcat when embedding is enabled — reliable for JSESSIONID (a header-rewriting filter cannot reach it). Scoped no-op when disabled; warns that embedding requires TLS so SameSite=None is Secure. No DHIS2 change, no Tomcat upgrade.
…se 3-4) server.use-forward-headers documented in the client template (set via openboxes.yml, not upstream application.yml). docker/dhis2-sso/ adds an nginx-TLS + OpenBoxes + mysql compose stack under *.localtest.me for live embedded/silent-SSO testing against a real DHIS2 (scaffolding, pending a manual run-through).
D1: CSP filter gated off by default. D2: pivot from the unreliable Set-Cookie filter to Rfc6265CookieProcessor (reaches JSESSIONID). Upstream touch points corrected to the single file actually edited (resources.groovy).
- breakout.gsp: JS-encode the interactive URL (encodeAsJavaScript) and only navigate window.top when actually framed, recovering in place top-level. - Strengthen cookie customizer test to assert Rfc6265 SameSite=None value. - Correct design rollback note (filter no-ops, not 'self').
The iframe CookieCustomizer and CSP filter declared `GrailsApplication grailsApplication` but were registered in resources.groovy without wiring it. Spring-DSL beans there are not autowired by name, so grailsApplication was null. IframeCookieCustomizer.customize() runs at embedded-Tomcat creation, so the null dereference aborted boot (NPE) for every deployment, embedding enabled or not. Wire grailsApplication explicitly via ref().
Add the iframe embedding block to the committed per-client config reference with placeholder values, alongside the existing DHIS2 OAuth example. Notes the SameSite=None/Secure requirement (TLS + use-forward-headers in prod, session.cookie.secure only for localhost testing). No secrets.
Make the forwarded-proto setting visible in the committed per-client config reference (root-level server.use-forward-headers), alongside the frameAncestors example. Required behind a TLS proxy so OB marks the SameSite=None cookie Secure. No secrets.
- sanitize attacker-influenceable `error` param before logging (CRLF strip + cap) - null-safe fallback for malformed id_token exception message - drop redundant @transactional (Grails services are transactional by default) - collapse redundant prepareAuthorize ternary (default param covers it) - add tests: login_required must not break out when not silent; tombstoned link whose user is already inactive; full-URL assertion for PKCE-omit
Confirmed in-frame silent SSO end-to-end on a local same-site rig (DHIS2 2.42.4.1 + skeleton installed as a DHIS2 app + OB on lvh.me): production topology is "embedder is a DHIS2 app", so DHIS2's session cookie is first-party and SameSite is a non-issue; only OB's cookie is cross-site. Captured DHIS2 config requirements: oauth2.server.enabled requires server.base.url, consent must be off for prompt=none, redirect URI needs a real TLD, DHIS2 2.42 needs a Tomcat-10 core. Documented the third-party cookie limitation (state mismatch when 3p cookies blocked) and its fixes (same-parent-domain deploy or Partitioned/CHIPS).
Full ./gradlew test green (973 tests, 0 failures). 3.2 folded into 5.7; 4.5 superseded by the skeleton-as-DHIS2-app rig; 5.7 updated with confirmed in-frame success. Frontend untouched; local npm test blocked by Node-18/Babel env (CI on Node 14 covers it).
Archive dhis2-iframe-embedding to archive/2026-06-11-dhis2-iframe-embedding. Apply spec deltas to permanent specs: dhis2-auth (+1 embedded-silent-auth requirement), iframe-embedding (new capability), local-dev-tls (new capability). Also fix pre-existing structural issues in openspec/specs/dhis2-auth/spec.md (delta headers in a main spec; SHALL keyword on the second line) so it validates.
Revert the simplify-pass removal. The AST-based @transactional reliably binds the Hibernate session for flush() in private methods; the default Grails wrapping does not, causing TransactionRequiredException at runtime (per oauth-branch commit 4aa42c9). Unit DataTest specs don't catch this — restore + comment so it isn't removed again.
- Registration: give DHIS2-only users an unguessable random local password instead of the shared '*DHIS2*' sentinel, closing an account-takeover path (local DB/API login matches cleartext). - OAuth HTTP client: set connect/socket/connection-request timeouts so a slow DHIS2 endpoint can't hang servlet threads. - Token/userinfo errors: keep the raw response body out of the (logged) exception; expose it only at guarded debug level. - id_token: clarify the comment — TLS secures transport but does not validate the JWT as an identity assertion (accepted risk).
# Conflicts: # grails-app/services/org/pih/warehouse/custom/dhis2auth/Dhis2RegistrationService.groovy
EST: DHIS2 OAuth
EST: DHIS2 iframe embedding (silent SSO)
EST: add DHIS2 break-out page i18n keys to root bundle
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Setup ai config