fix: close platform access re-review findings
This commit is contained in:
@@ -80,3 +80,12 @@ func RequirePlatformMenuAccess() gin.HandlerFunc {
|
||||
ctx.Next()
|
||||
}
|
||||
}
|
||||
|
||||
func requirePlatformRoot(ctx *gin.Context) bool {
|
||||
claims, err := middleware.ParseAuth(ctx)
|
||||
if err == nil && claims.Role == "root" {
|
||||
return true
|
||||
}
|
||||
infra.Response.Error(ctx, errcode.ErrPermissionDenied)
|
||||
return false
|
||||
}
|
||||
|
||||
@@ -157,6 +157,7 @@ func TestPlatformRoleStatusAndArchiveReturnNotFoundWhenUpdateAffectsZeroRows(t *
|
||||
mock.ExpectCommit()
|
||||
|
||||
ctx, recorder := updateContext(test.method, "/roles/role-a", "role-a", []byte(test.body))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "root"})
|
||||
test.handler(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, int32(status.Code(errcode.ErrRecordNotFound)))
|
||||
@@ -470,6 +471,105 @@ func TestExplicitPreciseScopeRetainsCoordinatesButStillMasksPII(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPublicResponseProjectionRemovesCredentialAndAttachmentSecrets(t *testing.T) {
|
||||
ctx, _ := updateContext(http.MethodGet, "/staff/credential/credential-a", "credential-a", nil)
|
||||
ctx.Set("Auth", &types.JwtClaims{Extend: map[string]string{"location_scope": "precise"}})
|
||||
response := protectPreciseLocation(ctx, &models.StaffCredential{}, map[string]any{
|
||||
"identity": "credential-a",
|
||||
"credential_type": "installer",
|
||||
"credential_no": "CERT-123456",
|
||||
"evidence_uri": "private://evidence",
|
||||
"file_uri": "private://file",
|
||||
"attachment_uri": "private://attachment",
|
||||
})
|
||||
encoded, err := json.Marshal(response)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
body := string(encoded)
|
||||
if !strings.Contains(body, `"identity":"credential-a"`) || !strings.Contains(body, `"credential_type":"installer"`) {
|
||||
t.Fatalf("safe credential fields were removed: %s", body)
|
||||
}
|
||||
for _, forbidden := range []string{"credential_no", "evidence_uri", "file_uri", "attachment_uri", "CERT-123456", "private://"} {
|
||||
if strings.Contains(body, forbidden) {
|
||||
t.Fatalf("credential response leaked %s: %s", forbidden, body)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlatformAccountDetailMasksDisplayNameAndAvatarWithPreciseScope(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
mock.ExpectQuery(regexp.QuoteMeta(`SELECT * FROM "platfrom_account" WHERE identity = $1 ORDER BY "platfrom_account"."id" LIMIT $2`)).
|
||||
WithArgs("account-a", 1).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id", "identity", "created_at", "updated_at", "status", "version", "username", "display_name", "avatar", "password_hash", "platform_role_code", "phone"}).
|
||||
AddRow(uint64(1), "account-a", nil, nil, "enabled", 1, "operator", "张三", "private://avatar", "hash", "finance_operator", "13800138000"))
|
||||
|
||||
ctx, recorder := updateContext(http.MethodGet, "/platform/platfrom_account/account-a", "account-a", nil)
|
||||
ctx.Set("Auth", &types.JwtClaims{Extend: map[string]string{"location_scope": "precise"}})
|
||||
GetPlatfromAccount(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, 0)
|
||||
body := recorder.Body.String()
|
||||
if !strings.Contains(body, `"display_name_masked"`) || !strings.Contains(body, `"phone_masked":"138****8000"`) {
|
||||
t.Fatalf("platform account detail omitted masked PII: %s", body)
|
||||
}
|
||||
for _, forbidden := range []string{`"display_name":`, `"avatar":`, "张三", "private://avatar"} {
|
||||
if strings.Contains(body, forbidden) {
|
||||
t.Fatalf("platform account detail leaked %s: %s", forbidden, body)
|
||||
}
|
||||
}
|
||||
assertMockExpectations(t, mock)
|
||||
}
|
||||
|
||||
func TestPlatformAccountListMasksDisplayNameAndAvatar(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
mock.ExpectQuery(regexp.QuoteMeta(`SELECT count(*) FROM "platfrom_account"`)).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"count"}).AddRow(1))
|
||||
mock.ExpectQuery(regexp.QuoteMeta(`SELECT * FROM "platfrom_account" ORDER BY created_at desc LIMIT $1`)).
|
||||
WithArgs(20).
|
||||
WillReturnRows(sqlmock.NewRows([]string{"id", "identity", "created_at", "updated_at", "status", "version", "username", "display_name", "avatar", "password_hash", "platform_role_code", "phone"}).
|
||||
AddRow(uint64(1), "account-a", nil, nil, "enabled", 1, "operator", "张三", "private://avatar", "hash", "finance_operator", "13800138000"))
|
||||
|
||||
ctx, recorder := updateContext(http.MethodGet, "/platform/platfrom_account", "", nil)
|
||||
ListPlatfromAccount(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, 0)
|
||||
body := recorder.Body.String()
|
||||
if !strings.Contains(body, `"display_name_masked"`) || strings.Contains(body, `"display_name":`) || strings.Contains(body, `"avatar":`) {
|
||||
t.Fatalf("platform account list did not apply the masked projection: %s", body)
|
||||
}
|
||||
assertMockExpectations(t, mock)
|
||||
}
|
||||
|
||||
func TestNonRootCannotManagePlatformRolesOrMenus(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
handler gin.HandlerFunc
|
||||
method string
|
||||
target string
|
||||
identity string
|
||||
body string
|
||||
}{
|
||||
{"create role", CreatePlatformRole, http.MethodPost, "/platform/platform_role", "", `{"role_code":"auditor","name":"Auditor"}`},
|
||||
{"create menu", CreatePlatformMenu, http.MethodPost, "/platform/platform_menu", "", `{"menu_code":"audit","name":"Audit","path":"/audit"}`},
|
||||
{"update menu status", UpdatePlatformMenuStatus, http.MethodPatch, "/platform/platform_menu/menu-a/status", "menu-a", `{"status":"disabled"}`},
|
||||
{"archive menu", ArchivePlatformMenu, http.MethodDelete, "/platform/platform_menu/menu-a", "menu-a", ``},
|
||||
{"replace role menus", ReplacePlatformRoleMenus, http.MethodPut, "/platform/platform_role/role-a/menu", "role-a", `{"menu_identities":[]}`},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
ctx, recorder := updateContext(test.method, test.target, test.identity, []byte(test.body))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "platform_operator"})
|
||||
|
||||
test.handler(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, int32(status.Code(errcode.ErrPermissionDenied)))
|
||||
assertMockExpectations(t, mock)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestPlatformMenuAllowsOnlyAssignedDomain(t *testing.T) {
|
||||
menus := []models.PlatformMenu{
|
||||
{MenuCode: "finance", Path: "/finance"},
|
||||
@@ -495,6 +595,7 @@ func TestCreatePlatformAccountRequiresAssignableNonRootRole(t *testing.T) {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
ctx, recorder := updateContext(http.MethodPost, "/platform/platfrom_account", "", []byte(test.body))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "root"})
|
||||
|
||||
CreatePlatfromAccount(ctx)
|
||||
|
||||
@@ -504,6 +605,31 @@ func TestCreatePlatformAccountRequiresAssignableNonRootRole(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestNonRootCannotAssignPlatformAccountRole(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
handler gin.HandlerFunc
|
||||
method string
|
||||
identity string
|
||||
body string
|
||||
}{
|
||||
{"create account", CreatePlatfromAccount, http.MethodPost, "", `{"username":"operator","password":"secure-password","platform_role_code":"auditor"}`},
|
||||
{"change account role", UpdatePlatfromAccount, http.MethodPut, "account-a", `{"platform_role_code":"auditor"}`},
|
||||
}
|
||||
for _, test := range tests {
|
||||
t.Run(test.name, func(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
ctx, recorder := updateContext(test.method, "/platform/platfrom_account/"+test.identity, test.identity, []byte(test.body))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "platform_operator"})
|
||||
|
||||
test.handler(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, int32(status.Code(errcode.ErrPermissionDenied)))
|
||||
assertMockExpectations(t, mock)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestListSafetyEventDisposalsReturnsOnlyTheRequestedEventHistory(t *testing.T) {
|
||||
_, mock := setupPlatformRoleDatabase(t)
|
||||
mock.ExpectQuery(regexp.QuoteMeta(`SELECT count(*) FROM "saf_event_disposal" WHERE saf_event_identity = $1`)).
|
||||
@@ -708,6 +834,7 @@ func TestReplacePlatformRoleMenusAllowsAnEmptySetToClearAssignmentsTransactional
|
||||
mock.ExpectCommit()
|
||||
|
||||
ctx, recorder := updateContext(http.MethodPut, "/roles/role-a/menus", "role-a", []byte(`{"menu_identities":[]}`))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "root"})
|
||||
ReplacePlatformRoleMenus(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, 0)
|
||||
@@ -723,6 +850,7 @@ func TestReplacePlatformRoleMenusRejectsSystemRoleBeforeChangingRelations(t *tes
|
||||
mock.ExpectRollback()
|
||||
|
||||
ctx, recorder := updateContext(http.MethodPut, "/roles/root-role/menus", "root-role", []byte(`{"menu_identities":[]}`))
|
||||
ctx.Set("Auth", &types.JwtClaims{Role: "root"})
|
||||
ReplacePlatformRoleMenus(ctx)
|
||||
|
||||
assertResponseCode(t, recorder, int32(status.Code(errcode.ErrInvalidArgument)))
|
||||
|
||||
@@ -22,6 +22,9 @@ func GetPlatformRole(ctx *gin.Context) { getByIdentity[models.PlatformRole](ctx)
|
||||
|
||||
// CreatePlatformRole 创建非内置平台角色。
|
||||
func CreatePlatformRole(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request models.PlatformRole
|
||||
if err := ctx.ShouldBindJSON(&request); err != nil || request.RoleCode == "" || request.Name == "" || request.RoleCode == "root" {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
@@ -41,6 +44,9 @@ func CreatePlatformRole(ctx *gin.Context) {
|
||||
|
||||
// UpdatePlatformRole 更新非内置平台角色。
|
||||
func UpdatePlatformRole(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request struct {
|
||||
Name string `json:"name" binding:"required,max=64"`
|
||||
DataScope string `json:"data_scope" binding:"required,max=32"`
|
||||
@@ -111,6 +117,9 @@ func GetPlatformMenu(ctx *gin.Context) {
|
||||
}
|
||||
|
||||
func CreatePlatformMenu(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request platformMenuRequest
|
||||
if err := ctx.ShouldBindJSON(&request); err != nil {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
@@ -130,6 +139,9 @@ func CreatePlatformMenu(ctx *gin.Context) {
|
||||
}
|
||||
|
||||
func UpdatePlatformMenu(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request platformMenuRequest
|
||||
if err := ctx.ShouldBindJSON(&request); err != nil {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
@@ -143,12 +155,29 @@ func UpdatePlatformMenu(ctx *gin.Context) {
|
||||
updateAllowedByIdentity(ctx, &models.PlatformMenu{}, gin.H{"parent_id": parentID, "name": request.Name, "icon": request.Icon, "path": request.Path, "sort_no": request.SortNo}, []string{"parent_id", "name", "icon", "path", "sort_no"})
|
||||
}
|
||||
|
||||
func UpdatePlatformMenuStatus(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
UpdateRecordStatus(ctx, &models.PlatformMenu{})
|
||||
}
|
||||
|
||||
func ArchivePlatformMenu(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
ArchiveRecord(ctx, &models.PlatformMenu{})
|
||||
}
|
||||
|
||||
type platformRoleMenusRequest struct {
|
||||
MenuIdentities []string `json:"menu_identities" binding:"required"`
|
||||
MenuIdentities []string `json:"menu_identities"`
|
||||
}
|
||||
|
||||
// ReplacePlatformRoleMenus replaces every menu assignment for a role atomically.
|
||||
func ReplacePlatformRoleMenus(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request platformRoleMenusRequest
|
||||
if err := ctx.ShouldBindJSON(&request); err != nil {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
@@ -198,6 +227,9 @@ func ReplacePlatformRoleMenus(ctx *gin.Context) {
|
||||
|
||||
// ListPlatformRoleMenuIdentities returns the current assignment for the role editor.
|
||||
func ListPlatformRoleMenuIdentities(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var role models.PlatformRole
|
||||
if err := impl.DBService.Where("identity = ?", ctx.Param("identity")).First(&role).Error; err != nil {
|
||||
respondRecordError(ctx, err)
|
||||
@@ -217,6 +249,9 @@ func ListPlatformRoleMenuIdentities(ctx *gin.Context) {
|
||||
|
||||
// UpdatePlatformRoleStatus 更新非内置平台角色状态,系统角色始终受保护。
|
||||
func UpdatePlatformRoleStatus(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request struct {
|
||||
Status string `json:"status" binding:"required,max=32"`
|
||||
}
|
||||
@@ -238,6 +273,9 @@ func UpdatePlatformRoleStatus(ctx *gin.Context) {
|
||||
|
||||
// ArchivePlatformRole 归档非内置平台角色,系统角色始终受保护。
|
||||
func ArchivePlatformRole(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var role models.PlatformRole
|
||||
if err := impl.DBService.Where("identity = ?", ctx.Param("identity")).First(&role).Error; err != nil {
|
||||
respondRecordError(ctx, err)
|
||||
@@ -279,9 +317,11 @@ func ListPlatfromAccount(ctx *gin.Context) {
|
||||
infra.Response.Error(ctx, err)
|
||||
return
|
||||
}
|
||||
views := make([]gin.H, 0, len(list))
|
||||
views := make([]map[string]any, 0, len(list))
|
||||
for _, item := range list {
|
||||
views = append(views, gin.H{"identity": item.Identity, "username": item.Username, "display_name": item.DisplayName, "avatar": item.Avatar, "phone_masked": maskPhone(item.Phone), "platform_role_code": item.PlatformRoleCode, "status": item.Status})
|
||||
view := platformAccountView(item)
|
||||
protectPreciseLocation(ctx, &models.PlatfromAccount{}, view)
|
||||
views = append(views, view)
|
||||
}
|
||||
infra.Response.Success(ctx, gin.H{"total": total, "list": views})
|
||||
}
|
||||
@@ -295,18 +335,12 @@ type platfromAccountRequest struct {
|
||||
Phone string `json:"phone" binding:"max=32"`
|
||||
}
|
||||
|
||||
type platfromAccountView struct {
|
||||
Identity string `json:"identity"`
|
||||
Username string `json:"username"`
|
||||
DisplayName string `json:"display_name"`
|
||||
Avatar string `json:"avatar"`
|
||||
PhoneMasked string `json:"phone_masked"`
|
||||
PlatformRoleCode string `json:"platform_role_code"`
|
||||
Status string `json:"status"`
|
||||
}
|
||||
|
||||
func platformAccountView(account models.PlatfromAccount) platfromAccountView {
|
||||
return platfromAccountView{Identity: account.Identity, Username: account.Username, DisplayName: account.DisplayName, Avatar: account.Avatar, PhoneMasked: maskPhone(account.Phone), PlatformRoleCode: account.PlatformRoleCode, Status: account.Status}
|
||||
func platformAccountView(account models.PlatfromAccount) map[string]any {
|
||||
return map[string]any{
|
||||
"identity": account.Identity, "username": account.Username,
|
||||
"display_name": account.DisplayName, "avatar": account.Avatar, "phone": account.Phone,
|
||||
"platform_role_code": account.PlatformRoleCode, "status": account.Status,
|
||||
}
|
||||
}
|
||||
|
||||
func GetPlatfromAccount(ctx *gin.Context) {
|
||||
@@ -315,10 +349,14 @@ func GetPlatfromAccount(ctx *gin.Context) {
|
||||
respondRecordError(ctx, err)
|
||||
return
|
||||
}
|
||||
infra.Response.Success(ctx, platformAccountView(account))
|
||||
view := platformAccountView(account)
|
||||
infra.Response.Success(ctx, protectPreciseLocation(ctx, &models.PlatfromAccount{}, view))
|
||||
}
|
||||
|
||||
func CreatePlatfromAccount(ctx *gin.Context) {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
var request platfromAccountRequest
|
||||
if err := ctx.ShouldBindJSON(&request); err != nil {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
@@ -338,7 +376,8 @@ func CreatePlatfromAccount(ctx *gin.Context) {
|
||||
infra.Response.Error(ctx, err)
|
||||
return
|
||||
}
|
||||
infra.Response.Success(ctx, platformAccountView(account))
|
||||
view := platformAccountView(account)
|
||||
infra.Response.Success(ctx, protectPreciseLocation(ctx, &models.PlatfromAccount{}, view))
|
||||
}
|
||||
|
||||
func UpdatePlatfromAccount(ctx *gin.Context) {
|
||||
@@ -354,6 +393,9 @@ func UpdatePlatfromAccount(ctx *gin.Context) {
|
||||
}
|
||||
values := gin.H{"display_name": request.DisplayName, "avatar": request.Avatar, "phone": request.Phone}
|
||||
if request.PlatformRoleCode != nil {
|
||||
if !requirePlatformRoot(ctx) {
|
||||
return
|
||||
}
|
||||
if !isAssignablePlatformRole(*request.PlatformRoleCode) {
|
||||
infra.Response.Error(ctx, errcode.ErrInvalidArgument)
|
||||
return
|
||||
|
||||
@@ -190,19 +190,29 @@ func isCreatedResponseField(key string) bool {
|
||||
func protectPreciseLocation(ctx *gin.Context, model, value any) any {
|
||||
maskPersonalName := reflect.TypeOf(model) == reflect.TypeOf(&models.UserAccount{}) ||
|
||||
reflect.TypeOf(model) == reflect.TypeOf(&models.StaffAccount{})
|
||||
protectPublicFields(value, maskPersonalName, hasPreciseLocationScope(ctx))
|
||||
maskDisplayName := reflect.TypeOf(model) == reflect.TypeOf(&models.PlatfromAccount{})
|
||||
protectPublicFields(value, maskPersonalName, maskDisplayName, hasPreciseLocationScope(ctx))
|
||||
return value
|
||||
}
|
||||
|
||||
func protectPublicFields(value any, maskPersonalName, retainCoordinates bool) {
|
||||
var sensitiveResponseFields = map[string]bool{
|
||||
"avatar": true, "address": true, "credential_no": true,
|
||||
"evidence_uri": true, "evidence_url": true, "file_uri": true,
|
||||
"attachment_uri": true, "attachment_url": true,
|
||||
"certificate_uri": true, "certificate_url": true,
|
||||
"credential_uri": true, "credential_url": true,
|
||||
}
|
||||
|
||||
func protectPublicFields(value any, maskPersonalName, maskDisplayName, retainCoordinates bool) {
|
||||
switch data := value.(type) {
|
||||
case map[string]any:
|
||||
if phone, ok := data["phone"].(string); ok && phone != "" {
|
||||
data["phone_masked"] = maskPhone(phone)
|
||||
}
|
||||
delete(data, "phone")
|
||||
delete(data, "avatar")
|
||||
delete(data, "address")
|
||||
for key := range sensitiveResponseFields {
|
||||
delete(data, key)
|
||||
}
|
||||
if maskPersonalName {
|
||||
if name, ok := data["name"].(string); ok && name != "" {
|
||||
data["name_masked"] = maskPersonalNameValue(name)
|
||||
@@ -213,16 +223,22 @@ func protectPublicFields(value any, maskPersonalName, retainCoordinates bool) {
|
||||
}
|
||||
delete(data, "real_name")
|
||||
}
|
||||
if maskDisplayName {
|
||||
if name, ok := data["display_name"].(string); ok && name != "" {
|
||||
data["display_name_masked"] = maskPersonalNameValue(name)
|
||||
}
|
||||
delete(data, "display_name")
|
||||
}
|
||||
if !retainCoordinates {
|
||||
delete(data, "longitude")
|
||||
delete(data, "latitude")
|
||||
}
|
||||
for _, item := range data {
|
||||
protectPublicFields(item, maskPersonalName, retainCoordinates)
|
||||
protectPublicFields(item, maskPersonalName, maskDisplayName, retainCoordinates)
|
||||
}
|
||||
case []any:
|
||||
for _, item := range data {
|
||||
protectPublicFields(item, maskPersonalName, retainCoordinates)
|
||||
protectPublicFields(item, maskPersonalName, maskDisplayName, retainCoordinates)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user