Merge pull request #360 from keel-hq/feature/basic_auth

Feature/basic auth

bot/slack/slack.go

@@ -63,7 +63,7 @@ func (b *Bot) Configure(approvalsRespCh chan *bot.ApprovalResponse, botMessagesC
 
 		b.approvalsChannel = "general"
 		if channel := os.Getenv(constants.EnvSlackApprovalsChannel); channel != "" {
-			b.approvalsChannel = channel
+			b.approvalsChannel = strings.TrimPrefix(channel, "#")
 		}
 
 		b.slackClient = client

cmd/keel/main.go

@@ -268,6 +268,8 @@ func setupTriggers(ctx context.Context, providers provider.Providers, approvalsM
 		Port:            types.KeelDefaultPort,
 		Providers:       providers,
 		ApprovalManager: approvalsManager,
+		Username:        os.Getenv(constants.EnvBasicAuthUser),
+		Password:        os.Getenv(constants.EnvBasicAuthPassword),
 	})
 
 	go func() {

tests/acceptance_test.go

@@ -4,14 +4,17 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"fmt"
 	"net/http"
 	"testing"
 	"time"
 
+	"github.com/keel-hq/keel/constants"
+
 	"github.com/keel-hq/keel/types"
 
 	apps_v1 "k8s.io/api/apps/v1"
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 
 	log "github.com/sirupsen/logrus"
@@ -405,3 +408,153 @@ func TestApprovals(t *testing.T) {
 		}
 	})
 }
+
+func TestApprovalsWithAuthentication(t *testing.T) {
+
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	username := "foobar"
+	password := "barfood"
+
+	// go startKeel(ctx)
+	keel := &KeelCmd{
+		env: []string{
+			fmt.Sprintf("%s=%s", constants.EnvBasicAuthUser, username),
+			fmt.Sprintf("%s=%s", constants.EnvBasicAuthPassword, password),
+		},
+	}
+	go func() {
+		err := keel.Start(ctx)
+		if err != nil {
+			log.WithFields(log.Fields{
+				"error": err,
+			}).Error("failed to start Keel process")
+		}
+	}()
+
+	defer func() {
+		err := keel.Stop()
+		if err != nil {
+			log.WithFields(log.Fields{
+				"error": err,
+			}).Error("failed to stop Keel process")
+		}
+	}()
+
+	_, kcs := getKubernetesClient()
+
+	t.Run("CreateDeploymentWithApprovals", func(t *testing.T) {
+
+		testNamespace := createNamespaceForTest()
+		defer deleteTestNamespace(testNamespace)
+
+		dep := &apps_v1.Deployment{
+			meta_v1.TypeMeta{},
+			meta_v1.ObjectMeta{
+				Name:      "dep-1-auth-test",
+				Namespace: testNamespace,
+				Labels: map[string]string{
+					types.KeelPolicyLabel:           "all",
+					types.KeelMinimumApprovalsLabel: "1",
+					types.KeelApprovalDeadlineLabel: "5",
+				},
+				Annotations: map[string]string{},
+			},
+			apps_v1.DeploymentSpec{
+				Selector: &meta_v1.LabelSelector{
+					MatchLabels: map[string]string{
+						"app": "wd-1",
+					},
+				},
+				Template: v1.PodTemplateSpec{
+					ObjectMeta: meta_v1.ObjectMeta{
+						Labels: map[string]string{
+							"app":     "wd-1",
+							"release": "1",
+						},
+					},
+					Spec: v1.PodSpec{
+						Containers: []v1.Container{
+							v1.Container{
+								Name:  "wd-1",
+								Image: "karolisr/webhook-demo:0.0.14",
+							},
+						},
+					},
+				},
+			},
+			apps_v1.DeploymentStatus{},
+		}
+
+		_, err := kcs.AppsV1().Deployments(testNamespace).Create(dep)
+		if err != nil {
+			t.Fatalf("failed to create deployment: %s", err)
+		}
+		// giving some time to get started
+		// TODO: replace with a readiness check function to wait for 1/1 READY
+		time.Sleep(2 * time.Second)
+
+		// sending webhook
+		client := http.DefaultClient
+		buf := bytes.NewBufferString(dockerHub0150Webhook)
+		req, err := http.NewRequest("POST", "http://localhost:9300/v1/webhooks/dockerhub", buf)
+		if err != nil {
+			t.Fatalf("failed to create req: %s", err)
+		}
+		resp, err := client.Do(req)
+		if err != nil {
+			t.Errorf("failed to make a webhook request to keel: %s", err)
+		}
+
+		if resp.StatusCode != 200 {
+			t.Errorf("unexpected webhook response from keel: %d", resp.StatusCode)
+		}
+
+		time.Sleep(2 * time.Second)
+
+		reqNoAuth, err := http.NewRequest("GET", "http://localhost:9300/v1/approvals", nil)
+		if err != nil {
+			t.Fatalf("failed to create req: %s", err)
+		}
+		respNoAuth, err := client.Do(reqNoAuth)
+		if err != nil {
+			t.Logf("failed to make req: %s", err)
+		}
+		defer respNoAuth.Body.Close()
+		if respNoAuth.StatusCode != http.StatusUnauthorized {
+			t.Errorf("expected 401, got: %d", respNoAuth.StatusCode)
+		}
+
+		// doing it again with authentication
+		reqAuth, err := http.NewRequest("GET", "http://localhost:9300/v1/approvals", nil)
+		if err != nil {
+			t.Fatalf("failed to create req: %s", err)
+		}
+		reqAuth.SetBasicAuth(username, password)
+		resp, err = client.Do(reqAuth)
+		if err != nil {
+			t.Errorf("failed to make req: %s", err)
+		}
+
+		var approvals []*types.Approval
+		dec := json.NewDecoder(resp.Body)
+		defer resp.Body.Close()
+		err = dec.Decode(&approvals)
+		if err != nil {
+			t.Fatalf("failed to decode approvals resp: %s", err)
+		}
+
+		if len(approvals) == 0 {
+			t.Errorf("no approvals found")
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), time.Second*10)
+		defer cancel()
+
+		err = waitFor(ctx, kcs, testNamespace, dep.ObjectMeta.Name, "karolisr/webhook-demo:0.0.14")
+		if err != nil {
+			t.Errorf("update failed: %s", err)
+		}
+	})
+}

tests/helpers.go

@@ -10,7 +10,7 @@ import (
 	"strings"
 	"time"
 
-	"k8s.io/api/core/v1"
+	v1 "k8s.io/api/core/v1"
 	meta_v1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/client-go/kubernetes"
 	"k8s.io/client-go/rest"
@@ -75,6 +75,8 @@ func deleteTestNamespace(namespace string) error {
 
 type KeelCmd struct {
 	cmd *exec.Cmd
+
+	env []string
 }
 
 func (kc *KeelCmd) Start(ctx context.Context) error {
@@ -87,6 +89,8 @@ func (kc *KeelCmd) Start(ctx context.Context) error {
 	c.Env = []string{
 		"DEBUG=true",
 	}
+	c.Env = append(c.Env, kc.env...)
+
 	c.Stdout = os.Stdout
 	c.Stderr = os.Stderr
 

trigger/http/approvals_endpoint.go

@@ -4,9 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
-	"os"
 
-	"github.com/keel-hq/keel/constants"
 	"github.com/keel-hq/keel/cache"
 	"github.com/keel-hq/keel/types"
 )
@@ -26,23 +24,6 @@ const (
 
 func (s *TriggerServer) approvalsHandler(resp http.ResponseWriter, req *http.Request) {
 
-	// basic auth
-	if os.Getenv(constants.EnvBasicAuthUser) != "" && os.Getenv(constants.EnvBasicAuthPassword) != "" {
-
-		resp.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
-
-		username, password, authOK := req.BasicAuth()
-		if authOK == false {
-			http.Error(resp, "Not authorized", 401)
-			return
-		}
-
-		if username != os.Getenv(constants.EnvBasicAuthUser) || password != os.Getenv(constants.EnvBasicAuthPassword) {
-			http.Error(resp, "Not authorized", 401)
-			return
-		}
-	}
-
 	// unknown lists all
 	approvals, err := s.approvalsManager.List()
 	if err != nil {
@@ -67,23 +48,6 @@ func (s *TriggerServer) approvalsHandler(resp http.ResponseWriter, req *http.Req
 
 func (s *TriggerServer) approvalApproveHandler(resp http.ResponseWriter, req *http.Request) {
 
-	// basic auth
-	if os.Getenv(constants.EnvBasicAuthUser) != "" && os.Getenv(constants.EnvBasicAuthPassword) != "" {
-
-		resp.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
-
-		username, password, authOK := req.BasicAuth()
-		if authOK == false {
-			http.Error(resp, "Not authorized", 401)
-			return
-		}
-
-		if username != os.Getenv(constants.EnvBasicAuthUser) || password != os.Getenv(constants.EnvBasicAuthPassword) {
-			http.Error(resp, "Not authorized", 401)
-			return
-		}
-	}
-	
 	var ar approveRequest
 	dec := json.NewDecoder(req.Body)
 	defer req.Body.Close()

trigger/http/approvals_endpoint_test.go

@@ -352,3 +352,108 @@ func TestReject(t *testing.T) {
 	}
 
 }
+
+func TestAuthListApprovalsA(t *testing.T) {
+
+	fp := &fakeProvider{}
+	mem := memory.NewMemoryCache()
+	am := approvals.New(mem)
+	providers := provider.New([]provider.Provider{fp}, am)
+	srv := NewTriggerServer(&Opts{
+		Providers:       providers,
+		ApprovalManager: am,
+		Username:        "user-1",
+		Password:        "secret",
+	})
+	srv.registerRoutes(srv.router)
+
+	err := am.Create(&types.Approval{
+		Identifier:     "123",
+		VotesRequired:  5,
+		NewVersion:     "2.0.0",
+		CurrentVersion: "1.0.0",
+	})
+
+	if err != nil {
+		t.Fatalf("failed to create approval: %s", err)
+	}
+
+	// listing
+	req, err := http.NewRequest("GET", "/v1/approvals", nil)
+	if err != nil {
+		t.Fatalf("failed to create req: %s", err)
+	}
+
+	rec := httptest.NewRecorder()
+
+	srv.router.ServeHTTP(rec, req)
+	if rec.Code != 401 {
+		t.Errorf("expected 401 status code, got: %d", rec.Code)
+
+		t.Log(rec.Body.String())
+	}
+}
+
+func TestAuthListApprovalsB(t *testing.T) {
+
+	fp := &fakeProvider{}
+	mem := memory.NewMemoryCache()
+	am := approvals.New(mem)
+	providers := provider.New([]provider.Provider{fp}, am)
+	srv := NewTriggerServer(&Opts{
+		Providers:       providers,
+		ApprovalManager: am,
+		Username:        "user-1",
+		Password:        "secret",
+	})
+	srv.registerRoutes(srv.router)
+
+	err := am.Create(&types.Approval{
+		Identifier:     "123",
+		VotesRequired:  5,
+		NewVersion:     "2.0.0",
+		CurrentVersion: "1.0.0",
+	})
+
+	if err != nil {
+		t.Fatalf("failed to create approval: %s", err)
+	}
+
+	// listing
+	req, err := http.NewRequest("GET", "/v1/approvals", nil)
+	if err != nil {
+		t.Fatalf("failed to create req: %s", err)
+	}
+
+	req.SetBasicAuth("user-1", "secret")
+
+	rec := httptest.NewRecorder()
+
+	srv.router.ServeHTTP(rec, req)
+	if rec.Code != 200 {
+		t.Errorf("expected 200 status code, got: %d", rec.Code)
+
+		t.Log(rec.Body.String())
+	}
+
+	var approvals []*types.Approval
+
+	err = json.Unmarshal(rec.Body.Bytes(), &approvals)
+	if err != nil {
+		t.Fatalf("failed to unmarshal response into approvals: %s", err)
+	}
+
+	if len(approvals) != 1 {
+		t.Fatalf("expected to find 1 approval but found: %d", len(approvals))
+	}
+
+	if approvals[0].VotesRequired != 5 {
+		t.Errorf("unexpected votes required")
+	}
+	if approvals[0].NewVersion != "2.0.0" {
+		t.Errorf("unexpected new version: %s", approvals[0].NewVersion)
+	}
+	if approvals[0].CurrentVersion != "1.0.0" {
+		t.Errorf("unexpected current version: %s", approvals[0].CurrentVersion)
+	}
+}

trigger/http/http.go

@@ -28,6 +28,10 @@ type Opts struct {
 	Providers provider.Providers
 
 	ApprovalManager approvals.Manager
+
+	// Username and password are used for basic auth
+	Username string
+	Password string
 }
 
 // TriggerServer - webhook trigger & healthcheck server
@@ -37,6 +41,10 @@ type TriggerServer struct {
 	port             int
 	server           *http.Server
 	router           *mux.Router
+
+	// basic auth
+	username string
+	password string
 }
 
 // NewTriggerServer - create new HTTP trigger based server
@@ -46,6 +54,8 @@ func NewTriggerServer(opts *Opts) *TriggerServer {
 		providers:        opts.Providers,
 		approvalsManager: opts.ApprovalManager,
 		router:           mux.NewRouter(),
+		username:         opts.Username,
+		password:         opts.Password,
 	}
 }
 
@@ -91,27 +101,25 @@ func (s *TriggerServer) registerRoutes(mux *mux.Router) {
 	mux.HandleFunc("/version", s.versionHandler).Methods("GET", "OPTIONS")
 
 	// approvals
-	mux.HandleFunc("/v1/approvals", s.approvalsHandler).Methods("GET", "OPTIONS")
+	mux.HandleFunc("/v1/approvals", s.requireAdminAuthorization(s.approvalsHandler)).Methods("GET", "OPTIONS")
 	// approving
-	mux.HandleFunc("/v1/approvals", s.approvalApproveHandler).Methods("POST", "OPTIONS")
+	mux.HandleFunc("/v1/approvals", s.requireAdminAuthorization(s.approvalApproveHandler)).Methods("POST", "OPTIONS")
 
-	// native webhooks handler
+	mux.Handle("/metrics", promhttp.Handler())
+
+	s.registerWebhookRoutes(mux)
+}
+
+func (s *TriggerServer) registerWebhookRoutes(mux *mux.Router) {
 	mux.HandleFunc("/v1/webhooks/native", s.nativeHandler).Methods("POST", "OPTIONS")
-
-	// dockerhub webhooks handler
 	mux.HandleFunc("/v1/webhooks/dockerhub", s.dockerHubHandler).Methods("POST", "OPTIONS")
-
-	// quay webhooks handler
 	mux.HandleFunc("/v1/webhooks/quay", s.quayHandler).Methods("POST", "OPTIONS")
-
 	mux.HandleFunc("/v1/webhooks/azure", s.azureHandler).Methods("POST", "OPTIONS")
 
 	// Docker registry notifications, used by Docker, Gitlab, Harbor
 	// https://docs.docker.com/registry/notifications/
 	//https://docs.gitlab.com/ee/administration/container_registry.html#configure-container-registry-notifications
 	mux.HandleFunc("/v1/webhooks/registry", s.registryNotificationHandler).Methods("POST", "OPTIONS")
-
-	mux.Handle("/metrics", promhttp.Handler())
 }
 
 func (s *TriggerServer) healthHandler(resp http.ResponseWriter, req *http.Request) {
@@ -135,3 +143,23 @@ func (s *TriggerServer) versionHandler(resp http.ResponseWriter, req *http.Reque
 func (s *TriggerServer) trigger(event types.Event) error {
 	return s.providers.Submit(event)
 }
+
+func (s *TriggerServer) requireAdminAuthorization(next http.HandlerFunc) http.HandlerFunc {
+	return func(rw http.ResponseWriter, r *http.Request) {
+
+		if s.username == "" && s.password == "" {
+			next(rw, r)
+			return
+		}
+
+		rw.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
+
+		username, password, ok := r.BasicAuth()
+		if ok && username == s.username && password == s.password {
+			next(rw, r)
+			return
+		}
+
+		http.Error(rw, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
+	}
+}