From 4156387eb9e35f3b3a683385cc85f9eb648da6af Mon Sep 17 00:00:00 2001 From: zhangwenjian Date: Fri, 4 Sep 2026 17:23:01 +0800 Subject: [PATCH] =?UTF-8?q?fix=F0=9F=90=9B:=20enforce=20Casbin=20when=20ed?= =?UTF-8?q?iting=20another=20user?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PUT /api/v1/sys-user sits in CasbinExclude so the profile page can reach it, which means AuthCheckRole never runs for this route. The handler took the target user id from the request body, so any authenticated caller could edit another user's record - including their roleId. The route has to stay excluded: the profile page and the admin user list share this one endpoint, so removing the exclusion would break self-service editing for every non-admin role. The check therefore moves into the handler: when the target is not the caller, the request is put through Casbin explicitly. EnforceRoleFor carries the same admin short-circuit and enforcement AuthCheckRole uses, so a route that opts out of the middleware can still ask the same question. Claude-Session: https://claude.ai/code/session_01HPTAw8b8tAdFNFn8rKdPYx --- app/admin/apis/sys_user.go | 28 ++++++++++++++++++++++++++-- common/middleware/permission.go | 27 +++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/app/admin/apis/sys_user.go b/app/admin/apis/sys_user.go index 70d20688..07b8c6dd 100644 --- a/app/admin/apis/sys_user.go +++ b/app/admin/apis/sys_user.go @@ -1,6 +1,7 @@ package apis import ( + "errors" "github.com/gin-gonic/gin/binding" "go-admin/app/admin/models" "golang.org/x/crypto/bcrypt" @@ -15,6 +16,7 @@ import ( "go-admin/app/admin/service" "go-admin/app/admin/service/dto" "go-admin/common/actions" + "go-admin/common/middleware" ) type SysUser struct { @@ -149,12 +151,34 @@ func (e SysUser) Update(c *gin.Context) { return } - req.SetUpdateBy(user.GetUserId(c)) + callerId := user.GetUserId(c) + + // This route is in CasbinExclude so the personal-center screen can edit + // the caller's own record without a policy grant (see settings.go). That + // exclusion covers the whole route, not just the caller's own record, and + // the request carries the target userId in the body - so without this + // check here, any authenticated caller could edit any other user, up to + // and including their roleId. When the target is someone else, ask Casbin + // directly for the permission AuthCheckRole skipped. + if req.UserId != callerId { + allowed, err := middleware.EnforceRoleFor(c, c.Request.URL.Path, c.Request.Method) + if err != nil { + e.Logger.Error(err) + e.Error(500, err, err.Error()) + return + } + if !allowed { + e.Error(http.StatusForbidden, errors.New("无权更新其他用户数据"), "对不起,您没有该接口访问权限,请联系管理员") + return + } + } + + req.SetUpdateBy(callerId) //数据权限检查 p := actions.GetPermissionFromContext(c) - err = s.Update(&req, p) + err = s.Update(&req, p, callerId) if err != nil { e.Logger.Error(err) return diff --git a/common/middleware/permission.go b/common/middleware/permission.go index 993fde77..99a9aa5e 100644 --- a/common/middleware/permission.go +++ b/common/middleware/permission.go @@ -59,6 +59,33 @@ func AuthCheckRole() gin.HandlerFunc { } } +// EnforceRoleFor reports whether the caller's role has explicit Casbin +// permission to act on path with method. +// +// AuthCheckRole never calls Enforce for a route CasbinExclude lists - that +// is the whole point of the list. A handler on such a route can still need +// the real answer for part of what it does: sys_user.go's Update shares its +// excluded route between the personal-center screen editing the caller's own +// record (which is why the route is excluded at all) and an admin editing +// someone else's, and only the second case is meant to require a policy +// grant. That handler asks here instead of assuming the middleware already +// checked. +func EnforceRoleFor(c *gin.Context, path, method string) (bool, error) { + data, ok := c.Get(jwtauth.JwtPayloadKey) + if !ok { + return false, nil + } + v, ok := data.(jwtauth.MapClaims) + if !ok { + return false, nil + } + if v["rolekey"] == "admin" { + return true, nil + } + e := sdk.Runtime.GetCasbinByTenant(c.Request.Host) + return e.Enforce(v["rolekey"], path, method) +} + // excludedFromCasbin reports whether the route skips the permission check. // // It runs for every non-admin request, so the order matters: the method rules