diff --git a/.github/release-notes/v0.10.2.md b/.github/release-notes/v0.10.2.md new file mode 100644 index 0000000..b054263 --- /dev/null +++ b/.github/release-notes/v0.10.2.md @@ -0,0 +1,9 @@ +# Webhook Automation Service v0.10.2 candidate + +This candidate builds on the published v0.10.1 release. It is not yet a published release. + +- Require the control-plane project header to match the requested project on receiver management routes, and enforce read-only roles in comma-separated role lists. +- Restrict receiver listing, lookup, deletion, name checks, and key or JWT execution validation to `webhookReceiver` generic objects. +- Add regression tests for project boundaries, unrelated generic-object kinds, and valid receiver execution. + +The candidate must pass the security release gate and isolated-VM acceptance checks before publication. The Server 8080 QA path has not been claimed as verified by this note. diff --git a/.github/workflows/security-release-gate.yml b/.github/workflows/security-release-gate.yml index 6c68124..2dc3f61 100644 --- a/.github/workflows/security-release-gate.yml +++ b/.github/workflows/security-release-gate.yml @@ -19,7 +19,7 @@ jobs: runs-on: ubuntu-24.04 timeout-minutes: 90 env: - CANDIDATE_VERSION: 0.10.1 + CANDIDATE_VERSION: 0.10.2 GO_VERSION: 1.27.0 GO_LINUX_AMD64_SHA256: 675c26c449cbb18fc24b74650de1eabbae6e16f64326fd85a283fb3b58280685 TRIVY_IMAGE: aquasec/trivy:0.74.0@sha256:62b1e65e8869bc4b4c6aa4fa2b21595256c7c2f6018a9d9ad61caf87187c1969 diff --git a/README.md b/README.md index fc4c6d4..952ce93 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ PastureStack is an independent community effort to preserve, audit, and moderniz The current public GitHub Release is [`v0.10.1`](https://github.com/PastureStack/webhook-automation-service/releases/tag/v0.10.1). +The next candidate is `v0.10.2`; see the [candidate release notes](.github/release-notes/v0.10.2.md). ## Runtime boundary @@ -32,7 +33,7 @@ make build make integration-test ``` -`make package` creates the deterministic flat GitHub Release asset `webhook-automation-service-0.10.1-linux-amd64.tar.xz`. The archive contains the executable plus compatibility, source, notice, and composite license files. The Server release verifies its SHA-256 digest before installation, so operators do not need an artifact mirror. +`make package` creates the deterministic flat candidate asset `webhook-automation-service-0.10.2-linux-amd64.tar.xz`. The archive contains the executable plus compatibility, source, notice, and composite license files. The Server release verifies its SHA-256 digest before installation, so operators do not need an artifact mirror. See [COMPATIBILITY.md](COMPATIBILITY.md), [ORIGIN.md](ORIGIN.md), [MODIFICATIONS.md](MODIFICATIONS.md), and [THIRD-PARTY-NOTICES.md](THIRD-PARTY-NOTICES.md). diff --git a/scripts/integration-test b/scripts/integration-test index 1f4646c..9edb32f 100755 --- a/scripts/integration-test +++ b/scripts/integration-test @@ -33,7 +33,8 @@ RSA_PRIVATE_KEY_CONTENTS='must-not-be-read-or-logged' \ service_pid=$! for _ in $(seq 1 100); do - if curl --silent --fail "http://${listen_address}/v1-webhooks?projectId=1a1" >"${work_root}/version.json"; then + if curl --silent --fail -H 'X-API-Project-Id: 1a1' \ + "http://${listen_address}/v1-webhooks?projectId=1a1" >"${work_root}/version.json"; then break fi if ! kill -0 "${service_pid}" 2>/dev/null; then @@ -44,6 +45,12 @@ for _ in $(seq 1 100); do done grep -F 'v1-webhooks' "${work_root}/version.json" >/dev/null +# A directly reached management route must not trust its query alone. The +# production engine supplies this header only after authorizing the project. +status="$(curl --silent --output /dev/null --write-out '%{http_code}' \ + "http://${listen_address}/v1-webhooks?projectId=1a1")" +test "${status}" = "403" + status="$(curl --silent --output "${work_root}/invalid-token.json" --write-out '%{http_code}' \ -X POST "http://${listen_address}/v1-webhooks/endpoint?token=invalid")" test "${status}" = "400" diff --git a/scripts/version b/scripts/version index 7406467..acaf579 100755 --- a/scripts/version +++ b/scripts/version @@ -3,7 +3,7 @@ version_script_dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" version_repo_root="$(cd "${version_script_dir}/.." && pwd)" -VERSION="${VERSION:-0.10.1}" +VERSION="${VERSION:-0.10.2}" COMMIT="${COMMIT:-$(git -c safe.directory="${version_repo_root}" -C "${version_repo_root}" rev-parse HEAD)}" ARCH="${ARCH:-amd64}" diff --git a/service/construct_handler.go b/service/construct_handler.go index 4884e31..59a036f 100644 --- a/service/construct_handler.go +++ b/service/construct_handler.go @@ -21,7 +21,7 @@ import ( const maximumConfigurationBodyBytes = 1 << 20 func (rh *RouteHandler) ConstructPayload(w http.ResponseWriter, r *http.Request) (int, error) { - if readonlyRoles[getRoles(r)] { + if hasReadonlyRole(r) { return http.StatusMethodNotAllowed, fmt.Errorf("user doesn't have the access to create webhook") } apiContext := api.GetApiContext(r) diff --git a/service/execute_handler.go b/service/execute_handler.go index 797e83b..ff3ecbe 100644 --- a/service/execute_handler.go +++ b/service/execute_handler.go @@ -85,6 +85,7 @@ func (rh *RouteHandler) ExecuteWithKey(uuid string, projectID string, request *h filters := make(map[string]interface{}) filters["key"] = uuid + filters["kind"] = webhookReceiverKind goCollection, err := apiClient.GenericObject.List(&client.ListOpts{ Filters: filters, }) @@ -92,11 +93,18 @@ func (rh *RouteHandler) ExecuteWithKey(uuid string, projectID string, request *h return 500, fmt.Errorf("Error %v filtering genericObjects by key", err) } - if len(goCollection.Data) == 0 { + var webhook *client.GenericObject + for i := range goCollection.Data { + if goCollection.Data[i].Kind == webhookReceiverKind { + webhook = &goCollection.Data[i] + break + } + } + if webhook == nil { return 403, fmt.Errorf("Requested webhook has been revoked/does not exist for this account") } - resourceData := goCollection.Data[0].ResourceData + resourceData := webhook.ResourceData driverID, ok := resourceData["driver"].(string) if !ok { return 400, fmt.Errorf("No driver provided") @@ -123,14 +131,17 @@ func (rh *RouteHandler) ExecuteWithKey(uuid string, projectID string, request *h func validateWebhook(uuid string, apiClient *client.RancherClient) (int, error) { filters := make(map[string]interface{}) filters["key"] = uuid + filters["kind"] = webhookReceiverKind webhookCollection, err := apiClient.GenericObject.List(&client.ListOpts{ Filters: filters, }) if err != nil { return 500, err } - if len(webhookCollection.Data) > 0 { - return 0, nil + for _, webhook := range webhookCollection.Data { + if webhook.Kind == webhookReceiverKind { + return 0, nil + } } return 403, fmt.Errorf("Requested webhook has been revoked") } diff --git a/service/execute_kind_test.go b/service/execute_kind_test.go new file mode 100644 index 0000000..ebb0821 --- /dev/null +++ b/service/execute_kind_test.go @@ -0,0 +1,133 @@ +package service + +import ( + "crypto/rand" + "crypto/rsa" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/PastureStack/webhook-automation-service/model" + "github.com/rancher/go-rancher/v2" +) + +// The mock honors the key filter but intentionally ignores kind, so the +// receiver boundary is checked even if the API returns other object kinds. +type executeKindObjectList struct { + client.GenericObjectOperations + objects []client.GenericObject + filters map[string]interface{} +} + +func (m *executeKindObjectList) List(opts *client.ListOpts) (*client.GenericObjectCollection, error) { + m.filters = opts.Filters + var matched []client.GenericObject + for _, object := range m.objects { + if object.Key == opts.Filters["key"] { + matched = append(matched, object) + } + } + return &client.GenericObjectCollection{Data: matched}, nil +} + +type executeKindClientFactory struct { + objects *executeKindObjectList +} + +func (f *executeKindClientFactory) GetClient(string) (*client.RancherClient, error) { + return &client.RancherClient{GenericObject: f.objects}, nil +} + +func newExecuteKindHandler(objects ...client.GenericObject) (*RouteHandler, *executeKindObjectList) { + list := &executeKindObjectList{objects: objects} + return &RouteHandler{ClientFactory: &executeKindClientFactory{objects: list}}, list +} + +func executeKindReceiver(kind string) client.GenericObject { + return client.GenericObject{ + Key: "receiver-key", + Kind: kind, + ResourceData: map[string]interface{}{ + "driver": "scaleService", + "config": model.ScaleService{ + ServiceID: "id", + ScaleAction: "up", + ScaleChange: 1, + }, + }, + } +} + +func TestExecuteWithKeyRejectsForeignGenericObject(t *testing.T) { + handler, list := newExecuteKindHandler(executeKindReceiver("otherGenericObject")) + request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint", nil) + code, err := handler.ExecuteWithKey("receiver-key", "1a1", request) + if code != http.StatusForbidden || err == nil { + t.Fatalf("foreign object returned (%d, %v), want 403", code, err) + } + if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind { + t.Fatalf("list filters = %#v, want receiver key and kind", list.filters) + } +} + +func TestExecuteWithSignedJWTRejectsForeignGenericObject(t *testing.T) { + privateKey, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatal(err) + } + handler, list := newExecuteKindHandler(executeKindReceiver("otherGenericObject")) + handler.PublicKey = &privateKey.PublicKey + token := signTestToken(t, privateKey, "RS256", map[string]interface{}{ + "driver": "scaleService", + "projectId": "1a1", + "uuid": "receiver-key", + "exp": time.Now().Add(time.Minute).Unix(), + }) + request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint?token="+token, nil) + code, err := handler.Execute(httptest.NewRecorder(), request) + if code != http.StatusForbidden || err == nil { + t.Fatalf("signed JWT for foreign object returned (%d, %v), want 403", code, err) + } + if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind { + t.Fatalf("list filters = %#v, want receiver key and kind", list.filters) + } +} + +func TestExecuteWithKeyUsesReceiverAfterForeignGenericObject(t *testing.T) { + foreign := executeKindReceiver("otherGenericObject") + foreign.ResourceData["driver"] = "unregistered" + handler, _ := newExecuteKindHandler( + foreign, + executeKindReceiver(webhookReceiverKind), + ) + request := httptest.NewRequest(http.MethodPost, "/v1-webhooks/endpoint", nil) + code, err := handler.ExecuteWithKey("receiver-key", "1a1", request) + if code != http.StatusOK || err != nil { + t.Fatalf("valid receiver returned (%d, %v), want 200", code, err) + } +} + +func TestValidateWebhookRequiresReceiverKind(t *testing.T) { + for _, test := range []struct { + name string + objects []client.GenericObject + want int + }{ + {"foreign only", []client.GenericObject{executeKindReceiver("otherGenericObject")}, http.StatusForbidden}, + {"valid receiver", []client.GenericObject{executeKindReceiver(webhookReceiverKind)}, 0}, + {"foreign then receiver", []client.GenericObject{executeKindReceiver("otherGenericObject"), executeKindReceiver(webhookReceiverKind)}, 0}, + } { + t.Run(test.name, func(t *testing.T) { + list := &executeKindObjectList{objects: test.objects} + apiClient := &client.RancherClient{GenericObject: list} + code, err := validateWebhook("receiver-key", apiClient) + if code != test.want || (err == nil) != (test.want == 0) { + t.Fatalf("validateWebhook returned (%d, %v), want status %d", code, err, test.want) + } + if list.filters["key"] != "receiver-key" || list.filters["kind"] != webhookReceiverKind { + t.Fatalf("list filters = %#v, want receiver key and kind", list.filters) + } + }) + } +} diff --git a/service/forward_post_test.go b/service/forward_post_test.go index 6d20d97..ec53cee 100644 --- a/service/forward_post_test.go +++ b/service/forward_post_test.go @@ -59,7 +59,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"forwardPost","name":"wh-name", "forwardPostConfig": {"projectId": "1a5","serviceName": "pipeline-server", "port": "60080", "path": "/v1"}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) requireNoError(t, err) request.Header.Set("Content-Type", "application/json") @@ -79,7 +79,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) { // Test getting the created webhook by id byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL) - request, err = http.NewRequest("GET", byID, nil) + request, err = managementRequest("GET", byID, nil) requireNoError(t, err) request.Header.Set("Content-Type", "application/json") @@ -105,7 +105,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) { requireEqual(t, "execute status", response.Code, http.StatusOK) //List webhooks - requestList, err := http.NewRequest("GET", constructURL, nil) + requestList, err := managementRequest("GET", constructURL, nil) requireNoError(t, err) requestList.Header.Set("Content-Type", "application/json") @@ -126,7 +126,7 @@ func TestCreateUpdateExecuteListAndDelete(t *testing.T) { requireSelfLink(t, wh.Links["self"]) //Delete - request, err = http.NewRequest("DELETE", byID, nil) + request, err = managementRequest("DELETE", byID, nil) requireNoError(t, err) request.Header.Set("Content-Type", "application/json") diff --git a/service/framework_test.go b/service/framework_test.go index 7a43f2f..74af84d 100644 --- a/service/framework_test.go +++ b/service/framework_test.go @@ -5,6 +5,7 @@ import ( "crypto/rand" "crypto/rsa" "fmt" + "io" "io/ioutil" "net/http" "net/http/httptest" @@ -22,6 +23,19 @@ var server *httptest.Server var router *mux.Router var r *RouteHandler +// managementRequest models the control plane's authenticated proxy. Public +// webhook endpoint tests may carry this header, but do not depend on it. +func managementRequest(method, url string, body io.Reader) (*http.Request, error) { + request, err := http.NewRequest(method, url, body) + if err != nil { + return nil, err + } + if projectID := request.URL.Query().Get("projectId"); projectID != "" { + request.Header.Set(projectAPIHeader, projectID) + } + return request, nil +} + // TODO Refactor this test to use gocheck func init() { drivers.Drivers = map[string]drivers.WebhookDriver{} @@ -86,7 +100,7 @@ func init() { func TestMissingProjectIdHeader(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks", server.URL) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`))) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`))) if err != nil { t.Fatal(err) } @@ -107,7 +121,7 @@ func TestMissingProjectIdHeader(t *testing.T) { func TestMissingContentTypeHeader(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks?projectId=1a1", server.URL) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`))) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer([]byte(`{}`))) if err != nil { t.Fatal(err) } diff --git a/service/handlers.go b/service/handlers.go index c7fa10c..38d70fc 100644 --- a/service/handlers.go +++ b/service/handlers.go @@ -1,8 +1,10 @@ package service import ( + "errors" "fmt" "net/http" + "strings" "github.com/PastureStack/webhook-automation-service/drivers" "github.com/PastureStack/webhook-automation-service/model" @@ -14,7 +16,9 @@ import ( ) const ( - RoleAPIHeader = "X-API-Roles" + RoleAPIHeader = "X-API-Roles" + projectAPIHeader = "X-API-Project-Id" + webhookReceiverKind = "webhookReceiver" ) var readonlyRoles = map[string]bool{ @@ -36,12 +40,18 @@ func (rh *RouteHandler) ListWebhooks(w http.ResponseWriter, r *http.Request) (in return 500, err } filters := make(map[string]interface{}) - filters["kind"] = "webhookReceiver" + filters["kind"] = webhookReceiverKind objs, err := apiClient.GenericObject.List(&client.ListOpts{ Filters: filters, }) + if err != nil { + return http.StatusInternalServerError, err + } response := []model.Webhook{} for _, obj := range objs.Data { + if obj.Kind != webhookReceiverKind { + continue + } webhook, err := rh.convertToWebhookGenericObject(obj) if err != nil { logrus.Warnf("Skipping webhook %s because: %v", obj.Id, err) @@ -60,7 +70,7 @@ func (rh *RouteHandler) ListWebhooks(w http.ResponseWriter, r *http.Request) (in continue } // we will hide the url to prevent readonly and restricted users to access endpoint - if readonlyRoles[getRoles(r)] { + if hasReadonlyRole(r) { respWebhook.URL = "" } @@ -95,7 +105,7 @@ func (rh *RouteHandler) GetWebhook(w http.ResponseWriter, r *http.Request) (int, return 500, err } - if obj == nil { + if obj == nil || obj.Kind != webhookReceiverKind { return 404, fmt.Errorf("Webhook not found") } @@ -115,7 +125,7 @@ func (rh *RouteHandler) GetWebhook(w http.ResponseWriter, r *http.Request) (int, return 500, fmt.Errorf("create webhook response: %w", err) } // we will hide the url to prevent readonly and restricted users to access endpoint - if readonlyRoles[getRoles(r)] { + if hasReadonlyRole(r) { respWebhook.URL = "" } @@ -124,7 +134,7 @@ func (rh *RouteHandler) GetWebhook(w http.ResponseWriter, r *http.Request) (int, } func (rh *RouteHandler) DeleteWebhook(w http.ResponseWriter, r *http.Request) (int, error) { - if readonlyRoles[getRoles(r)] { + if hasReadonlyRole(r) { return http.StatusMethodNotAllowed, fmt.Errorf("user doesn't have the access to delete webhook") } vars := mux.Vars(r) @@ -144,14 +154,17 @@ func (rh *RouteHandler) DeleteWebhook(w http.ResponseWriter, r *http.Request) (i return 500, err } - if obj == nil { + if obj == nil || obj.Kind != webhookReceiverKind { return 404, fmt.Errorf("Webhook not found") } err = apiClient.GenericObject.Delete(obj) if err != nil { - statusCode := err.(*client.ApiError).StatusCode - return statusCode, err + var apiErr *client.ApiError + if errors.As(err, &apiErr) { + return apiErr.StatusCode, err + } + return http.StatusInternalServerError, err } return 204, nil } @@ -161,6 +174,16 @@ func getProjectID(r *http.Request) (string, int, error) { if projectID == "" { return "", 400, fmt.Errorf("projectId must be supplied as query parameter") } + // The authenticated control-plane proxy supplies the effective project. + // Never let a different query project select this service's privileged API + // client after the caller was authorized for the header project. + authorized := r.Header.Get(projectAPIHeader) + if authorized == "" { + return "", http.StatusForbidden, fmt.Errorf("authorized project is required") + } + if authorized != projectID { + return "", http.StatusForbidden, fmt.Errorf("projectId does not match the authorized project") + } return projectID, 0, nil } @@ -231,18 +254,28 @@ func (rh *RouteHandler) convertToWebhookGenericObject(genericObject client.Gener func (rh *RouteHandler) isUniqueName(webhookName string, projectID string, apiClient *client.RancherClient) (int, error) { filters := make(map[string]interface{}) filters["name"] = webhookName + filters["kind"] = webhookReceiverKind obj, err := apiClient.GenericObject.List(&client.ListOpts{ Filters: filters, }) if err != nil { return 500, err } - if len(obj.Data) > 0 { - return 400, fmt.Errorf("Cannot have duplicate webhook name, webhook %s already exists", webhookName) + for _, existing := range obj.Data { + if existing.Kind == webhookReceiverKind && existing.Name == webhookName { + return 400, fmt.Errorf("Cannot have duplicate webhook name, webhook %s already exists", webhookName) + } } return 200, nil } -func getRoles(r *http.Request) string { - return r.Header.Get(RoleAPIHeader) +func hasReadonlyRole(r *http.Request) bool { + for _, header := range r.Header.Values(RoleAPIHeader) { + for _, role := range strings.Split(header, ",") { + if readonlyRoles[strings.ToLower(strings.TrimSpace(role))] { + return true + } + } + } + return false } diff --git a/service/receiver_boundary_test.go b/service/receiver_boundary_test.go new file mode 100644 index 0000000..c5d546f --- /dev/null +++ b/service/receiver_boundary_test.go @@ -0,0 +1,146 @@ +package service + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/rancher/go-rancher/v2" +) + +func TestReceiverDirectIDRejectsOtherGenericObjectKinds(t *testing.T) { + mock := r.ClientFactory.(*MockAPIClientFactory).mw + id := "wrong-kind-fixture" + foreign := &client.GenericObject{ + Kind: "otherGenericObject", + ResourceData: map[string]interface{}{ + "driver": "scaleService", "url": "http://example.invalid", + "config": map[string]interface{}{}, + }, + } + foreign.Id = id + mock.created[id] = foreign + t.Cleanup(func() { delete(mock.created, id) }) + + for _, method := range []string{http.MethodGet, http.MethodDelete} { + request := httptest.NewRequest(method, + "/v1-webhooks/receivers/"+id+"?projectId=1a1", nil) + request.Header.Set(projectAPIHeader, "1a1") + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusNotFound { + t.Fatalf("%s wrong-kind ID returned %d, want 404", method, response.Code) + } + if mock.created[id] != foreign { + t.Fatalf("%s changed an unrelated generic object", method) + } + } +} + +func TestReceiverListNeverSerializesOtherGenericObjectKinds(t *testing.T) { + mock := r.ClientFactory.(*MockAPIClientFactory).mw + id := "wrong-kind-list-fixture" + foreign := &client.GenericObject{ + Kind: "otherGenericObject", + ResourceData: map[string]interface{}{ + "driver": "scaleService", "url": "http://example.invalid", + "config": map[string]interface{}{}, + }, + } + foreign.Id = id + mock.created[id] = foreign + t.Cleanup(func() { delete(mock.created, id) }) + request := httptest.NewRequest(http.MethodGet, "/v1-webhooks/receivers?projectId=1a1", nil) + request.Header.Set(projectAPIHeader, "1a1") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusOK { + t.Fatalf("receiver list returned %d, want 200", response.Code) + } + var payload struct { + Data []struct { + ID string `json:"id"` + } `json:"data"` + } + if err := json.Unmarshal(response.Body.Bytes(), &payload); err != nil { + t.Fatal(err) + } + for _, item := range payload.Data { + if item.ID == id { + t.Fatal("receiver list serialized a foreign generic object") + } + } +} + +func TestReceiverNameUniquenessIgnoresOtherGenericObjectKinds(t *testing.T) { + mock := r.ClientFactory.(*MockAPIClientFactory).mw + id := "wrong-kind-name-fixture" + foreign := &client.GenericObject{Kind: "otherGenericObject", Name: "shared-name-fixture"} + foreign.Id = id + mock.created[id] = foreign + t.Cleanup(func() { delete(mock.created, id) }) + apiClient, err := r.ClientFactory.GetClient("1a1") + if err != nil { + t.Fatal(err) + } + code, err := r.isUniqueName(foreign.Name, "1a1", apiClient) + if err != nil || code != http.StatusOK { + t.Fatalf("foreign generic object blocked a receiver name: code=%d err=%v", code, err) + } + receiver := &client.GenericObject{Kind: webhookReceiverKind, Name: foreign.Name} + receiver.Id = "same-kind-name-fixture" + mock.created[receiver.Id] = receiver + t.Cleanup(func() { delete(mock.created, receiver.Id) }) + code, err = r.isUniqueName(receiver.Name, "1a1", apiClient) + if err == nil || code != http.StatusBadRequest { + t.Fatalf("duplicate receiver name was accepted: code=%d err=%v", code, err) + } +} + +func TestReceiverRejectsMismatchedAuthorizedProject(t *testing.T) { + mock := r.ClientFactory.(*MockAPIClientFactory).mw + id := "project-mismatch-fixture" + receiver := &client.GenericObject{Kind: webhookReceiverKind} + receiver.Id = id + mock.created[id] = receiver + t.Cleanup(func() { delete(mock.created, id) }) + + for _, method := range []string{http.MethodGet, http.MethodDelete} { + request := httptest.NewRequest(method, + "/v1-webhooks/receivers/"+id+"?projectId=1a2", nil) + request.Header.Set(projectAPIHeader, "1a1") + request.Header.Set("Content-Type", "application/json") + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusForbidden { + t.Fatalf("%s mismatched project returned %d, want 403", method, response.Code) + } + if mock.created[id] != receiver { + t.Fatalf("%s changed a receiver from another project", method) + } + } +} + +func TestReceiverRejectsMissingAuthorizedProject(t *testing.T) { + mock := r.ClientFactory.(*MockAPIClientFactory).mw + id := "missing-project-header-fixture" + receiver := &client.GenericObject{Kind: webhookReceiverKind} + receiver.Id = id + mock.created[id] = receiver + t.Cleanup(func() { delete(mock.created, id) }) + + for _, method := range []string{http.MethodGet, http.MethodDelete} { + request := httptest.NewRequest(method, + "/v1-webhooks/receivers/"+id+"?projectId=1a1", nil) + response := httptest.NewRecorder() + router.ServeHTTP(response, request) + if response.Code != http.StatusForbidden { + t.Fatalf("%s missing trusted project returned %d, want 403", method, response.Code) + } + if mock.created[id] != receiver { + t.Fatalf("%s changed a receiver without trusted project", method) + } + } +} diff --git a/service/roles_boundary_test.go b/service/roles_boundary_test.go new file mode 100644 index 0000000..734ae0a --- /dev/null +++ b/service/roles_boundary_test.go @@ -0,0 +1,138 @@ +package service + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/PastureStack/webhook-automation-service/model" + "github.com/rancher/go-rancher/v2" +) + +func roleBoundaryRequest(method, path, roles, body string) *http.Request { + request := httptest.NewRequest(method, path, strings.NewReader(body)) + request.Header.Set(projectAPIHeader, "1a1") + request.Header.Set(RoleAPIHeader, roles) + request.Header.Set("Content-Type", "application/json") + return request +} + +func roleBoundaryReceiver() *client.GenericObject { + object := &client.GenericObject{ + Kind: webhookReceiverKind, + ResourceData: map[string]interface{}{ + "driver": "scaleService", + "url": "http://example.invalid/receiver", + "config": model.ScaleService{ + ServiceID: "id", + ScaleAction: "up", + ScaleChange: 1, + Min: 1, + Max: 4, + }, + }, + } + object.Id = "role-boundary-receiver" + return object +} + +func TestCombinedReadonlyRolesBlockReceiverWritesAndHideURL(t *testing.T) { + for _, roles := range []string{ + "owner,readonly", + "member, restricted", + "v1-readonly,owner", + "member,v1-restricted", + } { + t.Run(roles, func(t *testing.T) { + receiver := roleBoundaryReceiver() + objects := &mockGenericObject{created: map[string]*client.GenericObject{receiver.Id: receiver}} + handler := NewRouter(&RouteHandler{ClientFactory: &MockAPIClientFactory{mw: objects}}) + collectionPath := "/v1-webhooks/receivers?projectId=1a1" + receiverPath := "/v1-webhooks/receivers/" + receiver.Id + "?projectId=1a1" + + response := httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodPost, collectionPath, roles, `{}`)) + if response.Code != http.StatusMethodNotAllowed { + t.Fatalf("create returned %d, want 405", response.Code) + } + + response = httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodGet, receiverPath, roles, "")) + if response.Code != http.StatusOK { + t.Fatalf("get returned %d, want 200", response.Code) + } + var got model.Webhook + if err := json.Unmarshal(response.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got.URL != "" { + t.Fatalf("get exposed receiver URL to roles %q", roles) + } + + response = httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodGet, collectionPath, roles, "")) + if response.Code != http.StatusOK { + t.Fatalf("list returned %d, want 200", response.Code) + } + var listed model.WebhookCollection + if err := json.Unmarshal(response.Body.Bytes(), &listed); err != nil { + t.Fatal(err) + } + if len(listed.Data) != 1 || listed.Data[0].URL != "" { + t.Fatalf("list exposed receiver URL to roles %q: %#v", roles, listed.Data) + } + + response = httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodDelete, receiverPath, roles, "")) + if response.Code != http.StatusMethodNotAllowed { + t.Fatalf("delete returned %d, want 405", response.Code) + } + if objects.created[receiver.Id] != receiver { + t.Fatal("readonly request changed the receiver") + } + }) + } +} + +func TestCombinedOwnerMemberRolesCanManageReceiver(t *testing.T) { + objects := &mockGenericObject{created: map[string]*client.GenericObject{}} + handler := NewRouter(&RouteHandler{ClientFactory: &MockAPIClientFactory{mw: objects}}) + roles := "owner, member" + collectionPath := "/v1-webhooks/receivers?projectId=1a1" + createBody := `{"driver":"scaleService","name":"role-boundary",` + + `"scaleServiceConfig":{"serviceId":"id","amount":1,"action":"up","min":1,"max":4}}` + + response := httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodPost, collectionPath, roles, createBody)) + if response.Code != http.StatusOK { + t.Fatalf("owner/member create returned %d, want 200: %s", response.Code, response.Body.String()) + } + receiverPath := "/v1-webhooks/receivers/1?projectId=1a1" + response = httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodGet, receiverPath, roles, "")) + if response.Code != http.StatusOK { + t.Fatalf("owner/member get returned %d, want 200", response.Code) + } + var got model.Webhook + if err := json.Unmarshal(response.Body.Bytes(), &got); err != nil { + t.Fatal(err) + } + if got.URL == "" { + t.Fatal("owner/member receiver URL was hidden") + } + response = httptest.NewRecorder() + handler.ServeHTTP(response, roleBoundaryRequest(http.MethodDelete, receiverPath, roles, "")) + if response.Code != http.StatusNoContent { + t.Fatalf("owner/member delete returned %d, want 204", response.Code) + } +} + +func TestRepeatedRoleHeadersHonorReadonlyRole(t *testing.T) { + request := roleBoundaryRequest(http.MethodGet, "/", "owner", "") + request.Header.Add(RoleAPIHeader, "restricted") + if !hasReadonlyRole(request) { + t.Fatal("a second role header bypassed the readonly guard") + } +} diff --git a/service/scale_host_test.go b/service/scale_host_test.go index db62130..c255a71 100644 --- a/service/scale_host_test.go +++ b/service/scale_host_test.go @@ -25,7 +25,7 @@ func TestWebhookCreateAndExecuteScaleHost(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": 1, "max": 4, "deleteOption": "mostRecent"}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -56,7 +56,7 @@ func TestWebhookCreateAndExecuteScaleHost(t *testing.T) { // Test getting the created webhook by id byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL) - request, err = http.NewRequest("GET", byID, nil) + request, err = managementRequest("GET", byID, nil) if err != nil { t.Fatal(err) } @@ -98,7 +98,7 @@ func TestWebhookCreateAndExecuteScaleHost(t *testing.T) { } //List webhooks - requestList, err := http.NewRequest("GET", constructURL, nil) + requestList, err := managementRequest("GET", constructURL, nil) if err != nil { t.Fatal(err) } @@ -131,7 +131,7 @@ func TestWebhookCreateAndExecuteScaleHost(t *testing.T) { } //Delete - request, err = http.NewRequest("DELETE", byID, nil) + request, err = managementRequest("DELETE", byID, nil) if err != nil { t.Fatal(err) } @@ -147,7 +147,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": -1, "max": 4, "deleteOption": "mostRecent"}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -161,7 +161,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": 1, "max": -4, "deleteOption": "mostRecent"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -175,7 +175,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1.5, "action": "up", "min": 1, "max": 4, "deleteOption": "mostRecent"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -189,7 +189,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": 1.5, "max": 4, "deleteOption": "mostRecent"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -203,7 +203,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": 1, "max": 4.5, "deleteOption": "mostRecent"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -217,7 +217,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "up", "min": 1, "max": 4, "deleteOption": "random"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -231,7 +231,7 @@ func TestWebhookCreateInvalidMinMaxActionScaleHost(t *testing.T) { jsonStr = []byte(`{"driver":"scaleHost","name":"wh-name", "scaleHostConfig": {"hostSelector": {"foo": "bar"}, "amount": 1, "action": "random", "min": 1, "max": 4, "deleteOption": "mostRecent"}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } diff --git a/service/scale_service_test.go b/service/scale_service_test.go index 751fe0c..16744b7 100644 --- a/service/scale_service_test.go +++ b/service/scale_service_test.go @@ -23,7 +23,7 @@ func TestWebhookCreateAndExecuteScaleService(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": 1, "max": 4}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -62,7 +62,7 @@ func TestWebhookCreateAndExecuteScaleService(t *testing.T) { // Test getting the created webhook by id byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL) - request, err = http.NewRequest("GET", byID, nil) + request, err = managementRequest("GET", byID, nil) if err != nil { t.Fatal(err) } @@ -104,7 +104,7 @@ func TestWebhookCreateAndExecuteScaleService(t *testing.T) { } // //List webhooks - requestList, err := http.NewRequest("GET", constructURL, nil) + requestList, err := managementRequest("GET", constructURL, nil) if err != nil { t.Fatal(err) } @@ -137,7 +137,7 @@ func TestWebhookCreateAndExecuteScaleService(t *testing.T) { } // //Delete - request, err = http.NewRequest("DELETE", byID, nil) + request, err = managementRequest("DELETE", byID, nil) if err != nil { t.Fatal(err) } @@ -181,7 +181,7 @@ func TestWebhookInvalidMinMaxActionScaleService(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": -1, "max": 4}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -195,7 +195,7 @@ func TestWebhookInvalidMinMaxActionScaleService(t *testing.T) { jsonStr = []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": 1, "max": -4}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -209,7 +209,7 @@ func TestWebhookInvalidMinMaxActionScaleService(t *testing.T) { jsonStr = []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1.5, "action": "up", "min": 1, "max": 4}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -223,7 +223,7 @@ func TestWebhookInvalidMinMaxActionScaleService(t *testing.T) { jsonStr = []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": 1.5, "max": 4}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -237,7 +237,7 @@ func TestWebhookInvalidMinMaxActionScaleService(t *testing.T) { jsonStr = []byte(`{"driver":"scaleService","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": 1, "max": 4.5}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -254,7 +254,7 @@ func TestCreateWithInvalidDriver(t *testing.T) { constructURL := fmt.Sprintf("%s/v1-webhooks/receivers?projectId=1a1", server.URL) jsonStr := []byte(`{"driver":"driverInvalid","name":"wh-name", "scaleServiceConfig": {"serviceId": "id", "amount": 1, "action": "up", "min": -1, "max": 4}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } diff --git a/service/upgrade_service_test.go b/service/upgrade_service_test.go index b438343..dbf53c6 100644 --- a/service/upgrade_service_test.go +++ b/service/upgrade_service_test.go @@ -27,7 +27,7 @@ func TestWebhookCreateAndExecuteServiceUpgrade(t *testing.T) { jsonStr := []byte(`{"driver":"serviceUpgrade","name":"wh-name", "serviceUpgradeConfig": {"serviceSelector": {"foo": "bar"}, "tag": "wh-tag", "batchSize": 1, "intervalMillis":2, "startFirst": true}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -63,7 +63,7 @@ func TestWebhookCreateAndExecuteServiceUpgrade(t *testing.T) { // Test getting the created webhook by id byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL) - request, err = http.NewRequest("GET", byID, nil) + request, err = managementRequest("GET", byID, nil) if err != nil { t.Fatal(err) } @@ -105,7 +105,7 @@ func TestWebhookCreateAndExecuteServiceUpgrade(t *testing.T) { } //List webhooks - requestList, err := http.NewRequest("GET", constructURL, nil) + requestList, err := managementRequest("GET", constructURL, nil) if err != nil { t.Fatal(err) } @@ -138,7 +138,7 @@ func TestWebhookCreateAndExecuteServiceUpgrade(t *testing.T) { } //Delete - request, err = http.NewRequest("DELETE", byID, nil) + request, err = managementRequest("DELETE", byID, nil) if err != nil { t.Fatal(err) } @@ -179,7 +179,7 @@ func TestWebhookTag(t *testing.T) { jsonStr := []byte(`{"driver":"serviceUpgrade","name":"` + name + `", "serviceUpgradeConfig": {"serviceSelector": {"foo": "bar"}, "tag": "` + tag + `", "batchSize": 1, "intervalMillis":2, "startFirst": true}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -191,7 +191,7 @@ func TestWebhookTag(t *testing.T) { t.Fatalf("Tag is valid") } byID := fmt.Sprintf("%s/v1-webhooks/receivers/1?projectId=1a1", server.URL) - request, err = http.NewRequest("DELETE", byID, nil) + request, err = managementRequest("DELETE", byID, nil) if err != nil { t.Fatal(err) } @@ -208,7 +208,7 @@ func TestWebhookTag(t *testing.T) { jsonStr := []byte(`{"driver":"serviceUpgrade","name":"` + name + `", "serviceUpgradeConfig": {"serviceSelector": {"foo": "bar"}, "tag": "` + tag + `", "batchSize": 1, "intervalMillis":2, "startFirst": true}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -228,7 +228,7 @@ func TestWebhookInvalidBatchSizeInterval(t *testing.T) { jsonStr := []byte(`{"driver":"serviceUpgrade","name":"wh-name", "serviceUpgradeConfig": {"serviceSelector": {"foo": "bar"}, "tag": "wh-tag", "batchSize": 0, "intervalMillis":2, "startFirst": true}}`) - request, err := http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err := managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) } @@ -243,7 +243,7 @@ func TestWebhookInvalidBatchSizeInterval(t *testing.T) { jsonStr = []byte(`{"driver":"serviceUpgrade","name":"wh-name", "serviceUpgradeConfig": {"serviceSelector": {"foo": "bar"}, "tag": "wh-tag", "batchSize": 1, "intervalMillis":0, "startFirst": true}}`) - request, err = http.NewRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) + request, err = managementRequest("POST", constructURL, bytes.NewBuffer(jsonStr)) if err != nil { t.Fatal(err) }