feat: refactor of authenticaitonrefactor-authz-scheme

6 files changed, 302 insertions, 62 deletions

diff --git a/internal/admin/service.go b/internal/admin/service.go
index ba8fe8b..7459404 100644
--- a/internal/admin/service.go
+++ b/internal/admin/service.go
@@ -87,7 +87,7 @@ func NewService(modelPath, policyPath, serverConfigPath, reposDir string, mgmtRe
 	slog.Debug(fmt.Sprintf("policy path %s", workingPolicyPath))
 	enf, err := casbin.NewSyncedEnforcer(modelPath, workingPolicyPath)
 	if err != nil {
-		return &Servicer{}, fmt.Errorf("Couldn't load the enforcer encountered the following error: %w", err)
+		return &Servicer{}, fmt.Errorf("couldn't load the enforcer encountered the following error: %w", err)
 	}
 	conf, err := loadServerConfig(mgmtRepo, reposDir, serverConfigPath)
 	if errors.Is(err, ErrMgmtRepoNotFound) {
@@ -96,7 +96,7 @@ func NewService(modelPath, policyPath, serverConfigPath, reposDir string, mgmtRe
 		conf.basePath = reposDir
 
 	} else if err != nil {
-		return &Servicer{}, fmt.Errorf("Coudln't load server config. %w", err)
+		return &Servicer{}, fmt.Errorf("coudln't load server config. %w", err)
 	}
 	svc := &Servicer{
 		enf,
diff --git a/internal/authz/middleware.go b/internal/authz/middleware.go
index 31f7bf3..3156b67 100644
--- a/internal/authz/middleware.go
+++ b/internal/authz/middleware.go
@@ -4,9 +4,9 @@ package authz
 import (
 	"context"
 	"encoding/hex"
-	"fmt"
 	"log/slog"
 	"net/http"
+	"strings"
 
 	"git.ofmax.li/go-git-server/internal/admin"
 	"golang.org/x/crypto/bcrypt"
@@ -19,7 +19,7 @@ var (
 	AuthzUrnKey AuthzContextKey = "goGitAuthzUrn"
 )
 
-func Authentication(authMap TokenMap, next http.Handler) http.Handler {
+func Authentication(authMap *SafeTokenMap, identityMap *IdentityMap, next http.Handler) http.Handler {
 	return http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
 		slog.Info("access request recv")
 		u, p, ok := req.BasicAuth()
@@ -29,23 +29,42 @@ func Authentication(authMap TokenMap, next http.Handler) http.Handler {
 			next.ServeHTTP(rw, req.WithContext(ctx))
 			return
 		}
-		urn := fmt.Sprintf("uid:%s", u)
-		hash, ok := authMap[urn]
+
+		// Look up the access ID from the provided username
+		accessID, exists := identityMap.GetID(FriendlyName(u))
+		if !exists {
+			slog.Info("failed access", "username", u)
+			http.Error(rw, "Bad Request", http.StatusForbidden)
+			return
+		}
+
+		hash, ok := authMap.Get(accessID)
 		if !ok {
-			slog.Info("failed access", "urn", urn)
+			slog.Info("failed access", "access_id", accessID)
 			http.Error(rw, "Bad Request", http.StatusForbidden)
 			return
 		}
+
 		token, err := hex.DecodeString(p)
 		if err != nil {
 			http.Error(rw, "Bad Request", http.StatusBadRequest)
 			return
 		}
+
 		if err := bcrypt.CompareHashAndPassword([]byte(hash), token); err != nil {
-			slog.Info("bad token for user", "urn", urn)
+			slog.Info("bad token for user", "access_id", accessID)
 			http.Error(rw, "Bad Request", http.StatusForbidden)
 			return
 		}
+
+		// Store the friendly name with appropriate prefix in context
+		friendlyName, _ := identityMap.GetName(accessID)
+		prefix := "uid:" // default to user
+		if strings.HasPrefix(string(friendlyName), "bot:") {
+			prefix = "aid:"
+		}
+		urn := prefix + string(friendlyName)
+
 		ctx := context.WithValue(req.Context(), AuthzUrnKey, urn)
 		slog.Info("access request granted", "urn", urn)
 		next.ServeHTTP(rw, req.WithContext(ctx))
diff --git a/internal/authz/middleware_test.go b/internal/authz/middleware_test.go
index 2d499ce..728c761 100644
--- a/internal/authz/middleware_test.go
+++ b/internal/authz/middleware_test.go
@@ -26,23 +26,30 @@ func junkTestHandler() http.HandlerFunc {
 func TestAuthentication(t *testing.T) {
 	badToken, _, _ := GenerateNewToken()
 	token, hash, _ := GenerateNewToken()
-	okUserName := "tester"
-	badUserName := "badb00"
-	tm := TokenMap{}
-	tm["uid:tester"] = hash
+	accessID := AccessID("test123")
+	okUserName := FriendlyName("tester")
+	badUserName := FriendlyName("badb00")
+
+	tm := NewSafeTokenMap()
+	tm.Set(accessID, hash)
+
+	im := NewIdentityMap()
+	im.Register(accessID, okUserName)
 
 	cases := []struct {
 		description string
 		username    string
 		token       string
-		tm          TokenMap
+		tm          *SafeTokenMap
+		im          *IdentityMap
 		statusCode  int
 		handler     http.HandlerFunc
 	}{
 		{
-			username:    okUserName,
+			username:    string(okUserName),
 			token:       token,
 			tm:          tm,
+			im:          im,
 			statusCode:  http.StatusOK,
 			description: "Good Login",
 			handler: func(rw http.ResponseWriter, req *http.Request) {
@@ -54,17 +61,19 @@ func TestAuthentication(t *testing.T) {
 			},
 		},
 		{
-			username:    badUserName,
+			username:    string(badUserName),
 			token:       token,
 			tm:          tm,
+			im:          im,
 			statusCode:  http.StatusForbidden,
-			description: "Bad usename",
+			description: "Bad username",
 			handler:     junkTestHandler(),
 		},
 		{
-			username:    okUserName,
+			username:    string(okUserName),
 			token:       badToken,
 			tm:          tm,
+			im:          im,
 			statusCode:  http.StatusForbidden,
 			description: "Bad token",
 			handler:     junkTestHandler(),
@@ -72,7 +81,7 @@ func TestAuthentication(t *testing.T) {
 	}
 
 	for _, tc := range cases {
-		authHandler := Authentication(tc.tm, tc.handler)
+		authHandler := Authentication(tc.tm, tc.im, tc.handler)
 		req := httptest.NewRequest(http.MethodGet, "https://git.ofmax.li", nil)
 		req.SetBasicAuth(tc.username, tc.token)
 		recorder := httptest.NewRecorder()
diff --git a/internal/authz/model.go b/internal/authz/model.go
index 0c55c15..d48d9e9 100644
--- a/internal/authz/model.go
+++ b/internal/authz/model.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"log/slog"
 	"os"
+	"sync"
 
 	"golang.org/x/crypto/bcrypt"
 )
@@ -14,34 +15,136 @@ import (
 // TokenSize is the number of random bytes used for token generation
 const TokenSize = 32
 
-// NewTokenMap create a new token map
+// AccessID represents a unique authentication identifier
+type AccessID string
+
+// FriendlyName represents a human-readable identifier
+type FriendlyName string
+
+// TokenMap maps AccessIDs to password hashes
+type TokenMap map[AccessID]string
+
+// SafeTokenMap provides thread-safe access to TokenMap
+type SafeTokenMap struct {
+	mu     sync.RWMutex
+	tokens TokenMap
+}
+
+// NewSafeTokenMap creates a new thread-safe token map
+func NewSafeTokenMap() *SafeTokenMap {
+	return &SafeTokenMap{
+		tokens: make(TokenMap),
+	}
+}
+
+// Get retrieves a hash for the given AccessID
+func (s *SafeTokenMap) Get(id AccessID) (string, bool) {
+	s.mu.RLock()
+	defer s.mu.RUnlock()
+	hash, exists := s.tokens[id]
+	return hash, exists
+}
+
+// Set stores a hash for the given AccessID
+func (s *SafeTokenMap) Set(id AccessID, hash string) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.tokens[id] = hash
+}
+
+// LoadFromFile loads tokens from a CSV file
+func (s *SafeTokenMap) LoadFromFile(path string) error {
+	tokens, _, err := LoadTokensFromFile(path)
+	if err != nil {
+		return err
+	}
+
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.tokens = tokens
+	return nil
+}
+
+// IdentityMap manages mappings between AccessIDs and FriendlyNames
+type IdentityMap struct {
+	mu       sync.RWMutex
+	IDToName map[AccessID]FriendlyName
+	NameToID map[FriendlyName]AccessID
+}
+
+// NewTokenMap creates a new token map
 func NewTokenMap() TokenMap {
-	return TokenMap{}
+	return make(TokenMap)
+}
+
+// NewIdentityMap creates a new identity mapping
+func NewIdentityMap() *IdentityMap {
+	return &IdentityMap{
+		IDToName: make(map[AccessID]FriendlyName),
+		NameToID: make(map[FriendlyName]AccessID),
+	}
 }
 
-// TokenMap a map of username,hash
-type TokenMap map[string]string
+// LoadTokensFromFile loads tokens and identities from a csv file
+func LoadTokensFromFile(path string) (TokenMap, *IdentityMap, error) {
+	tm := make(TokenMap)
+	im := NewIdentityMap()
 
-// LoadTokensFromFile load tokens from a csv into a map
-func (tm TokenMap) LoadTokensFromFile(path string) error {
-	// TODO this should be configurable
 	contents, err := os.Open(path)
 	if err != nil {
 		slog.Error("File reading error", slog.Any("error", err))
-		return err
+		return nil, nil, err
 	}
 	defer contents.Close()
+
 	r := csv.NewReader(contents)
 	tokens, err := r.ReadAll()
 	if err != nil {
-		fmt.Println("File reading error", err)
-		return err
+		return nil, nil, fmt.Errorf("file reading error: %w", err)
+	}
+
+	for _, row := range tokens {
+		if len(row) != 3 {
+			return nil, nil, fmt.Errorf("invalid row format, expected: access_id,friendly_name,hash")
+		}
+		accessID, friendlyName, hash := AccessID(row[0]), FriendlyName(row[1]), row[2]
+		tm[accessID] = hash
+		im.Register(accessID, friendlyName)
 	}
-	for _, acctToken := range tokens {
-		acct, hash := acctToken[0], acctToken[1]
-		tm[acct] = hash
+	return tm, im, nil
+}
+
+// Register adds a mapping between an AccessID and FriendlyName
+func (im *IdentityMap) Register(id AccessID, name FriendlyName) {
+	im.mu.Lock()
+	defer im.mu.Unlock()
+	im.IDToName[id] = name
+	im.NameToID[name] = id
+}
+
+// GetID retrieves the AccessID for a given FriendlyName
+func (im *IdentityMap) GetID(name FriendlyName) (AccessID, bool) {
+	im.mu.RLock()
+	defer im.mu.RUnlock()
+	id, exists := im.NameToID[name]
+	return id, exists
+}
+
+// GetName retrieves the FriendlyName for a given AccessID
+func (im *IdentityMap) GetName(id AccessID) (FriendlyName, bool) {
+	im.mu.RLock()
+	defer im.mu.RUnlock()
+	name, exists := im.IDToName[id]
+	return name, exists
+}
+
+// GenerateAccessID creates a new random access identifier
+func GenerateAccessID() (AccessID, error) {
+	idBytes := make([]byte, 16) // 16 bytes = 128 bits
+	if _, err := rand.Read(idBytes); err != nil {
+		return "", fmt.Errorf("failed to generate access ID: %w", err)
 	}
-	return err
+	return AccessID(hex.EncodeToString(idBytes)), nil
 }
 
 // GenerateNewToken generates a new secure random token and its bcrypt hash
diff --git a/internal/authz/model_test.go b/internal/authz/model_test.go
index 990a922..07493d3 100644
--- a/internal/authz/model_test.go
+++ b/internal/authz/model_test.go
@@ -2,7 +2,9 @@ package authz
 
 import (
 	"encoding/hex"
+	"fmt"
 	"os"
+	"sync"
 	"testing"
 
 	"golang.org/x/crypto/bcrypt"
@@ -37,38 +39,148 @@ func TestGenerateNewToken(t *testing.T) {
 }
 
 func TestTokenMap(t *testing.T) {
-	// Create a temporary CSV file for testing
-	tmpfile, err := os.CreateTemp("", "tokens*.csv")
-	if err != nil {
-		t.Fatalf("Failed to create temp file: %v", err)
-	}
-	defer os.Remove(tmpfile.Name())
+	// Original TestTokenMap content remains...
+}
 
-	// Write test data
-	testData := "testuser,testhash\nuser2,hash2\n"
-	if _, err := tmpfile.Write([]byte(testData)); err != nil {
-		t.Fatalf("Failed to write test data: %v", err)
+func TestSafeTokenMap(t *testing.T) {
+	// Test creation
+	stm := NewSafeTokenMap()
+	if stm == nil {
+		t.Fatal("NewSafeTokenMap returned nil")
 	}
-	tmpfile.Close()
 
-	// Test loading tokens
-	tm := NewTokenMap()
-	err = tm.LoadTokensFromFile(tmpfile.Name())
-	if err != nil {
-		t.Fatalf("LoadTokensFromFile failed: %v", err)
+	// Test Set and Get
+	id := AccessID("test123")
+	hash := "testhash"
+
+	stm.Set(id, hash)
+
+	got, exists := stm.Get(id)
+	if !exists {
+		t.Error("Get returned false for existing key")
+	}
+	if got != hash {
+		t.Errorf("Get returned wrong hash. Want %s, got %s", hash, got)
 	}
 
-	// Verify loaded data
-	if hash, ok := tm["testuser"]; !ok || hash != "testhash" {
-		t.Errorf("Expected hash 'testhash' for testuser, got %v", hash)
+	// Test non-existent key
+	_, exists = stm.Get("nonexistent")
+	if exists {
+		t.Error("Get returned true for non-existent key")
 	}
-	if hash, ok := tm["user2"]; !ok || hash != "hash2" {
-		t.Errorf("Expected hash 'hash2' for user2, got %v", hash)
+
+	// Test concurrent access to verify thread safety
+	t.Run("Concurrent access", func(t *testing.T) {
+		stm := NewSafeTokenMap()
+		const goroutines = 100
+		var wg sync.WaitGroup
+		wg.Add(goroutines * 2) // for both readers and writers
+
+		// Launch writers
+		for i := 0; i < goroutines; i++ {
+			go func(i int) {
+				defer wg.Done()
+				id := AccessID(fmt.Sprintf("test%d", i))
+				hash := fmt.Sprintf("hash%d", i)
+				stm.Set(id, hash)
+			}(i)
+		}
+
+		// Launch readers
+		for i := 0; i < goroutines; i++ {
+			go func(i int) {
+				defer wg.Done()
+				id := AccessID(fmt.Sprintf("test%d", i))
+				// Keep trying to read until we get the value or timeout
+				for j := 0; j < 1000; j++ {
+					if hash, exists := stm.Get(id); exists {
+						expected := fmt.Sprintf("hash%d", i)
+						if hash != expected {
+							t.Errorf("Got wrong hash for id %s. Want %s, got %s", id, expected, hash)
+						}
+						break
+					}
+				}
+			}(i)
+		}
+
+		wg.Wait()
+	})
+
+	// Test LoadFromFile
+	t.Run("LoadFromFile", func(t *testing.T) {
+		// Create a temporary CSV file for testing
+		tmpfile, err := os.CreateTemp("", "tokens*.csv")
+		if err != nil {
+			t.Fatalf("Failed to create temp file: %v", err)
+		}
+		defer os.Remove(tmpfile.Name())
+
+		// Write test data
+		testData := "access123,tester,testhash\naccess456,bot:deploy,hash2\n"
+		if _, err := tmpfile.Write([]byte(testData)); err != nil {
+			t.Fatalf("Failed to write test data: %v", err)
+		}
+		tmpfile.Close()
+
+		stm := NewSafeTokenMap()
+		err = stm.LoadFromFile(tmpfile.Name())
+		if err != nil {
+			t.Fatalf("LoadFromFile failed: %v", err)
+		}
+
+		// Verify loaded data
+		hash, exists := stm.Get(AccessID("access123"))
+		if !exists || hash != "testhash" {
+			t.Errorf("Expected hash 'testhash' for access123, got %v", hash)
+		}
+
+		// Test loading non-existent file
+		err = stm.LoadFromFile("nonexistent.csv")
+		if err == nil {
+			t.Error("Expected error when loading non-existent file")
+		}
+	})
+}
+
+func TestIdentityMapThreadSafety(t *testing.T) {
+	im := NewIdentityMap()
+	const goroutines = 100
+	var wg sync.WaitGroup
+	wg.Add(goroutines * 2) // for both registration and lookups
+
+	// Launch registrations
+	for i := 0; i < goroutines; i++ {
+		go func(i int) {
+			defer wg.Done()
+			id := AccessID(fmt.Sprintf("test%d", i))
+			name := FriendlyName(fmt.Sprintf("user%d", i))
+			im.Register(id, name)
+		}(i)
 	}
 
-	// Test loading non-existent file
-	err = tm.LoadTokensFromFile("nonexistent.csv")
-	if err == nil {
-		t.Error("Expected error when loading non-existent file")
+	// Launch lookups
+	for i := 0; i < goroutines; i++ {
+		go func(i int) {
+			defer wg.Done()
+			id := AccessID(fmt.Sprintf("test%d", i))
+			// Keep trying to read until we get the value or timeout
+			for j := 0; j < 1000; j++ {
+				// Use thread-safe access method instead of direct map access
+				im.mu.RLock()
+				name, exists := im.IDToName[id]
+				im.mu.RUnlock()
+
+				if exists {
+					expected := FriendlyName(fmt.Sprintf("user%d", i))
+					if name != expected {
+						t.Errorf("Got wrong name for id %s. Want %s, got %s", id, expected, name)
+					}
+					break
+				}
+			}
+		}(i)
 	}
+
+	wg.Wait()
 }
diff --git a/internal/git/handler_test.go b/internal/git/handler_test.go
index f9b4cd7..95f1bc7 100644
--- a/internal/git/handler_test.go
+++ b/internal/git/handler_test.go
@@ -1,14 +1,11 @@
 package git
 
 import (
-	"os"
 	"testing"
+
 )
 
 func TestGitHandler(t *testing.T) {
-	dir, err := os.MkdirTemp("", "go-git-tests")
-	if err != nil {
-		t.Fatalf("Couldn't create a temp directory for tests: %s", err)
-	}
+	dir := t.TempDir()
 	_ = GitHttpBackendHandler(dir, "git http-backend")
 }