Repository: avenga/couper
Pull request #609 (2022-10-21 08:59:14)

Reqauth refinements
* Don't merge backend body with token request body
* Only append backend block to oauth2/beta_token_request block if necessary
* Check if referenced/refined backend is defined

---
<details>
    <summary>Reviewer checklist</summary>
    <ul>
        <li>Read PR description: a summary about the changes is required</li>
        <li>Changelog updated</li>
        <li>Documentation: docs/{Reference, Cli, ...}, Docker and cli help/usage</li>
        <li>Pulled branch, manually tested</li>
        <li>Verified requirements are met</li>
        <li>Reviewed the code</li>
        <li>Reviewed the related tests</li>
    </ul>
</details>



Patch:
diff --git a/config/configload/backend.go b/config/configload/backend.go
index 1939aebc..428b453e 100644
--- a/config/configload/backend.go
+++ b/config/configload/backend.go
@@ -62,11 +62,11 @@ func PrepareBackend(helper *helper, attrName, attrValue string, block config.Bod
 	if reference != "" {
 		refBody, ok := helper.defsBackends[reference]
 		if !ok {
-			r := block.HCLBody().MissingItemRange()
+			r := block.HCLBody().SrcRange
 			if backendBody != nil {
-				r = backendBody.MissingItemRange()
+				r = backendBody.SrcRange
 			}
-			return nil, newDiagErr(&r, "backend reference is not defined: "+reference)
+			return nil, newDiagErr(&r, fmt.Sprintf("referenced backend %q is not defined", reference))
 		}
 
 		if backendBody == nil {
@@ -168,11 +168,14 @@ func setOAuth2Backend(helper *helper, parent *hclsyntax.Body) (*hclsyntax.Body,
 		return nil, err
 	}
 
-	backendBlock := &hclsyntax.Block{
-		Type: backend,
-		Body: backendBody,
+	if len(hclbody.BlocksOfType(oauthBody, backend)) == 0 {
+		// only add backend block, if not already there
+		backendBlock := &hclsyntax.Block{
+			Type: backend,
+			Body: backendBody,
+		}
+		oauthBody.Blocks = append(oauthBody.Blocks, backendBlock)
 	}
-	oauthBody.Blocks = append(oauthBody.Blocks, backendBlock)
 
 	return parent, nil
 }
@@ -223,7 +226,7 @@ func setTokenRequestBackend(helper *helper, parent *hclsyntax.Body) (*hclsyntax.
 			return nil, diags
 		}
 
-		if err := verifyBodyAttributes(tokenRequest, tokenRequestBody); err != nil {
+		if err = verifyBodyAttributes(tokenRequest, tokenRequestBody); err != nil {
 			return nil, err
 		}
 
@@ -235,11 +238,14 @@ func setTokenRequestBackend(helper *helper, parent *hclsyntax.Body) (*hclsyntax.
 			return nil, berr
 		}
 
-		backendBlock := &hclsyntax.Block{
-			Type: backend,
-			Body: hclbody.MergeBodies(backendBody, tokenRequestBody, false),
+		if len(hclbody.BlocksOfType(tokenRequestBody, backend)) == 0 {
+			// only add backend block, if not already there
+			backendBlock := &hclsyntax.Block{
+				Type: backend,
+				Body: backendBody,
+			}
+			tokenRequestBody.Blocks = append(tokenRequestBody.Blocks, backendBlock)
 		}
-		tokenRequestBody.Blocks = append(tokenRequestBody.Blocks, backendBlock)
 	}
 
 	return parent, nil
diff --git a/config/configload/endpoint.go b/config/configload/endpoint.go
index e57c2ec7..2e5aede5 100644
--- a/config/configload/endpoint.go
+++ b/config/configload/endpoint.go
@@ -50,7 +50,7 @@ func refineEndpoints(helper *helper, endpoints config.Endpoints, check bool) err
 		}
 
 		if check && len(endpoint.Proxies)+len(endpoint.Requests) == 0 && endpoint.Response == nil {
-			r := endpointBody.MissingItemRange()
+			r := endpointBody.SrcRange
 			return newDiagErr(&r,
 				"missing 'default' proxy or request block, or a response definition",
 			)
@@ -60,7 +60,7 @@ func refineEndpoints(helper *helper, endpoints config.Endpoints, check bool) err
 
 		names := map[string]*hclsyntax.Body{}
 		unique := map[string]struct{}{}
-		subject := endpoint.Remain.MissingItemRange()
+		subject := endpoint.HCLBody().SrcRange
 
 		for _, proxyConfig := range endpoint.Proxies {
 			if proxyConfig.Name == "" {
@@ -69,6 +69,7 @@ func refineEndpoints(helper *helper, endpoints config.Endpoints, check bool) err
 
 			names[proxyConfig.Name] = proxyConfig.HCLBody()
 
+			subject = proxyConfig.HCLBody().SrcRange
 			if err = validLabel(proxyConfig.Name, &subject); err != nil {
 				return err
 			}
@@ -110,6 +111,7 @@ func refineEndpoints(helper *helper, endpoints config.Endpoints, check bool) err
 
 			names[reqConfig.Name] = reqConfig.HCLBody()
 
+			subject = reqConfig.HCLBody().SrcRange
 			if err = validLabel(reqConfig.Name, &subject); err != nil {
 				return err
 			}
diff --git a/config/configload/helper.go b/config/configload/helper.go
index a2bfe4bf..fd634726 100644
--- a/config/configload/helper.go
+++ b/config/configload/helper.go
@@ -68,7 +68,10 @@ func (h *helper) configureDefinedBackends() error {
 	}
 
 	for _, name := range backendNames {
-		b := h.defsBackends[name]
+		b, set := h.defsBackends[name]
+		if !set {
+			return errors.Configuration.Messagef("referenced backend %q is not defined", name)
+		}
 		be, err := PrepareBackend(h, "_init", "", &config.Backend{Name: name, Remain: b})
 		if err != nil {
 			return err
diff --git a/config/runtime/backend.go b/config/runtime/backend.go
index ffe7a07b..c7d5bbcd 100644
--- a/config/runtime/backend.go
+++ b/config/runtime/backend.go
@@ -155,7 +155,7 @@ func newRequestAuthorizer(evalCtx *hcl.EvalContext, block *hclsyntax.Block,
 
 	backendBlocks := hclbody.BlocksOfType(block.Body, "backend")
 	if len(backendBlocks) == 0 {
-		r := block.Body.MissingItemRange()
+		r := block.Body.SrcRange
 		diag := &hcl.Diagnostics{&hcl.Diagnostic{
 			Subject: &r,
 			Summary: "missing backend initialization",
diff --git a/config/runtime/endpoint.go b/config/runtime/endpoint.go
index 3673f284..82208621 100644
--- a/config/runtime/endpoint.go
+++ b/config/runtime/endpoint.go
@@ -150,7 +150,7 @@ func newEndpointOptions(confCtx *hcl.EvalContext, endpointConf *config.Endpoint,
 
 	// TODO: redirect
 	if endpointConf.Response == nil && len(proxies)+len(requests)+len(sequences) == 0 { // && redirect == nil
-		r := endpointConf.Remain.MissingItemRange()
+		r := endpointConf.HCLBody().SrcRange
 		m := fmt.Sprintf("configuration error: endpoint %q requires at least one proxy, request, response or redirect block", endpointConf.Pattern)
 		return nil, hcl.Diagnostics{&hcl.Diagnostic{
 			Severity: hcl.DiagError,
@@ -161,7 +161,7 @@ func newEndpointOptions(confCtx *hcl.EvalContext, endpointConf *config.Endpoint,
 
 	bodyLimit, err := parseBodyLimit(endpointConf.RequestBodyLimit)
 	if err != nil {
-		r := endpointConf.Remain.MissingItemRange()
+		r := endpointConf.HCLBody().SrcRange
 		return nil, hcl.Diagnostics{&hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  "parsing endpoint request body limit: " + endpointConf.Pattern,


Test patch:
diff --git a/config/configload/load_test.go b/config/configload/load_test.go
index 3f929645..1c92e221 100644
--- a/config/configload/load_test.go
+++ b/config/configload/load_test.go
@@ -471,71 +471,166 @@ func TestConfigErrors(t *testing.T) {
 	}{
 		{
 			"websockets attribute and block",
-			`proxy {
-		      backend = "foo"
-		      websockets = true
-		      websockets {}
-		    }`,
-			"couper.hcl:6,9-26: either websockets attribute or block is allowed; ",
+			`server {
+			  endpoint "/" {
+			    proxy {
+			      backend = "foo"
+			      websockets = true
+			      websockets {}
+			    }
+			  }
+			}`,
+			"couper.hcl:5,10-27: either websockets attribute or block is allowed; ",
 		},
 		{
 			"unlabeled proxy and request",
-			`proxy {
-		      backend {}
-		    }
-		    request {
-		      url = "http://foo/bar"
-		      backend {}
-		    }`,
-			`couper.hcl:3,18-18: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
+			`server {
+			  endpoint "/" {
+			    proxy {
+			      backend {}
+			    }
+			    request {
+			      url = "http://foo/bar"
+			      backend {}
+			    }
+			  }
+			}`,
+			`couper.hcl:6,16-9,9: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
 		},
 		{
 			"unlabeled proxy and default labeled request",
-			`proxy {
-		      backend {}
-		    }
-		    request "default" {
-		      url = "http://foo/bar"
-		      backend {}
-		    }`,
-			`couper.hcl:3,18-18: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
+			`server {
+			  endpoint "/" {
+			    proxy {
+			      backend {}
+			    }
+			    request "default" {
+			      url = "http://foo/bar"
+			      backend {}
+			    }
+			  }
+			}`,
+			`couper.hcl:6,26-9,9: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
 		},
 		{
 			"default labeled proxy and unlabeled request",
-			`proxy "default" {
-		      backend {}
-		    }
-		    request {
-		      url = "http://foo/bar"
-		      backend {}
-		    }`,
-			`couper.hcl:3,18-18: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
+			`server {
+			  endpoint "/" {
+			    proxy "default" {
+			      backend {}
+			    }
+			    request {
+			      url = "http://foo/bar"
+			      backend {}
+			    }
+			  }
+			}`,
+			`couper.hcl:6,16-9,9: proxy and request names (either default or explicitly set via label) must be unique: "default"; `,
 		},
 		{
 			"labeled proxy and request",
-			`proxy "foo" {
-		      backend {}
-		    }
-		    request "foo" {
-		      url = "http://foo/bar"
-		      backend {}
-		    }`,
-			`couper.hcl:3,18-18: proxy and request names (either default or explicitly set via label) must be unique: "foo"; `,
+			`server {
+			  endpoint "/" {
+			    proxy "foo" {
+			      backend {}
+			    }
+			    request "foo" {
+			      url = "http://foo/bar"
+			      backend {}
+			    }
+			  }
+			}`,
+			`couper.hcl:6,22-9,9: proxy and request names (either default or explicitly set via label) must be unique: "foo"; `,
+		},
+		{
+			"undefined referenced proxy backend",
+			`server {
+			  endpoint "/" {
+			    proxy "foo" {
+			      backend = "rs"
+			    }
+			  }
+			}`,
+			`couper.hcl:3,20-5,9: referenced backend "rs" is not defined; `,
+		},
+		{
+			"undefined refined proxy backend",
+			`server {
+			  endpoint "/" {
+			    proxy "foo" {
+			      backend "rs" {}
+			    }
+			  }
+			}`,
+			`couper.hcl:4,23-25: referenced backend "rs" is not defined; `,
+		},
+		{
+			"undefined referenced oauth2 backend",
+			`server {}
+			definitions {
+			  backend "foo" {
+			    oauth2 {
+			      token_endpoint = "https://as/token"
+			      backend = "as"
+			    }
+			  }
+			}`,
+			`configuration error: referenced backend "as" is not defined`,
+		},
+		{
+			"undefined refined oauth2 backend",
+			`server {}
+			definitions {
+			  backend "foo" {
+			    oauth2 {
+			      token_endpoint = "https://as/token"
+			      backend "as" {}
+			    }
+			  }
+			}`,
+			`configuration error: referenced backend "as" is not defined`,
+		},
+		{
+			"undefined referenced token request backend",
+			`server {}
+			definitions {
+			  backend "foo" {
+			    beta_token_request {
+			      url = "https://as/token"
+			      backend = "as"
+			    }
+			  }
+			}`,
+			`configuration error: referenced backend "as" is not defined`,
+		},
+		{
+			"undefined refined token request backend",
+			`server {}
+			definitions {
+			  backend "foo" {
+			    beta_token_request {
+			      url = "https://as/token"
+			      backend = "as"
+			    }
+			  }
+			}`,
+			`configuration error: referenced backend "as" is not defined`,
 		},
 	}
 
-	template := `
-		server {
-		  endpoint "/" {
-		    %%
-		  }
-		}`
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(subT *testing.T) {
-			_, err := configload.LoadBytes([]byte(strings.Replace(template, "%%", tt.hcl, -1)), "couper.hcl")
+			_, err := configload.LoadBytes([]byte(tt.hcl), "couper.hcl")
+
+			var errorMsg = ""
+			if err != nil {
+				if gErr, ok := err.(errors.GoError); ok {
+					errorMsg = gErr.LogError()
+				} else {
+					errorMsg = err.Error()
+				}
+			}
 
-			errorMsg := err.Error()
 			if tt.error != errorMsg {
 				subT.Errorf("%q: Unexpected configuration error:\n\tWant: %q\n\tGot:  %q", tt.name, tt.error, errorMsg)
 			}