Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security review PR - Collection of all "team project" feature related changes #89

Closed
wants to merge 31 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
c90591b
feat: add sec_ tables + view and test data
pieterlukasse Nov 29, 2023
d009aa7
feat: added "team project" to /cohortdefinition-stats/by-source-id/ e…
pieterlukasse Dec 1, 2023
8c3ee95
feat: integrate extra 'team project' validation for concept endpoints
pieterlukasse Dec 6, 2023
fb06d28
fix: fix PrepareNewArboristRequest tests
pieterlukasse Dec 8, 2023
317e132
feat: move "team project" checks to TeamProjectAuthzI
pieterlukasse Dec 8, 2023
522afa0
feat: add tests for TeamProjectAuthz
pieterlukasse Dec 8, 2023
e76c6bc
fix: fix the usage of TeamProjectValidation
pieterlukasse Dec 11, 2023
cc06ba8
fix: fix minor code issues
pieterlukasse Dec 11, 2023
88c2289
feat: simplify filtering on allowedCohortDefinitionIds
pieterlukasse Dec 12, 2023
6751594
Merge pull request #81 from uc-cdis/feat/integrate_team_project_checks
pieterlukasse Dec 13, 2023
e783f33
Merge branch 'feat/team_project_feature' into feat/integrate_arborist…
pieterlukasse Dec 13, 2023
faed7c2
feat: improve variable name to reflect its real nature
pieterlukasse Dec 14, 2023
198efbf
Merge pull request #82 from uc-cdis/feat/integrate_arborist_validatio…
pieterlukasse Dec 14, 2023
8e04f96
feat: adding "team project" check to cohortdata methods
pieterlukasse Dec 14, 2023
c38cfad
feat: remove unused endpoints
pieterlukasse Dec 15, 2023
8b40f1c
fix: add query parameter by-team-project?team-project= to stats endpoint
pieterlukasse Dec 18, 2023
6852e94
fix: prefer method TeamProjectValidation
pieterlukasse Dec 18, 2023
f7b7fc7
fix: fix tests after by-team-project?team-project= to stats endpoint …
pieterlukasse Dec 18, 2023
30fb6b4
Merge pull request #83 from uc-cdis/feat/integrate_arborist_validatio…
pieterlukasse Dec 19, 2023
1836c2b
Revert "feat: remove unused endpoints"
pieterlukasse Jan 10, 2024
9ac8935
wip: add validation TODOs
pieterlukasse Jan 23, 2024
ecb2237
Merge pull request #86 from uc-cdis/feat/team_project_feature
pieterlukasse Jan 23, 2024
75e4b48
feat: add missing arborist checks
pieterlukasse Jan 26, 2024
45b3110
fix: return StatusForbidden instead of StatusBadRequest
pieterlukasse Jan 26, 2024
d4f3520
feat: add new test for RetriveById
pieterlukasse Jan 29, 2024
91a922d
feat: add new test for RetriveStatsBySourceIdAndTeamProject
pieterlukasse Jan 29, 2024
9b451e7
feat: extra tests to cover some blind spots in teamprojectauthz
pieterlukasse Jan 29, 2024
aa75cb3
fix: better test name
pieterlukasse Jan 30, 2024
5e1839b
Merge pull request #87 from uc-cdis/feat/add_missing_arborist_checks
pieterlukasse Jan 30, 2024
922c748
feat: updated overview diagram
pieterlukasse Jan 30, 2024
9ab34c7
feat: improve tests for GetTeamProjectsThatMatchAllCohortDefinitionIds
pieterlukasse Feb 6, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ cd tests/setup_local_db/
JSON summary data endpoints:
```bash
curl http://localhost:8080/sources | python -m json.tool
curl http://localhost:8080/cohortdefinition-stats/by-source-id/1 | python -m json.tool
curl "http://localhost:8080/cohortdefinition-stats/by-source-id/1/by-team-project?team-project=test" | python -m json.tool
curl http://localhost:8080/concept/by-source-id/1 | python -m json.tool
curl -d '{"ConceptIds":[2000000324,2000006885]}' -H "Content-Type: application/json" -X POST http://localhost:8080/concept/by-source-id/1 | python -m json.tool
curl -d '{"ConceptTypes":["Measurement","Person"]}' -H "Content-Type: application/json" -X POST http://localhost:8080/concept/by-source-id/1/by-type | python -m json.tool
Expand Down
1 change: 1 addition & 0 deletions config/mocktest.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
arborist_endpoint: 'https://arboristdummyurl'
41 changes: 35 additions & 6 deletions controllers/cohortdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,21 @@ import (
"strconv"

"github.com/gin-gonic/gin"
"github.com/uc-cdis/cohort-middleware/middlewares"
"github.com/uc-cdis/cohort-middleware/models"
"github.com/uc-cdis/cohort-middleware/utils"
)

type CohortDataController struct {
cohortDataModel models.CohortDataI
cohortDataModel models.CohortDataI
teamProjectAuthz middlewares.TeamProjectAuthzI
}

func NewCohortDataController(cohortDataModel models.CohortDataI) CohortDataController {
return CohortDataController{cohortDataModel: cohortDataModel}
func NewCohortDataController(cohortDataModel models.CohortDataI, teamProjectAuthz middlewares.TeamProjectAuthzI) CohortDataController {
return CohortDataController{
cohortDataModel: cohortDataModel,
teamProjectAuthz: teamProjectAuthz,
}
}

func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Context) {
Expand All @@ -44,6 +49,14 @@ func (u CohortDataController) RetrieveHistogramForCohortIdAndConceptId(c *gin.Co
cohortId, _ := strconv.Atoi(cohortIdStr)
histogramConceptId, _ := strconv.ParseInt(histogramIdStr, 10, 64)

validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

cohortData, err := u.cohortDataModel.RetrieveHistogramDataBySourceIdAndCohortIdAndConceptIdsAndCohortPairs(sourceId, cohortId, histogramConceptId, filterConceptIds, cohortPairs)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving concept details", "error": err.Error()})
Expand Down Expand Up @@ -85,6 +98,14 @@ func (u CohortDataController) RetrieveDataBySourceIdAndCohortIdAndVariables(c *g
sourceId, _ := strconv.Atoi(sourceIdStr)
cohortId, _ := strconv.Atoi(cohortIdStr)

validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

// call model method:
cohortData, err := u.cohortDataModel.RetrieveDataBySourceIdAndCohortIdAndConceptIdsOrderedByPersonId(sourceId, cohortId, conceptIds)
if err != nil {
Expand All @@ -111,7 +132,7 @@ func generateCohortPairsHeaders(cohortPairs []utils.CustomDichotomousVariableDef
cohortPairsHeaders := []string{}

for _, cohortPair := range cohortPairs {
cohortPairsHeaders = append(cohortPairsHeaders, utils.GetCohortPairKey(cohortPair.CohortId1, cohortPair.CohortId2))
cohortPairsHeaders = append(cohortPairsHeaders, utils.GetCohortPairKey(cohortPair.CohortDefinitionId1, cohortPair.CohortDefinitionId2))
}

return cohortPairsHeaders
Expand Down Expand Up @@ -230,6 +251,14 @@ func (u CohortDataController) RetrieveCohortOverlapStatsWithoutFilteringOnConcep
controlCohortId, errors[2] = utils.ParseNumericArg(c, "controlcohortid")
conceptIds, cohortPairs, errors[3] = utils.ParseConceptIdsAndDichotomousDefs(c)

validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{caseCohortId, controlCohortId}, cohortPairs)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

if utils.ContainsNonNil(errors) {
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"})
c.Abort()
Expand Down Expand Up @@ -298,8 +327,8 @@ func (u CohortDataController) RetrievePeopleIdAndCohort(sourceId int, cohortId i
*/
personIdToCSVValues := make(map[int64]map[string]string)
for _, cohortPair := range cohortPairs {
firstCohortDefinitionId := cohortPair.CohortId1
secondCohortDefinitionId := cohortPair.CohortId2
firstCohortDefinitionId := cohortPair.CohortDefinitionId1
secondCohortDefinitionId := cohortPair.CohortDefinitionId2
cohortPairKey := utils.GetCohortPairKey(firstCohortDefinitionId, secondCohortDefinitionId)

firstCohortPeopleData, err1 := u.cohortDataModel.RetrieveDataByOriginalCohortAndNewCohort(sourceId, cohortId, firstCohortDefinitionId)
Expand Down
57 changes: 29 additions & 28 deletions controllers/cohortdefinition.go
Original file line number Diff line number Diff line change
@@ -1,27 +1,41 @@
package controllers

import (
"log"
"net/http"
"strconv"

"github.com/gin-gonic/gin"
"github.com/uc-cdis/cohort-middleware/middlewares"
"github.com/uc-cdis/cohort-middleware/models"
"github.com/uc-cdis/cohort-middleware/utils"
)

type CohortDefinitionController struct {
cohortDefinitionModel models.CohortDefinitionI
teamProjectAuthz middlewares.TeamProjectAuthzI
}

func NewCohortDefinitionController(cohortDefinitionModel models.CohortDefinitionI) CohortDefinitionController {
return CohortDefinitionController{cohortDefinitionModel: cohortDefinitionModel}
func NewCohortDefinitionController(cohortDefinitionModel models.CohortDefinitionI, teamProjectAuthz middlewares.TeamProjectAuthzI) CohortDefinitionController {
return CohortDefinitionController{
cohortDefinitionModel: cohortDefinitionModel,
teamProjectAuthz: teamProjectAuthz,
}
}

func (u CohortDefinitionController) RetriveById(c *gin.Context) {
cohortDefinitionId := c.Param("id")

if cohortDefinitionId != "" {
cohortDefinitionId, _ := strconv.Atoi(cohortDefinitionId)
// validate teamproject access permission for cohort:
validAccessRequest := u.teamProjectAuthz.TeamProjectValidationForCohort(c, cohortDefinitionId)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}
cohortDefinition, err := u.cohortDefinitionModel.GetCohortDefinitionById(cohortDefinitionId)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving cohortDefinition", "error": err.Error()})
Expand All @@ -35,39 +49,26 @@ func (u CohortDefinitionController) RetriveById(c *gin.Context) {
c.Abort()
}

func (u CohortDefinitionController) RetriveByName(c *gin.Context) {
cohortDefinitionName := c.Param("name")

if cohortDefinitionName != "" {
cohortDefinition, err := u.cohortDefinitionModel.GetCohortDefinitionByName(cohortDefinitionName)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving cohortDefinition", "error": err.Error()})
c.Abort()
return
}
c.JSON(http.StatusOK, gin.H{"CohortDefinition": cohortDefinition})
func (u CohortDefinitionController) RetriveStatsBySourceIdAndTeamProject(c *gin.Context) {
// This method returns ALL cohortdefinition entries with cohort size statistics (for a given source)
sourceId, err1 := utils.ParseNumericArg(c, "sourceid")
teamProject := c.Query("team-project")
if teamProject == "" {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error while parsing request", "error": "team-project is a mandatory parameter but was found to be empty!"})
c.Abort()
return
}
c.JSON(http.StatusBadRequest, gin.H{"message": "bad request"})
c.Abort()
}

func (u CohortDefinitionController) RetriveAll(c *gin.Context) {
cohortDefinitions, err := u.cohortDefinitionModel.GetAllCohortDefinitions()
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving cohortDefinition", "error": err.Error()})
// validate teamproject access permission:
validAccessRequest := u.teamProjectAuthz.HasAccessToTeamProject(c, teamProject)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}
c.JSON(http.StatusOK, gin.H{"cohort_definitions": cohortDefinitions})
}

func (u CohortDefinitionController) RetriveStatsBySourceId(c *gin.Context) {
// This method returns ALL cohortdefinition entries with cohort size statistics (for a given source)

sourceId, err1 := utils.ParseNumericArg(c, "sourceid")
if err1 == nil {
cohortDefinitionsAndStats, err := u.cohortDefinitionModel.GetAllCohortDefinitionsAndStatsOrderBySizeDesc(sourceId)
cohortDefinitionsAndStats, err := u.cohortDefinitionModel.GetAllCohortDefinitionsAndStatsOrderBySizeDesc(sourceId, teamProject)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{"message": "Error retrieving cohortDefinition", "error": err.Error()})
c.Abort()
Expand Down
35 changes: 33 additions & 2 deletions controllers/concept.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,23 @@ import (
"strconv"

"github.com/gin-gonic/gin"
"github.com/uc-cdis/cohort-middleware/middlewares"
"github.com/uc-cdis/cohort-middleware/models"
"github.com/uc-cdis/cohort-middleware/utils"
)

type ConceptController struct {
conceptModel models.ConceptI
cohortDefinitionModel models.CohortDefinitionI
teamProjectAuthz middlewares.TeamProjectAuthzI
}

func NewConceptController(conceptModel models.ConceptI, cohortDefinitionModel models.CohortDefinitionI) ConceptController {
return ConceptController{conceptModel: conceptModel, cohortDefinitionModel: cohortDefinitionModel}
func NewConceptController(conceptModel models.ConceptI, cohortDefinitionModel models.CohortDefinitionI, teamProjectAuthz middlewares.TeamProjectAuthzI) ConceptController {
return ConceptController{
conceptModel: conceptModel,
cohortDefinitionModel: cohortDefinitionModel,
teamProjectAuthz: teamProjectAuthz,
}
}

func (u ConceptController) RetriveAllBySourceId(c *gin.Context) {
Expand Down Expand Up @@ -93,6 +99,14 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortId(c *gin.Co
c.Abort()
return
}
validAccessRequest := u.teamProjectAuthz.TeamProjectValidationForCohort(c, cohortId)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

breakdownConceptId, err := utils.ParseBigNumericArg(c, "breakdownconceptid")
if err != nil {
log.Printf("Error: %s", err.Error())
Expand All @@ -118,6 +132,14 @@ func (u ConceptController) RetrieveBreakdownStatsBySourceIdAndCohortIdAndVariabl
c.Abort()
return
}
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

breakdownConceptId, err := utils.ParseBigNumericArg(c, "breakdownconceptid")
if err != nil {
log.Printf("Error: %s", err.Error())
Expand Down Expand Up @@ -175,6 +197,15 @@ func (u ConceptController) RetrieveAttritionTable(c *gin.Context) {
c.Abort()
return
}
_, cohortPairs := utils.GetConceptIdsAndCohortPairsAsSeparateLists(conceptIdsAndCohortPairs)
validAccessRequest := u.teamProjectAuthz.TeamProjectValidation(c, []int{cohortId}, cohortPairs)
if !validAccessRequest {
log.Printf("Error: invalid request")
c.JSON(http.StatusForbidden, gin.H{"message": "access denied"})
c.Abort()
return
}

breakdownConceptId, err := utils.ParseBigNumericArg(c, "breakdownconceptid")
if err != nil {
log.Printf("Error: %s", err.Error())
Expand Down
Binary file modified docs/cohort-middleware-overview.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
23 changes: 18 additions & 5 deletions middlewares/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ func AuthMiddleware() gin.HandlerFunc {
}

return func(ctx *gin.Context) {
req, err := PrepareNewArboristRequest(ctx, c.GetString("arborist_endpoint"))
req, err := PrepareNewArboristRequest(ctx)
if err != nil {
ctx.AbortWithStatus(500)
log.Printf("Error while preparing Arborist request: %s", err.Error())
return
}
client := &http.Client{}
// send the request to Arborist:
Expand All @@ -44,21 +45,33 @@ func AuthMiddleware() gin.HandlerFunc {
}

// this function will take the request from the given ctx, validated it for the presence of an "Authorization / Bearer" token
// and then return the URL that can be used to consult Arborist regarding access permissions. This function
// and then return the URL that can be used to consult Arborist regarding cohort-middleware access permissions. This function
// returns an error if "Authorization / Bearer" token is missing in ctx
func PrepareNewArboristRequest(ctx *gin.Context, arboristEndpoint string) (*http.Request, error) {
func PrepareNewArboristRequest(ctx *gin.Context) (*http.Request, error) {

resourcePath := fmt.Sprintf("/cohort-middleware%s", ctx.Request.URL.Path)
service := "cohort-middleware"

return PrepareNewArboristRequestForResourceAndService(ctx, resourcePath, service)
}

// this function will take the request from the given ctx, validated it for the presence of an "Authorization / Bearer" token
// and then return the URL that can be used to consult Arborist regarding access permissions for the given
// resource path and service.
func PrepareNewArboristRequestForResourceAndService(ctx *gin.Context, resourcePath string, service string) (*http.Request, error) {
c := config.GetConfig()
arboristEndpoint := c.GetString("arborist_endpoint")
// validate:
authorization := ctx.Request.Header.Get("Authorization")
if authorization == "" {
return nil, errors.New("missing Authorization header")
}

// build up the request URL string:
resourcePath := fmt.Sprintf("/cohort-middleware%s", ctx.Request.URL.Path)
arboristAuth := fmt.Sprintf("%s/auth/proxy?resource=%s&service=%s&method=%s",
arboristEndpoint,
resourcePath,
"cohort-middleware",
service,
"access")

// make request object / validate URL:
Expand Down
Loading
Loading