Merge pull request #96 from iandyh/ownership-2

Ownership check at collection level

shibuya/api/collection.go

@@ -25,7 +25,7 @@ func getCollection(collectionID string) (*model.Collection, error) {
 }
 
 func (s *ShibuyaAPI) collectionConfigGetHandler(w http.ResponseWriter, req *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(req, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return

shibuya/api/main.go

@@ -5,7 +5,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"io/ioutil"
+	"io"
 	"net/http"
 	"strconv"
 	"time"
@@ -353,7 +353,7 @@ func (s *ShibuyaAPI) collectionFilesGetHandler(w http.ResponseWriter, _ *http.Re
 }
 
 func (s *ShibuyaAPI) collectionFilesUploadHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -373,7 +373,7 @@ func (s *ShibuyaAPI) collectionFilesUploadHandler(w http.ResponseWriter, r *http
 }
 
 func (s *ShibuyaAPI) collectionFilesDeleteHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -448,25 +448,11 @@ func (s *ShibuyaAPI) collectionCreateHandler(w http.ResponseWriter, r *http.Requ
 }
 
 func (s *ShibuyaAPI) collectionDeleteHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	account := model.GetAccountBySession(r)
-	if account == nil {
-		s.handleErrors(w, makeLoginError())
-		return
-	}
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
 	}
-	project, err := model.GetProject(collection.ProjectID)
-	if err != nil {
-		s.handleErrors(w, err)
-		return
-	}
-	if _, ok := account.MLMap[project.Owner]; !ok {
-		s.handleErrors(w, makeNoPermissionErr("You don't have permission"))
-		return
-	}
 	if config.SC.ExecutorConfig.Cluster.OnDemand {
 		operator := controller.NewGCPOperator(collection.ID, 0)
 		pool := operator.GetNodePool()
@@ -492,7 +478,7 @@ func (s *ShibuyaAPI) collectionDeleteHandler(w http.ResponseWriter, r *http.Requ
 }
 
 func (s *ShibuyaAPI) collectionGetHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -531,14 +517,19 @@ func (s *ShibuyaAPI) collectionUpdateHandler(w http.ResponseWriter, _ *http.Requ
 }
 
 func (s *ShibuyaAPI) collectionUploadHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
+	collection, err := checkCollectionOwnership(r, params)
+	if err != nil {
+		s.handleErrors(w, err)
+		return
+	}
 	e := new(model.ExecutionWrapper)
 	r.ParseMultipartForm(1 << 20) //parse 1 MB of data
 	file, _, err := r.FormFile("collectionYAML")
 	if err != nil {
 		s.handleErrors(w, makeInvalidResourceError("file"))
 		return
 	}
-	raw, err := ioutil.ReadAll(file)
+	raw, err := io.ReadAll(file)
 	if err != nil {
 		s.handleErrors(w, makeInvalidRequestError("invalid file"))
 		return
@@ -552,22 +543,16 @@ func (s *ShibuyaAPI) collectionUploadHandler(w http.ResponseWriter, r *http.Requ
 		s.handleErrors(w, makeInvalidResourceError("YAML file"))
 		return
 	}
-	collectionID, _ := strconv.ParseInt(params.ByName("collection_id"), 10, 64)
-	if (e.Content.CollectionID != 0) && (e.Content.CollectionID != collectionID) {
+	if e.Content.CollectionID != collection.ID {
 		s.handleErrors(w, makeInvalidRequestError("collection ID mismatch"))
 		return
 	}
-	collection, err := model.GetCollection(collectionID)
+	project, err := model.GetProject(collection.ProjectID)
 	if err != nil {
+		log.Error(err)
 		s.handleErrors(w, err)
 		return
 	}
-	project, _ := model.GetProject(collection.ProjectID)
-	account := model.GetAccountBySession(r)
-	if _, ok := account.MLMap[project.Owner]; !ok {
-		s.handleErrors(w, makeNoPermissionErr("You don't own the project"))
-		return
-	}
 	totalEnginesRequired := 0
 	for _, ep := range e.Content.Tests {
 		plan, err := model.GetPlan(ep.PlanID)
@@ -626,7 +611,7 @@ func (s *ShibuyaAPI) collectionUploadHandler(w http.ResponseWriter, r *http.Requ
 }
 
 func (s *ShibuyaAPI) collectionEnginesDetailHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -640,7 +625,7 @@ func (s *ShibuyaAPI) collectionEnginesDetailHandler(w http.ResponseWriter, r *ht
 }
 
 func (s *ShibuyaAPI) collectionDeploymentHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -657,7 +642,7 @@ func (s *ShibuyaAPI) collectionDeploymentHandler(w http.ResponseWriter, r *http.
 }
 
 func (s *ShibuyaAPI) collectionTriggerHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -669,7 +654,7 @@ func (s *ShibuyaAPI) collectionTriggerHandler(w http.ResponseWriter, r *http.Req
 }
 
 func (s *ShibuyaAPI) collectionTermHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -681,7 +666,7 @@ func (s *ShibuyaAPI) collectionTermHandler(w http.ResponseWriter, r *http.Reques
 }
 
 func (s *ShibuyaAPI) collectionStatusHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -694,7 +679,7 @@ func (s *ShibuyaAPI) collectionStatusHandler(w http.ResponseWriter, r *http.Requ
 }
 
 func (s *ShibuyaAPI) collectionPurgeHandler(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	collection, err := getCollection(params.ByName("collection_id"))
+	collection, err := checkCollectionOwnership(r, params)
 	if err != nil {
 		s.handleErrors(w, err)
 		return
@@ -748,7 +733,11 @@ func (s *ShibuyaAPI) planLogHandler(w http.ResponseWriter, r *http.Request, para
 }
 
 func (s *ShibuyaAPI) streamCollectionMetrics(w http.ResponseWriter, r *http.Request, params httprouter.Params) {
-	// Currently we don't do authentication for simplicity.
+	collection, err := checkCollectionOwnership(r, params)
+	if err != nil {
+		s.handleErrors(w, err)
+		return
+	}
 	flusher, ok := w.(http.Flusher)
 	if !ok {
 		http.Error(w, "Streaming unsupported!", http.StatusInternalServerError)
@@ -759,11 +748,6 @@ func (s *ShibuyaAPI) streamCollectionMetrics(w http.ResponseWriter, r *http.Requ
 	w.Header().Set("Connection", "keep-alive")
 	w.Header().Set("Access-Control-Allow-Origin", "*")
 
-	collection, err := getCollection(params.ByName("collection_id"))
-	if err != nil {
-		s.handleErrors(w, err)
-		return
-	}
 	clientIP := retrieveClientIP(r)
 	item := &controller.ApiMetricStream{
 		StreamClient: make(chan *controller.ApiMetricStreamEvent),

shibuya/api/utils.go

@@ -3,6 +3,9 @@ package api
 import (
 	"net/http"
 	"strings"
+
+	"github.com/julienschmidt/httprouter"
+	"github.com/rakutentech/shibuya/shibuya/model"
 )
 
 func retrieveClientIP(r *http.Request) string {
@@ -12,3 +15,24 @@ func retrieveClientIP(r *http.Request) string {
 	}
 	return strings.Split(t, ",")[0]
 }
+
+func checkCollectionOwnership(r *http.Request, params httprouter.Params) (*model.Collection, error) {
+	account := model.GetAccountBySession(r)
+	if account == nil {
+		return nil, makeLoginError()
+	}
+	collection, err := getCollection(params.ByName("collection_id"))
+	if err != nil {
+		return nil, err
+	}
+	project, err := model.GetProject(collection.ProjectID)
+	if err != nil {
+		return nil, err
+	}
+	if _, ok := account.MLMap[project.Owner]; !ok {
+		if !account.IsAdmin() {
+			return nil, makeNoPermissionErr("You are not the owner of the collection")
+		}
+	}
+	return collection, nil
+}

shibuya/model/user.go

@@ -39,3 +39,14 @@ func GetAccountBySession(r *http.Request) *Account {
 	}
 	return a
 }
+
+func (a *Account) IsAdmin() bool {
+	for _, ml := range a.ML {
+		for _, admin := range config.SC.AuthConfig.AdminUsers {
+			if ml == admin {
+				return true
+			}
+		}
+	}
+	return false
+}

shibuya/ui/handler.go

@@ -45,16 +45,7 @@ func (u *UI) homeHandler(w http.ResponseWriter, r *http.Request, params httprout
 		http.Redirect(w, r, "/login", http.StatusSeeOther)
 		return
 	}
-	IsAdmin := false
-outer:
-	for _, ml := range account.ML {
-		for _, admin := range config.SC.AuthConfig.AdminUsers {
-			if ml == admin {
-				IsAdmin = true
-				break outer
-			}
-		}
-	}
+	IsAdmin := account.IsAdmin()
 	enableSid := config.SC.EnableSid
 	resultDashboardURL := config.SC.DashboardConfig.Url + config.SC.DashboardConfig.RunDashboard
 	engineHealthDashboardURL := config.SC.DashboardConfig.Url + config.SC.DashboardConfig.EnginesDashboard

shibuya/ui/static/js/collection.js

@@ -161,7 +161,7 @@ var Collection = Vue.component("collection", {
                     this.collection = resp.body;
                 },
                 function (resp) {
-                    console.log(resp.body);
+                    alert(resp.body.message);
                 }
             );
             this.$http.get("collections/" + this.collection_id + '/status').then(
@@ -170,7 +170,7 @@ var Collection = Vue.component("collection", {
                     this.updateCache(this.collection_status.status);
                 },
                 function (resp) {
-                    console.log(resp.body);
+                    alert(resp.body.message);
                 }
             );
         },
@@ -394,4 +394,4 @@ var Collection = Vue.component("collection", {
             );
         }
     }
-});
+});