Skip to content

feat(member-base-nestjs-module): let applications name the casbin policy table - #214

Merged
fantasywind merged 2 commits into
mainfrom
feat/casbin-rule-entity
Sep 2, 2026
Merged

feat(member-base-nestjs-module): let applications name the casbin policy table#214
fantasywind merged 2 commits into
mainfrom
feat/casbin-rule-entity

Conversation

@luca-tw

@luca-tw luca-tw commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Why

Casbin policies went into typeorm-adapter's own casbin_rule and nowhere else. newAdapter accepts a second argument carrying customCasbinRuleEntity, but the module only ever passed the first one, so the table name was not reachable through MemberBaseModule.forRoot.

Two applications pointed at the same database therefore shared one policy table. Since the usual startup routine is "load the authoritative rules, clear the table, write them back", each one's boot briefly empties the table the other is enforcing against. Converging at the end is not the same as being isolated in between.

What

A new casbinRuleEntity option. Subclass typeorm-adapter's CasbinRule, name the table, hand it over:

@Entity('backend_casbin_rule')
export class BackendCasbinRule extends CasbinRule {}

MemberBaseModule.forRoot({
  casbinAdapterOptions: { type: 'postgres' /* ... */ },
  casbinRuleEntity: BackendCasbinRule,
});

Two decisions worth flagging for review

The config is undefined when the option is unset, not {}. newAdapter is then called exactly as it was before this option existed, so no existing deployment changes behaviour or leaves casbin_rule. The spec covers all three unset shapes (no options, {}, adapter configured but no entity).

Building that config is a standalone function rather than an inline ternary. The CASBIN_ENFORCER provider around it cannot be unit tested — it dynamic-imports typeorm-adapter (which jest.mock cannot intercept under ESM) and then opens a real connection, and option-providers.ts is excluded from coverage for that reason. Extracting it is what gives the new option test coverage at all. This is the same split, for the same reason, as load-typeorm-adapter.ts.

TypeORMAdapterConfig is restated locally because typeorm-adapter declares it but does not re-export it from its entry point; the alternative was a deep import of typeorm-adapter/lib/adapter.

Scope

This separates the cache, not the permissions. If both applications rebuild their policies from the same upstream tables, both still end up with identical contents, and a permission missing upstream stays missing in both. Setting the option on an application that already has policies starts it from an empty table — it does not migrate rows. Both caveats are stated in the README section.

Verification

  • New spec: 5 passing.
  • Package suite: 742 passed / 3 suites failing. Those 3 failures are pre-existing — verified by running the same suite on a clean tree at this commit's parent (737 passed, same 3 failures). All three come from ldapts, an optional peer dependency that is not installed in this checkout; tsc reports the same single unrelated error on the clean tree.
  • ESLint clean on the changed files; the commit also passed the repo's lint-staged typecheck/lint/format hooks.

🤖 Generated with Claude Code

…icy table

Casbin policies went into typeorm-adapter's own `casbin_rule` and nowhere
else, because `newAdapter` was called with only its first argument and the
second one — where `customCasbinRuleEntity` lives — was never exposed.

Two applications sharing a database therefore shared the table. Since the
usual startup routine is "load the authoritative rules, clear the table,
write them back", each one's boot briefly empties the table the other is
enforcing against. `casbinRuleEntity` gives each of them its own.

Building the adapter config is a standalone function rather than an inline
ternary because the provider around it cannot be unit tested: it
dynamic-imports typeorm-adapter and then opens a real connection. Same
split, and the same reason, as load-typeorm-adapter.ts.

Left unset the config stays `undefined`, so `newAdapter` is called exactly
as before and existing deployments keep `casbin_rule`.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 2, 2026

Copy link
Copy Markdown

Deploying utils-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4eaff8e
Status:🚫  Build failed.

View logs

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 2, 2026

Copy link
Copy Markdown

Deploying rytass-utils-storybook with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4eaff8e
Status: ✅  Deploy successful!
Preview URL: https://8051c1eb.rytass-utils-storybook.pages.dev
Branch Preview URL: https://feat-casbin-rule-entity.rytass-utils-storybook.pages.dev

View logs

…ly controls

The section said the entity is "read for its table name only — the columns
must remain those of CasbinRule". Both halves were wrong. typeorm-adapter
builds every policy row from the class (savePolicyLine does `new
(this.getCasbinRuleConstructor())()`), resolves the repository through it and,
where it opens the connection itself, creates the table from it. Extra columns
are explicitly supported upstream; @CreateDateColumn / @UpdateDateColumn are
that README's own example.

Which is also why the option is inert on the other branch. `newAdapter` builds
`entities: [getCasbinRuleType(...)]` only when handed connection options; given
`{ connection: dataSource }` it takes that DataSource's entity list as it finds
it while still resolving the repository through the custom class. An
application reusing its main DataSource — the natural choice in exactly the
two-apps-one-database setup this option is for — gets EntityMetadataNotFoundError
out of loadPolicy() at boot, naming TypeORM rather than the option that caused
it. Documented rather than fixed: registering entities on someone else's
DataSource is not this module's call.

llms.txt gains the section it was missing entirely, and the same caveat lands
on the option's JSDoc so it shows up at the call site.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TYPZe3xb4ZNVzHGhLeJE5U
@fantasywind
fantasywind merged commit d936d4f into main Sep 2, 2026
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants