Merge pull request #530 from goblogplatform/fix/admin-strict-with-wizard-mode

Make IsAdmin strict, gate wizard upload on IsWizardMode

Author: compscidr

admin/admin.go

@@ -199,7 +199,8 @@ func (a *Admin) CreatePost(c *gin.Context) {
 func (a *Admin) UploadFile(c *gin.Context) {
 	log.Println("Upload file API hit")
 
-	if !a.auth.IsAdmin(c) {
+	// Allow the install wizard (pre-admin) to upload images during setup.
+	if !a.auth.IsAdmin(c) && !a.auth.IsWizardMode(c) {
 		log.Println("IS ADMIN RETURNED FALSE")
 		c.JSON(http.StatusUnauthorized, "Not Authorized")
 		return
@@ -530,7 +531,8 @@ func (a *Admin) UpdateSettings(c *gin.Context) {
 		return
 	}
 
-	if !a.auth.IsAdmin(c) {
+	// Allow the install wizard (pre-admin) to write the initial settings.
+	if !a.auth.IsAdmin(c) && !a.auth.IsWizardMode(c) {
 		log.Println("IS ADMIN RETURNED FALSE")
 		c.JSON(http.StatusUnauthorized, "Not Authorized")
 		return

admin/admin_test.go

@@ -41,6 +41,11 @@ func (m *Auth) IsLoggedIn(c *gin.Context) bool {
 	return args.Bool(0)
 }
 
+func (m *Auth) IsWizardMode(c *gin.Context) bool {
+	args := m.Called(c)
+	return args.Bool(0)
+}
+
 func TestCreatePost(t *testing.T) {
 	db, _ := gorm.Open(sqlite.Open(":memory:"))
 	db.AutoMigrate(&auth.BlogUser{})
@@ -404,15 +409,28 @@ func TestCreatePost(t *testing.T) {
 	//	t.Fatalf("Expected to get status %d but instead got %d\n%s", http.StatusBadRequest, w.Code, body)
 	//}
 
-	//file upload, not admin
+	//file upload, not admin and not wizard mode
 	a.On("IsAdmin", mock.Anything).Return(false).Once()
+	a.On("IsWizardMode", mock.Anything).Return(false).Once()
 	w = httptest.NewRecorder()
 	router.ServeHTTP(w, req)
 	if w.Code != http.StatusUnauthorized {
 		body, _ := ioutil.ReadAll(w.Body)
 		t.Fatalf("Expected to get status %d but instead got %d\n%s", http.StatusUnauthorized, w.Code, body)
 	}
 
+	//file upload, not admin but in wizard mode (fresh install) — should be allowed
+	a.On("IsAdmin", mock.Anything).Return(false).Once()
+	a.On("IsWizardMode", mock.Anything).Return(true).Once()
+	w = httptest.NewRecorder()
+	req = httptest.NewRequest("POST", "/api/v1/upload", nil)
+	router.ServeHTTP(w, req)
+	if w.Code != http.StatusBadRequest {
+		// expecting 400 (no file in form) rather than 401 — proves the auth gate let us through
+		body, _ := ioutil.ReadAll(w.Body)
+		t.Fatalf("Expected to get status %d (past auth gate, missing file) but instead got %d\n%s", http.StatusBadRequest, w.Code, body)
+	}
+
 	//file upload, missing file
 	a.On("IsAdmin", mock.Anything).Return(true).Once()
 	w = httptest.NewRecorder()

auth/auth.go

@@ -21,6 +21,7 @@ import (
 type IAuth interface {
 	IsAdmin(c *gin.Context) bool
 	IsLoggedIn(c *gin.Context) bool
+	IsWizardMode(c *gin.Context) bool
 }
 
 // Auth API
@@ -203,16 +204,17 @@ func (a *Auth) DisplayUserTable() {
 	log.Println(users)
 }
 
-// IsAdmin returns true if the user logged in is the admin user
-// First tries for a session token, and if that fails falls back on an auth token
+// IsAdmin returns true if the user logged in is the admin user.
+// First tries for a session token, and if that fails falls back on an auth token.
+//
+// Returns false when no admin user exists in the DB. Pre-admin handlers that
+// the install wizard genuinely needs (e.g. UploadFile, UpdateSettings) should
+// also accept IsWizardMode as an exemption.
 func (a *Auth) IsAdmin(c *gin.Context) bool {
-
-	// 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
-	// so the wizard can upload images
 	var adminUser AdminUser
 	err := (*a.db).First(&adminUser).Error
 	if err != nil {
-		return true
+		return false
 	}
 
 	session := sessions.Default(c)
@@ -224,9 +226,6 @@ func (a *Auth) IsAdmin(c *gin.Context) bool {
 		}
 	}
 
-	//debug
-	//a.DisplayUserTable()
-
 	// first make sure the access token matches a logged in user
 	var existingUser BlogUser
 	err = (*a.db).Where("access_token = ?", token).First(&existingUser).Error
@@ -241,6 +240,17 @@ func (a *Auth) IsAdmin(c *gin.Context) bool {
 	return true
 }
 
+// IsWizardMode returns true when the install wizard has not yet completed,
+// detected by the absence of any row in the admin_users table. The wizard's
+// own pre-admin endpoints (image upload, initial settings) gate on this so
+// that fresh-install setup can complete before an admin user exists, without
+// IsAdmin itself being permissive to anonymous traffic.
+func (a *Auth) IsWizardMode(c *gin.Context) bool {
+	var adminUser AdminUser
+	err := (*a.db).First(&adminUser).Error
+	return err != nil
+}
+
 // IsLoggedIn Returns true if the user is logged in, false otherwise
 func (a *Auth) IsLoggedIn(c *gin.Context) bool {
 	session := sessions.Default(c)

auth/auth_test.go

@@ -0,0 +1,62 @@
+package auth_test
+
+import (
+	"net/http/httptest"
+	"testing"
+
+	"goblog/auth"
+
+	"github.com/gin-contrib/sessions"
+	"github.com/gin-contrib/sessions/cookie"
+	"github.com/gin-gonic/gin"
+	"gorm.io/driver/sqlite"
+	"gorm.io/gorm"
+)
+
+func newCtx() *gin.Context {
+	gin.SetMode(gin.TestMode)
+	w := httptest.NewRecorder()
+	c, _ := gin.CreateTestContext(w)
+	r := gin.Default()
+	store := cookie.NewStore([]byte("test"))
+	r.Use(sessions.Sessions("session", store))
+	c.Request = httptest.NewRequest("GET", "/", nil)
+	r.HandleContext(c)
+	return c
+}
+
+func newAuth(t *testing.T) (*auth.Auth, *gorm.DB) {
+	db, err := gorm.Open(sqlite.Open(":memory:"))
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	if err := db.AutoMigrate(&auth.BlogUser{}, &auth.AdminUser{}); err != nil {
+		t.Fatalf("migrate: %v", err)
+	}
+	a := auth.New(db, "test")
+	return &a, db
+}
+
+func TestIsAdmin_NoAdminUser_ReturnsFalse(t *testing.T) {
+	a, _ := newAuth(t)
+	if a.IsAdmin(newCtx()) {
+		t.Fatal("IsAdmin must be false on a fresh install with no admin_users row; otherwise anonymous traffic is treated as admin")
+	}
+}
+
+func TestIsWizardMode_NoAdminUser_ReturnsTrue(t *testing.T) {
+	a, _ := newAuth(t)
+	if !a.IsWizardMode(newCtx()) {
+		t.Fatal("IsWizardMode must be true when no admin_users row exists")
+	}
+}
+
+func TestIsWizardMode_WithAdminUser_ReturnsFalse(t *testing.T) {
+	a, db := newAuth(t)
+	user := auth.BlogUser{ID: 1, Login: "admin"}
+	db.Create(&user)
+	db.Create(&auth.AdminUser{BlogUserID: user.ID, BlogUser: user})
+	if a.IsWizardMode(newCtx()) {
+		t.Fatal("IsWizardMode must be false once an admin_users row exists")
+	}
+}

blog/blog_test.go

@@ -40,6 +40,11 @@ func (m *Auth) IsLoggedIn(c *gin.Context) bool {
 	return args.Bool(0)
 }
 
+func (m *Auth) IsWizardMode(c *gin.Context) bool {
+	args := m.Called(c)
+	return args.Bool(0)
+}
+
 func TestBlogWorkflow(t *testing.T) {
 	db, _ := gorm.Open(sqlite.Open(":memory:"))
 	db.AutoMigrate(&auth.BlogUser{})