From bc9c31bdaa0c70e423edf1395db5adbf7e7dc771 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?David=20Sedl=C3=A1=C4=8Dek?= <david.littlefarmer@gmail.com>
Date: Thu, 24 Oct 2024 16:04:23 +0200
Subject: [PATCH] VerifyACL and merge acl.Get and ParseRequest (#4)

---
 common.go      | 77 ++++++++++++++++++++------------------------------
 common_test.go | 74 +++++++++++++++++++++++++++++++++++-------------
 middleware.go  | 12 ++------
 3 files changed, 87 insertions(+), 76 deletions(-)

diff --git a/common.go b/common.go
index 8696666..943d2d2 100644
--- a/common.go
+++ b/common.go
@@ -4,8 +4,8 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
-	"reflect"
 	"strings"
 
 	"github.com/0xsequence/authcontrol/proto"
@@ -37,44 +37,47 @@ type UserStore interface {
 type Config[T any] map[string]map[string]T
 
 // Get returns the config value for the given request.
-func (c Config[T]) Get(r *WebRPCRequest) (v T, ok bool) {
+func (c Config[T]) Get(path string) (*T, error) {
 	if c == nil {
-		return v, false
+		return nil, fmt.Errorf("cofig is nil")
 	}
 
-	methodCfg, ok := c[r.ServiceName][r.MethodName]
-	if !ok {
-		return v, false
+	p := strings.Split(path, "/")
+	if len(p) < 4 {
+		return nil, fmt.Errorf("path has not enough parts")
 	}
 
-	return methodCfg, true
-}
+	var (
+		packageName = p[len(p)-3]
+		serviceName = p[len(p)-2]
+		methodName  = p[len(p)-1]
+	)
 
-// WebRPCRequest is a parsed RPC request.
-type WebRPCRequest struct {
-	PackageName string
-	ServiceName string
-	MethodName  string
-}
-
-// newRequest parses a path into an rcpRequest.
-func ParseRequest(path string) *WebRPCRequest {
-	p := strings.Split(path, "/")
-	if len(p) < 4 {
-		return nil
+	if packageName != "rpc" {
+		return nil, fmt.Errorf("path doesn't include rpc")
 	}
 
-	r := &WebRPCRequest{
-		PackageName: p[len(p)-3],
-		ServiceName: p[len(p)-2],
-		MethodName:  p[len(p)-1],
+	methodCfg, ok := c[serviceName][methodName]
+	if !ok {
+		return nil, fmt.Errorf("acl not found")
 	}
 
-	if r.PackageName != "rpc" {
-		return nil
+	return &methodCfg, nil
+}
+
+// VerifyACL checks that the given ACL config is valid for the given service.
+// It can be used in unit tests to ensure that all methods are covered.
+func (acl Config[any]) VerifyACL(webrpcServices map[string][]string) error {
+	var errList []error
+	for service, methods := range webrpcServices {
+		for _, method := range methods {
+			if _, ok := acl[service][method]; !ok {
+				errList = append(errList, fmt.Errorf("%s.%s not found", service, method))
+			}
+		}
 	}
 
-	return r
+	return errors.Join(errList...)
 }
 
 // ACL is a list of session types, encoded as a bitfield.
@@ -102,23 +105,3 @@ func (a ACL) And(session ...proto.SessionType) ACL {
 func (t ACL) Includes(session proto.SessionType) bool {
 	return t&ACL(1<<session) != 0
 }
-
-// VerifyACL checks that the given ACL config is valid for the given service.
-// It can be used in unit tests to ensure that all methods are covered.
-func VerifyACL[T any](acl Config[ACL]) error {
-	var t T
-	iType := reflect.TypeOf(&t).Elem()
-	service := iType.Name()
-	m, ok := acl[service]
-	if !ok {
-		return errors.New("service " + service + " not found")
-	}
-	var errList []error
-	for i := 0; i < iType.NumMethod(); i++ {
-		method := iType.Method(i)
-		if _, ok := m[method.Name]; !ok {
-			errList = append(errList, errors.New(""+service+"."+method.Name+" not found"))
-		}
-	}
-	return errors.Join(errList...)
-}
diff --git a/common_test.go b/common_test.go
index f75b46b..cc5aa91 100644
--- a/common_test.go
+++ b/common_test.go
@@ -3,11 +3,13 @@ package authcontrol_test
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"net/http"
 	"net/http/httptest"
 	"testing"
 
 	"github.com/go-chi/jwtauth/v5"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/0xsequence/authcontrol"
@@ -58,38 +60,70 @@ func executeRequest(t *testing.T, ctx context.Context, handler http.Handler, pat
 }
 
 func TestVerifyACL(t *testing.T) {
-	type Service interface {
-		Method1() error
-		Method2() error
-		Method3() error
+	services := map[string][]string{
+		"Service1": {
+			"Method1",
+			"Method2",
+			"Method3",
+		},
+		"Service2": {
+			"Method1",
+		},
 	}
 
-	err := authcontrol.VerifyACL[Service](nil)
-	require.Error(t, err)
-
-	err = authcontrol.VerifyACL[Service](authcontrol.Config[authcontrol.ACL]{
-		"WrongName": {
+	// Valid ACL config
+	acl := authcontrol.Config[any]{
+		"Service1": {
 			"Method1": authcontrol.NewACL(proto.SessionType_User),
 			"Method2": authcontrol.NewACL(proto.SessionType_User),
 			"Method3": authcontrol.NewACL(proto.SessionType_User),
 		},
-	})
-	require.Error(t, err)
+		"Service2": {
+			"Method1": authcontrol.NewACL(proto.SessionType_User),
+		},
+	}
+
+	err := acl.VerifyACL(services)
+	assert.NoError(t, err)
 
-	err = authcontrol.VerifyACL[Service](authcontrol.Config[authcontrol.ACL]{
-		"Service": {
+	// Wrong Service
+	acl = authcontrol.Config[any]{
+		"WrongService1": {
 			"Method1": authcontrol.NewACL(proto.SessionType_User),
 			"Method2": authcontrol.NewACL(proto.SessionType_User),
+			"Method3": authcontrol.NewACL(proto.SessionType_User),
 		},
-	})
+		"Service2": {
+			"Method1": authcontrol.NewACL(proto.SessionType_User),
+		},
+	}
+
+	err = acl.VerifyACL(services)
 	require.Error(t, err)
 
-	err = authcontrol.VerifyACL[Service](authcontrol.Config[authcontrol.ACL]{
-		"Service": {
+	expectedErrors := []error{
+		errors.New("Service1.Method1 not found"),
+		errors.New("Service1.Method2 not found"),
+		errors.New("Service1.Method3 not found"),
+	}
+	assert.Equal(t, errors.Join(expectedErrors...).Error(), err.Error())
+
+	// Wrong Methods
+	acl = authcontrol.Config[any]{
+		"Service1": {
 			"Method1": authcontrol.NewACL(proto.SessionType_User),
-			"Method2": authcontrol.NewACL(proto.SessionType_User),
-			"Method3": authcontrol.NewACL(proto.SessionType_User),
 		},
-	})
-	require.NoError(t, err)
+		"Service2": {
+			"Method1": authcontrol.NewACL(proto.SessionType_User),
+		},
+	}
+
+	err = acl.VerifyACL(services)
+	require.Error(t, err)
+
+	expectedErrors = []error{
+		errors.New("Service1.Method2 not found"),
+		errors.New("Service1.Method3 not found"),
+	}
+	assert.Equal(t, errors.Join(expectedErrors...).Error(), err.Error())
 }
diff --git a/middleware.go b/middleware.go
index 01ae8b0..66f9918 100644
--- a/middleware.go
+++ b/middleware.go
@@ -129,15 +129,9 @@ func AccessControl(acl Config[ACL], o *Options) func(next http.Handler) http.Han
 
 	return func(next http.Handler) http.Handler {
 		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-			req := ParseRequest(r.URL.Path)
-			if req == nil {
-				eh(r, w, proto.ErrUnauthorized.WithCausef("invalid rpc method"))
-				return
-			}
-
-			acl, ok := acl.Get(req)
-			if !ok {
-				eh(r, w, proto.ErrUnauthorized.WithCausef("rpc method not found"))
+			acl, err := acl.Get(r.URL.Path)
+			if err != nil {
+				eh(r, w, proto.ErrUnauthorized.WithCausef("get acl: %w", err))
 				return
 			}