From 6c58e9db9b7eb465de6b081ecf5ea76068ef594a Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Sat, 8 Jul 2023 18:09:34 +0200
Subject: [PATCH] Fix agent auth (#1952)

if no global agent secret set, disable agent registration via it
---
 server/api/login.go                |  9 ++----
 server/grpc/auth_server.go         | 52 +++++++++++++++++++-----------
 server/model/settings.go           |  1 -
 server/router/middleware/config.go |  7 +++-
 4 files changed, 41 insertions(+), 28 deletions(-)

diff --git a/server/api/login.go b/server/api/login.go
index 2b1bddaf86..8aaf58ded6 100644
--- a/server/api/login.go
+++ b/server/api/login.go
@@ -26,6 +26,7 @@ import (
 
 	"github.com/woodpecker-ci/woodpecker/server"
 	"github.com/woodpecker-ci/woodpecker/server/model"
+	"github.com/woodpecker-ci/woodpecker/server/router/middleware"
 	"github.com/woodpecker-ci/woodpecker/server/store"
 	"github.com/woodpecker-ci/woodpecker/server/store/types"
 	"github.com/woodpecker-ci/woodpecker/shared/httputil"
@@ -63,7 +64,7 @@ func HandleAuth(c *gin.Context) {
 	if tmpuser == nil {
 		return
 	}
-	config := ToConfig(c)
+	config := middleware.GetConfig(c)
 
 	// get the user from the database
 	u, err := _store.GetUserRemoteID(tmpuser.ForgeRemoteID, tmpuser.Login)
@@ -227,9 +228,3 @@ type tokenPayload struct {
 	Refresh string `json:"refresh_token,omitempty"`
 	Expires int64  `json:"expires_in,omitempty"`
 }
-
-// ToConfig returns the config from the Context
-func ToConfig(c *gin.Context) *model.Settings {
-	v := c.MustGet("config")
-	return v.(*model.Settings)
-}
diff --git a/server/grpc/auth_server.go b/server/grpc/auth_server.go
index 9646808eff..672cc62924 100644
--- a/server/grpc/auth_server.go
+++ b/server/grpc/auth_server.go
@@ -16,13 +16,15 @@ package grpc
 
 import (
 	"context"
+	"errors"
+	"fmt"
 
 	"github.com/rs/zerolog/log"
 
 	"github.com/woodpecker-ci/woodpecker/pipeline/rpc/proto"
-	"github.com/woodpecker-ci/woodpecker/server"
 	"github.com/woodpecker-ci/woodpecker/server/model"
 	"github.com/woodpecker-ci/woodpecker/server/store"
+	"github.com/woodpecker-ci/woodpecker/server/store/types"
 )
 
 type WoodpeckerAuthServer struct {
@@ -39,7 +41,7 @@ func NewWoodpeckerAuthServer(jwtManager *JWTManager, agentMasterToken string, st
 func (s *WoodpeckerAuthServer) Auth(_ context.Context, req *proto.AuthRequest) (*proto.AuthResponse, error) {
 	agent, err := s.getAgent(req.AgentId, req.AgentToken)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("Agent could not auth: %w", err)
 	}
 
 	accessToken, err := s.jwtManager.Generate(agent.ID)
@@ -55,25 +57,37 @@ func (s *WoodpeckerAuthServer) Auth(_ context.Context, req *proto.AuthRequest) (
 }
 
 func (s *WoodpeckerAuthServer) getAgent(agentID int64, agentToken string) (*model.Agent, error) {
-	if agentToken == s.agentMasterToken && agentID == -1 {
-		agent := new(model.Agent)
-		agent.Name = ""
-		agent.OwnerID = -1 // system agent
-		agent.Token = server.Config.Server.AgentToken
-		agent.Backend = ""
-		agent.Platform = ""
-		agent.Capacity = -1
-		err := s.store.AgentCreate(agent)
-		if err != nil {
-			log.Err(err).Msgf("Error creating system agent: %s", err)
-			return nil, err
+	// global agent secret auth
+	if s.agentMasterToken != "" {
+		if agentToken == s.agentMasterToken && agentID == -1 {
+			agent := new(model.Agent)
+			agent.Name = ""
+			agent.OwnerID = -1 // system agent
+			agent.Token = s.agentMasterToken
+			agent.Backend = ""
+			agent.Platform = ""
+			agent.Capacity = -1
+			err := s.store.AgentCreate(agent)
+			if err != nil {
+				log.Err(err).Msgf("Error creating system agent: %s", err)
+				return nil, err
+			}
+			return agent, nil
+		}
+
+		if agentToken == s.agentMasterToken {
+			agent, err := s.store.AgentFind(agentID)
+			if err != nil && errors.Is(err, types.RecordNotExist) {
+				return nil, fmt.Errorf("AgentID not found in database")
+			}
+			return agent, err
 		}
-		return agent, nil
 	}
 
-	if agentToken == s.agentMasterToken {
-		return s.store.AgentFind(agentID)
+	// individual agent token auth
+	agent, err := s.store.AgentFindByToken(agentToken)
+	if err != nil && errors.Is(err, types.RecordNotExist) {
+		return nil, fmt.Errorf("individual agent not found by token: %w", err)
 	}
-
-	return s.store.AgentFindByToken(agentToken)
+	return agent, err
 }
diff --git a/server/model/settings.go b/server/model/settings.go
index 3e0f52f42d..d82cbf7ae0 100644
--- a/server/model/settings.go
+++ b/server/model/settings.go
@@ -17,7 +17,6 @@ package model
 // Settings defines system configuration parameters.
 type Settings struct {
 	Open            bool            // Enables open registration
-	Secret          string          // Secret token used to authenticate agents
 	Admins          map[string]bool // Administrative users
 	Orgs            map[string]bool // Organization whitelist
 	OwnersWhitelist map[string]bool // Owners whitelist
diff --git a/server/router/middleware/config.go b/server/router/middleware/config.go
index 839affe518..b335e756c9 100644
--- a/server/router/middleware/config.go
+++ b/server/router/middleware/config.go
@@ -36,7 +36,6 @@ func Config(cli *cli.Context) gin.HandlerFunc {
 func setupConfig(c *cli.Context) *model.Settings {
 	return &model.Settings{
 		Open:            c.Bool("open"),
-		Secret:          c.String("agent-secret"),
 		Admins:          sliceToMap2(c.StringSlice("admin")),
 		Orgs:            sliceToMap2(c.StringSlice("orgs")),
 		OwnersWhitelist: sliceToMap2(c.StringSlice("repo-owners")),
@@ -54,3 +53,9 @@ func sliceToMap2(s []string) map[string]bool {
 	}
 	return v
 }
+
+// GetConfig returns the config from the Context
+func GetConfig(c *gin.Context) *model.Settings {
+	v := c.MustGet(configKey)
+	return v.(*model.Settings)
+}