mirror of
https://gitcode.com/JianFeeeee/ModelRouter.git
synced 2026-11-07 09:47:23 +00:00
refactor(plugins): 统一授权词汇 —— AccessRule
## 问题
同一个问题「谁能看到 / 能调用什么」有两套规则、相反的默认值,并排住在两个文件里:
audienceAllows(want, role): "" / "all" → 全开 页面可见性
HTTPHandler.allows(role,key): "" → 仅 admin 注册端点
两个默认值各自都对:页面没人看得见就等于没渲染,页面不该被未声明的 audience
藏起来;而端点「忘了声明」绝不能等于对公网敞开。
危害不在默认值本身,在于**同一个问题有两套词汇**。运维配「谁能看 billing」和
「谁能调 /balance」得先学会两套规则,才能回答同一个问题。
## 改动
新增 internal/lua/plugin_access.go,把两套规则收敛成一个 AccessRule,
并把**仍然不同的那一点——默认值——写成数据而不是留成隐含**:
- AllowsCapability:空 = 拒绝(端点、函数)
- AllowsVisibility:空 = 继承(页面、元素)
语法也统一成一种,`ParseAccessRule` / `Rule` 双向:
"admin" / "admin, user" / "@alice, @bob" / "user, @alice" / "public"
`@name` 是密钥、其余是角色,一��字符串表达得完规则编辑器要提供的全部选项。
## 抽取时差点弄丢一个真实语义
`"all"` 是既有插件文件和**已保存的运维 override** 里的写法。统一后的解析器
不认识它,于是一个已发布的「全开」授权静默变成了关闭 —— 已部署的
billing、quota 可见性和所有存量 override 都会受影响。
是既有的 TestAudienceAllows 抓到的,它当场变红。这正是抽取的实质收益:
两个函数并排住着的时候,没人会想到去动那个共用字符串。
`TestAllIsAnAliasForPublic` 把它写成新类型的性质,而不是留给碰巧跑到的那
个套件去守着。
## 顺手加的东西我又撤了
accessRuleFor 第一版把 roles 里写成 `@alice` 的项重分类成密钥,理由是
「这大概是个自然的笔误」。那是在重构里偷带行为变更:会把写了莫名其妙内容的
端点的可达范围悄悄放宽。已撤,注释里写明为什么这个笔误值得修是另一件事、
需要它自己的理由和判据。
## 判据
七条性质,每条一个默认/边界:capability 默认关、visibility 默认继承、
keys 替换而非追加、keys 空时未认证调用者不匹配、admin 恒通、
"all" 别名、Rule() 往返稳定、角色与密钥名大小写/空格无关。
变异自证四次,全部变红:keys 变 OR、capability 默认全开、visibility 默认
拒绝、丢掉 "all" 别名。
头两次跑变异时报「没抓到」,是注入脚本自己写坏了(NameError / 替换串不
匹配)—— 变异没生效不等于判据无力,重写后确认四条都能抓到。
## 一条被记录下来的意外
`TestRoleAndKeyUnionAdmitsBoth` 最初断言「同时列出 role 和 key 时两者都放行」。
debug 跑出来是相反的:keys 非空就整体替换掉 role,carol 被拒。
这个意外正是不把它放在注释里的理由。判据现在断言实际语义(替换),与既有
HTTPHandler.allows 一致,因此没有任何已部署行为改变;但它把「这是我们的选择」
写成了可执行的一句话,而不是默默沿用旧代码碰巧做的事。
Co-Authored-By: ModelRouter <noreply@modelrouter.dev>
This commit is contained in:
180
internal/lua/plugin_access.go
Normal file
180
internal/lua/plugin_access.go
Normal file
@ -0,0 +1,180 @@
|
||||
package lua
|
||||
|
||||
import "strings"
|
||||
|
||||
// One vocabulary for "who may see / read / call this".
|
||||
//
|
||||
// Before this there were two rules with OPPOSITE defaults, living side by side:
|
||||
//
|
||||
// - audienceAllows(want, role): "" or "all" → everybody
|
||||
// Used by UI visibility and per-page/per-element audiences.
|
||||
// - HTTPHandler.allows(role, key): "" → admin ONLY
|
||||
// Used by registered endpoints, where "I forgot to declare" must not mean
|
||||
// "open to the internet".
|
||||
//
|
||||
// Both defaults are right for their own surface, and having them in one file
|
||||
// with one name is how a reader ends up assuming the other one's rule applies.
|
||||
// An operator configuring "who can see billing" and "who can call /balance" had
|
||||
// to learn two vocabularies to answer the same question.
|
||||
//
|
||||
// The distinction that remains is not the RULE, it is the DEFAULT, so it is
|
||||
// stated as data rather than left implicit: an empty grant is "deny" for
|
||||
// capabilities and "inherit" for visibility, and every call site names which it
|
||||
// wants.
|
||||
|
||||
// AccessRule is a parsed grant: a set of roles, an optional set of named gateway
|
||||
// keys, and an explicit public flag.
|
||||
//
|
||||
// Keys REPLACE roles rather than adding to them. That is not an arbitrary
|
||||
// choice: "allow these two keys" read as "these two keys AND everyone in the
|
||||
// user role" silently widens access to every user key, which is the opposite of
|
||||
// what someone tightening a rule meant. `keys = { "@alice" }` therefore means
|
||||
// exactly alice.
|
||||
type AccessRule struct {
|
||||
// Roles admit a gateway role: "admin", "user", …
|
||||
Roles []string
|
||||
// Keys admit a named gateway key, by CONFIG NAME (never the secret itself, so
|
||||
// a rule file contains no credential and survives key rotation).
|
||||
Keys []string
|
||||
// Public admits anyone, including an unauthenticated caller.
|
||||
Public bool
|
||||
}
|
||||
|
||||
// AccessAll is the grant that admits everyone.
|
||||
func AccessAll() AccessRule { return AccessRule{Public: true} }
|
||||
|
||||
// ParseAccessRule reads the one grant syntax the plugin API accepts:
|
||||
//
|
||||
// "admin" roles
|
||||
// "admin, user" roles
|
||||
// "@alice, @bob" named keys, replacing the role rule
|
||||
// "user, @alice" the named keys plus the roles (union)
|
||||
// "public" anyone
|
||||
// "all" anyone (the older spelling of public)
|
||||
// "" empty — meaning depends on the caller
|
||||
//
|
||||
// An "@name" token is a key, anything else is a role. That is unambiguous to
|
||||
// type and to parse, and it means one string can express everything the rules
|
||||
// editor needs to offer.
|
||||
//
|
||||
// "all" is kept as an alias for "public" because it is what existing plugin
|
||||
// files and saved operator overrides already contain. Dropping it would have
|
||||
// silently turned a published grant into a closed one.
|
||||
func ParseAccessRule(s string) AccessRule {
|
||||
var r AccessRule
|
||||
for _, part := range strings.Split(s, ",") {
|
||||
part = strings.TrimSpace(part)
|
||||
if part == "" {
|
||||
continue
|
||||
}
|
||||
if strings.EqualFold(part, "public") || strings.EqualFold(part, "all") {
|
||||
r.Public = true
|
||||
continue
|
||||
}
|
||||
if strings.HasPrefix(part, "@") {
|
||||
if k := strings.TrimSpace(strings.TrimPrefix(part, "@")); k != "" {
|
||||
r.Keys = append(r.Keys, k)
|
||||
}
|
||||
continue
|
||||
}
|
||||
r.Roles = append(r.Roles, part)
|
||||
}
|
||||
return r
|
||||
}
|
||||
|
||||
// Rule renders the grant back to the syntax ParseAccessRule accepts. The admin
|
||||
// page uses it to fill its input, so a value that round-trips is a value the
|
||||
// operator can edit without translating.
|
||||
func (r AccessRule) Rule() string {
|
||||
var parts []string
|
||||
for _, k := range r.Keys {
|
||||
parts = append(parts, "@"+k)
|
||||
}
|
||||
for _, role := range r.Roles {
|
||||
parts = append(parts, role)
|
||||
}
|
||||
if r.Public {
|
||||
parts = append(parts, "public")
|
||||
}
|
||||
return strings.Join(parts, ", ")
|
||||
}
|
||||
|
||||
// Empty reports whether the grant names nobody.
|
||||
func (r AccessRule) Empty() bool {
|
||||
return len(r.Roles) == 0 && len(r.Keys) == 0 && !r.Public
|
||||
}
|
||||
|
||||
// AllowsCapability reports whether a CAPABILITY (an endpoint, a callable
|
||||
// function) is reachable. Empty means DENY: a plugin that forgets to declare
|
||||
// gets a 403 naming the field to set, never an open door.
|
||||
//
|
||||
// Admin is always admitted. An operator has to be able to inspect what a plugin
|
||||
// exposes even while locked out of it, or a typo in a key list would leave
|
||||
// nobody able to diagnose the endpoint they were trying to tighten.
|
||||
func (r AccessRule) AllowsCapability(role, keyName string) bool {
|
||||
if role == "admin" {
|
||||
return true
|
||||
}
|
||||
if len(r.Keys) > 0 {
|
||||
// A non-empty key list REPLACES the role rule (see the type comment).
|
||||
// An unauthenticated caller has no key name, so a key-scoped capability
|
||||
// is unreachable for them — even when the same rule also lists roles,
|
||||
// because a caller with no key is also not the kind of caller a role
|
||||
// rule was written for.
|
||||
if keyName == "" {
|
||||
return false
|
||||
}
|
||||
for _, k := range r.Keys {
|
||||
if strings.EqualFold(k, keyName) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
// The key list replaced the role rule, so a miss is a deny even though
|
||||
// roles may also be listed.
|
||||
return false
|
||||
}
|
||||
if r.Public {
|
||||
return true
|
||||
}
|
||||
for _, want := range r.Roles {
|
||||
if strings.EqualFold(want, role) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// AllowsVisibility reports whether a VIEW (a page, a UI element) is rendered.
|
||||
// Empty means INHERIT: visibility is additive by nature — a page nobody can see
|
||||
// is simply not rendered — so an undeclared audience must not hide a page the
|
||||
// plugin clearly intends to publish.
|
||||
func (r AccessRule) AllowsVisibility(role string) bool {
|
||||
if r.Empty() || r.Public {
|
||||
return true
|
||||
}
|
||||
if role == "admin" && len(r.Keys) > 0 {
|
||||
// A key-scoped view still has to be inspectable by an admin, for the
|
||||
// same reason capabilities do.
|
||||
return true
|
||||
}
|
||||
for _, want := range r.Roles {
|
||||
if strings.EqualFold(want, role) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
// accessRuleFor folds the separate declaration fields of an endpoint into one
|
||||
// grant, so the rules below live in one place instead of beside each field.
|
||||
//
|
||||
// It is a plain fold, deliberately. An earlier version also reclassified a role
|
||||
// that happened to start with "@" into a key, on the theory that "roles =
|
||||
// { '@alice' }" is a natural mistake. That is a behaviour change smuggled into a
|
||||
// refactor: it would have quietly widened who reaches an endpoint whose author
|
||||
// wrote something nonsensical. If that mistake is worth fixing, it is a change
|
||||
// with its own justification and its own test, not a side effect of unifying
|
||||
// two functions into one.
|
||||
func accessRuleFor(roles, keys []string, public bool) AccessRule {
|
||||
return AccessRule{Roles: roles, Keys: keys, Public: public}
|
||||
}
|
||||
152
internal/lua/plugin_access_test.go
Normal file
152
internal/lua/plugin_access_test.go
Normal file
@ -0,0 +1,152 @@
|
||||
package lua
|
||||
|
||||
import (
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// The two functions this file replaces had OPPOSITE defaults:
|
||||
//
|
||||
// audienceAllows: "" / "all" → everybody
|
||||
// HTTPHandler.allows: "" → admin ONLY
|
||||
//
|
||||
// Both are correct for their surface. The hazard is that they are two
|
||||
// vocabularies for the same question, so a reader — or an operator — assumes
|
||||
// the rule that is more familiar applies. AccessRule keeps one rule and states
|
||||
// the default as data at each call site.
|
||||
|
||||
// TestCapabilityDefaultIsClosed: an endpoint that declares nobody must not be
|
||||
// reachable. This is the property that makes "I forgot" fail safe.
|
||||
func TestCapabilityDefaultIsClosed(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name string
|
||||
rule string
|
||||
}{
|
||||
{"empty", ""},
|
||||
{"nil-ish", " "},
|
||||
} {
|
||||
if ParseAccessRule(tc.rule).AllowsCapability("user", "alice") {
|
||||
t.Errorf("%s: an undeclared capability is reachable by a user", tc.name)
|
||||
}
|
||||
if !ParseAccessRule(tc.rule).AllowsCapability("admin", "alice") {
|
||||
t.Errorf("%s: admin locked out of a capability they must be able to inspect", tc.name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestVisibilityDefaultIsInherits: the opposite default, on purpose. A page
|
||||
// nobody can see is not rendered; an undeclared audience must not hide a page
|
||||
// the author clearly meant to publish.
|
||||
func TestVisibilityDefaultIsInherits(t *testing.T) {
|
||||
if !ParseAccessRule("").AllowsVisibility("user") {
|
||||
t.Error("an undeclared audience hides a page")
|
||||
}
|
||||
if ParseAccessRule("admin").AllowsVisibility("user") {
|
||||
t.Error("an admin-only page was shown to a user")
|
||||
}
|
||||
}
|
||||
|
||||
// TestNamedKeysReplaceRolesNotAdd: the single most consequential rule, and the
|
||||
// one an "OR" implementation gets wrong. "allow these two keys" must not also
|
||||
// admit every user key.
|
||||
func TestNamedKeysReplaceRolesNotAdd(t *testing.T) {
|
||||
r := ParseAccessRule("@alice, @bob")
|
||||
if !r.AllowsCapability("user", "alice") {
|
||||
t.Error("a named key was not admitted")
|
||||
}
|
||||
if r.AllowsCapability("user", "carol") {
|
||||
t.Error("a key list leaked to every other user — the rule must REPLACE, not extend")
|
||||
}
|
||||
if r.AllowsCapability("user", "") {
|
||||
t.Error("an unauthenticated caller — no key name at all — matched a named key")
|
||||
}
|
||||
if !r.AllowsCapability("admin", "") {
|
||||
t.Error("admin locked out; a typo in a key list would leave nobody able to diagnose it")
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoleAndKeyUnionAdmitsNeitherSibling: what a rule naming BOTH a role and a
|
||||
// key actually does.
|
||||
//
|
||||
// Written first as "both are admitted", which is the intuitive reading of a
|
||||
// union — and the debug run showed the opposite: a non-empty key list REPLACES
|
||||
// the role rule outright, so carol is denied even though "user" is listed.
|
||||
//
|
||||
// That surprise is exactly why the rule is named, commented and now tested. The
|
||||
// question is whether the behaviour is the one we want; asserting it here
|
||||
// documents the choice rather than quietly preserving whatever the old code
|
||||
// happened to do. The behaviour matches the pre-existing HTTPHandler.allows, so
|
||||
// nothing deployed changes.
|
||||
func TestRoleAndKeyUnionAdmitsNeitherSibling(t *testing.T) {
|
||||
r := ParseAccessRule("user, @alice")
|
||||
if !r.AllowsCapability("user", "alice") {
|
||||
t.Error("the named key was not admitted")
|
||||
}
|
||||
if r.AllowsCapability("user", "carol") {
|
||||
t.Error("a listed role was honoured despite a non-empty key list; " +
|
||||
"the key list must replace it, or a listed role silently widens " +
|
||||
"access to every user key")
|
||||
}
|
||||
if r.AllowsCapability("user", "mallory") {
|
||||
t.Error("an unrelated user was admitted")
|
||||
}
|
||||
}
|
||||
|
||||
// TestAllIsAnAliasForPublic guards a real regression this refactor nearly
|
||||
// shipped. "all" is what existing plugin files and already-saved operator
|
||||
// overrides contain; when the unified parser did not know it, a published grant
|
||||
// silently became a closed one. The pre-existing TestAudienceAllows is what
|
||||
// caught it — this test states it as a property of the new type so the
|
||||
// compatibility requirement is not left to whichever suite happens to run.
|
||||
func TestAllIsAnAliasForPublic(t *testing.T) {
|
||||
r := ParseAccessRule("all")
|
||||
if !r.Public {
|
||||
t.Fatal(`"all" did not parse as public`)
|
||||
}
|
||||
if !r.AllowsCapability("user", "carol") {
|
||||
t.Error(`"all" closed a grant that was published as open`)
|
||||
}
|
||||
// It must also render back to the form the editor understands, or the next
|
||||
// save of an untouched field would rewrite it.
|
||||
if !strings.Contains(strings.ToLower(r.Rule()), "public") {
|
||||
t.Errorf("Rule() = %q, want it to render as public", r.Rule())
|
||||
}
|
||||
}
|
||||
|
||||
// TestRuleRoundTrips: the admin page fills its input from Rule(), so a grant
|
||||
// that does not round-trip is one an operator cannot edit without translating.
|
||||
func TestRuleRoundTrips(t *testing.T) {
|
||||
for _, in := range []string{
|
||||
"admin",
|
||||
"admin, user",
|
||||
"@alice",
|
||||
"@alice, @bob",
|
||||
"user, @alice",
|
||||
"public",
|
||||
"all",
|
||||
} {
|
||||
once := ParseAccessRule(in)
|
||||
twice := ParseAccessRule(once.Rule())
|
||||
if once.Rule() != twice.Rule() {
|
||||
t.Errorf("%q -> %q -> %q, not stable", in, once.Rule(), twice.Rule())
|
||||
}
|
||||
if len(twice.Roles) != len(once.Roles) || len(twice.Keys) != len(once.Keys) {
|
||||
t.Errorf("%q lost data across a round trip: %+v -> %+v", in, once, twice)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestRoleMatchingIsCaseAndSpaceInsensitive: grants arrive typed by humans into
|
||||
// a text input, and "Admin" not matching "admin" would lock an admin out of
|
||||
// their own plugin.
|
||||
func TestRoleMatchingIsCaseAndSpaceInsensitive(t *testing.T) {
|
||||
if !ParseAccessRule(" Admin , User ").AllowsCapability("admin", "x") {
|
||||
t.Error("casing and spacing defeated a role match")
|
||||
}
|
||||
if ParseAccessRule("Admin").AllowsCapability("user", "x") {
|
||||
t.Error("a user was admitted by a case variant")
|
||||
}
|
||||
if !ParseAccessRule(" @Alice ").AllowsCapability("user", "alice") {
|
||||
t.Error("casing defeated a named-key match")
|
||||
}
|
||||
}
|
||||
@ -1134,18 +1134,9 @@ func (ps *Plugins) HookErrors() map[string]map[string]interface{} {
|
||||
// "*" is deliberately NOT a synonym for "all": an audience that looks
|
||||
// restrictive in a config file while meaning everything is exactly the kind of
|
||||
// setting that fails silently.
|
||||
// audienceAllows answers a VISIBILITY question, so an empty grant inherits.
|
||||
func audienceAllows(want, role string) bool {
|
||||
want = strings.ToLower(strings.TrimSpace(want))
|
||||
if want == "" || want == "all" {
|
||||
return true
|
||||
}
|
||||
role = strings.ToLower(strings.TrimSpace(role))
|
||||
for _, part := range strings.Split(want, ",") {
|
||||
if strings.TrimSpace(part) == role {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return ParseAccessRule(want).AllowsVisibility(role)
|
||||
}
|
||||
|
||||
// visibilityOverrides holds the operator's per-plugin audience overrides.
|
||||
@ -2091,40 +2082,11 @@ func (h HTTPHandler) routePath() string {
|
||||
//
|
||||
// keyName is the caller key's configured name ("" when it has none), not the
|
||||
// secret.
|
||||
// allows answers a CAPABILITY question, so an empty grant denies. See
|
||||
// AccessRule.AllowsCapability for why the default is closed and why a non-empty
|
||||
// key list replaces the role rule instead of adding to it.
|
||||
func (h HTTPHandler) allows(role, keyName string) bool {
|
||||
// Admin is always admitted. An operator must be able to inspect what a plugin
|
||||
// exposes even while locked out of it, or a typo in a key list would leave
|
||||
// nobody able to diagnose the endpoint they were trying to tighten.
|
||||
if role == "admin" {
|
||||
return true
|
||||
}
|
||||
// Named keys REPLACE the role rule rather than adding to it.
|
||||
if len(h.Keys) > 0 {
|
||||
if keyName == "" {
|
||||
return false
|
||||
}
|
||||
for _, k := range h.Keys {
|
||||
if strings.EqualFold(strings.TrimSpace(k), keyName) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
if h.Public {
|
||||
return true
|
||||
}
|
||||
if len(h.Roles) == 0 {
|
||||
// No declared roles and no public opt-out: treat as admin-only. A plugin
|
||||
// author who forgets this gets a 403 rather than an open endpoint, and
|
||||
// the error says which field to set.
|
||||
return role == "admin"
|
||||
}
|
||||
for _, r := range h.Roles {
|
||||
if strings.EqualFold(strings.TrimSpace(r), role) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
return accessRuleFor(h.Roles, h.Keys, h.Public).AllowsCapability(role, keyName)
|
||||
}
|
||||
|
||||
func (ps *Plugins) StateVisibleTo(name, role string) bool {
|
||||
|
||||
Reference in New Issue
Block a user