From 7a7e73dda8cf32681cc31bedf64055f0094a2e21 Mon Sep 17 00:00:00 2001
From: Ayman Bagabas <ayman.bagabas@gmail.com>
Date: Thu, 17 Nov 2022 17:24:30 -0500
Subject: [PATCH] fix(auth): wrong auth for anon users

This fixes an issue with using anon-access with registered users.

Fixes: d88ccb97d3e7 ("ref(config): clarify repo auth for key")
---
 config/auth.go      | 86 ++++++++++++++++++++++++++-------------------
 config/auth_test.go | 61 +++++++++++++++++++++++++++++++-
 2 files changed, 109 insertions(+), 38 deletions(-)

diff --git a/config/auth.go b/config/auth.go
index ff1d3c056..30e8d6719 100644
--- a/config/auth.go
+++ b/config/auth.go
@@ -81,10 +81,8 @@ func (cfg *Config) anonAccessLevel() gm.AccessLevel {
 // If repo exists, and private, then admins and collabs are allowed access.
 // If repo exists, and not private, then access is based on config.AnonAccess.
 func (cfg *Config) accessForKey(repo string, pk ssh.PublicKey) gm.AccessLevel {
-	var u *User
-	var r *RepoConfig
 	anon := cfg.anonAccessLevel()
-OUT:
+	private := cfg.isPrivate(repo)
 	// Find user
 	for _, user := range cfg.Users {
 		for _, k := range user.PublicKeys {
@@ -94,49 +92,63 @@ OUT:
 				return gm.NoAccess
 			}
 			if ssh.KeysEqual(pk, apk) {
-				us := user
-				u = &us
-				break OUT
+				if user.Admin {
+					return gm.AdminAccess
+				}
+				u := user
+				if cfg.isCollab(repo, &u) {
+					if anon > gm.ReadWriteAccess {
+						return anon
+					}
+					return gm.ReadWriteAccess
+				}
+				if !private {
+					if anon > gm.ReadOnlyAccess {
+						return anon
+					}
+					return gm.ReadOnlyAccess
+				}
 			}
 		}
 	}
-	// Find repo
-	for _, rp := range cfg.Repos {
-		if rp.Repo == repo {
-			rr := rp
-			r = &rr
-			break
-		}
+	// Don't restrict access to private repos if no users are configured.
+	// Return anon access level.
+	if private && len(cfg.Users) > 0 {
+		return gm.NoAccess
 	}
-	if u != nil && u.Admin {
-		return gm.AdminAccess
+	return anon
+}
+
+func (cfg *Config) findRepo(repo string) *RepoConfig {
+	for _, r := range cfg.Repos {
+		if r.Repo == repo {
+			return &r
+		}
 	}
-	if r == nil || len(cfg.Users) == 0 {
-		return anon
+	return nil
+}
+
+func (cfg *Config) isPrivate(repo string) bool {
+	if r := cfg.findRepo(repo); r != nil {
+		return r.Private
 	}
-	// Collabs default access is read-write
-	if u != nil {
-		ac := gm.ReadWriteAccess
-		if anon > ac {
-			ac = anon
-		}
-		for _, c := range r.Collabs {
-			if c == u.Name {
-				return ac
+	return false
+}
+
+func (cfg *Config) isCollab(repo string, user *User) bool {
+	if user != nil {
+		for _, r := range user.CollabRepos {
+			if r == repo {
+				return true
 			}
 		}
-		for _, rr := range u.CollabRepos {
-			if rr == r.Repo {
-				return ac
+		if r := cfg.findRepo(repo); r != nil {
+			for _, c := range r.Collabs {
+				if c == user.Name {
+					return true
+				}
 			}
 		}
 	}
-	// Users default access is read-only
-	if !r.Private {
-		if anon > gm.ReadOnlyAccess {
-			return anon
-		}
-		return gm.ReadOnlyAccess
-	}
-	return gm.NoAccess
+	return false
 }
diff --git a/config/auth_test.go b/config/auth_test.go
index 7e59b59b3..4f48bafb4 100644
--- a/config/auth_test.go
+++ b/config/auth_test.go
@@ -34,6 +34,27 @@ func TestAuth(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:   "anon access: no-access, anonymous user with admin user",
+			access: git.NoAccess,
+			repo:   "foo",
+			cfg: Config{
+				AnonAccess: "no-access",
+				Repos: []RepoConfig{
+					{
+						Repo: "foo",
+					},
+				},
+				Users: []User{
+					{
+						Admin: true,
+						PublicKeys: []string{
+							adminKey,
+						},
+					},
+				},
+			},
+		},
 		{
 			name:   "anon access: no-access, authd user",
 			key:    dummyPk,
@@ -55,6 +76,28 @@ func TestAuth(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:   "anon access: no-access, anonymous user with admin user",
+			key:    dummyPk,
+			repo:   "foo",
+			access: git.NoAccess,
+			cfg: Config{
+				AnonAccess: "no-access",
+				Repos: []RepoConfig{
+					{
+						Repo: "foo",
+					},
+				},
+				Users: []User{
+					{
+						Admin: true,
+						PublicKeys: []string{
+							adminKey,
+						},
+					},
+				},
+			},
+		},
 		{
 			name:   "anon access: no-access, admin user",
 			repo:   "foo",
@@ -429,7 +472,7 @@ func TestAuth(t *testing.T) {
 			name:   "anon access: no-access, authd user, new repo",
 			key:    dummyPk,
 			repo:   "foo",
-			access: git.NoAccess,
+			access: git.ReadOnlyAccess,
 			cfg: Config{
 				AnonAccess: "no-access",
 				Users: []User{
@@ -441,6 +484,22 @@ func TestAuth(t *testing.T) {
 				},
 			},
 		},
+		{
+			name:   "anon access: no-access, authd user, new repo, with user",
+			key:    dummyPk,
+			repo:   "foo",
+			access: git.NoAccess,
+			cfg: Config{
+				AnonAccess: "no-access",
+				Users: []User{
+					{
+						PublicKeys: []string{
+							adminKey,
+						},
+					},
+				},
+			},
+		},
 		{
 			name:   "anon access: no-access, admin user, new repo",
 			repo:   "foo",