Harden Admin Panel access

server/admin/admin.go

@@ -11,19 +11,21 @@ import (
 type Group struct {
 	*echo.Group
 	db *db.DB
+	au *auth.AdminAuth
 }
 
 // New creates a new group.
-func New(db *db.DB, unauthed *echo.Group) (*Group, error) {
+func New(db *db.DB, au *auth.AdminAuth, unauthed *echo.Group) (*Group, error) {
 	grp := &Group{
 		Group: unauthed,
 		db:    db,
+		au:    au,
 	}
 	// Authentication
 	unauthed.GET("/login", grp.LoginGet)
 	unauthed.POST("/login", grp.LoginPost)
 
-	g := unauthed.Group("", auth.MustAdmin)
+	g := unauthed.Group("", au.MustAdmin)
 	g.GET("/logout", grp.LogoutPost)
 	g.GET("", grp.Home)
 	// Contest List

server/admin/auth.go

@@ -23,7 +23,7 @@ func (l *LoginCtx) Render(c echo.Context) error {
 
 // LoginGet implements GET /admin/login.
 func (g *Group) LoginGet(c echo.Context) error {
-	if ok, err := alreadyLoggedIn(c); err != nil {
+	if ok, err := g.alreadyLoggedIn(c); err != nil {
 		return err
 	} else if ok {
 		return nil
@@ -34,14 +34,14 @@ func (g *Group) LoginGet(c echo.Context) error {
 
 // LoginPost implements POST /admin/login.
 func (g *Group) LoginPost(c echo.Context) error {
-	if ok, err := alreadyLoggedIn(c); err != nil {
+	if ok, err := g.alreadyLoggedIn(c); err != nil {
 		return err
 	} else if ok {
 		return nil
 	}
 
 	key := c.FormValue("key")
-	if err := auth.SaveAdmin(key, c); err != nil {
+	if err := g.au.SaveAdmin(key, c); err != nil {
 		return (&LoginCtx{Error: err}).Render(c)
 	}
 	last := c.QueryParam("last")
@@ -64,8 +64,8 @@ func (g *Group) LogoutPost(c echo.Context) error {
 }
 
 // Redirect if already logged in.
-func alreadyLoggedIn(c echo.Context) (bool, error) {
-	ok, err := auth.AuthenticateAdmin(c)
+func (g *Group) alreadyLoggedIn(c echo.Context) (bool, error) {
+	ok, err := g.au.AuthenticateAdmin(c)
 	if err != nil {
 		return false, err
 	}

server/admin/auth_test.go

@@ -0,0 +1,74 @@
+package admin_test
+
+import (
+	"net/http"
+	"net/url"
+	"os"
+	"testing"
+
+	"github.com/natsukagami/kjudge/server/auth"
+	"github.com/natsukagami/kjudge/test"
+)
+
+func TestAdminLogin(t *testing.T) {
+	ts := test.NewServer(t)
+
+	run := func(key string) *http.Response {
+		form := url.Values{}
+		form.Set("key", key)
+
+		return ts.Serve(ts.PostForm(t, "/admin/login", form))
+	}
+
+	t.Run("success", func(t *testing.T) {
+		resp := run(os.Getenv(auth.AdminKeyEnv))
+
+		if resp.StatusCode >= 400 {
+			t.Errorf("Expected OK got %d", resp.StatusCode)
+		}
+		cookies := resp.Cookies()
+		if len(cookies) != 1 {
+			t.Errorf("Expected one cookie got %#v", cookies)
+		}
+		cookie := cookies[0]
+		if cookie.Name != auth.AdminSessionName {
+			t.Errorf("Expected cookie name `%s` got `%s`", auth.AdminSessionName, cookie.Name)
+		}
+		if !cookie.HttpOnly {
+			t.Error("Cookie is not Http-Only")
+		}
+		if cookie.SameSite != http.SameSiteStrictMode {
+			t.Error("Cookie is not strict same-site")
+		}
+	})
+
+	t.Run("fail", func(t *testing.T) {
+		resp := run("ababababababa")
+
+		if resp.StatusCode < 400 {
+			t.Errorf("Expected error got %d", resp.StatusCode)
+		}
+		cookies := resp.Cookies()
+		if len(cookies) != 0 {
+			t.Errorf("Expected no cookie got %#v", cookies)
+		}
+	})
+}
+
+func TestAdminLoginCheck(t *testing.T) {
+	ts := test.NewServer(t)
+	withAdmin := ts.WithAdmin(t)
+
+	t.Run("with admin cookie", func(t *testing.T) {
+		resp := ts.Serve(ts.Get(t, "/admin", nil), withAdmin)
+		if resp.StatusCode != http.StatusOK {
+			t.Errorf("Expected OK got %d", resp.StatusCode)
+		}
+	})
+	t.Run("without admin cookie", func(t *testing.T) {
+		resp := ts.Serve(ts.Get(t, "/admin", nil))
+		if resp.StatusCode == http.StatusOK {
+			t.Errorf("Expected redirect got %d", resp.StatusCode)
+		}
+	})
+}

server/auth/admin.go

@@ -13,38 +13,56 @@ import (
 	"github.com/pkg/errors"
 )
 
-// The string that will be used as the admin key.
-var adminKey string
+// AdminAuth hosts the authentication module for admin.
+type AdminAuth struct {
+	// The string that will be used as the admin key.
+	hashedAdminKey []byte
+	// Random session key, will change every restart. Guarantees re-login of admin panel.
+	sessionKey string
+}
 
-// The admin key environment variable.
-const adminKeyEnv = "ADMIN_KEY"
+// AdminKeyEnv is the admin key environment variable to look for.
+const AdminKeyEnv = "ADMIN_KEY"
 
-// The key name for the admin session.
-const adminSessionName = "kjudge_admin"
+const (
+	// AdminSessionName is the key name for the admin session cookie.
+	AdminSessionName     = "kjudge_admin"
+	adminSessionKeyField = "key"
+)
 
-func init() {
-	val := os.Getenv(adminKeyEnv)
+// NewAdmin creates a new AdminAuth.
+func NewAdmin() (*AdminAuth, error) {
+	val := os.Getenv(AdminKeyEnv)
 	if val != "" {
 		if len(val) < 6 {
-			log.Fatalf("The admin key should be at least 6 characters long.")
+			return nil, errors.New("The admin key should be at least 6 characters long.")
+		}
+	} else {
+		log.Printf("%s variable not set. A random key will be generated and displayed.", AdminKeyEnv)
+		b := make([]byte, 16)
+		if _, err := rand.Read(b); err != nil {
+			log.Fatalf("%+v", errors.WithStack(err))
 		}
-		adminKey = val
-		return
+		val = fmt.Sprintf("%x", b)
+		log.Printf("The Admin Panel access key is `%s`\n", val)
 	}
-
-	log.Println("ADMIN_KEY variable not set. A random key will be generated and displayed.")
-	b := make([]byte, 16)
-	if _, err := rand.Read(b); err != nil {
-		log.Fatalf("%+v", errors.WithStack(err))
+	// store the hashed version of it in memory
+	adminKey, err := PasswordHash(val)
+	if err != nil {
+		return nil, errors.Wrap(err, "error generating hashed key")
 	}
-	adminKey = fmt.Sprintf("%x", b)
-	log.Printf("The Admin Panel access key is `%s`\n", adminKey)
+	// generate a session key
+	sessionKey := make([]byte, 64)
+	if _, err := rand.Read(sessionKey); err != nil {
+		return nil, errors.Wrap(err, "error generating session key")
+	}
+	return &AdminAuth{hashedAdminKey: adminKey, sessionKey: fmt.Sprintf("%x", sessionKey)}, nil
 }
 
 // MustAdmin is a middleware that ensures admin access.
-func MustAdmin(h echo.HandlerFunc) echo.HandlerFunc {
+func (au *AdminAuth) MustAdmin(h echo.HandlerFunc) echo.HandlerFunc {
 	return func(c echo.Context) error {
-		res, err := AuthenticateAdmin(c)
+		res, err := au.AuthenticateAdmin(c)
 		if err != nil {
 			return nil
 		}
@@ -57,40 +75,40 @@ func MustAdmin(h echo.HandlerFunc) echo.HandlerFunc {
 }
 
 // AuthenticateAdmin returns whether the context has admin panel access.
-func AuthenticateAdmin(c echo.Context) (bool, error) {
-	sess, err := session.Get(adminSessionName, c)
+func (au *AdminAuth) AuthenticateAdmin(c echo.Context) (bool, error) {
+	sess, err := session.Get(AdminSessionName, c)
 	if err != nil {
 		return false, errors.Wrapf(RemoveAdmin(c), "handling err %v", err)
 	}
 	if sess.IsNew {
 		return false, nil
 	}
-	hashedKey, ok := sess.Values["key"].(string)
+	key, ok := sess.Values[adminSessionKeyField].(string)
 	if !ok {
 		return false, nil
 	}
 
-	return CheckPassword(adminKey, hashedKey)
+	return key == au.sessionKey, nil
 }
 
 // SaveAdmin saves the admin cookie.
-func SaveAdmin(key string, c echo.Context) error {
-	if key != adminKey {
+func (au *AdminAuth) SaveAdmin(key string, c echo.Context) error {
+	if ok, err := CheckPassword(key, string(au.hashedAdminKey)); err != nil {
+		return err
+	} else if !ok {
 		return errors.New("Invalid admin key")
 	}
-	sess, _ := session.Get(adminSessionName, c)
+	sess, _ := session.Get(AdminSessionName, c)
 	sess.Options.MaxAge = 0
-	hashedKey, err := PasswordHash(adminKey)
-	if err != nil {
-		return err
-	}
-	sess.Values["key"] = string(hashedKey)
+	sess.Options.HttpOnly = true
+	sess.Options.SameSite = http.SameSiteStrictMode
+	sess.Values[adminSessionKeyField] = au.sessionKey
 	return errors.WithStack(sess.Save(c.Request(), c.Response()))
 }
 
 // RemoveAdmin removes the admin cookie session.
 func RemoveAdmin(c echo.Context) error {
-	sess, _ := session.Get(adminSessionName, c)
+	sess, _ := session.Get(AdminSessionName, c)
 	sess.Options.MaxAge = -1
 	return errors.WithStack(sess.Save(c.Request(), c.Response()))
 }

server/server.go

@@ -17,6 +17,7 @@ import (
 	"github.com/natsukagami/kjudge/models"
 	"github.com/natsukagami/kjudge/models/verify"
 	"github.com/natsukagami/kjudge/server/admin"
+	"github.com/natsukagami/kjudge/server/auth"
 	"github.com/natsukagami/kjudge/server/contests"
 	"github.com/natsukagami/kjudge/server/template"
 	"github.com/natsukagami/kjudge/server/user"
@@ -59,7 +60,11 @@ func New(db *db.DB) (*Server, error) {
 
 	s.SetupProfiling()
 
-	if _, err := admin.New(s.db, s.echo.Group("/admin")); err != nil {
+	au, err := auth.NewAdmin()
+	if err != nil {
+		return nil, err
+	}
+	if _, err := admin.New(s.db, au, s.echo.Group("/admin")); err != nil {
 		return nil, err
 	}
 	if _, err := user.New(s.db, s.echo.Group("/user")); err != nil {

test/testing.go

@@ -2,7 +2,9 @@
 package test
 
 import (
+	"crypto/rand"
 	_ "embed"
+	"fmt"
 	"log"
 	"net/http"
 	"net/http/httptest"
@@ -15,6 +17,7 @@ import (
 	"github.com/labstack/echo/v4"
 	"github.com/natsukagami/kjudge/db"
 	"github.com/natsukagami/kjudge/server"
+	"github.com/natsukagami/kjudge/server/auth"
 	"github.com/pkg/errors"
 )
 
@@ -72,6 +75,13 @@ func NewDB(t *testing.T) *db.DB {
 
 // NewServer creates a new kjudge server running on a test database.
 func NewServer(t *testing.T) *TestServer {
+	// generate an admin key
+	adminKey := make([]byte, 32)
+	if _, err := rand.Read(adminKey); err != nil {
+		t.Fatal("generating admin key:", err)
+	}
+	t.Setenv(auth.AdminKeyEnv, fmt.Sprintf("%x", adminKey))
+
 	db := NewDB(t)
 	s, err := server.New(db)
 	if err != nil {
@@ -88,6 +98,16 @@ func (ts *TestServer) PostForm(t *testing.T, path string, body url.Values) *http
 	return req
 }
 
+// Get fires a new GET request with URL queries.
+func (ts *TestServer) Get(t *testing.T, path string, queries url.Values) *http.Request {
+	if len(queries) > 0 {
+		path = path + "?" + queries.Encode()
+	}
+	req := httptest.NewRequest(http.MethodGet, path, nil)
+
+	return req
+}
+
 // Serve serves a HTTP request and returns its response.
 func (ts *TestServer) Serve(req *http.Request, opts ...ReqOpt) *http.Response {
 	for _, opt := range opts {
@@ -119,5 +139,25 @@ func (ts *TestServer) WithMisaka(t *testing.T) ReqOpt {
 	}
 }
 
+// WithAdmin logs in with the admin panel cookie for the next request.
+func (ts *TestServer) WithAdmin(t *testing.T) ReqOpt {
+	// Perform the log in.
+	form := url.Values{}
+	form.Set("key", os.Getenv(auth.AdminKeyEnv))
+	resp := ts.Serve(ts.PostForm(t, "/admin/login", form))
+
+	if resp.StatusCode >= 400 {
+		t.Fatalf("Cannot login to admin panel: got code %d", resp.StatusCode)
+	}
+	cookies := resp.Cookies()
+	if len(cookies) != 1 {
+		t.Fatalf("Cannot login to admin panel: expect one cookie, got %#v", cookies)
+	}
+
+	return func(req *http.Request) {
+		req.AddCookie(cookies[0])
+	}
+}
+
 // ReqOpt is an option for sending requests.
 type ReqOpt func(*http.Request)