fix: use an OU's handle as its mail domain by maneesha-xyz · Pull Request #308 · OpenGovMail/raven

maneesha-xyz · 2026-09-28T07:32:11Z

📌 Description

An OU's handle is now its mail domain. Raven no longer works out the domain from where the OU sits in the Thunder tree, so a domain can have any number of labels, a child OU's domain doesn't have to be a subdomain of its parent's, and a grouping OU (such as foundations) can sit above OUs that receive mail.

Closes [BUG] Mail domain is derived from the OU tree, so deep domains and OUs under a grouping OU don't resolve #307

🔍 Changes Made

internal/maildomain: new FromHandle, which returns an OU handle as a lowercased domain, or reports that the handle isn't a domain (foundations, default).
internal/socketmap/thunder: buildOUPath and the never-expiring ouCache are removed. ValidateDomain and GetOrgUnitIDForDomain now look the domain up in a domain → OU ID index built by walking GET /organization-units and GET /organization-units/{id}/ous (paged at 100, following next links).
- The index is rebuilt every CACHE_TTL_SECONDS. If a rebuild fails, the previous index stays in use.
- A domain that is the handle of more than one OU is left out of the index.
internal/server/auth and internal/delivery/groupresolver: the parent-walk in resolveOrganizationUnitDomain is replaced with a single GET /organization-units/{id} and FromHandle. The callers (IMAP LOGIN domain check, OAUTHBEARER, group expansion) are unchanged.
docs/THUNDER_CONSUMER_CONTRACT_GUIDE.md: endpoint [TASK] Write Test for "LOGIN" Command #3 is now the list/children endpoints, and [BUG] Copy of the send mail was not saved on the sqlite database. #9 no longer describes a parent walk.

✅ Checklist (Email System)

Core IMAP commands tested (LOGIN, CAPABILITY, LIST, SELECT, FETCH, LOGOUT).
Authentication is tested.
Docker build & run validated.
Configuration loading verified for default and custom paths.
Persistent storage with Docker volume verified.
Error handling and logging verified
Documentation updated (README, config samples).

🧪 Testing Instructions

To test the server, use the instructions in the README in the test directory.

go test ./... passes. The new and updated tests:

internal/maildomain: handle parsing, including deep domains, co.uk, mixed case, trailing dot, non-domain handles and path-traversal characters.
internal/socketmap/thunder/index_test.go: a child OU silver.x.y under foundations resolves with no x.y OU present; duplicate handles resolve to nothing; pagination; index reuse within the TTL; the previous index is served when a rebuild fails; an error when no index was ever built.
internal/server/auth and internal/delivery/groupresolver: fixtures use full-domain handles and fail the test if a parent OU is fetched.

Against a real Thunder, with raven's own IDP_CLIENT_ID/IDP_CLIENT_SECRET, run the socketmap service and check that virtual-domains <child OU domain> returns OK 1 and virtual-domains <grouping OU handle> returns NOTFOUND. This also confirms raven's token may call the list endpoints; so far they have only been checked with an admin token.

⚠️ Notes for Reviewers

Breaking for existing trees. Before upgrading, rename every OU that receives mail so that its handle is its full mail domain. For example, a child silver under x.y becomes silver.x.y. Otherwise mail for that OU stops resolving and its users can't log in.
A new or renamed OU takes up to CACHE_TTL_SECONDS (default 300) to take effect. Socketmap answers from the domain index, and a stale index is refreshed in the background, so Postfix rejects mail for a newly created domain until the next refresh lands. A renamed OU's old domain is accepted for the same window.
Handles of OUs that receive mail must be unique across the whole tree, not just among siblings. Socketmap leaves a duplicated domain out of the index and logs both OU IDs, but IMAP LOGIN and group expansion read a single OU's handle and can't see the duplicate. Two users with the same username in those OUs would open the same mailbox. Thunder-side uniqueness for mailDomains would enforce this.
Each OU has one domain for now. Multiple domains wait for a mailDomains attribute on OUs in Thunder.
Out of scope: SASL still ignores the domain when authenticating senders ([BUG] IMAP authentication allows login with username-only and mismatched domain #267), and a Thunder outage still makes domains look nonexistent.

greptile-apps · 2026-09-28T07:35:41Z

Confidence Score: 0/5

[Critical risk] Changes how mail domains are resolved from organization units.

The PR does not appear safe to merge: early lookups can time out after SocketMap reports readiness, and several previously reported mail-routing and mailbox-identity failures remain outstanding.

Findings

Cold lookups can time out ▶
New domains remain invisible ▶
Duplicate domains share mailboxes ▶
Old domains stay accepted ▶
Domain casing splits mailboxes ▶
Startup can walk twice ▶
Listing permissions remain untested ▶

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  A[SocketMap startup] --> B[Background OU-tree walk]
  A --> C[Bind listener]
  C --> D[Cold domain or address lookup]
  D --> E{Index available?}
  E -- No --> F[Wait for tree walk]
  E -- Yes --> G[Answer from index]
  F --> G
  F --> H[LMTP deadline may expire]
  H --> I[Fall back to recipient identity]

_{Reviews (3) · Last reviewed commit: "fix(socketmap): build the domain index w..."}

greptile-apps · 2026-09-28T07:35:50Z

+			parent = strings.TrimSuffix(strings.TrimPrefix(r.URL.Path, "/organization-units/"), "/ous")
+		default:
+			w.WriteHeader(http.StatusNotFound)
+			return
+		}
+


Listing permissions remain untested

The fake Thunder accepts a hard-coded bearer token, so these tests cannot establish whether Raven’s service-account credentials can read both new listing endpoints. If Thunder returns 403 for any page, the first index build fails and no domains resolve. Add a contract or deployment smoke test using Raven’s credentials to catch that configuration failure before rollout.

Knowledge Base Used: Thunder directory integration

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

greptile-apps · 2026-09-28T08:38:27Z

+	} else if stale && refreshing.CompareAndSwap(false, true) {
+		go func() {
+			defer refreshing.Store(false)
+			if err := rebuildIndex(host, port, tokenRefreshSeconds, true); err != nil {
+				log.Printf("      │ ⚠ Domain index refresh failed, serving the previous one: %v", err)
+			}
+		}()
+	}
+
+	ouID, found := index[domain]


New domains remain invisible

When an OU is added after the index is built, repeated lookups for its domain still use the old snapshot and return NOTFOUND. A lookup after the configured TTL only starts a background rebuild; it does not wait for the new domain to appear. Mail to a valid newly configured domain can therefore be rejected until that rebuild finishes.

Knowledge Base Used:

Directory lookup service

SocketMap server and protocol

greptile-apps · 2026-09-28T08:38:27Z


-	if len(handles) == 0 {
-		return "", fmt.Errorf("no OU handles found")
+	domain, ok := maildomain.FromHandle(ou.Handle)


Duplicate domains share mailboxes

When two OUs have the same domain handle, SocketMap excludes that domain as ambiguous, but this resolver and IMAP login still accept each OU's handle. If users in those OUs share a local username, group expansion collapses their addresses into one. LMTP still delivers after SocketMap returns NOTFOUND, and both users can select the same email-keyed mailbox. Mail intended for distinct organizations can thus be mixed.

How this was verified: Both OU resolvers accept the duplicate handle, while group expansion and mailbox storage identify recipients by email rather than OU ID.

Knowledge Base Used:

Thunder directory integration

Recipient and group resolution

IMAP authentication and transport security

greptile-apps · 2026-09-28T08:38:27Z

+	} else if stale && refreshing.CompareAndSwap(false, true) {
+		go func() {
+			defer refreshing.Store(false)
+			if err := rebuildIndex(host, port, tokenRefreshSeconds, true); err != nil {
+				log.Printf("      │ ⚠ Domain index refresh failed, serving the previous one: %v", err)
+			}
+		}()
+	}
+
+	ouID, found := index[domain]
+	return ouID, found, nil


Old domains stay accepted

If an OU's handle changes after the index is built, domain validation continues to accept its former domain while a refresh is pending or failing. Address lookups also use the stored OU ID without checking its current handle, so users still in that OU can be accepted under the former domain. This now affects even domains that were never queried before the handle changed.

How this was verified: Domain validation answers from the stored index, and user validation compares against its OU ID without rereading the OU's handle.

Knowledge Base Used:

Thunder directory integration

Directory lookup service

greptile-apps · 2026-09-28T08:38:27Z

+// FromHandle returns the mail domain declared by an OU handle, lowercased and without a
+// trailing dot. It reports false when the handle is not a domain.
+func FromHandle(handle string) (string, bool) {
+	domain := strings.TrimSuffix(strings.ToLower(strings.TrimSpace(handle)), ".")


Domain casing splits mailboxes

If an OU handle or a user's login domain contains uppercase letters, group expansion now lowercases the handle when forming the member's address, but LOGIN accepts the domain case-insensitively and keeps the spelling supplied by the client. Mail delivery and IMAP then open different email-keyed database files, so a user logged in with the uppercase spelling cannot see mail delivered to the lowercased address.

Knowledge Base Used:

IMAP authentication and transport security

Recipient and group resolution

greptile-apps · 2026-09-29T04:44:51Z

+	go func() {
+		if err := thunder.WarmIndex(cfg.ThunderHost, cfg.ThunderPort, cfg.TokenRefreshSeconds); err != nil {
+			log.Printf("⚠ Initial domain index build failed: %v", err)
+			log.Printf("  Check that raven's service account can list organization units")
+			log.Printf("  Service will retry on first request")
+		}
+	}()


Cold lookups can time out

If the initial OU-tree walk lasts longer than LMTP’s two-second SocketMap deadline, SocketMap accepts connections while cold lookups are still waiting for the index. LMTP then falls back to the recipient address instead of the resolved mailbox identity, so mail for a role mailbox can be stored under the wrong identity. Keep readiness from signaling usable lookups until the initial index is available.

Knowledge Base Used:

Recipient and group resolution

Runtime configuration and deployment

greptile-apps · 2026-09-29T04:44:51Z

+	go func() {
+		if err := thunder.WarmIndex(cfg.ThunderHost, cfg.ThunderPort, cfg.TokenRefreshSeconds); err != nil {


Startup can walk twice

If a cold lookup starts building the index before the new startup goroutine does, WarmIndex still forces a second complete OU-tree walk after the first finishes. On a large tree, this doubles startup listing traffic to Thunder without improving the fresh index. The new test covers only the ordering where the warm build starts first.

Knowledge Base Used: Directory lookup service

maneesha-xyz added 4 commits September 28, 2026 10:33

feat(maildomain): read an OU's mail domain from its handle

93da264

fix(socketmap): look up a domain's OU by handle, not tree path

d23fc46

fix(auth,groupresolver): take an OU's mail domain from its own handle

9864e25

docs: describe OU handle as mail domain

be9ef3a

maneesha-xyz self-assigned this Sep 28, 2026

maneesha-xyz added the bug Something isn't working label Sep 28, 2026

greptile-apps Bot reviewed Sep 28, 2026

View reviewed changes

fix(socketmap): refresh the domain index without blocking lookups

713d830

greptile-apps Bot reviewed Sep 28, 2026

View reviewed changes

fix(socketmap): build the domain index without delaying startup

13ca5b4

greptile-apps Bot reviewed Sep 29, 2026

View reviewed changes

maneesha-xyz marked this pull request as draft September 29, 2026 05:07

Navigation Menu

Repository navigation

fix: use an OU's handle as its mail domain - #308

fix: use an OU's handle as its mail domain#308
maneesha-xyz wants to merge 6 commits into
mainfrom
ou-handle-as-mail-domain

maneesha-xyz commented Sep 28, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Sep 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 28, 2026

Uh oh!

greptile-apps Bot Sep 29, 2026

Uh oh!

greptile-apps Bot Sep 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

		go func() {
		if err := thunder.WarmIndex(cfg.ThunderHost, cfg.ThunderPort, cfg.TokenRefreshSeconds); err != nil {

Conversation

maneesha-xyz commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📌 Description

🔍 Changes Made

✅ Checklist (Email System)

🧪 Testing Instructions

⚠️ Notes for Reviewers

Uh oh!

greptile-apps Bot commented Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Confidence Score: 0/5

Findings

Uh oh!

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 28, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot Sep 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

maneesha-xyz commented Sep 28, 2026 •

edited

Loading

greptile-apps Bot commented Sep 28, 2026 •

edited

Loading