Merge pull request #249 from neicnordic/feature/use-gin-router

Replace mux with gin router

Author: norling

.github/integration/tests/common/50_check_endpoint.sh

@@ -59,7 +59,7 @@ fi
 
 echo "expected dataset found"
 
-## Test datasets/files endpoint 
+## Test datasets/files endpoint
 
 check_files=$(curl --cacert certs/ca.pem -H "Authorization: Bearer $token" "https://localhost:8443/metadata/datasets/https://doi.example/ty009.sfrrss/600.45asasga/files" | jq -r '.[0].fileId')
 
@@ -91,7 +91,7 @@ else
 fi
 
 # ------------------
-# Test bad token
+# Test get visas failed
 
 token=$(curl --cacert certs/ca.pem "https://localhost:8000/tokens" | jq -r  '.[1]')
 

.github/integration/tests/s3notls/52_check_endpoint.sh

@@ -55,7 +55,7 @@ fi
 
 echo "expected dataset found"
 
-## Test datasets/files endpoint 
+## Test datasets/files endpoint
 
 check_files=$(curl -H "Authorization: Bearer $token" "http://localhost:8080/metadata/datasets/https://doi.example/ty009.sfrrss/600.45asasga/files" | jq -r '.[0].fileId')
 
@@ -78,7 +78,7 @@ crypt4gh decrypt --sk c4gh.sec.pem < dummy_data.c4gh > old-file.txt
 curl -H "Authorization: Bearer $token" "http://localhost:8080/files/urn:neic:001-002" --output test-download.txt
 
 
-cmp --silent old-file.txt test-download.txt 
+cmp --silent old-file.txt test-download.txt
 status=$?
 if [[ $status = 0 ]]; then
     echo "Files are the same"
@@ -87,7 +87,7 @@ else
 fi
 
 # ------------------
-# Test bad token
+# Test get visas failed
 
 token=$(curl --cacert certs/ca.pem "https://localhost:8000/tokens" | jq -r  '.[1]')
 

README.md

@@ -26,7 +26,7 @@ There is an README file in the [dev_utils](/dev_utils) folder with sections for
 | Component     | Role |
 |---------------|------|
 | middleware     | Performs access token verification and validation |
-| sda        | Constructs the main API endpoints fort the NeIC SDA Data Out API. |
+| sda        | Constructs the main API endpoints for the NeIC SDA Data Out API. |
 
 
 ## Internal Components
@@ -43,4 +43,4 @@ There is an README file in the [dev_utils](/dev_utils) folder with sections for
 | Component     | Role |
 |---------------|------|
 | auth        | Auth pkg is used by the middleware to parse OIDC Details and extract GA4GH Visas from a [GA4GH Passport](https://github.com/ga4gh-duri/ga4gh-duri.github.io/blob/master/researcher_ids/ga4gh_passport_v1.md) |
-| request       | This pkg Stores a HTTP client, so that it doesn't need to be initialised on every request. |
+| request       | This pkg Stores a HTTP client, so that it doesn't need to be initialised on every request. |

api/api.go

@@ -6,29 +6,32 @@ import (
 	"net/http"
 	"time"
 
-	"github.com/gorilla/mux"
+	"github.com/gin-gonic/gin"
 	"github.com/neicnordic/sda-download/api/middleware"
 	"github.com/neicnordic/sda-download/api/sda"
 	"github.com/neicnordic/sda-download/internal/config"
 	log "github.com/sirupsen/logrus"
 )
 
 // healthResponse
-func healthResponse(w http.ResponseWriter, r *http.Request) {
+func healthResponse(c *gin.Context) {
 	// ok response to health
-	w.WriteHeader(http.StatusOK)
+	c.Writer.WriteHeader(http.StatusOK)
 }
 
 // Setup configures the web server and registers the routes
 func Setup() *http.Server {
 	// Set up routing
 	log.Info("(2/5) Registering endpoint handlers")
-	r := mux.NewRouter().SkipClean(true)
 
-	r.Handle("/metadata/datasets", middleware.TokenMiddleware(http.HandlerFunc(sda.Datasets))).Methods("GET")
-	r.Handle("/metadata/datasets/{dataset:[^\\s/$.?#].[^\\s]+|[A-Za-z0-9-_:.]+}/files", middleware.TokenMiddleware(http.HandlerFunc(sda.Files))).Methods("GET")
-	r.Handle("/files/{fileid:[A-Za-z0-9-_:.]+}", middleware.TokenMiddleware(http.HandlerFunc(sda.Download))).Methods("GET")
-	r.HandleFunc("/health", healthResponse).Methods("GET")
+	router := gin.Default()
+
+	router.HandleMethodNotAllowed = true
+
+	router.GET("/metadata/datasets", middleware.TokenMiddleware(), sda.Datasets)
+	router.GET("/metadata/datasets/*dataset", middleware.TokenMiddleware(), sda.Files)
+	router.GET("/files/:fileid", middleware.TokenMiddleware(), sda.Download)
+	router.GET("/health", healthResponse)
 
 	// Configure TLS settings
 	log.Info("(3/5) Configuring TLS")
@@ -45,7 +48,7 @@ func Setup() *http.Server {
 	log.Info("(4/5) Configuring server")
 	srv := &http.Server{
 		Addr:              config.Config.App.Host + ":" + fmt.Sprint(config.Config.App.Port),
-		Handler:           r,
+		Handler:           router,
 		TLSConfig:         cfg,
 		TLSNextProto:      make(map[string]func(*http.Server, *tls.Conn, http.Handler)),
 		ReadHeaderTimeout: 20 * time.Second,

api/middleware/middleware.go

@@ -1,46 +1,43 @@
 package middleware
 
 import (
-	"context"
 	"net/http"
 
+	"github.com/gin-gonic/gin"
 	"github.com/neicnordic/sda-download/internal/config"
 	"github.com/neicnordic/sda-download/internal/session"
 	"github.com/neicnordic/sda-download/pkg/auth"
 	log "github.com/sirupsen/logrus"
 )
 
-type stringVariable string
-
-// as specified in docs: https://pkg.go.dev/context#WithValue
-var datasetsKey = stringVariable("datasets")
+var datasetsKey = "datasets"
 
 // TokenMiddleware performs access token verification and validation
 // JWTs are verified and validated by the app, opaque tokens are sent to AAI for verification
 // Successful auth results in list of authorised datasets
-func TokenMiddleware(nextHandler http.Handler) http.Handler {
-
-	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+func TokenMiddleware() gin.HandlerFunc {
 
+	return func(c *gin.Context) {
 		// Check if dataset permissions are cached to session
-		sessionCookie, err := r.Cookie(config.Config.Session.Name)
+		sessionCookie, err := c.Cookie(config.Config.Session.Name)
 		if err != nil {
 			log.Debugf("no session cookie received")
 		}
 		var datasets []string
 		var exists bool
-		if sessionCookie != nil {
+		if sessionCookie != "" {
 			log.Debug("session cookie received")
-			datasets, exists = session.Get(sessionCookie.Value)
+			datasets, exists = session.Get(sessionCookie)
 		}
 
 		if !exists {
 			log.Debug("no session found, create new session")
 
 			// Check that a token is provided
-			token, code, err := auth.GetToken(r.Header.Get("Authorization"))
+			token, code, err := auth.GetToken(c.Request.Header.Get("Authorization"))
 			if err != nil {
-				http.Error(w, err.Error(), code)
+				c.String(code, err.Error())
+				c.AbortWithStatus(code)
 
 				return
 			}
@@ -49,7 +46,8 @@ func TokenMiddleware(nextHandler http.Handler) http.Handler {
 			visas, err := auth.GetVisas(auth.Details, token)
 			if err != nil {
 				log.Debug("failed to validate token at AAI")
-				http.Error(w, "bad token", 401)
+				c.String(http.StatusUnauthorized, "get visas failed")
+				c.AbortWithStatus(code)
 
 				return
 			}
@@ -64,47 +62,40 @@ func TokenMiddleware(nextHandler http.Handler) http.Handler {
 			// Start a new session and store datasets under the session key
 			key := session.NewSessionKey()
 			session.Set(key, datasets)
-			sessionCookie := &http.Cookie{
-				Name:     config.Config.Session.Name,
-				Value:    key,
-				Domain:   config.Config.Session.Domain,
-				Secure:   config.Config.Session.Secure,
-				HttpOnly: config.Config.Session.HTTPOnly,
-				// time.Duration is stored in nanoseconds, but MaxAge wants seconds
-				MaxAge: int(config.Config.Session.Expiration) / 1e9,
-			}
-			http.SetCookie(w, sessionCookie)
+			c.SetCookie(config.Config.Session.Name, // name
+				key, // value
+				int(config.Config.Session.Expiration)/1e9, // max age
+				"/",                            // path
+				config.Config.Session.Domain,   // domain
+				config.Config.Session.Secure,   // secure
+				config.Config.Session.HTTPOnly, // httpOnly
+			)
 			log.Debug("authorization check passed")
 		}
 
 		// Store dataset list to request context, for use in the endpoint handlers
-		modifiedContext := storeDatasets(r.Context(), datasets)
-		modifiedRequest := r.WithContext(modifiedContext)
+		c = storeDatasets(c, datasets)
 
 		// Forward request to the next endpoint handler
-		nextHandler.ServeHTTP(w, modifiedRequest)
-	})
+		c.Next()
+	}
 
 }
 
 // storeDatasets stores the dataset list to the request context
-func storeDatasets(ctx context.Context, datasets []string) context.Context {
+func storeDatasets(c *gin.Context, datasets []string) *gin.Context {
 	log.Debugf("storing %v datasets to request context", datasets)
 
-	ctx = context.WithValue(ctx, datasetsKey, datasets)
+	c.Set(datasetsKey, datasets)
 
-	return ctx
+	return c
 }
 
 // GetDatasets extracts the dataset list from the request context
-var GetDatasets = func(ctx context.Context) []string {
-	datasets := ctx.Value(datasetsKey)
-	if datasets == nil {
-		log.Debug("request datasets context is empty")
+var GetDatasets = func(c *gin.Context) []string {
+	datasets := c.GetStringSlice(datasetsKey)
 
-		return []string{}
-	}
 	log.Debugf("returning %v from request context", datasets)
 
-	return datasets.([]string)
+	return datasets
 }