From f7a46f972f1a35f38002f248a34fb75b77047aaa Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Sun, 10 May 2020 22:07:03 -0400 Subject: [PATCH 1/2] support other subject options --- src/e2e/e2etest.go | 30 ++++++++++++++++++++++- src/route/handlers.go | 44 ++++++++++++++++++++++++++++------ src/unit-test/handlers_test.go | 7 ++++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/src/e2e/e2etest.go b/src/e2e/e2etest.go index 5c9fd8c..b79e13e 100644 --- a/src/e2e/e2etest.go +++ b/src/e2e/e2etest.go @@ -84,11 +84,12 @@ func addWebhookToDb() string { log.Fatal("Invalid topic config ", err) } + // test 403 add a configuration under another topic reqJSON, err := json.Marshal(topicConfig) if err != nil { log.Fatal("Topic marshalling error Error reading request. ", err) } - log.Println("create topic and webhook with REST call") + log.Println("create topic under another tenant and webhook with REST call") req, err := http.NewRequest("POST", restURL, bytes.NewBuffer(reqJSON)) if err != nil { log.Fatal("Error reading request. ", err) @@ -106,6 +107,33 @@ func addWebhookToDb() string { } defer resp.Body.Close() + log.Printf("post call to rest API statusCode %d", resp.StatusCode) + eval(resp.StatusCode == http.StatusForbidden, "expected rest api status code is 403") + + // successful test with matching subject and tenant + topicConfig.TopicFullName = webhookTopic + reqJSON, err = json.Marshal(topicConfig) + if err != nil { + log.Fatal("Topic marshalling error Error reading request. ", err) + } + log.Println("create topic and webhook with REST call") + req, err = http.NewRequest("POST", restURL, bytes.NewBuffer(reqJSON)) + if err != nil { + log.Fatal("Error reading request. ", err) + } + + req.Header.Set("Authorization", restAPIToken) + + // Set client timeout + client = &http.Client{Timeout: time.Second * 10} + + // Send request + resp, err = client.Do(req) + if err != nil { + log.Fatal("Error reading response from Beam. ", err) + } + defer resp.Body.Close() + log.Printf("post call to rest API statusCode %d", resp.StatusCode) eval(resp.StatusCode == 201, "expected rest api status code is 201") return topicConfig.Key diff --git a/src/route/handlers.go b/src/route/handlers.go index 2cd9c6d..c3956e6 100644 --- a/src/route/handlers.go +++ b/src/route/handlers.go @@ -78,6 +78,10 @@ func ReceiveHandler(w http.ResponseWriter, r *http.Request) { util.ResponseErrorJSON(err, w, http.StatusServiceUnavailable) return } + if pulsarAsync { + w.WriteHeader(http.StatusAccepted) + return + } w.WriteHeader(http.StatusOK) return } @@ -125,13 +129,13 @@ func UpdateTopicHandler(w http.ResponseWriter, r *http.Request) { return } - if _, err = model.ValidateTopicConfig(doc); err != nil { - util.ResponseErrorJSON(err, w, http.StatusUnprocessableEntity) + if !VerifySubjectBasedOnTopic(doc.TopicFullName, r.Header.Get("injectedSubs")) { + w.WriteHeader(http.StatusForbidden) return } - if !VerifySubjectBasedOnTopic(doc.TopicFullName, r.Header.Get("injectedSubs")) { - w.WriteHeader(http.StatusForbidden) + if _, err = model.ValidateTopicConfig(doc); err != nil { + util.ResponseErrorJSON(err, w, http.StatusUnprocessableEntity) return } @@ -234,15 +238,17 @@ func VerifySubjectBasedOnTopic(topicFN, tokenSub string) bool { // VerifySubject verifies the subject can meet the requirement. // Subject verification requires role or tenant name in the jwt subject func VerifySubject(requiredSubject, tokenSubjects string) bool { - for _, v := range strings.Split(tokenSubjects, ",") { + for _, pv := range strings.Split(tokenSubjects, ",") { + v := strings.TrimSpace(pv) + if util.StrContains(util.SuperRoles, v) { return true } if requiredSubject == v { return true } - sub := extractTenant(v) - if sub != "" && requiredSubject == sub { + subCase1, subCase2 := ExtractTenant(v) + if true == (requiredSubject == subCase1 || requiredSubject == subCase2) { return true } } @@ -250,6 +256,30 @@ func VerifySubject(requiredSubject, tokenSubjects string) bool { return false } +// ExtractTenant attempts to extract tenant based on delimiter `-` and `-client-` +// so that it will covercases such as 1. my-tenant-12345qbc +// 2. my-tenant-client-12345qbc +// 3. my-tenant +// 4. my-tenant-client-client-12345qbc +func ExtractTenant(tokenSub string) (string, string) { + var case1 string + // expect `-` in subject unless it is superuser, or admin + // so return them as is + parts := strings.Split(tokenSub, subDelimiter) + if len(parts) < 2 { + return tokenSub, tokenSub + } + + // cases to cover with only `-` as delimiter + validLength := len(parts) - 1 + case1 = strings.Join(parts[:validLength], subDelimiter) + + if parts[validLength-1] == "client" { + return case1, strings.Join(parts[:(validLength-1)], subDelimiter) + } + return case1, case1 +} + func extractTenant(tokenSub string) string { // expect - in subject unless it is superuser parts := strings.Split(tokenSub, subDelimiter) diff --git a/src/unit-test/handlers_test.go b/src/unit-test/handlers_test.go index 23a21a7..e37411e 100644 --- a/src/unit-test/handlers_test.go +++ b/src/unit-test/handlers_test.go @@ -168,6 +168,13 @@ func TestSubjectMatch(t *testing.T) { assert(t, VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "picasso-1234,myadmin"), "") assert(t, !VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "picaso-1234,myadmin"), "") + // support -client- subject + assert(t, VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "picasso-client-1234"), "") + assert(t, VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "ad, picasso-client-1234"), "") + assert(t, VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "picasso-client-1234, admin"), "") + assert(t, VerifySubjectBasedOnTopic("persistent://picasso-client/local-useast1-gcp/yet-another-test-topic", "picasso-client-client-1234, admin"), "") + assert(t, !VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "picasso-client-client-1234, admin"), "") + originalSuperRoles := util.SuperRoles util.SuperRoles = []string{} assert(t, !VerifySubjectBasedOnTopic("persistent://picasso/local-useast1-gcp/yet-another-test-topic", "superuser"), "") From c1d7ba14ca8e6edbc1a41c6475fac49494cedb5c Mon Sep 17 00:00:00 2001 From: Ming Luo Date: Sun, 10 May 2020 22:42:16 -0400 Subject: [PATCH 2/2] move failure path test from e2e to unit-test --- src/e2e/e2etest.go | 30 +----------------------------- src/unit-test/handlers_test.go | 13 +++++++++++++ 2 files changed, 14 insertions(+), 29 deletions(-) diff --git a/src/e2e/e2etest.go b/src/e2e/e2etest.go index b79e13e..5c9fd8c 100644 --- a/src/e2e/e2etest.go +++ b/src/e2e/e2etest.go @@ -84,12 +84,11 @@ func addWebhookToDb() string { log.Fatal("Invalid topic config ", err) } - // test 403 add a configuration under another topic reqJSON, err := json.Marshal(topicConfig) if err != nil { log.Fatal("Topic marshalling error Error reading request. ", err) } - log.Println("create topic under another tenant and webhook with REST call") + log.Println("create topic and webhook with REST call") req, err := http.NewRequest("POST", restURL, bytes.NewBuffer(reqJSON)) if err != nil { log.Fatal("Error reading request. ", err) @@ -107,33 +106,6 @@ func addWebhookToDb() string { } defer resp.Body.Close() - log.Printf("post call to rest API statusCode %d", resp.StatusCode) - eval(resp.StatusCode == http.StatusForbidden, "expected rest api status code is 403") - - // successful test with matching subject and tenant - topicConfig.TopicFullName = webhookTopic - reqJSON, err = json.Marshal(topicConfig) - if err != nil { - log.Fatal("Topic marshalling error Error reading request. ", err) - } - log.Println("create topic and webhook with REST call") - req, err = http.NewRequest("POST", restURL, bytes.NewBuffer(reqJSON)) - if err != nil { - log.Fatal("Error reading request. ", err) - } - - req.Header.Set("Authorization", restAPIToken) - - // Set client timeout - client = &http.Client{Timeout: time.Second * 10} - - // Send request - resp, err = client.Do(req) - if err != nil { - log.Fatal("Error reading response from Beam. ", err) - } - defer resp.Body.Close() - log.Printf("post call to rest API statusCode %d", resp.StatusCode) eval(resp.StatusCode == 201, "expected rest api status code is 201") return topicConfig.Key diff --git a/src/unit-test/handlers_test.go b/src/unit-test/handlers_test.go index e37411e..112c8d4 100644 --- a/src/unit-test/handlers_test.go +++ b/src/unit-test/handlers_test.go @@ -69,6 +69,19 @@ func TestTopicHandler(t *testing.T) { handler.ServeHTTP(rr, req) equals(t, http.StatusCreated, rr.Code) + // test create topic config under a different tenant + topic.TopicFullName = "persistent://another-tenant/local-useast1-gcp/yet-another-test-topic" + reqJSON, err = json.Marshal(topic) + errNil(t, err) + // test create a new topic + req, err = http.NewRequest(http.MethodPost, "/v2/topic", bytes.NewReader(reqJSON)) + errNil(t, err) + + handler = http.HandlerFunc(UpdateTopicHandler) + + handler.ServeHTTP(rr, req) + equals(t, http.StatusForbidden, rr.Code) + // test update the newly created topic req, err = http.NewRequest(http.MethodPost, "/v2/topic", bytes.NewReader(reqJSON)) errNil(t, err)