Route unmapped email domains to review instead of inventing agencies - #207
Open
collinschreyer-dev wants to merge 14 commits into
Open
Route unmapped email domains to review instead of inventing agencies#207collinschreyer-dev wants to merge 14 commits into
collinschreyer-dev wants to merge 14 commits into
Conversation
…cies Two related defects in how a user's agency is derived at login. grabAgencyFromEmail resolved a domain by looking the first domain label up in AGENCY_LOOKUP, and getConfig returns its input unchanged on a miss. An unrecognised domain therefore became an agency name: @wv.gov produced an agency called "wv", @gmail.com produced "gmail", @mit.edu "mit", @deca.mil "deca", @cfpb.gov "cfpb". There was no unknown state, so the corpus accumulated junk agencies that later have to be reconciled by hand. Unresolved domains now return a 'Needs Review' sentinel. Detection tests dictionary membership rather than comparing the result to the input, because 25 AGENCY_LOOKUP entries legitimately map to themselves (for instance "department of agriculture"), and a string comparison would misclassify those as unresolved. The original domain is retained on Users.unresolvedDomain so an administrator can see what needs categorising; 'Needs Review' on its own is not actionable. The migration is additive, nullable and guarded. Separately, the Login.gov scope was "openid email profile". getGovernmentEmail already existed and was already wired into createUser, but all_emails was never requested, so it always received an empty array and a user whose primary address is personal arrived as that address. Adds all_emails to the scope; no logic change was needed. Verified: navy.mil, us.navy.mil, army.mil, cms.hhs.gov, hhs.gov, gsa.gov and bis.doc.gov all resolve exactly as before; the five junk cases now return Needs Review; and the three translateCASAgencyName assertions in token.test.js are unchanged. Note: Needs Review users match no solicitation under the current exact-agency visibility check, so they receive no feed until an administrator categorises them. That is the intended interim behaviour and needs Laura's confirmation. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… scopes
Phase 2 of the agency mapping work. Extends the existing Agencies table rather
than creating a parallel one, because Agencies and agency_alias already exist
and are populated with 629 rows.
Schema:
Agencies.parentId parent/component hierarchy
Agencies.agencyType federal_agency | federal_component | state_local |
education | other | needs_review, check-constrained
Agencies.deviationSourceId whose deviation applies; NULL inherits from parent
Agencies.active, .provenance
agency_domains email domain -> agency, replaces the 7 hardcoded
UNIQUE_EMAIL_AGENCY_MAPPING entries
agency_solicitation_scope which agencies' solicitations a user may see
Users.agencyId added alongside Users.agency, which stays
The architectural point is that solicitation access and deviation inheritance
are separate relationships. Access lives in agency_solicitation_scope, deviation
in Agencies.deviationSourceId, and neither derives from the other. Scope is an
explicit join table rather than derived from parentId because the rules are not
uniform: a Navy user sees only Navy while a CMS user may be scoped to CMS and
HHS, and a derived model cannot express both.
Seeding takes the 165 AGENCY_LOOKUP names and 7 domain mappings already encoded
in config. It deliberately does not infer parents: the config has no reliable
parent data and guessing would produce a hierarchy that looks authoritative but
is not. Parents come from the Phase 5 spreadsheet reconciliation or from admin.
Every agency is given a self-scope row, which reproduces today's exact-match
visibility behaviour. Nothing changes for existing users until scopes are
configured.
Also removes AGENCY_HIERARCHY from config.js. 108 lines, six parent agencies,
19 offices, 21 domains, and zero references anywhere in srt-api or srt-ui. It
was an earlier unfinished attempt at this same problem, superseded by this work.
Verified against a copy of the local SRT database: both migrations apply
cleanly, producing 11 new agencies (629 pre-existing, correctly distinguished by
provenance), 7 domain mappings, 640 self-scope rows, and 10 of 11 users linked
by exact name match. Rollback restores the database exactly, with no residual
columns or tables.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…n inheritance Replaces the exact-string agency match in getPredictions with a lookup against agency_solicitation_scope, and adds the deviation resolver that walks the parent hierarchy. The two are computed from different tables and neither is derived from the other, which is the separation this phase exists to establish: a component can inherit its parent's deviation without gaining sight of the parent's solicitations, and can be scoped to another agency's solicitations without changing whose deviation applies to it. The read filter still matches both the agency and office columns, as before. The GSA admin bypass is unchanged. Deployment is behaviour-preserving. solicitationScopeFor falls back to [user.agency] when the user has no agencyId, when the agency has no scope rows, and when the database call fails, so every user currently in the system sees exactly what they saw before. Phase 2 seeded a self-scope row for each agency, which reproduces the old filter exactly. Failures narrow visibility rather than widen it. Tests cover the three unchanged-behaviour states, the new scoping capability, inactive agency exclusion, cycle termination in the inheritance walk, and both directions of the visibility/deviation independence property. Mutation-checked: reintroducing parent-derived visibility fails 3 tests, removing the fallback fails 1, and removing the depth cap hangs the cycle test. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
23 of the 24 /api/rag-analytics/* routes were registered with no middleware at all. SRT applies authentication per route rather than through a blanket middleware, and these were never given any, so they were reachable unauthenticated from the internet. The handlers perform no authorization of their own. This exposed more than analytics reads. saveStage, deleteStage, savePipeline and deletePipeline are unauthenticated writes to pipeline configuration, and execute-pipeline, execute-stage, test-completion, test-embeddings, generate-prompt and package-synthesis all invoke LLM and embedding backends, so an anonymous caller could both alter pipeline config and spend model budget. Guards follow what the UI already enforces on the client side. Every analytics page is behind AdminGuardFn in app.routing.ts, so those routes get token(), admin_only(). The two endpoints the regular home page calls, playground/analyze and playground/package-synthesis, get token() only, since requiring admin there would break the normal user workflow. The cause of the gap is visible in how the calls are written. art-lookup was the one guarded route and the one called through Angular HttpClient, which attaches a bearer token via TokenInterceptor. Every unguarded route was called with raw fetch(), which bypasses the interceptor. The companion srt-ui change attaches the header explicitly at those call sites. Verified no tests and no other services call these endpoints. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ation
Adds ten admin-only endpoints so the agency model can be managed without a
config change and a deploy. Everything these routes cover previously lived in
AGENCY_LOOKUP and UNIQUE_EMAIL_AGENCY_MAPPING in config.js.
GET /api/admin/agency-management full view: hierarchy, domains,
user counts, access, deviation
POST /api/admin/agencies create agency or component
PUT /api/admin/agencies/:id edit, deactivate, reparent
PUT /api/admin/agencies/:id/scope set solicitation access
PUT /api/admin/agencies/:id/deviation set deviation source
POST /api/admin/agency-domains map a domain
PUT /api/admin/agency-domains/:id reassign or deactivate a domain
DELETE /api/admin/agency-domains/:id remove a mapping
GET /api/admin/needs-review unresolved domain queue
POST /api/admin/needs-review/resolve resolve one domain for all its users
Access and deviation are edited through separate endpoints that touch separate
tables. Setting one cannot move the other. The management view returns both side
by side so it is visible in the UI that they differ.
Guardrails:
Cycle prevention on both parentId and deviationSourceId. Phase 3 capped the read
path at depth 10 so a loop could not hang a request; that cap is a backstop, and
this refuses the write with an actionable error instead. It also refuses an edit
when a cycle already exists upstream rather than extending it.
A component cannot sit at the top of the hierarchy, and resolveNeedsReview will
attach a domain to an existing agency or to a new component under a named
parent, but will never create a top-level agency. That stays a deliberate act
through createAgency.
An agency always retains sight of its own solicitations; scope edits cannot
remove it. Agencies are deactivated rather than deleted and there is no delete
endpoint for them. Scope replacement runs in a transaction, as does needs-review
resolution.
Also fixes a Phase 2 gap: the migration added Users.agencyId but the model never
declared it, so user.agencyId was always undefined and Phase 3 scope resolution
would have fallen back to exact name matching for every user, permanently. The
column is now declared with its association.
14 tests cover the cycle walk in both columns, including the indirect and
pre-existing-loop cases, and the top-level guardrail. Mutation-checked:
reducing the cycle check to self-reference only fails 4 of them.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reads Laura's mapping spreadsheet, normalises it, classifies every row, compares against what SRT already holds, and writes a Markdown report describing exactly what an import would change. It writes nothing. --apply is deliberately not implemented until the report has been reviewed and the flagged rows resolved. Findings from the 256-row source: 83 rows have the affiliation and association columns transposed, a contiguous block where the department sits in the affiliation column and its component in the association column. Detected structurally rather than by line number, so re-exporting the sheet cannot silently break the correction. Imported as written, these would have inverted ten departments into components of their own bureaus. 7 agency names appear under more than one spelling, mostly the US prefix. Department of Agriculture appears three ways. Left unmerged they would become separate agencies splitting their own users and solicitations. 1 suspected typo, Defense Commisary Agency. Reported, never auto-corrected: a name that looks misspelled may be the legal name. 1 row cannot be classified from its domain and needs a human decision. 149 rows carry no domain and are created without a domain mapping. All 256 Agency Deviation cells are empty; the column is not written. Reconciliation against the 629 agencies in the local SRT database: 168 already present, 88 would be created, 104 new domain mappings, no conflicts, 438 in SRT but absent from the sheet and left alone. The spreadsheet is not treated as authoritative for deletion. Separately, the pass found 62 pairs of agencies already in SRT that are the same body under two spellings, nearly all X and U.S. X. Visibility matches on the agency name string, so a user on one spelling cannot see work tagged with the other. None of the pairs currently carries users or predictions, so nothing is split today, but they are worth merging. The comparison runs over a plain read-only pg connection rather than the Sequelize models, because requiring server/models/index.js calls umzug.up() at import time and would apply pending migrations as a side effect of asking for a report. It also verifies the connected database actually has an Agencies table before comparing, so it cannot silently reconcile against an unrelated local database. 30 tests cover transposition in both directions, the same-name and US-prefix cases, whitespace and zero-width normalisation, typo detection, classification, and CSV quoting. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds --apply to the reconciliation script and runs it against the local SRT database. The transposition reading was confirmed correct before applying. Result on the local database: 640 agencies to 718. 78 created, 159 existing agencies given the parent the sheet names, 162 given a real type in place of the needs_review placeholder the Phase 2 seed left, 96 domains mapped. Every new agency gets a self-scope row, so it behaves exactly like one that has always been there. No deviation was written and nothing was deleted. The import runs in one transaction and rolls back whole. It fills blanks but never overwrites: a domain already pointing somewhere is left alone, and an agency that already has a parent keeps it. Both cases are reported rather than merged. One real conflict surfaced, Office of Inspector General, which SRT holds under Interior and the sheet places under Transportation. Every department has one, so a single undifferentiated row is genuinely ambiguous and was left as it was. Three defects found and fixed while rehearsing against a throwaway clone: Skipping agencies that already existed dropped most of the hierarchy. The sheet names a parent for 169 entities but only 65 were getting one, because the transposed rows describe components that are already in SRT as flat rows. The import now enriches an existing agency when the field is blank, which is the spreadsheet's main contribution. A single pass was not enough. A row can name a parent that only takes its place later in the same pass, so the child read a stale value. The pass now repeats until nothing changes, capped at five. Matching was not deterministic. SRT holds 62 pairs of agencies that are the same body under two spellings, so a canonical key matches more than one row, and the lookup kept whichever row Postgres happened to return last. Consecutive imports enriched different halves of the same pair. Ordering by id and keeping the first occurrence fixes it, and also makes the parent spelling stable. Verified on a clone before touching anything real: three consecutive invocations now produce 78/0/0 created, the second and third being pure no-ops. On the real database afterwards: 718 of 718 agencies reachable from a root so no cycles, no self-parents, no dangling foreign keys, every agency holding a self-scope row, and zero deviation values written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Completes the remaining backend work. Auto-decline of personal email registrations. A self-registration from a consumer mail provider is declined on arrival and the person is emailed to say why and how to appeal. Off by default, and deliberately reversible: the decline sets isRejected, which an administrator clears from the Users screen, so it filters the approval queue rather than blocking anyone permanently. That distinction is load-bearing. State and local staff may have no government address available, and the Login.gov work in flight may leave a personal address as their only way in, so a hard block could shut out exactly the users we are otherwise trying to support. Government addresses are never declined whatever else is configured, and individual domains or addresses can be exempted without a deploy. Email templates are now stored. The three SRT ships with were a hardcoded array in the Angular component, so an administrator could edit one for a single send but never save it or add their own. They are seeded into a table with CRUD endpoints behind admin_only. templateKey is immutable because admin_audit_log rows already reference it, and a built-in is deactivated rather than deleted so the shipped set can always be restored. Duplicate agencies are resolved through aliases rather than deletion. SRT held 65 agency records that are the same body under two spellings, nearly all X and U.S. X. Visibility matches on the agency name string, so a user under one spelling could not see solicitations tagged with the other. The obvious fix is to merge the rows and rewrite the losing name wherever it appears, which would mean rewriting tens of thousands of solicitation rows and changing what people see if it went half right. Instead the canonical record keeps its id, the duplicate's name is recorded as an alias, and the duplicate is deactivated. Solicitation data is never touched. That works because it revives agency_alias, a table created in 2021, seeded with 31 aliases, and never read by anything since. It now has a model, and solicitationScopeFor includes an agency's aliases in the names it matches on. Visibility only widens where an alias row exists, so an agency without one behaves exactly as before. Verified against a clone before applying. Two defects surfaced there: the merge ran several updates concurrently on a single pg client, which cannot do that, and repointing scope rows tripped the uniqueness constraint because the surviving agency already held the pair being moved onto it. Colliding rows are now removed before repointing, and the queries are serialised. The transaction rolled back cleanly both times, which is how the bugs were caught without consequence. After the merge on the local database: 718 agency rows retained, 653 active, 65 deactivated, aliases 31 to 96, no orphaned domains or scope rows, no user or domain pointing at a deactivated agency, and no active agency still sharing a name with another. Re-running reports nothing to do. 81 tests. New coverage for the decline policy, including that a state .gov address survives even if misconfigured as personal, and for alias resolution, including that an alias lookup failure narrows visibility rather than failing the request. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…the UI Two adjustments so the Phase 6 backend is legible from the admin console. The agency management view now returns each agency's alternate spellings. A duplicate that was merged survives as an alias rather than a row, so without this an administrator looking for an agency that has apparently disappeared has no way to see where it went or which names it now answers to. Auto-decline now writes 'Declined (Personal Email)' rather than a raw auto_declined key. The admin Users screen already had that exact string in its list of review statuses and renders reviewStatus directly, so an auto-declined account now displays, filters, and can be changed exactly like one declined by hand. Inventing a second vocabulary would have shown the raw key in the status column and left it unfilterable. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e test matrix Login resolution now consults agency_domains before falling back to the config maps. This was plan item 3.1 and it had been missed: Phase 3 delivered scope-aware visibility but left grabAgencyFromEmail reading only the two hardcoded maps, so the 96 domains imported from Laura's spreadsheet sat in the database unused and login still answered from config. The import was only half a feature without this. resolveAgencyForEmail queries the domain table first and falls back to the existing function for anything it does not know, so behaviour is unchanged for unmapped domains and a database problem degrades to today's answer rather than failing the login. The observable difference on real data: cfpb.gov config: Needs Review table: Bureau of Consumer Financial Protection ams.usda.gov config: Agricultural Marketing Service table: same ftb.ca.gov config: Needs Review table: Needs Review usss.dhs.gov now resolves to "Secret Service" rather than "US SECRET SERVICE", because the duplicate merge made the former canonical. Those users keep sight of the same solicitations through the alias, which is what the alias mechanism is for. Adds the Phase 6 matrix covering government email selection, .mil handling, deterministic selection when an account holds several government addresses, and the regression cases from the plan. Those regressions are the point of the original change: @wv.gov, @gmail.com, @mit.edu and @deca.mil each used to become an agency named after the first label of the domain, so a user at wv.gov joined an agency called "wv". Each is now asserted to reach Needs Review instead. One test in the first draft asserted that cfpb.gov resolves through the config maps. It does not, and never did. The assertion was wrong rather than the code, and correcting it is what surfaced the missing 3.1 work above. 102 tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds items 6.1 through 6.6, so all twelve scenarios from the plan now map to a
named test and the matrix is auditable against the document rather than against
memory.
6.1 Navy sees Navy work only and inherits DOD's deviation.
6.2 CMS sees both CMS and HHS work, and still inherits HHS's deviation. The
contrast with Navy is the point: the rules are not uniform, which is why
access is stored rather than derived from the hierarchy.
6.3 FEMA sees FEMA work only and inherits DHS's deviation.
6.4 A state body sees no federal solicitations and inherits no federal
deviation, including the realistic case of a state user with no agency
record at all, which is where Needs Review leaves them today.
6.5 A component with its own deviation overrides the parent chain.
6.6 Both directions of independence: inheriting a deviation upward grants no
sight of the parent, and being scoped to another agency does not change
whose deviation applies.
Mutation-checked. Making visibility follow the parent chain fails 6 tests,
removing the isKnownAgencyKey guard that stops unmapped domains becoming agency
names fails 10, and selecting the first email regardless of its domain fails 3.
A first attempt at the middle mutation edited a string that does not exist in
the file and so changed nothing, which briefly looked like the regression tests
were not guarding anything. They are: the corrected mutation fails 10 tests.
114 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…egration/auth-and-agency
getConfig prefers environment variables over the config file, and they always arrive as strings. The flag was read with plain truthiness, so setting autoDeclinePersonalEmail to "false" in order to turn the feature off would have turned it on instead, and every personal-email registration would have been declined by the act of trying to disable it. Only a real boolean true or the string "true" enables it now. Covered for "false", "0", "no", "off", "" and "False". 121 tests.
Seeds the six templates from the program's email SOP: Access Granted, Request
Access, Government Email Needed, Non-Agency Account Access Granted, Login.gov
Troubleshooting, and Which Agency. These were being sent by hand from the
SRT@gsa.gov mailbox against a written procedure, which is why the SOP exists.
Wording is the SOP's own. Two deliberate changes. The [First name] marker
becomes {{first_name}} to match the placeholder style the existing templates
use. And the Login.gov troubleshooting template had one specific person's
address hardcoded as the one for the recipient to select, so everyone who
received it was told to choose someone else's email; that is now
{{government_email}}.
Auto-decline now sends the SOP's own "Government Email Needed" wording rather
than text invented for this feature, so an automatic decline reads identically
to one sent by hand. The built-in text remains only as a fallback if the
template is removed. The recipient's first name is threaded through, and an
unresolved placeholder is stripped rather than greeting someone as
"Hello {{first_name}}".
Two things the SOP settles that were previously open questions.
Step 7 grants access to .edu and non-federal .gov addresses, and the
Non-Agency template describes exactly the upload-tool-only experience. State and
local users are in scope by the program's own documented procedure, which is
worth knowing while the Login.gov barrier for those users is unresolved.
The Login.gov Troubleshooting template states that the cause is "a problem with
the default configuration of our app", independently confirming what the
authorization request and the Login.gov screenshot suggested. That template also
becomes largely unnecessary once SRT requests the all_emails scope, since that
is the underlying fix.
123 tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
Phase 1 of the agency/domain mapping work. Two related defects in how a user's
agency is derived at login.
The passthrough
grabAgencyFromEmailresolves a domain by looking the first domain label up inAGENCY_LOOKUP, andgetConfigreturns its input unchanged on a miss. So anunrecognised domain became an agency name:
@wv.govwv@gmail.comgmail@mit.edumit@deca.mildeca@cfpb.govcfpbThere was no unknown state, so the corpus accumulated junk agencies that have to
be reconciled by hand later.
Unresolved domains now return a
Needs Reviewsentinel, and the original domainis kept on
Users.unresolvedDomainso an administrator can see what needscategorising. The migration is additive, nullable and guarded.
One subtlety worth reviewing: detection tests dictionary membership rather
than comparing the result to the input. 25
AGENCY_LOOKUPentries legitimatelymap to themselves (
department of agriculture,department of commerce, andothers), so a string comparison would have misclassified real agencies as
unresolved.
The Login.gov scope
The scope was
openid email profile.getGovernmentEmail()already existed andwas already wired into
createUser, butall_emailswas never requested, so italways received an empty array and a user whose Login.gov primary address is
personal arrived as that address and resolved to the wrong agency.
Adds
all_emails. No logic change was needed. Note the Login.gov consent screenwill now list the additional claim.
Verification
All previously-resolving domains are unchanged:
navy.mil,us.navy.mil,army.mil,cms.hhs.gov,hhs.gov,gsa.gov,bis.doc.gov. The five junkcases return
Needs Review. The threetranslateCASAgencyNameassertions intoken.test.jsare unaffected, verified directly because Jest could not runlocally (
jest-extendedis not installed).Needs a decision before merge
A
Needs Reviewuser matches no solicitation under the current exact-agencyvisibility check, so they receive no feed until an administrator categorises
them. That is deliberate, but it is a policy change and should be confirmed with
Laura.
Not yet done: end-to-end confirmation against a real Login.gov account holding
both a personal and a
.govaddress.