Refactor permissions checks to avoid questions/works leaks between promotions/groups/start-availability

Thanks-To François Dautrême <francois.dautreme@epita.fr>

questions.go

@@ -39,7 +39,7 @@ func declareAPIAuthQuestionsRoutes(router *gin.RouterGroup) {
 				c.JSON(http.StatusOK, questions)
 			}
 		} else {
-			if (!s.Shown || s.Direct != nil) && !u.IsAdmin {
+			if s.Direct != nil && !u.IsAdmin {
 				c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not accessible"})
 				return
 			}
@@ -62,24 +62,7 @@ func declareAPIAuthQuestionsRoutes(router *gin.RouterGroup) {
 	questionsRoutes.Use(questionHandler)
 
 	questionsRoutes.GET("", func(c *gin.Context) {
-		q := c.MustGet("question").(*Question)
+		c.JSON(http.StatusOK, c.MustGet("question").(*Question))
-		u := c.MustGet("LoggedUser").(*User)
-
-		if !u.IsAdmin {
-			s, err := getSurvey(int(q.IdSurvey))
-			if err != nil {
-				log.Println("Unable to getSurvey:", err)
-				c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"errmsg": "An error occurs during survey retrieval. Please try again later."})
-				return
-			}
-
-			if !s.Shown || (s.Direct != nil && *s.Direct != q.Id) {
-				c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not authorized"})
-				return
-			}
-		}
-
-		c.JSON(http.StatusOK, q)
 	})
 
 	declareAPIAuthProposalsRoutes(questionsRoutes)
@@ -171,6 +154,8 @@ func declareAPIAdminUserQuestionsRoutes(router *gin.RouterGroup) {
 }
 
 func questionHandler(c *gin.Context) {
+	u := c.MustGet("LoggedUser").(*User)
+
 	var survey *Survey
 	if s, ok := c.Get("survey"); ok {
 		survey = s.(*Survey)
@@ -190,6 +175,15 @@ func questionHandler(c *gin.Context) {
 			c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"errmsg": "Question not found"})
 			return
 		}
+
+		s, err := getSurvey(int(question.IdSurvey))
+		if err != nil {
+			log.Println("Unable to getSurvey:", err)
+			c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"errmsg": "An error occurs during survey retrieval. Please try again later."})
+			return
+		}
+
+		survey = s
 	} else {
 		question, err = survey.GetQuestion(qid)
 		if err != nil {
@@ -198,6 +192,15 @@ func questionHandler(c *gin.Context) {
 		}
 	}
 
+	if !u.IsAdmin && (!survey.checkUserAccessToSurvey(u) || (survey.Direct != nil && *survey.Direct != question.Id)) {
+		c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not authorized"})
+		return
+	}
+	if !u.IsAdmin && survey.StartAvailability.After(time.Now()) {
+		c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not accessible yet"})
+		return
+	}
+
 	c.Set("question", question)
 
 	c.Next()

surveys.go

@@ -62,19 +62,14 @@ func declareAPISurveysRoutes(router *gin.RouterGroup) {
 			return
 		}
 
-		s := c.MustGet("survey").(*Survey)
+		c.JSON(http.StatusOK, c.MustGet("survey").(*Survey))
-
-		if (s.Promo == u.Promo && (s.Group == "" || strings.Contains(u.Groups, ","+s.Group+",") && s.Shown)) || u.IsAdmin {
-			c.JSON(http.StatusOK, s)
-		} else {
-			c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not accessible"})
-		}
 	})
 }
 
 func declareAPIAuthSurveysRoutes(router *gin.RouterGroup) {
 	surveysRoutes := router.Group("/surveys/:sid")
 	surveysRoutes.Use(surveyHandler)
+	surveysRoutes.Use(surveyUserAccessHandler)
 
 	surveysRoutes.GET("/score", func(c *gin.Context) {
 		loggedUser := c.MustGet("LoggedUser").(*User)
@@ -219,18 +214,20 @@ func surveyHandler(c *gin.Context) {
 	}
 }
 
+func (s *Survey) checkUserAccessToSurvey(u *User) bool {
+	return u.IsAdmin || (u.Promo == s.Promo && s.Shown && (s.Group == "" || strings.Contains(u.Groups, ","+s.Group+",")))
+}
+
 func surveyUserAccessHandler(c *gin.Context) {
 	u := c.MustGet("LoggedUser").(*User)
-	w := c.MustGet("survey").(*Survey)
+	s := c.MustGet("survey").(*Survey)
 
-	if u.IsAdmin {
+	if !s.checkUserAccessToSurvey(u) {
-		c.Next()
-	} else if w.Shown && (w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",")) {
-		c.Next()
-	} else {
 		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"errmsg": "Survey not found."})
 		return
 	}
+
+	c.Next()
 }
 
 type Survey struct {

works.go

@@ -43,7 +43,11 @@ func declareAPIWorksRoutes(router *gin.RouterGroup) {
 		} else {
 			for _, w := range works {
 				if w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",") {
+					// Remove informations not needed on front page for students
+					w.Promo = 0
 					w.Group = ""
+					w.DescriptionRaw = ""
+
 					response = append(response, w)
 				}
 			}
@@ -79,7 +83,10 @@ func declareAPIWorksRoutes(router *gin.RouterGroup) {
 		} else {
 			for _, w := range works {
 				if w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",") {
+					// Remove informations not needed on front page for students
+					w.Promo = 0
 					w.Group = ""
+
 					response = append(response, w)
 				}
 			}
@@ -193,16 +200,7 @@ func declareAPIAuthWorksRoutes(router *gin.RouterGroup) {
 	worksRoutes.Use(workUserAccessHandler)
 
 	worksRoutes.GET("", func(c *gin.Context) {
-		u := c.MustGet("LoggedUser").(*User)
+		c.JSON(http.StatusOK, c.MustGet("work").(*Work))
-		w := c.MustGet("work").(*Work)
-
-		if u.IsAdmin {
-			c.JSON(http.StatusOK, w)
-		} else if w.Shown && w.StartAvailability.Before(time.Now()) && (w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",")) {
-			c.JSON(http.StatusOK, w)
-		} else {
-			c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Permission denied"})
-		}
 	})
 
 	// Grades related to works
@@ -239,18 +237,24 @@ func workHandler(c *gin.Context) {
 	}
 }
 
+func (w *Work) checkUserAccessToWork(u *User) bool {
+	return u.IsAdmin || (u.Promo == w.Promo && w.Shown && (w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",")))
+}
+
 func workUserAccessHandler(c *gin.Context) {
 	u := c.MustGet("LoggedUser").(*User)
 	w := c.MustGet("work").(*Work)
 
-	if u.IsAdmin {
+	if !w.checkUserAccessToWork(u) {
-		c.Next()
-	} else if w.Shown && (w.Group == "" || strings.Contains(u.Groups, ","+w.Group+",")) {
-		c.Next()
-	} else {
 		c.AbortWithStatusJSON(http.StatusNotFound, gin.H{"errmsg": "Work not found."})
 		return
 	}
+	if !u.IsAdmin && w.StartAvailability.After(time.Now()) {
+		c.AbortWithStatusJSON(http.StatusForbidden, gin.H{"errmsg": "Not accessible yet"})
+		return
+	}
+
+	c.Next()
 }
 
 type OneWork struct {
@@ -291,14 +295,14 @@ func allWorks(cnd string, param ...interface{}) (items []*OneWork, err error) {
 type Work struct {
 	Id                int64     `json:"id"`
 	Title             string    `json:"title"`
-	Promo             uint      `json:"promo"`
+	Promo             uint      `json:"promo,omitempty"`
-	Group             string    `json:"group"`
+	Group             string    `json:"group,omitempty"`
 	Shown             bool      `json:"shown"`
-	Description       string    `json:"description"`
+	Description       string    `json:"description,omitempty"`
 	DescriptionRaw    string    `json:"descr_raw,omitempty"`
-	Tag               string    `json:"tag"`
+	Tag               string    `json:"tag,omitempty"`
-	SubmissionURL     *string   `json:"submission_url"`
+	SubmissionURL     *string   `json:"submission_url,omitempty"`
-	Corrected         bool      `json:"corrected"`
+	Corrected         bool      `json:"corrected,omitempty"`
 	StartAvailability time.Time `json:"start_availability"`
 	EndAvailability   time.Time `json:"end_availability"`
 }