ver 2.1 -> 2.2

2026-07-22 22:01:51 +08:00
parent 1deae876c3
commit 40f057030c
3 changed files with 4 additions and 61 deletions
@@ -0,0 +1,147 @@
+# AGENT.md
+
+    Violin IAM service: unified login, authentication, and authorization
+    microservice for the Violin suite.
+
+## Rules
+
+    1. Do not use PowerShell to read or write files. Prefer the built-in
+       file tools or Python instead.
+    2. Use English in code. Avoid Chinese, Japanese, or any other non-ASCII
+       natural language inside source files.
+
+## Boot rules
+
+The very first time I ran `java -cp ... cn.violin.iam.ViolinIamApplication`,
+Spring failed with "Parameter 0 of constructor in
+`cn.violin.core.config.WebMvcConfig` required a bean of type
+`cn.violin.core.interceptor.AuthenticationInterceptor` that could not be
+found". The unit-test suite was green but had never actually booted the
+context. Concrete gotchas to fix:
+
+- **`@SpringBootApplication` MUST set `scanBasePackages = {"cn.violin"}`**
+  explicitly. The default only scans the app's own sub-package
+  (`cn.violin.iam`), which excludes `cn.violin.core.interceptor.AuthenticationInterceptor`,
+  `CurrentUserHandlerMethodArgumentResolver`, `TraceIdFilter`, and other
+  framework beans. Spring Boot auto-config registers the
+  `@AutoConfiguration` classes themselves, but their `@Bean` dependencies
+  that are not themselves exposed via another `@Bean` still need component-scan
+  coverage. Compare with `violin-auth`, which has always used
+  `scanBasePackages = {"cn.violin"}`; it never tripped over this.
+
+- **`@MapperScan("cn.violin.*.mapper")` — never use bare `cn.violin`.**
+  The broad form hoovers up every interface in the project, including
+  framework-internal ones like `JwksClient`, and MyBatis then complains
+  at runtime:
+  `Invalid bound statement (not found): cn.violin.core.security.JwksClient.loadKeys`.
+  Narrowing the pattern prevents MyBatis from treating non-mapper
+  interfaces as proxies.
+
+- **`@Service` impls whose interface is in the same package should be
+  marked `@Primary` when `scanBasePackages` is widened.** Once
+  `cn.violin.*` is scanned, Spring ends up holding two `BeanDefinition`s
+  of the same bean type (the impl + some derived candidate), and
+  `@Autowired` of the interface fails with `NoUniqueBeanDefinitionException:
+  expected single matching bean but found 2: OAuthServiceImpl,OAuthService`.
+  Adding `@Primary` to the impl collapses that to a single candidate.
+
+- **Spring 6 forbids `@PostConstruct` methods with parameters.**
+  `SnowflakeId.init(SnowflakeProperties props)` looked innocuous but
+  `IllegalStateException: Lifecycle annotation requires a no-arg method`
+  hit on `@SpringBootTest`. Use `InitializingBean` + `@Autowired` for
+  the setter (properties injection remains valid via
+  `wireProperties`). Same pattern applies to any other singleton that
+  needs configuration at boot.
+
+- **`@ConfigurationProperties` on Java `record` types needs
+  `@ConstructorBinding`** when the canonical constructor does not match
+  Spring's expected discovery. Without it, Spring fails with
+  `No default constructor found`.
+
+- **Adding `@SpringBootTest` early catches all four of these.** Place a
+  smoke test (`ApplicationContextLoadsTest`) in
+  `src/test/java/.../boot/` and run `mvn test`. If the unit tests are
+  green but the boot test fails, the bug is precisely the kind that
+  silently passes when scanning only `@Component`-annotated classes
+  but blows up at full startup.
+
+## Design notes (do not skip)
+
+### JWT revocation propagation — what we tried, what we picked, and why
+
+The naive question "should we drop `customerId` from the JWT claim so
+downstream must call IAM every request?" was the wrong frame. Below is
+the actual decision tree we walked.
+
+#### Problem
+
+A user's `t_user.customer_id` is migrated from tenant A to tenant B. The
+JWT issued at login still carries `customerId=A`. Without propagation
+the user keeps operating under A's RBAC scope (and SAR cache) for the
+remainder of the token TTL.
+
+#### Industry patterns surveyed
+
+1. **Short TTL + RFC 7662 introspection** (Google / AWS / GitHub /
+   Slack). Token TTL = 5-15 min; each request consults an introspection
+   endpoint (with local cache) to check the token is still live.
+   Revocation propagates in &lt;1 s via shared cache / pub-sub.
+
+2. **`revokedCustomerBindings` cache** (medium-scale).
+   `Cache<userId, oldCustomerId>` with TTL covering the longest possible
+   token lifetime. `UserService.changeCustomer(...)` writes the DB row
+   then publishes `(userId, oldCustomerId)` into the cache. The cache
+   TTL ensures stale entries eventually fall out without a cron.
+
+3. **Active binding flag** (per-user cache of currently-valid
+   customerId). cache miss → fall back to DB lookup. cache hit but
+   cache.customerId != JWT claim → treat as revoke signal.
+
+4. **Kafka / event-driven revocation** (large clusters, &gt;10 services).
+   IAM emits `user.tenant.changed` to a topic; subscribers invalidate
+   local cache on receipt. Strictly over-engineered for a 1-issuer /
+   5-subscriber fleet.
+
+#### What we picked and why
+
+We picked **option 2** (`revokedCustomerBindings` cache) and combined
+it with **token TTL reduction** from 24 h to **1 h**. Net result:
+
+- Worst-case staleness window = `min(token TTL, cache TTL)` = 1 h.
+- No per-request IAM call (the user's framing-question was "is the
+  extra IAM call worth it?" — answer: no, because it moves IAM onto
+  the hot path and turns it into a single point of failure).
+- Single-instance IAM means cache state is authoritative without
+  sticky sessions or shared cache.
+
+#### What we did NOT do (and the reasons)
+
+- **Drop `customerId` from JWT claim.** Would force every permission
+  check to call `POST /api/v1/internal/check-permission`, adding
+  2-5 ms latency per request and making IAM the hot path. Token
+  claim is now treated as **informational only** — never trusted
+  for authorization. All decisions go through IAM SAR.
+- **Re-sign JWT on every customer change.** Requires Kafka or
+  webhook infra to broadcast — engineering cost far exceeds risk
+  reduction given the 1 h TTL already covers the window.
+- **Use Authentik's session revocation directly.** Adds an upstream
+  dependency IAM cannot fail without.
+
+#### Implementation hooks already in place
+
+| File | Role |
+| --- | --- |
+| `cn.violin.common.context.RequestContext` | exposes `setCustomerIdHint(...)` and `getVerifiedCustomerId()` — verified provenance is now mandatory for SAR / PermissionAspect / RbacController / ResourcePermissionAspect |
+| `SubjectAccessReviewRequest.customerVerified` | explicit boolean; SAR denies on `false` |
+| `PermissionAspect` | reads `getVerifiedCustomerId()` first, throws `UnauthorizedException` if null |
+| `ResourcePermissionAspect` | same |
+| `RbacController.access-check` | same |
+| `InternalController.check-permission` | accepts only verified (set from HMAC + ServiceAllowlist mapping) |
+
+#### Followup still pending (backlog, not on the critical path)
+
+Add `CustomerBindingRevocationCache` and have `UserService.changeCustomer(...)`
+push `(sub, oldCustomerId)` into it. Have `PermissionAspect` consult the cache
+as a secondary check (after verified gate). Reduce default `expirationMs`
+in `ViolinJwtProperties` from `86_400_000` (24 h) to `3_600_000` (1 h).
+These three together close the revocation window to the bound of TTL.