From ec08e895b5ef4317518a03dc4e1494e825b3f299 Mon Sep 17 00:00:00 2001 From: Drew Dara-Abrams Date: Mon, 30 Oct 2023 11:23:59 -0700 Subject: [PATCH 1/6] separate auth roles for downloading current FV and historic FVs --- server/rest/rest.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/rest/rest.go b/server/rest/rest.go index 49624907..bb970f38 100644 --- a/server/rest/rest.go +++ b/server/rest/rest.go @@ -55,14 +55,14 @@ func NewServer(cfg config.Config, srv http.Handler) (http.Handler, error) { r.HandleFunc("/feeds", feedHandler) r.HandleFunc("/feeds/{feed_key}.{format}", feedHandler) r.HandleFunc("/feeds/{feed_key}", feedHandler) - r.HandleFunc("/feeds/{feed_key}/download_latest_feed_version", makeHandlerFunc(restcfg, "feedVersionDownloadLatest", feedVersionDownloadLatestHandler)) + r.HandleFunc("/feeds/{feed_key}/download_latest_feed_version", ancheck.RoleRequired("tl_download_fv_current")(makeHandlerFunc(restcfg, "feedVersionDownloadLatest", feedVersionDownloadLatestHandler))) r.HandleFunc("/feed_versions.{format}", feedVersionHandler) r.HandleFunc("/feed_versions", feedVersionHandler) r.HandleFunc("/feed_versions/{feed_version_key}.{format}", feedVersionHandler) r.HandleFunc("/feed_versions/{feed_version_key}", feedVersionHandler) r.HandleFunc("/feeds/{feed_key}/feed_versions", feedVersionHandler) - r.Handle("/feed_versions/{feed_version_key}/download", ancheck.RoleRequired("tl_user_pro")(makeHandlerFunc(restcfg, "feedVersionDownload", feedVersionDownloadHandler))) + r.Handle("/feed_versions/{feed_version_key}/download", ancheck.RoleRequired("tl_download_fv_historic")(makeHandlerFunc(restcfg, "feedVersionDownload", feedVersionDownloadHandler))) r.HandleFunc("/agencies.{format}", agencyHandler) r.HandleFunc("/agencies", agencyHandler) From 93ed10899401b5143e9df5b68812654c7090feb7 Mon Sep 17 00:00:00 2001 From: Ian Rees Date: Mon, 30 Oct 2023 11:43:34 -0700 Subject: [PATCH 2/6] Fix --- server/rest/rest.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/rest/rest.go b/server/rest/rest.go index bb970f38..8cbee863 100644 --- a/server/rest/rest.go +++ b/server/rest/rest.go @@ -55,7 +55,7 @@ func NewServer(cfg config.Config, srv http.Handler) (http.Handler, error) { r.HandleFunc("/feeds", feedHandler) r.HandleFunc("/feeds/{feed_key}.{format}", feedHandler) r.HandleFunc("/feeds/{feed_key}", feedHandler) - r.HandleFunc("/feeds/{feed_key}/download_latest_feed_version", ancheck.RoleRequired("tl_download_fv_current")(makeHandlerFunc(restcfg, "feedVersionDownloadLatest", feedVersionDownloadLatestHandler))) + r.Handle("/feeds/{feed_key}/download_latest_feed_version", ancheck.RoleRequired("tl_download_fv_current")(makeHandlerFunc(restcfg, "feedVersionDownloadLatest", feedVersionDownloadLatestHandler))) r.HandleFunc("/feed_versions.{format}", feedVersionHandler) r.HandleFunc("/feed_versions", feedVersionHandler) From 950b7967a089143ac890522075a067f1067caa51 Mon Sep 17 00:00:00 2001 From: Ian Rees Date: Mon, 30 Oct 2023 12:31:07 -0700 Subject: [PATCH 3/6] Tests --- server/rest/feed_version_download_test.go | 117 ++++++++++++++++++++-- test_setup.sh | 12 +-- 2 files changed, 114 insertions(+), 15 deletions(-) diff --git a/server/rest/feed_version_download_test.go b/server/rest/feed_version_download_test.go index 61aaea2e..081b857a 100644 --- a/server/rest/feed_version_download_test.go +++ b/server/rest/feed_version_download_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/interline-io/transitland-server/auth/ancheck" + "github.com/interline-io/transitland-server/auth/authn" "github.com/interline-io/transitland-server/internal/testutil" ) @@ -21,12 +22,12 @@ func TestFeedVersionDownloadRequest(t *testing.T) { if err != nil { t.Fatal(err) } - restSrv = ancheck.AdminDefaultMiddleware("test")(restSrv) t.Run("ok", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feed_versions/d2813c293bcfd7a97dde599527ae6c62c98e66c6/download", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asAdmin := ancheck.AdminDefaultMiddleware("test")(restSrv) + asAdmin.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 200 { t.Errorf("got status code %d, expected 200", sc) } @@ -34,10 +35,67 @@ func TestFeedVersionDownloadRequest(t *testing.T) { t.Errorf("got %d bytes, expected 59324", sc) } }) - t.Run("not authorized", func(t *testing.T) { + t.Run("not authorized as anon", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feed_versions/d2813c293bcfd7a97dde599527ae6c62c98e66c6/download", nil) + rr := httptest.NewRecorder() + asAnon := restSrv + asAnon.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("not authorized as user, missing role", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feed_versions/d2813c293bcfd7a97dde599527ae6c62c98e66c6/download", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("testrole") + })(restSrv) + asUser.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("not authorized as user, only current download role", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feed_versions/d2813c293bcfd7a97dde599527ae6c62c98e66c6/download", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("tl_download_fv_current") + })(restSrv) + asUser.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("authorized as user", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feed_versions/d2813c293bcfd7a97dde599527ae6c62c98e66c6/download", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("tl_download_fv_historic") + })(restSrv) + asUser.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 200 { + t.Errorf("got status code %d, expected 200", sc) + } + if sc := len(rr.Body.Bytes()); sc != 59324 { + t.Errorf("got %d bytes, expected 59324", sc) + } + }) + t.Run("not authorized as anon, not redistributable", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feed_versions/dd7aca4a8e4c90908fd3603c097fabee75fea907/download", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asAnon := restSrv + asAnon.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("not authorized as user, not redistributable", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feed_versions/dd7aca4a8e4c90908fd3603c097fabee75fea907/download", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("tl_download_fv_historic") + })(restSrv) + asUser.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 401 { t.Errorf("got status code %d, expected 401", sc) } @@ -45,7 +103,8 @@ func TestFeedVersionDownloadRequest(t *testing.T) { t.Run("not found", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feed_versions/asdxyz/download", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asAdmin := ancheck.AdminDefaultMiddleware("test")(restSrv) + asAdmin.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 404 { t.Errorf("got status code %d, expected 404", sc) } @@ -68,7 +127,22 @@ func TestFeedDownloadLatestRequest(t *testing.T) { t.Run("ok", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feeds/CT/download_latest_feed_version", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asAdmin := ancheck.AdminDefaultMiddleware("test")(restSrv) + asAdmin.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 200 { + t.Errorf("got status code %d, expected 200", sc) + } + if sc := len(rr.Body.Bytes()); sc != 59324 { + t.Errorf("got %d bytes, expected 59324", sc) + } + }) + t.Run("ok as user", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feeds/CT/download_latest_feed_version", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("tl_download_fv_current") + })(restSrv) + asUser.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 200 { t.Errorf("got status code %d, expected 200", sc) } @@ -76,18 +150,43 @@ func TestFeedDownloadLatestRequest(t *testing.T) { t.Errorf("got %d bytes, expected 59324", sc) } }) - t.Run("not authorized", func(t *testing.T) { + t.Run("not authorized as anon", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feeds/CT/download_latest_feed_version", nil) + rr := httptest.NewRecorder() + asAnon := restSrv + asAnon.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("not authorized as user, missing role", func(t *testing.T) { + req, _ := http.NewRequest("GET", "/feeds/CT/download_latest_feed_version", nil) + rr := httptest.NewRecorder() + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("testrole") + })(restSrv) + asUser.ServeHTTP(rr, req) + if sc := rr.Result().StatusCode; sc != 401 { + t.Errorf("got status code %d, expected 401", sc) + } + }) + t.Run("not authorized as user,license", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feeds/BA/download_latest_feed_version", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { + return authn.NewCtxUser("testuser", "", "").WithRoles("testrole") + })(restSrv) + asUser.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 401 { t.Errorf("got status code %d, expected 401", sc) } }) + t.Run("not found", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feeds/asdxyz/download_latest_feed_version", nil) rr := httptest.NewRecorder() - restSrv.ServeHTTP(rr, req) + asAdmin := ancheck.AdminDefaultMiddleware("test")(restSrv) + asAdmin.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 404 { t.Errorf("got status code %d, expected 404", sc) } diff --git a/test_setup.sh b/test_setup.sh index 730bdb62..787188ca 100755 --- a/test_setup.sh +++ b/test_setup.sh @@ -1,17 +1,17 @@ #!/bin/bash # Remove import files -rm *.zip +export TL_TEST_STORAGE="${PWD}/tmp" +mkdir -p "${TL_TEST_STORAGE}"; rm ${TL_TEST_STORAGE}/*.zip # export TL_LOG=debug (cd cmd/tlserver && go install .) tlserver sync -dburl="$TL_TEST_SERVER_DATABASE_URL" test/data/server/server-test.dmfr.json # older data -tlserver fetch -dburl="$TL_TEST_SERVER_DATABASE_URL" -allow-local-fetch -feed-url=test/data/external/bart-old.zip BA # old data -tlserver import -dburl="$TL_TEST_SERVER_DATABASE_URL" +tlserver fetch -dburl="$TL_TEST_SERVER_DATABASE_URL" -storage="$TL_TEST_STORAGE" -allow-local-fetch -feed-url=test/data/external/bart-old.zip BA # old data +tlserver import -dburl="$TL_TEST_SERVER_DATABASE_URL" -storage="$TL_TEST_STORAGE" # current data -tlserver fetch -dburl="$TL_TEST_SERVER_DATABASE_URL" -allow-local-fetch -tlserver import -dburl="$TL_TEST_SERVER_DATABASE_URL" -activate +tlserver fetch -dburl="$TL_TEST_SERVER_DATABASE_URL" -storage="$TL_TEST_STORAGE" -allow-local-fetch +tlserver import -dburl="$TL_TEST_SERVER_DATABASE_URL" -storage="$TL_TEST_STORAGE" -activate # sync again tlserver sync -dburl="$TL_TEST_SERVER_DATABASE_URL" test/data/server/server-test.dmfr.json # supplemental data psql -f test_supplement.pgsql -rm *.zip From 610b1e9ac590266b1015c84d2cdb59a0da6791a0 Mon Sep 17 00:00:00 2001 From: Ian Rees Date: Mon, 30 Oct 2023 12:35:53 -0700 Subject: [PATCH 4/6] TL_TEST_STORAGE values --- .github/workflows/test.yml | 1 + test_setup.sh | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 54bdf64a..12c50701 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,6 +17,7 @@ jobs: PGPASSWORD: for_testing PGHOST: localhost PGPORT: 5432 + TL_TEST_STORAGE: tmp TL_TEST_SERVER_DATABASE_URL: postgres://root:for_testing@localhost:5432/tlv2_test_server?sslmode=disable TL_DATABASE_URL: postgres://root:for_testing@localhost:5432/tlv2_test_server?sslmode=disable services: diff --git a/test_setup.sh b/test_setup.sh index 787188ca..77c5650d 100755 --- a/test_setup.sh +++ b/test_setup.sh @@ -1,6 +1,6 @@ #!/bin/bash # Remove import files -export TL_TEST_STORAGE="${PWD}/tmp" +TL_TEST_STORAGE="${TL_TEST_STORAGE:-tmp}" mkdir -p "${TL_TEST_STORAGE}"; rm ${TL_TEST_STORAGE}/*.zip # export TL_LOG=debug (cd cmd/tlserver && go install .) From 18fd7442e2a574a452d6fc2f15bd6c5eabe274d7 Mon Sep 17 00:00:00 2001 From: Ian Rees Date: Mon, 30 Oct 2023 12:48:15 -0700 Subject: [PATCH 5/6] Fix? --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 12c50701..b52d7489 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -17,7 +17,7 @@ jobs: PGPASSWORD: for_testing PGHOST: localhost PGPORT: 5432 - TL_TEST_STORAGE: tmp + TL_TEST_STORAGE: /tmp/tlserver TL_TEST_SERVER_DATABASE_URL: postgres://root:for_testing@localhost:5432/tlv2_test_server?sslmode=disable TL_DATABASE_URL: postgres://root:for_testing@localhost:5432/tlv2_test_server?sslmode=disable services: From 7366b5a3e5178718769fdf58b318002d1e7c9fc2 Mon Sep 17 00:00:00 2001 From: Ian Rees Date: Mon, 30 Oct 2023 12:57:02 -0700 Subject: [PATCH 6/6] Clarify test --- server/rest/feed_version_download_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/rest/feed_version_download_test.go b/server/rest/feed_version_download_test.go index 081b857a..3949ba83 100644 --- a/server/rest/feed_version_download_test.go +++ b/server/rest/feed_version_download_test.go @@ -170,11 +170,11 @@ func TestFeedDownloadLatestRequest(t *testing.T) { t.Errorf("got status code %d, expected 401", sc) } }) - t.Run("not authorized as user,license", func(t *testing.T) { + t.Run("not authorized as user, not redistributable", func(t *testing.T) { req, _ := http.NewRequest("GET", "/feeds/BA/download_latest_feed_version", nil) rr := httptest.NewRecorder() asUser := ancheck.NewUserDefaultMiddleware(func() authn.User { - return authn.NewCtxUser("testuser", "", "").WithRoles("testrole") + return authn.NewCtxUser("testuser", "", "").WithRoles("download_latest_feed_version") })(restSrv) asUser.ServeHTTP(rr, req) if sc := rr.Result().StatusCode; sc != 401 {