-
Notifications
You must be signed in to change notification settings - Fork 1.7k
RBAC middleware support #2144
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: development
Are you sure you want to change the base?
RBAC middleware support #2144
Changes from all commits
9170451
a123a03
e59fd18
993dd2e
23073f7
0a0334f
2cb75bf
21b6309
c57a2e1
0f9226d
03b7ab9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
{ | ||
"route": { | ||
"*": ["admin"], | ||
"/post/*": ["admin","editor"], | ||
"/dashboard": ["admin","editor"], | ||
"/profile": ["admin","editor","user"], | ||
"/home":["admin","editor","user"], | ||
"/sayhello/*":["admin","editor","user"], | ||
"/greet":["admin","editor","user"] | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package main | ||
|
||
import ( | ||
"net/http" | ||
|
||
"gofr.dev/pkg/gofr" | ||
"gofr.dev/pkg/gofr/rbac" | ||
) | ||
|
||
func main() { | ||
app := gofr.New() | ||
|
||
// loading the rbac config file which is required | ||
rbacConfigs, err := rbac.LoadPermissions("config.json") | ||
if err != nil { | ||
return | ||
} | ||
|
||
// example of setting override for a specific role | ||
overrides := map[string]bool{"/greet": true} | ||
rbacConfigs.OverRides = overrides | ||
|
||
// setting the role extractor function | ||
rbacConfigs.RoleExtractorFunc = extractor | ||
|
||
// applying the middleware | ||
app.UseMiddleware(rbac.Middleware(rbacConfigs)) | ||
|
||
// sample routes | ||
app.GET("/sayhello/321", handler) | ||
app.GET("/greet", rbac.RequireRole("user1", handler)) | ||
|
||
app.Run() // listens and serves on localhost:8000 | ||
} | ||
|
||
func extractor(req *http.Request, _ ...any) (string, error) { | ||
return req.Header.Get("X-USER-ROLE"), nil | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. User role passed in the header cannot be trusted as is. While this may have been added as an example only, it'd be better to have a proper example. |
||
} | ||
|
||
func handler(ctx *gofr.Context) (any, error) { | ||
return "Hello World!", nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package rbac | ||
|
||
import ( | ||
"encoding/json" | ||
"net/http" | ||
"os" | ||
) | ||
|
||
type Config struct { | ||
RouteWithPermissions map[string][]string `json:"route"` // route: [Allowed roles] | ||
RoleExtractorFunc func(req *http.Request, args ...any) (string, error) | ||
OverRides map[string]bool // route: [override bool] | ||
} | ||
|
||
func LoadPermissions(path string) (*Config, error) { | ||
data, err := os.ReadFile(path) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
var config Config | ||
|
||
if err := json.Unmarshal(data, &config); err != nil { | ||
return nil, err | ||
} | ||
|
||
return &config, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package rbac | ||
|
||
import ( | ||
"os" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestLoadPermissions_Success(t *testing.T) { | ||
jsonContent := `{ | ||
"route": {"admin":["read", "write"], "user":["read"]}, | ||
"OverRides": {"admin":true, "user":false} | ||
}` | ||
tempFile, err := os.CreateTemp("", "test_permissions_*.json") | ||
assert.NoError(t, err) | ||
defer os.Remove(tempFile.Name()) | ||
|
||
_, err = tempFile.Write([]byte(jsonContent)) | ||
assert.NoError(t, err) | ||
tempFile.Close() | ||
|
||
cfg, err := LoadPermissions(tempFile.Name()) | ||
assert.NoError(t, err) | ||
assert.Equal(t, map[string][]string{"admin": {"read", "write"}, "user": {"read"}}, cfg.RouteWithPermissions) | ||
assert.Equal(t, map[string]bool{"admin": true, "user": false}, cfg.OverRides) | ||
} | ||
|
||
func TestLoadPermissions_FileNotFound(t *testing.T) { | ||
cfg, err := LoadPermissions("non_existent_file.json") | ||
assert.Nil(t, cfg) | ||
assert.Error(t, err) | ||
} | ||
|
||
func TestLoadPermissions_InvalidJSON(t *testing.T) { | ||
tempFile, err := os.CreateTemp("", "badjson_*.json") | ||
assert.NoError(t, err) | ||
defer os.Remove(tempFile.Name()) | ||
|
||
_, err = tempFile.Write([]byte(`{"route": [INVALID JSON}`)) | ||
assert.NoError(t, err) | ||
tempFile.Close() | ||
|
||
cfg, err := LoadPermissions(tempFile.Name()) | ||
assert.Nil(t, cfg) | ||
assert.Error(t, err) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package rbac | ||
|
||
import "gofr.dev/pkg/gofr" | ||
|
||
func HasRole(ctx *gofr.Context, role string) bool { | ||
expRole, _ := ctx.Context.Value(userRole).(string) | ||
return expRole == role | ||
} | ||
|
||
func GetUserRole(ctx *gofr.Context) string { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Errors should not be silently ignored There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it returns a bool, so have ignored it |
||
role, _ := ctx.Context.Value(userRole).(string) | ||
return role | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
package rbac | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
"gofr.dev/pkg/gofr" | ||
) | ||
|
||
func TestHasRole(t *testing.T) { | ||
tests := []struct { | ||
name string | ||
ctxRoleVal string | ||
checkRole string | ||
expectedRes bool | ||
}{ | ||
{"matching role", "admin", "admin", true}, | ||
{"non-matching role", "viewer", "admin", false}, | ||
{"empty role in context", "", "admin", false}, | ||
{"nil role in context", "", "", true}, | ||
} | ||
|
||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
// Create base context with the userRole value | ||
baseCtx := context.WithValue(t.Context(), userRole, tt.ctxRoleVal) | ||
|
||
// Wrap baseCtx in gofr.Context | ||
gofrCtx := &gofr.Context{Context: baseCtx} | ||
|
||
got := HasRole(gofrCtx, tt.checkRole) | ||
if got != tt.expectedRes { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using assert.Equal/Equalf to make it more concise |
||
t.Errorf("HasRole() = %v, want %v", got, tt.expectedRes) | ||
} | ||
}) | ||
} | ||
} | ||
func TestGetUserRole(t *testing.T) { | ||
expectedRole := "editor" | ||
baseCtx := context.WithValue(t.Context(), userRole, expectedRole) | ||
gofrCtx := &gofr.Context{Context: baseCtx} | ||
|
||
if role := GetUserRole(gofrCtx); role != expectedRole { | ||
t.Errorf("GetUserRole() = %v, want %v", role, expectedRole) | ||
} | ||
|
||
// Test no role set should return "" | ||
emptyCtx := &gofr.Context{Context: t.Context()} | ||
if role := GetUserRole(emptyCtx); role != "" { | ||
t.Errorf("GetUserRole() with no role = %v, want empty string", role) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
package rbac | ||
|
||
import ( | ||
"path" | ||
) | ||
|
||
func isRoleAllowed(role, apiroute string, config *Config) bool { | ||
var routePermissions []string | ||
|
||
// find the matched route from config | ||
for route, allowedRoles := range config.RouteWithPermissions { | ||
if isMatched, _ := path.Match(route, apiroute); isMatched && route != "" { | ||
// check if override is set for the matched route | ||
if config.OverRides[apiroute] { | ||
return true | ||
} | ||
routePermissions = allowedRoles | ||
break | ||
} | ||
} | ||
|
||
// append global permissions if any | ||
routePermissions = append(routePermissions, config.RouteWithPermissions["*"]...) | ||
|
||
// check if role is in allowed roles for the matched route | ||
for _, allowedRole := range routePermissions { | ||
if allowedRole == role || allowedRole == "*" { | ||
return true | ||
} | ||
} | ||
|
||
return false | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,42 @@ | ||
package rbac | ||
|
||
import ( | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestIsRoleAllowed(t *testing.T) { | ||
config := &Config{ | ||
RouteWithPermissions: map[string][]string{ | ||
"/admin/*": {"admin"}, | ||
"/user/*": {"user", "admin"}, | ||
"*": {"guest"}, | ||
}, | ||
OverRides: map[string]bool{ | ||
"/admin/home": true, | ||
}, | ||
} | ||
|
||
tests := []struct { | ||
name string | ||
role string | ||
route string | ||
expected bool | ||
}{ | ||
{"Override true", "anyone", "/admin/home", true}, | ||
{"Pattern match /admin/*", "admin", "/admin/dashboard", true}, | ||
{"Pattern match negative", "user", "/admin/dashboard", false}, | ||
{"Non-pattern route", "user", "/user/profile", true}, | ||
{"Wildcard permission", "guest", "/anything", true}, | ||
{"No route or global match", "unknown", "/private", false}, | ||
{"Not matched or globally allowed", "nobody", "/wildcard", false}, | ||
} | ||
|
||
for _, tc := range tests { | ||
t.Run(tc.name, func(t *testing.T) { | ||
got := isRoleAllowed(tc.role, tc.route, config) | ||
assert.Equal(t, tc.expected, got, tc.name) | ||
}) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
package rbac | ||
|
||
import ( | ||
"context" | ||
"errors" | ||
"net/http" | ||
|
||
"gofr.dev/pkg/gofr" | ||
) | ||
|
||
type authMethod int | ||
|
||
const userRole authMethod = 4 | ||
|
||
var ErrAccessDenied = errors.New("forbidden: access denied") | ||
|
||
func Middleware(config *Config, args ...any) func(handler http.Handler) http.Handler { | ||
return func(handler http.Handler) http.Handler { | ||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | ||
role, err := config.RoleExtractorFunc(r, args) | ||
if err != nil { | ||
http.Error(w, "Unauthorized: Missing or invalid role", http.StatusUnauthorized) | ||
|
||
return | ||
} | ||
|
||
if !isRoleAllowed(role, r.URL.Path, config) { | ||
http.Error(w, "Forbidden: Access denied", http.StatusForbidden) | ||
|
||
return | ||
} | ||
|
||
ctx := context.WithValue(r.Context(), userRole, role) | ||
|
||
handler.ServeHTTP(w, r.WithContext(ctx)) | ||
}) | ||
} | ||
} | ||
|
||
func RequireRole(allowedRole string, handlerFunc gofr.Handler) gofr.Handler { | ||
return func(ctx *gofr.Context) (any, error) { | ||
role, _ := ctx.Context.Value(userRole).(string) | ||
|
||
if role == allowedRole { | ||
return handlerFunc(ctx) | ||
} | ||
|
||
return nil, ErrAccessDenied | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is defeating the purpose of middleware if every route needs to add this RequireRole
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
requireRole is not needed for every route, can be directly used as app.GET("/sayhello/123", handler)