The Only Admin Module Without a Write Guard
An authorization audit found seven of eight admin modules guarded. The last one closed by reusing the pattern already in the codebase.
TL;DR
The KotaPortal service portal project had an admin module without a scope check. The fix reused a guard pattern from neighboring modules. The handler now passes middleware into the service with a two-function guard.
The audit table of the KotaPortal service portal project showed one uncomfortable row: of eight admin modules, seven already carried a per-scope write guard and one did not. The service types module, where service types are created, changed and deleted, wrote without any scope check at all. The fix landed as commit 7f2df04.
The first instinct was to write the guard from scratch: read the context, take the scope, match it against the permission list, throw an error. The neighbouring modules already carried that pattern, down to a comment naming it as parity with rentalpackage.ErrForbidden and mediaadmin.ErrForbidden. OWASP compresses the rule: implement access control once and reuse it throughout the application [1], because an API missing access controls for POST, PUT and DELETE is still among the most frequent findings [1]. Authorization itself is defined as the process of verifying that a requested action or service is approved for a specific entity [2], and the most common weakness is horizontal escalation, where an authenticated user reaches another user's resources [2].
Reuse the guard pattern from the modules that already have it
The handler now passes middleware.BidangFrom(r.Context()) into the service, and the guard itself is two short functions:
func guardTypeScope(bidang, module string) error {
if canManage(bidang, module) {
return nil
}
return fmt.Errorf("%w: module di luar scope bidang Anda", ErrForbidden)
}
func canManage(bidang, module string) bool {
if module == "" {
return scope.IsGlobal(bidang)
}
return scope.In(scope.ModulesFor(bidang), module)
}The first two lines of canManage hold the whole decision. A service type without a category is stored with a category_id of NULL and counts as cross-scope, so only the global actor (ALL) may write it. An empty module never means everyone may; it means no ordinary scope passes. The second line closes the opposite case: an unknown scope produces an empty module list, and scope.In over an empty list is always false, so nothing passes. Deny by default, exactly as OWASP words it [1].
The category-to-module mapping comes from a single JOIN query in the repository. A category with no match reads as NULL on the Module *string pointer, so a cross-scope row looks like ordinary data instead of a special case. Before any insert, Create resolves the category module first and then calls the guard; Update and Delete call the guard before a row is touched.
Keep reads open, tighten the write path
The service type list deliberately stays open. The public form and the admin grid both need to see every active type, while the write path is what gets guarded. The ?module= parameter is only a narrowing: when its value falls outside the caller's scope or is unknown, the handler returns an empty page without touching the database.
That decision produced a contract finding. The OpenAPI specification promised a 400 response for module values outside the enum, while the code answered 200 with an empty list. The fix went into the specification rather than the code: the parameter description now states that out-of-scope or unknown values are answered with an empty list. A contract should describe the behaviour that is actually tested, not the behaviour that was hoped for.
The write side is guarded twice. The guard rejects before a row is touched, and the HTTP answer is kept as short as possible. RFC 9110 states that 403 means the server understood the request but refuses to fulfil it, and a server may describe the reason in the response content if it wishes to make it public [3]. The handler answers with one fixed sentence that never names the module just resolved nor the caller's scope. The detail stays in the internal error for debugging; what reaches the user is only the generic sentence.
Add the trace without changing the function signature
The audit trail landed in the same commit, but the old constructor must not break. The solution is a variadic parameter, auditLoggers ...*audit.Logger: the final parameter carries the three-dot prefix, so the function can still be called with no argument for it [4], and when no argument is passed the value is nil [4]. At the other end, Log checks whether the receiver is empty and returns immediately, long before any database call, so an unconfigured audit trail is a no-op rather than a panic. One wiring line in main.go adds the logger, while the old test helper still calls the single-argument form unchanged.
The tests that decide the matter are the new ones: TestServiceTypeScopedWriteForbidden, TestServiceTypeLintasAllOnly, and TestServiceTypeListNarrowing. They need data that is deliberately out of reach, so an extra seed category of a different module is prepared for them. Run the service type integration suite; the expected result is that every test passes, including those three scope tests.
What changed after the audit was not the number of guards but the way modules get added. A new admin module now arrives with a row that has to be filled in the audit matrix, and the empty row is visible before its code is written.
Sources: