Skip to content

Commit 78c8f1b

Browse files
reinkrulreinkrul
authored
PDP: Decode HTTP request parameters, normalize scope (dash-to-underscore) (#375)
1 parent 7a25a8c commit 78c8f1b

File tree

6 files changed

+104
-18
lines changed

6 files changed

+104
-18
lines changed

‎component/pdp/component.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ func (c *Component) HandleMainPolicy(w http.ResponseWriter, r *http.Request) {
121121

122122
// TODO: Implement support for multiple scopes
123123
policy := qualifications[0]
124+
// OPA doesn't support dashes in package and rule names, so we replace them with underscores.
125+
policy = strings.ReplaceAll(policy, "-", "_")
124126

125127
// Step 2: Parse the PDP input and translate to the policy input
126128
policyInput, policyResult := NewPolicyInput(reqBody)

‎component/pdp/component_test.go‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,18 @@ func TestHandleMainPolicy_Integration(t *testing.T) {
196196
})
197197
t.Run("pzp", func(t *testing.T) {
198198
testCases := []testCase{
199+
{
200+
name: "allow - dash is normalized to underscore",
201+
clientQualifications: []string{"pzp-gf"},
202+
httpRequest: `GET /Patient?identifier=http://fhir.nl/fhir/NamingSystem/bsn|123456789`,
203+
decision: true,
204+
},
205+
{
206+
name: "allow - patient identifier is encoded",
207+
clientQualifications: []string{"pzp-gf"},
208+
httpRequest: `GET /Patient?identifier=http://fhir.nl/fhir/NamingSystem/bsn%7C123456789`,
209+
decision: true,
210+
},
199211
{
200212
name: "allow - Patient search with BSN identifier",
201213
clientQualifications: []string{"pzp_gf"},

‎component/pdp/fhirreq.go‎

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,11 @@ var definitions = []PathDef{
7070
PathDef: []string{"[type]", "_search?"},
7171
Verb: "POST",
7272
},
73+
{
74+
Interaction: fhir.TypeRestfulInteractionSearchType,
75+
PathDef: []string{"[type]", "_search"},
76+
Verb: "POST",
77+
},
7378
{
7479
Interaction: fhir.TypeRestfulInteractionSearchSystem,
7580
PathDef: []string{"?"},
@@ -368,8 +373,19 @@ func getSingleParameter(params url.Values, name string) (string, error) {
368373
func NewPolicyInput(request PDPRequest) (PolicyInput, PolicyResult) {
369374
var policyInput PolicyInput
370375

376+
// URL decode query parameters
377+
decodeHTTPRequest := request.Input.Request
378+
decodedQueryParams, err := urlValuesDecode(request.Input.Request.QueryParams)
379+
if err != nil {
380+
return policyInput, Deny(ResultReason{
381+
Code: TypeResultCodeUnexpectedInput,
382+
Description: "unable to decode query parameters: " + err.Error(),
383+
})
384+
}
385+
decodeHTTPRequest.QueryParams = *decodedQueryParams
386+
371387
policyInput.Subject = request.Input.Subject
372-
policyInput.Action.Request = request.Input.Request
388+
policyInput.Action.Request = decodeHTTPRequest
373389
policyInput.Context.DataHolderOrganizationId = request.Input.Context.DataHolderOrganizationId
374390
policyInput.Context.DataHolderFacilityType = request.Input.Context.DataHolderFacilityType
375391
policyInput.Context.PatientBSN = request.Input.Context.PatientBSN
@@ -385,7 +401,7 @@ func NewPolicyInput(request PDPRequest) (PolicyInput, PolicyResult) {
385401
if !ok {
386402
reason := ResultReason{
387403
Code: TypeResultCodeUnexpectedInput,
388-
Description: "unexpected input, unable to parse fhir request",
404+
Description: "unable to parse FHIR request",
389405
}
390406
return policyInput, Deny(reason)
391407
}
@@ -417,17 +433,17 @@ func NewPolicyInput(request PDPRequest) (PolicyInput, PolicyResult) {
417433
hasFormData
418434

419435
if paramsInBody {
420-
values, err := url.ParseQuery(request.Input.Request.Body)
436+
decodedBody, err := url.ParseQuery(request.Input.Request.Body)
421437
if err != nil {
422438
reason := ResultReason{
423439
Code: TypeResultCodeUnexpectedInput,
424-
Description: "Could not parse form encoded data",
440+
Description: fmt.Sprintf("could not parse form encoded request body: %v", err),
425441
}
426442
return PolicyInput{}, Deny(reason)
427443
}
428-
rawParams = values
444+
rawParams = decodedBody
429445
} else {
430-
rawParams = request.Input.Request.QueryParams
446+
rawParams = *decodedQueryParams
431447
}
432448

433449
params := groupParams(rawParams)
@@ -462,3 +478,17 @@ func NewPolicyInput(request PDPRequest) (PolicyInput, PolicyResult) {
462478

463479
return policyInput, result
464480
}
481+
482+
func urlValuesDecode(in url.Values) (*url.Values, error) {
483+
out := make(url.Values)
484+
for key, values := range in {
485+
for _, value := range values {
486+
decodedValue, err := url.QueryUnescape(value)
487+
if err != nil {
488+
return nil, fmt.Errorf("parameter '%s': %w", key, err)
489+
}
490+
out.Add(key, decodedValue)
491+
}
492+
}
493+
return &out, nil
494+
}

‎component/pdp/fhirreq_test.go‎

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -299,13 +299,13 @@ func TestNewPolicyInput(t *testing.T) {
299299
})
300300
})
301301
t.Run("patient BSN parsing", func(t *testing.T) {
302-
t.Run("ok", func(t *testing.T) {
302+
t.Run("in query parameter, unencoded", func(t *testing.T) {
303303
pdpRequest := PDPRequest{
304304
Input: PDPInput{
305305
Request: HTTPRequest{
306306
Method: "GET",
307307
Protocol: "HTTP/1.1",
308-
Path: "/Patient",
308+
Path: "/Patient?",
309309
QueryParams: url.Values{
310310
"identifier": []string{"http://fhir.nl/fhir/NamingSystem/bsn|900186021"},
311311
},
@@ -318,6 +318,46 @@ func TestNewPolicyInput(t *testing.T) {
318318
policyInput, _ := NewPolicyInput(pdpRequest)
319319
assert.Equal(t, "900186021", policyInput.Context.PatientBSN)
320320
})
321+
t.Run("in query parameter, encoded", func(t *testing.T) {
322+
pdpRequest := PDPRequest{
323+
Input: PDPInput{
324+
Request: HTTPRequest{
325+
Method: "GET",
326+
Protocol: "HTTP/1.1",
327+
Path: "/Patient?",
328+
QueryParams: url.Values{
329+
"identifier": []string{"http://fhir.nl/fhir/NamingSystem/bsn%7C900186021"},
330+
},
331+
},
332+
Context: PDPContext{
333+
ConnectionTypeCode: "hl7-fhir-rest",
334+
},
335+
},
336+
}
337+
policyInput, _ := NewPolicyInput(pdpRequest)
338+
assert.Equal(t, "900186021", policyInput.Context.PatientBSN)
339+
})
340+
t.Run("in POST body, encoded", func(t *testing.T) {
341+
pdpRequest := PDPRequest{
342+
Input: PDPInput{
343+
Request: HTTPRequest{
344+
Method: "POST",
345+
Protocol: "HTTP/1.1",
346+
Path: "/Patient/_search",
347+
Header: http.Header{
348+
"Content-Type": []string{"application/x-www-form-urlencoded"},
349+
},
350+
Body: "identifier=http://fhir.nl/fhir/NamingSystem/bsn%7C900186021",
351+
},
352+
Context: PDPContext{
353+
ConnectionTypeCode: "hl7-fhir-rest",
354+
},
355+
},
356+
}
357+
policyInput, policyResult := NewPolicyInput(pdpRequest)
358+
assert.Equal(t, "900186021", policyInput.Context.PatientBSN)
359+
assert.Empty(t, policyResult.Reasons)
360+
})
321361
t.Run("incorrect system", func(t *testing.T) {
322362
pdpRequest := PDPRequest{
323363
Input: PDPInput{

‎component/pdp/opa.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,10 @@ func createOPAService(ctx context.Context, opaBundleBaseURL string) (*sdk.OPA, e
4747
return result, nil
4848
}
4949

50-
// evalRegoPolicy evaluates a Rego policy using Open Policy Agent for the given scope and input
51-
func (c *Component) evalRegoPolicy(ctx context.Context, scope string, policyInput PolicyInput) (*PolicyResult, error) {
50+
// evalRegoPolicy evaluates a Rego policy using Open Policy Agent for the given policy and input
51+
func (c *Component) evalRegoPolicy(ctx context.Context, policy string, policyInput PolicyInput) (*PolicyResult, error) {
5252
// get the named policy decision for the specified input
53-
result, err := c.opaService.Decision(ctx, sdk.DecisionOptions{Path: "/" + scope + "/allow", Input: policyInput})
53+
result, err := c.opaService.Decision(ctx, sdk.DecisionOptions{Path: "/" + policy + "/allow", Input: policyInput})
5454
if err != nil {
5555
return nil, fmt.Errorf("failed to evaluate policy: %w", err)
5656
} else if _, ok := result.Result.(bool); !ok {
@@ -59,7 +59,8 @@ func (c *Component) evalRegoPolicy(ctx context.Context, scope string, policyInpu
5959

6060
allowed := result.Result.(bool)
6161
policyResult := PolicyResult{
62-
Allow: allowed,
62+
Allow: allowed,
63+
Policy: policy,
6364
}
6465
if !allowed {
6566
policyResult.Reasons = []ResultReason{

‎component/pdp/shared.go‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package pdp
33
import (
44
"fmt"
55
"net/http"
6+
"net/url"
67

78
fhirclient "github.com/SanteonNL/go-fhir-client"
89
"github.com/nuts-foundation/nuts-knooppunt/component/mitz"
@@ -33,12 +34,12 @@ type SubjectProperties struct {
3334
}
3435

3536
type HTTPRequest struct {
36-
Method string `json:"method"`
37-
Protocol string `json:"protocol"` // "HTTP/1.0"
38-
Path string `json:"path"`
39-
QueryParams map[string][]string `json:"query_params"`
40-
Header http.Header `json:"header"`
41-
Body string `json:"body"`
37+
Method string `json:"method"`
38+
Protocol string `json:"protocol"` // "HTTP/1.0"
39+
Path string `json:"path"`
40+
QueryParams url.Values `json:"query_params"`
41+
Header http.Header `json:"header"`
42+
Body string `json:"body"`
4243
}
4344

4445
type PDPContext struct {

0 commit comments

Comments
 (0)