Skip to content

Commit 9b58ff9

Browse files
Carl 9782 block v1v2 access (#1311)
## 🎫 Ticket https://jira.cms.gov/browse/BCDA-9782 ## 🛠 Changes Add a v1/v2 deny access configuration check to v1/v2 request middleware. ## ℹ️ Context Part of making sure new clients that start with v3 are not making old request. <!-- 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 Local linting and testing.
1 parent 0cbf04b commit 9b58ff9

8 files changed

Lines changed: 115 additions & 19 deletions

File tree

bcda/service/config.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ type Config struct {
3333
V3EnabledACOs []string `conf:"v3_enabled_acos"` // Simple list of ACOs with v3 access
3434
CutoffDuration time.Duration
3535
RateLimitConfig RateLimitConfig `conf:"rate_limit_config"`
36+
V1V2DenyRegexes []string `conf:"v1_v2_deny_regexes"`
3637
// Use the squash tag to allow the RunoutConfigs to avoid requiring the parameters
3738
// to be defined as a child of RunoutConfig.
3839
// Ex: Without the ,squash, we would have to have RunoutConfig.RUNOUT_CUTOFF_DATE_DAYS

bcda/web/middleware/middleware.go

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

78
"github.com/CMSgov/bcda-app/bcda/auth"
89
"github.com/CMSgov/bcda-app/bcda/responseutils"
@@ -34,14 +35,10 @@ func SecurityHeader(next http.Handler) http.Handler {
3435
func ACOEnabled(cfg *service.Config) func(next http.Handler) http.Handler {
3536
return func(next http.Handler) http.Handler {
3637
fn := func(w http.ResponseWriter, r *http.Request) {
37-
ctx := r.Context()
38-
ad, ok := ctx.Value(auth.AuthDataContextKey).(auth.AuthData)
38+
ad, ok := handleAuthData(r)
3939
if !ok {
40-
// We cannot get the correct FHIR response writer from here, so
41-
// return a non-FHIR-compliant HTTP response
42-
logger := log.GetCtxLogger(ctx)
43-
logger.WithField("resp_status", http.StatusInternalServerError).Error("AuthData should be set before calling this handler")
4440
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
41+
return
4542
}
4643

4744
rw, _ := getResponseWriterFromRequestPath(w, r)
@@ -50,8 +47,8 @@ func ACOEnabled(cfg *service.Config) func(next http.Handler) http.Handler {
5047
}
5148

5249
if cfg.IsACODisabled(ad.CMSID) {
53-
ctx, _ = log.WriteWarnWithFields(
54-
ctx,
50+
ctx, _ := log.WriteWarnWithFields(
51+
r.Context(),
5552
fmt.Sprintf("%s: Failed to complete request, CMSID %s is not enabled", responseutils.UnauthorizedErr, ad.CMSID),
5653
logrus.Fields{"resp_status": http.StatusUnauthorized},
5754
)
@@ -68,13 +65,8 @@ func ACOEnabled(cfg *service.Config) func(next http.Handler) http.Handler {
6865
func V3AccessControl(cfg *service.Config) func(next http.Handler) http.Handler {
6966
return func(next http.Handler) http.Handler {
7067
fn := func(w http.ResponseWriter, r *http.Request) {
71-
ctx := r.Context()
72-
ad, ok := ctx.Value(auth.AuthDataContextKey).(auth.AuthData)
68+
ad, ok := handleAuthData(r)
7369
if !ok {
74-
// We cannot get the correct FHIR response writer from here, so
75-
// return a non-FHIR-compliant HTTP response
76-
logger := log.GetCtxLogger(ctx)
77-
logger.WithField("resp_status", http.StatusInternalServerError).Error("AuthData should be set before calling this handler")
7870
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
7971
return
8072
}
@@ -85,8 +77,8 @@ func V3AccessControl(cfg *service.Config) func(next http.Handler) http.Handler {
8577
}
8678

8779
if !cfg.IsACOV3Enabled(ad.CMSID) {
88-
ctx, _ = log.WriteWarnWithFields(
89-
ctx,
80+
ctx, _ := log.WriteWarnWithFields(
81+
r.Context(),
9082
fmt.Sprintf("%s: Failed to begin v3 request, CMSID %s does not have v3 access", responseutils.UnauthorizedErr, ad.CMSID),
9183
logrus.Fields{"resp_status": http.StatusForbidden},
9284
)
@@ -99,3 +91,60 @@ func V3AccessControl(cfg *service.Config) func(next http.Handler) http.Handler {
9991
return http.HandlerFunc(fn)
10092
}
10193
}
94+
95+
func V1V2DenyControl(cfg *service.Config) func(next http.Handler) http.Handler {
96+
return func(next http.Handler) http.Handler {
97+
fn := func(w http.ResponseWriter, r *http.Request) {
98+
ad, ok := handleAuthData(r)
99+
if !ok {
100+
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
101+
return
102+
}
103+
104+
rw, _ := getResponseWriterFromRequestPath(w, r)
105+
if rw == nil {
106+
return
107+
}
108+
109+
if isACOV1V2DeniedAccess(cfg, ad.CMSID) {
110+
ctx, _ := log.WriteWarnWithFields(
111+
r.Context(),
112+
fmt.Sprintf("%s: Failed to begin v1/v2 request, CMSID %s does not have v1/v2 access", responseutils.UnauthorizedErr, ad.CMSID),
113+
logrus.Fields{"resp_status": http.StatusForbidden},
114+
)
115+
rw.OpOutcome(ctx, w, http.StatusForbidden, responseutils.UnauthorizedErr, "v1 nor v2 access not enabled for this ACO")
116+
return
117+
}
118+
119+
next.ServeHTTP(w, r)
120+
}
121+
122+
return http.HandlerFunc(fn)
123+
}
124+
}
125+
126+
func handleAuthData(r *http.Request) (auth.AuthData, bool) {
127+
ctx := r.Context()
128+
ad, ok := ctx.Value(auth.AuthDataContextKey).(auth.AuthData)
129+
if !ok {
130+
// We cannot get the correct FHIR response writer from here, so
131+
// return a non-FHIR-compliant HTTP response
132+
logger := log.GetCtxLogger(ctx)
133+
logger.WithField("resp_status", http.StatusInternalServerError).Error("AuthData should be set before calling this handler")
134+
135+
return ad, false
136+
}
137+
138+
return ad, true
139+
}
140+
141+
func isACOV1V2DeniedAccess(cfg *service.Config, ACOID string) bool {
142+
for _, str := range cfg.V1V2DenyRegexes {
143+
regex := regexp.MustCompile(str)
144+
if regex.MatchString(ACOID) {
145+
return true
146+
}
147+
}
148+
149+
return false
150+
}

bcda/web/middleware/middleware_test.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,48 @@ func (s *MiddlewareTestSuite) TestV3AccessControl() {
239239
}
240240
}
241241

242+
func (s *MiddlewareTestSuite) TestV1V2DenyControl() {
243+
tests := []struct {
244+
name string
245+
acoID string
246+
regexes []string
247+
expectedCode int
248+
}{
249+
{"V1V2 deny empty", "A1234", []string{}, http.StatusOK},
250+
{"V1V2 deny list nil", "A1234", nil, http.StatusOK},
251+
{"V1V2 deny list mismatch", "A1234", []string{"D\\d{4}"}, http.StatusOK},
252+
{"V1V2 deny list mismatch specific", "A1234", []string{"A1235", "A1236"}, http.StatusOK},
253+
{"V1V2 deny list mismatch case sensitive", "a1234", []string{"A1234", "A5678"}, http.StatusOK},
254+
{"V1V2 deny list match", "A1234", []string{"A\\d{4}"}, http.StatusForbidden},
255+
{"V1V2 deny list match specific", "A1234", []string{"A1234"}, http.StatusForbidden},
256+
{"V1V2 deny all", "A9999", []string{".*"}, http.StatusForbidden},
257+
{"V1V2 deny many regexes", "A9999", []string{"A1234", "A9998", "A999\\d"}, http.StatusForbidden},
258+
}
259+
260+
for _, tt := range tests {
261+
cfg := &service.Config{
262+
RunoutConfig: service.RunoutConfig{CutoffDurationDays: 180, ClaimThruDate: "2020-12-31"},
263+
V1V2DenyRegexes: tt.regexes,
264+
}
265+
266+
rr := httptest.NewRecorder()
267+
268+
V1V2DenyControl(cfg)(http.HandlerFunc(
269+
func(rw http.ResponseWriter, r *http.Request) {}),
270+
).ServeHTTP(rr, testRequest(RequestParameters{}, tt.acoID))
271+
272+
assert.Equal(s.T(), tt.expectedCode, rr.Code)
273+
}
274+
}
275+
276+
func (s *MiddlewareTestSuite) TestV1V2DenyControl_PanicOnBadRegex() {
277+
cfg := &service.Config{
278+
RunoutConfig: service.RunoutConfig{CutoffDurationDays: 180, ClaimThruDate: "2020-12-31"},
279+
V1V2DenyRegexes: []string{"?!.*"}, // invalid regex (invalid target for quantifier)
280+
}
281+
assert.Panics(s.T(), func() { isACOV1V2DeniedAccess(cfg, "A1234") })
282+
}
283+
242284
func testRequest(rp RequestParameters, cmsid string) *http.Request {
243285
ctx := context.WithValue(context.Background(), auth.AuthDataContextKey, auth.AuthData{CMSID: cmsid, ACOID: cmsid})
244286
ctx = SetRequestParamsCtx(ctx, rp)

bcda/web/router.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,10 @@ func NewAPIRouter(db *sql.DB, pool *pgxv5Pool.Pool, provider auth.Provider) http
4545

4646
rlm := middleware.NewRateLimitMiddleware(cfg, db)
4747
var requestValidators = []func(http.Handler) http.Handler{
48-
middleware.ACOEnabled(cfg), middleware.ValidateRequestURL, middleware.ValidateRequestHeaders, rlm.CheckConcurrentJobs,
48+
middleware.ACOEnabled(cfg), middleware.V1V2DenyControl(cfg), middleware.ValidateRequestURL, middleware.ValidateRequestHeaders, rlm.CheckConcurrentJobs,
4949
}
5050
nonExportRequestValidators := []func(http.Handler) http.Handler{
51-
middleware.ACOEnabled(cfg), middleware.ValidateRequestURL, middleware.ValidateRequestHeaders,
51+
middleware.ACOEnabled(cfg), middleware.V1V2DenyControl(cfg), middleware.ValidateRequestURL, middleware.ValidateRequestHeaders,
5252
}
5353

5454
if conf.GetEnv("DEPLOYMENT_TARGET") != "prod" {

conf/configs/dev.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,4 @@ rate_limit_config:
7979
all: false
8080
acos: []
8181
v3_enabled_acos: []
82+
v1_v2_deny_regexes: []

conf/configs/prod.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,4 +79,5 @@ rate_limit_config:
7979
all: true
8080
acos: []
8181
v3_enabled_acos:
82-
- "TEST994"
82+
- "TEST994"
83+
v1_v2_deny_regexes: ["IOTA\\d{3}"]

conf/configs/sandbox.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,4 @@ rate_limit_config:
8686
all: false
8787
acos: ["SBXBM0002"]
8888
v3_enabled_acos: []
89+
v1_v2_deny_regexes: []

conf/configs/test.yml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,4 @@ rate_limit_config:
7979
all: true
8080
acos: []
8181
v3_enabled_acos: []
82+
v1_v2_deny_regexes: []

0 commit comments

Comments
 (0)