Skip to content

Commit 7444e04

Browse files
BCDA-9669: Correct OpOutcome response body in v3 (#1294)
## 🎫 Ticket https://jira.cms.gov/browse/BCDA-9669 ## 🛠 Changes Updated `getRespWriter` conditional in auth/web/logging middleware to check for v3. ## ℹ️ Context V3 has details.coding removed since we are not following the the valueset or codes in the operation-outcome specification. For more details, see this ticket: https://jira.cms.gov/browse/BCDA-8927 <!-- If any of the following security implications apply, this PR must not be merged without Stephen Walter's approval. Explain in this section and add @SJWalter11 as a reviewer. - Adds a new software dependency or dependencies. - Modifies or invalidates one or more of our security controls. - Stores or transmits data that was not stored or transmitted before. - Requires additional review of security implications for other reasons. --> ## 🧪 Validation Tests added and passing.
1 parent 285c977 commit 7444e04

File tree

6 files changed

+98
-8
lines changed

6 files changed

+98
-8
lines changed

bcda/auth/middleware.go

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import (
2020
"github.com/CMSgov/bcda-app/bcda/models/postgres"
2121
responseutils "github.com/CMSgov/bcda-app/bcda/responseutils"
2222
responseutilsv2 "github.com/CMSgov/bcda-app/bcda/responseutils/v2"
23+
responseutilsv3 "github.com/CMSgov/bcda-app/bcda/responseutils/v3"
2324
"github.com/CMSgov/bcda-app/log"
2425
)
2526

@@ -58,7 +59,7 @@ func (m AuthMiddleware) ParseToken(next http.Handler) http.Handler {
5859
return
5960
}
6061

61-
rw := getRespWriter(r.URL.Path)
62+
rw := GetRespWriter(r.URL.Path)
6263

6364
authRegexp := regexp.MustCompile(`^Bearer (\S+)$`)
6465
authSubmatches := authRegexp.FindStringSubmatch(authHeader)
@@ -170,7 +171,7 @@ func handleTokenVerificationError(ctx context.Context, w http.ResponseWriter, rw
170171
// This depends on ParseToken being called beforehand in the routing middleware.
171172
func RequireTokenAuth(next http.Handler) http.Handler {
172173
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
173-
rw := getRespWriter(r.URL.Path)
174+
rw := GetRespWriter(r.URL.Path)
174175
ctx := r.Context()
175176

176177
token := ctx.Value(TokenContextKey)
@@ -193,7 +194,7 @@ func RequireTokenAuth(next http.Handler) http.Handler {
193194
// CheckBlacklist checks the auth data is associated with a blacklisted entity
194195
func CheckBlacklist(next http.Handler) http.Handler {
195196
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
196-
rw := getRespWriter(r.URL.Path)
197+
rw := GetRespWriter(r.URL.Path)
197198
ctx := r.Context()
198199

199200
ad, ok := ctx.Value(AuthDataContextKey).(AuthData)
@@ -223,7 +224,7 @@ func CheckBlacklist(next http.Handler) http.Handler {
223224
func (m AuthMiddleware) RequireTokenJobMatch(db *sql.DB) func(next http.Handler) http.Handler {
224225
return func(next http.Handler) http.Handler {
225226
fn := func(w http.ResponseWriter, r *http.Request) {
226-
rw := getRespWriter(r.URL.Path)
227+
rw := GetRespWriter(r.URL.Path)
227228
ctx := r.Context()
228229

229230
ad, ok := ctx.Value(AuthDataContextKey).(AuthData)
@@ -295,13 +296,13 @@ type fhirResponseWriter interface {
295296
OpOutcome(context.Context, http.ResponseWriter, int, string, string)
296297
}
297298

298-
func getRespWriter(path string) fhirResponseWriter {
299+
func GetRespWriter(path string) fhirResponseWriter {
299300
if strings.Contains(path, "/v1/") {
300301
return responseutils.NewFhirResponseWriter()
301302
} else if strings.Contains(path, "/v2/") {
302303
return responseutilsv2.NewFhirResponseWriter()
303304
} else if strings.Contains(path, fmt.Sprintf("/%s/", constants.V3Version)) {
304-
return responseutilsv2.NewFhirResponseWriter() // TODO: V3
305+
return responseutilsv3.NewFhirResponseWriter()
305306
} else {
306307
// CommonAuth is used in requests not exclusive to v1 or v2 (ie data requests or /_version).
307308
// In the cases we cannot discern a version we default to v1

bcda/auth/middleware_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,34 @@ func (s *MiddlewareTestSuite) TestCheckBlacklist() {
567567

568568
}
569569

570+
func TestGetRespWriter(t *testing.T) {
571+
ctx := context.Background()
572+
tests := []struct {
573+
name string
574+
path string
575+
}{
576+
{"v1", constants.V1Path},
577+
{"v2", constants.V2Path},
578+
{"v3", constants.V3Path},
579+
}
580+
for _, tt := range tests {
581+
t.Run(tt.name, func(t *testing.T) {
582+
rr := httptest.NewRecorder()
583+
rw := auth.GetRespWriter(tt.path)
584+
rw.OpOutcome(ctx, rr, http.StatusUnauthorized, responseutils.TokenErr, responseutils.TokenErr)
585+
resp := rr.Body.String()
586+
switch tt.name {
587+
case "v1":
588+
assert.NotContains(t, resp, "coding")
589+
case "v2":
590+
assert.Contains(t, resp, "coding")
591+
case "v3":
592+
assert.NotContains(t, resp, "coding")
593+
}
594+
})
595+
}
596+
}
597+
570598
func TestMiddlewareTestSuite(t *testing.T) {
571599
suite.Run(t, new(MiddlewareTestSuite))
572600
}

bcda/logging/middleware.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"github.com/CMSgov/bcda-app/bcda/responseutils"
2020

2121
responseutilsv2 "github.com/CMSgov/bcda-app/bcda/responseutils/v2"
22+
responseutilsv3 "github.com/CMSgov/bcda-app/bcda/responseutils/v3"
2223
"github.com/CMSgov/bcda-app/bcda/servicemux"
2324
"github.com/CMSgov/bcda-app/log"
2425
appMiddleware "github.com/CMSgov/bcda-app/middleware"
@@ -156,7 +157,7 @@ func getRespWriter(path string) fhirResponseWriter {
156157
} else if strings.Contains(path, "/v2/") {
157158
return responseutilsv2.NewFhirResponseWriter()
158159
} else if strings.Contains(path, fmt.Sprintf("/%s/", constants.V3Version)) {
159-
return responseutilsv2.NewFhirResponseWriter() // TODO: V3
160+
return responseutilsv3.NewFhirResponseWriter()
160161
} else {
161162
return responseutils.NewFhirResponseWriter()
162163
}

bcda/logging/middleware_test.go

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
"github.com/CMSgov/bcda-app/bcda/constants"
2323
"github.com/CMSgov/bcda-app/bcda/logging"
2424
"github.com/CMSgov/bcda-app/bcda/models"
25+
"github.com/CMSgov/bcda-app/bcda/responseutils"
2526
"github.com/CMSgov/bcda-app/bcda/testUtils"
2627
"github.com/CMSgov/bcda-app/conf"
2728
"github.com/CMSgov/bcda-app/log"
@@ -268,3 +269,31 @@ func TestMiddlewareTransactionCtx(t *testing.T) {
268269
handlerToTest.ServeHTTP(httptest.NewRecorder(), req)
269270

270271
}
272+
273+
func TestGetRespWriter(t *testing.T) {
274+
ctx := context.Background()
275+
tests := []struct {
276+
name string
277+
path string
278+
}{
279+
{"v1", constants.V1Path},
280+
{"v2", constants.V2Path},
281+
{"v3", constants.V3Path},
282+
}
283+
for _, tt := range tests {
284+
t.Run(tt.name, func(t *testing.T) {
285+
rr := httptest.NewRecorder()
286+
rw := auth.GetRespWriter(tt.path)
287+
rw.OpOutcome(ctx, rr, http.StatusUnauthorized, responseutils.TokenErr, responseutils.TokenErr)
288+
resp := rr.Body.String()
289+
switch tt.name {
290+
case "v1":
291+
assert.NotContains(t, resp, "coding")
292+
case "v2":
293+
assert.Contains(t, resp, "coding")
294+
case "v3":
295+
assert.NotContains(t, resp, "coding")
296+
}
297+
})
298+
}
299+
}

bcda/web/middleware/validation.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
"github.com/CMSgov/bcda-app/bcda/constants"
1414
responseutils "github.com/CMSgov/bcda-app/bcda/responseutils"
1515
responseutilsv2 "github.com/CMSgov/bcda-app/bcda/responseutils/v2"
16+
responseutilsv3 "github.com/CMSgov/bcda-app/bcda/responseutils/v3"
1617
"github.com/CMSgov/bcda-app/log"
1718
"github.com/sirupsen/logrus"
1819
)
@@ -405,7 +406,7 @@ func getRespWriter(version string) (fhirResponseWriter, error) {
405406
case "v2":
406407
return responseutilsv2.NewFhirResponseWriter(), nil
407408
case constants.V3Version:
408-
return responseutilsv2.NewFhirResponseWriter(), nil
409+
return responseutilsv3.NewFhirResponseWriter(), nil
409410
default:
410411
return nil, fmt.Errorf("unexpected API version: %s", version)
411412
}

bcda/web/middleware/validation_test.go

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import (
88
"testing"
99
"time"
1010

11+
"github.com/CMSgov/bcda-app/bcda/auth"
1112
"github.com/CMSgov/bcda-app/bcda/constants"
13+
"github.com/CMSgov/bcda-app/bcda/responseutils"
1214
"github.com/CMSgov/bcda-app/log"
1315
"github.com/sirupsen/logrus"
1416
"github.com/stretchr/testify/assert"
@@ -263,3 +265,31 @@ func TestValidateTypeFilterTagCodes(t *testing.T) {
263265
})
264266
}
265267
}
268+
269+
func TestGetRespWriter(t *testing.T) {
270+
ctx := context.Background()
271+
tests := []struct {
272+
name string
273+
path string
274+
}{
275+
{"v1", constants.V1Path},
276+
{"v2", constants.V2Path},
277+
{"v3", constants.V3Path},
278+
}
279+
for _, tt := range tests {
280+
t.Run(tt.name, func(t *testing.T) {
281+
rr := httptest.NewRecorder()
282+
rw := auth.GetRespWriter(tt.path)
283+
rw.OpOutcome(ctx, rr, http.StatusUnauthorized, responseutils.TokenErr, responseutils.TokenErr)
284+
resp := rr.Body.String()
285+
switch tt.name {
286+
case "v1":
287+
assert.NotContains(t, resp, "coding")
288+
case "v2":
289+
assert.Contains(t, resp, "coding")
290+
case "v3":
291+
assert.NotContains(t, resp, "coding")
292+
}
293+
})
294+
}
295+
}

0 commit comments

Comments
 (0)