EnableRBAC fails open: unusable RBAC config silently disables authorization instead of stopping the app
- Dominant language
- Go
- Stars
- 20.9k
- Forks
- 1.8k
- Avg merge
- 5d 18h
- Merged PRs (30d)
- 39
Description
### Description
`App.EnableRBAC` **fails open**: when the RBAC config cannot be loaded (file missing, unreadable, malformed YAML/JSON, or failing validation), it logs an error and `return`s without installing the middleware. The application then starts and serves **every route with no authorization**, and the caller has no way to detect this because `EnableRBAC` returns `void`.
For an authorization layer this is a dangerous default: a misconfiguration silently disables access control rather than stopping the app.
https://github.com/gofr-dev/gofr/blob/main/pkg/gofr/rbac.go#L42-L53
```go
// Load configuration directly with dependencies
config, err := rbac.LoadPermissions(path, logger, metrics, tracer)
if err != nil {
a.Logger().Errorf("Failed to load RBAC config: %v", err)
return // <-- no middleware installed; app serves ungated
}
a.Logger().Infof("Loaded RBAC config successfully")
middlewareFunc := rbac.Middleware(config)
a.UseMiddleware(middlewareFunc)
```
### Impact
We hit this in production. Our container image placed `rbac.yaml` at a path the running process didn't resolve; GoFr logged `Failed to load RBAC config: ...` and then served **all** routes with no role checks. An integration test later demonstrated the effect concretely: a `salesman`-role token successfully called an admin-only endpoint. The single log line was easy to miss amid startup output, and nothing in the API surface signals that RBAC is off.
### Expected behavior
A security middleware should **fail closed**. When configured but unable to load its policy, `EnableRBAC` should stop the application (e.g. `Fatalf`) or return an `error` so the caller can decide — not start ungated. Options, roughly in order of preference:
1. **Return an `error`** — `EnableRBAC(configPath ...string) error`. Most flexible; lets the caller `log.Fatal` or handle it. (API change.)
2. **`Fatalf` on load failure** — treat an unusable authz config like any other fatal boot dependency. (No signature change; safest default.)
3. At minimum, make the fail-open path opt-in and loud (a distinct `WARN`/`ERROR` that authorization is DISABLED), never the silent default.
### Steps to reproduce
1. `app.EnableRBAC("does-not-exist.yaml")` (or point it at a malformed file).
2. Start the app and hit any route that should require a permission.
3. Observe a single `Failed to load RBAC config` log line, then the request succeeds with no role/permission check.
### Version
- `gofr.dev v1.56.1` (observed), and the same code is on `main` (v1.58.0) — `pkg/gofr/rbac.go`, `EnableRBAC`.
Contributor guide
Research direction
Read pkg/gofr/rbac.go at App.EnableRBAC and reproduce the failure with a missing or malformed RBAC config. Check how the application exposes startup errors and how RBAC middleware is installed. Done means an unusable configured policy cannot leave the application serving ungated routes, with the chosen failure behavior covered by tests.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go
- Domain
- authorization, backend, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 58/100