Skip to content

Commit 5f93476

Browse files
compscidrclaude
andcommitted
Make IsAdmin strict, gate wizard upload on IsWizardMode
Previously IsAdmin returned true when admin_users was empty, so the wizard could upload images during setup. But that branch was reached on every request, not just wizard endpoints — any deployment that provisioned .env from configuration management would skip the wizard, leave admin_users empty, and grant admin authority to anonymous visitors on all routes guarded by IsAdmin. Split the concern: IsAdmin now returns false when no admin user exists. IsWizardMode is a new predicate (true iff admin_users is empty) used only by UploadFile and UpdateSettings — the two endpoints the wizard hits before the admin user is created. Refs #529 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 05100b5 commit 5f93476

5 files changed

Lines changed: 109 additions & 12 deletions

File tree

admin/admin.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,8 @@ func (a *Admin) CreatePost(c *gin.Context) {
199199
func (a *Admin) UploadFile(c *gin.Context) {
200200
log.Println("Upload file API hit")
201201

202-
if !a.auth.IsAdmin(c) {
202+
// Allow the install wizard (pre-admin) to upload images during setup.
203+
if !a.auth.IsAdmin(c) && !a.auth.IsWizardMode(c) {
203204
log.Println("IS ADMIN RETURNED FALSE")
204205
c.JSON(http.StatusUnauthorized, "Not Authorized")
205206
return
@@ -530,7 +531,8 @@ func (a *Admin) UpdateSettings(c *gin.Context) {
530531
return
531532
}
532533

533-
if !a.auth.IsAdmin(c) {
534+
// Allow the install wizard (pre-admin) to write the initial settings.
535+
if !a.auth.IsAdmin(c) && !a.auth.IsWizardMode(c) {
534536
log.Println("IS ADMIN RETURNED FALSE")
535537
c.JSON(http.StatusUnauthorized, "Not Authorized")
536538
return

admin/admin_test.go

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ func (m *Auth) IsLoggedIn(c *gin.Context) bool {
4141
return args.Bool(0)
4242
}
4343

44+
func (m *Auth) IsWizardMode(c *gin.Context) bool {
45+
args := m.Called(c)
46+
return args.Bool(0)
47+
}
48+
4449
func TestCreatePost(t *testing.T) {
4550
db, _ := gorm.Open(sqlite.Open(":memory:"))
4651
db.AutoMigrate(&auth.BlogUser{})
@@ -404,15 +409,28 @@ func TestCreatePost(t *testing.T) {
404409
// t.Fatalf("Expected to get status %d but instead got %d\n%s", http.StatusBadRequest, w.Code, body)
405410
//}
406411

407-
//file upload, not admin
412+
//file upload, not admin and not wizard mode
408413
a.On("IsAdmin", mock.Anything).Return(false).Once()
414+
a.On("IsWizardMode", mock.Anything).Return(false).Once()
409415
w = httptest.NewRecorder()
410416
router.ServeHTTP(w, req)
411417
if w.Code != http.StatusUnauthorized {
412418
body, _ := ioutil.ReadAll(w.Body)
413419
t.Fatalf("Expected to get status %d but instead got %d\n%s", http.StatusUnauthorized, w.Code, body)
414420
}
415421

422+
//file upload, not admin but in wizard mode (fresh install) — should be allowed
423+
a.On("IsAdmin", mock.Anything).Return(false).Once()
424+
a.On("IsWizardMode", mock.Anything).Return(true).Once()
425+
w = httptest.NewRecorder()
426+
req = httptest.NewRequest("POST", "/api/v1/upload", nil)
427+
router.ServeHTTP(w, req)
428+
if w.Code != http.StatusBadRequest {
429+
// expecting 400 (no file in form) rather than 401 — proves the auth gate let us through
430+
body, _ := ioutil.ReadAll(w.Body)
431+
t.Fatalf("Expected to get status %d (past auth gate, missing file) but instead got %d\n%s", http.StatusBadRequest, w.Code, body)
432+
}
433+
416434
//file upload, missing file
417435
a.On("IsAdmin", mock.Anything).Return(true).Once()
418436
w = httptest.NewRecorder()

auth/auth.go

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
type IAuth interface {
2222
IsAdmin(c *gin.Context) bool
2323
IsLoggedIn(c *gin.Context) bool
24+
IsWizardMode(c *gin.Context) bool
2425
}
2526

2627
// Auth API
@@ -203,16 +204,17 @@ func (a *Auth) DisplayUserTable() {
203204
log.Println(users)
204205
}
205206

206-
// IsAdmin returns true if the user logged in is the admin user
207-
// First tries for a session token, and if that fails falls back on an auth token
207+
// IsAdmin returns true if the user logged in is the admin user.
208+
// First tries for a session token, and if that fails falls back on an auth token.
209+
//
210+
// Returns false when no admin user exists in the DB. Pre-admin handlers that
211+
// the install wizard genuinely needs (e.g. UploadFile, UpdateSettings) should
212+
// also accept IsWizardMode as an exemption.
208213
func (a *Auth) IsAdmin(c *gin.Context) bool {
209-
210-
// if there is no admin user in the db, then we can't have an admin user, so we'll just return true for now, mostly
211-
// so the wizard can upload images
212214
var adminUser AdminUser
213215
err := (*a.db).First(&adminUser).Error
214216
if err != nil {
215-
return true
217+
return false
216218
}
217219

218220
session := sessions.Default(c)
@@ -224,9 +226,6 @@ func (a *Auth) IsAdmin(c *gin.Context) bool {
224226
}
225227
}
226228

227-
//debug
228-
//a.DisplayUserTable()
229-
230229
// first make sure the access token matches a logged in user
231230
var existingUser BlogUser
232231
err = (*a.db).Where("access_token = ?", token).First(&existingUser).Error
@@ -241,6 +240,17 @@ func (a *Auth) IsAdmin(c *gin.Context) bool {
241240
return true
242241
}
243242

243+
// IsWizardMode returns true when the install wizard has not yet completed,
244+
// detected by the absence of any row in the admin_users table. The wizard's
245+
// own pre-admin endpoints (image upload, initial settings) gate on this so
246+
// that fresh-install setup can complete before an admin user exists, without
247+
// IsAdmin itself being permissive to anonymous traffic.
248+
func (a *Auth) IsWizardMode(c *gin.Context) bool {
249+
var adminUser AdminUser
250+
err := (*a.db).First(&adminUser).Error
251+
return err != nil
252+
}
253+
244254
// IsLoggedIn Returns true if the user is logged in, false otherwise
245255
func (a *Auth) IsLoggedIn(c *gin.Context) bool {
246256
session := sessions.Default(c)

auth/auth_test.go

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
package auth_test
2+
3+
import (
4+
"net/http/httptest"
5+
"testing"
6+
7+
"goblog/auth"
8+
9+
"github.com/gin-contrib/sessions"
10+
"github.com/gin-contrib/sessions/cookie"
11+
"github.com/gin-gonic/gin"
12+
"gorm.io/driver/sqlite"
13+
"gorm.io/gorm"
14+
)
15+
16+
func newCtx() *gin.Context {
17+
gin.SetMode(gin.TestMode)
18+
w := httptest.NewRecorder()
19+
c, _ := gin.CreateTestContext(w)
20+
r := gin.Default()
21+
store := cookie.NewStore([]byte("test"))
22+
r.Use(sessions.Sessions("session", store))
23+
c.Request = httptest.NewRequest("GET", "/", nil)
24+
r.HandleContext(c)
25+
return c
26+
}
27+
28+
func newAuth(t *testing.T) (*auth.Auth, *gorm.DB) {
29+
db, err := gorm.Open(sqlite.Open(":memory:"))
30+
if err != nil {
31+
t.Fatalf("open: %v", err)
32+
}
33+
if err := db.AutoMigrate(&auth.BlogUser{}, &auth.AdminUser{}); err != nil {
34+
t.Fatalf("migrate: %v", err)
35+
}
36+
a := auth.New(db, "test")
37+
return &a, db
38+
}
39+
40+
func TestIsAdmin_NoAdminUser_ReturnsFalse(t *testing.T) {
41+
a, _ := newAuth(t)
42+
if a.IsAdmin(newCtx()) {
43+
t.Fatal("IsAdmin must be false on a fresh install with no admin_users row; otherwise anonymous traffic is treated as admin")
44+
}
45+
}
46+
47+
func TestIsWizardMode_NoAdminUser_ReturnsTrue(t *testing.T) {
48+
a, _ := newAuth(t)
49+
if !a.IsWizardMode(newCtx()) {
50+
t.Fatal("IsWizardMode must be true when no admin_users row exists")
51+
}
52+
}
53+
54+
func TestIsWizardMode_WithAdminUser_ReturnsFalse(t *testing.T) {
55+
a, db := newAuth(t)
56+
user := auth.BlogUser{ID: 1, Login: "admin"}
57+
db.Create(&user)
58+
db.Create(&auth.AdminUser{BlogUserID: user.ID, BlogUser: user})
59+
if a.IsWizardMode(newCtx()) {
60+
t.Fatal("IsWizardMode must be false once an admin_users row exists")
61+
}
62+
}

blog/blog_test.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@ func (m *Auth) IsLoggedIn(c *gin.Context) bool {
4040
return args.Bool(0)
4141
}
4242

43+
func (m *Auth) IsWizardMode(c *gin.Context) bool {
44+
args := m.Called(c)
45+
return args.Bool(0)
46+
}
47+
4348
func TestBlogWorkflow(t *testing.T) {
4449
db, _ := gorm.Open(sqlite.Open(":memory:"))
4550
db.AutoMigrate(&auth.BlogUser{})

0 commit comments

Comments
 (0)