NM-324: PenTest Fixes

[abhishek9686]

Describe your changes

• An Auditor User Can Add New Devices To Network

• A Platform User Can View Unassigned Network Activity

• A Platform User Can View JIT Requests Of Other Networks

• Platform User Can View Metrics Of Unassigned Network

• A Platform User Can View Unauthorised Network Dns Details

Provide Issue ticket number if applicable/not in title

Provide testing steps

Checklist before requesting a review

• My changes affect only 10 files or less.
• I have performed a self-review of my code and tested it.
• If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
• If it is a bugfix, my code is <= 200 lines.
• My functions are <= 80 lines.
• I have had my code reviewed by a peer.
• My unit tests pass locally.
• Netmaker is awesome.

[tenki-reviewer]

Tenki Code Review - Complete

Files Reviewed: 4
Findings: 1

By Severity:

• 🟠 Medium: 1

---

This PR introduces a NetworkActivityRsrc type and associated endpoint permission routing, tightens auditor access to enrollment keys, and removes blanket PlatformUser+GET and MetricRsrc fast-paths from network permission checks. One finding warrants attention: the removal of TrafficFlow from the unconditional IS_GLOBAL_ACCESS trigger means platform-users who filter /api/v1/flows by network_id now hit NetworkPermissionsCheck and are denied unless they hold an explicit network role.

Files Reviewed (4 files)

controllers/middleware.go
logic/user_mgmt.go
pro/logic/security.go
schema/user_roles.go

[tenki-reviewer]

Summary

The PR adds a new NetworkActivityRsrc / NetworkActivityRsrc resource type (network_activity) with its corresponding AllNetworkActivityRsrcID, wires it into the middleware route-to-header mapping, and maps it in GetAllRsrcIDForRsrc. It also tightens the authorization surface in several ways:

• Auditor denied enrollment-keys (pro/logic/security.go:174): A new guard explicitly returns access denied before the generic GET → nil auditor fast-path when the request path contains /api/v1/enrollment-keys. This correctly prevents auditors from listing enrollment keys even with GET.
• Removed PlatformUser+GET network fast-path (pro/logic/security.go): The blanket allowance for any PlatformUser doing a GET on network resources is removed. Network-scoped access now requires explicit group/role assignments.
• Removed MetricRsrc network fast-path (pro/logic/security.go): Metric access is no longer automatically granted to all authenticated users in NetworkPermissionsCheck; it must be role-configured.
• IS_GLOBAL_ACCESS condition refactor (controllers/middleware.go:141-145): TrafficFlow is removed as an unconditional global trigger. UserActivityRsrc is now included inside the NET_ID == "" gate.

Potential Issue: Platform-Users Lose Flow Access with network_id Filter

With TrafficFlow no longer forcing IS_GLOBAL_ACCESS=yes unconditionally, calls to GET /api/v1/flows?network_id=<id> now have NET_ID set (by the newly added network_id query-param block at lines 35-37), which causes the request to be routed to NetworkPermissionsCheck. Since the PlatformUser+GET fast-path in NetworkPermissionsCheck was also removed in this PR, platform-users will receive 403 Forbidden unless they hold a network-level role explicitly granting traffic_flow read permission.

If this is intentional (network-scoped flow access now requires a role), it should be communicated as a breaking change. If unintentional, a targeted fast-path for TrafficFlow+GET can be added back in NetworkPermissionsCheck.

Other Changes

All other changes (new NetworkActivityRsrc constant, enum alignment, GetAllRsrcIDForRsrc case) are correct and consistent with the existing pattern.

[tenki-reviewer]

🟠 Platform-users lose /api/v1/flows access when filtering by network_id (security)

The PR removes TrafficFlow from the IS_GLOBAL_ACCESS trigger in controllers/middleware.go. Previously the condition had || r.Header.Get("TARGET_RSRC") == schema.TrafficFlow.String() as an unconditional OR clause (not gated on NET_ID == ""), so any request to /api/v1/flows always set IS_GLOBAL_ACCESS=yes and was handled by GlobalPermissionsCheck, which grants PlatformUser a fast-path return nil for that route.

With the new code, when a caller supplies ?network_id=<id>, the newly added network_id query-param block (lines 35-37 of controllers/middleware.go) sets the NET_ID header to a non-empty value. Since NET_ID != "", the IS_GLOBAL_ACCESS condition evaluates to false, routing the request to NetworkPermissionsCheck instead. The PlatformUser+GET fast-path in NetworkPermissionsCheck was also removed in this PR, so platform-users are denied unless they hold an explicit network role with traffic_flow read permission.

A platform-user who could previously list flow logs filtered to a specific network (GET /api/v1/flows?network_id=net1) will now receive 403 Forbidden in deployments that do not configure matching network roles.

💡 Suggestion: If platform-users should retain read access to /api/v1/flows (including when filtered by network_id), either restore TrafficFlow as an unconditional IS_GLOBAL_ACCESS trigger (so it always routes to GlobalPermissionsCheck), or add an explicit fast-path in NetworkPermissionsCheck for TrafficFlow+GET+PlatformUser. If this is an intentional tightening of access, document the breaking change in release notes.

📋 Prompt for AI Agents

In pro/logic/security.go, in the NetworkPermissionsCheck function, after the if netID == "" check (around line 83), add a guard that allows platform-users to read TrafficFlow resources: if userRole.ID == schema.PlatformUser && r.Method == http.MethodGet && targetRsrc == schema.TrafficFlow.String() { return nil }. This restores the implicit read permission that platform-users had before the blanket PlatformUser+GET fast-path was removed. Alternatively, in controllers/middleware.go at lines 141-145, restore TrafficFlow as an unconditional global trigger by adding || r.Header.Get("TARGET_RSRC") == schema.TrafficFlow.String() back as an OR clause outside the NET_ID guard, so TrafficFlow requests always route to GlobalPermissionsCheck where the /api/v1/flows fast-path for PlatformUser already exists.