Development (#53)

* tests can now fail

* ci: fixed tests

* get tests working again

* remove flag from workflow

* fix: fixed a couple of TLS related issues. TLS1.3 is now required

* fix: minor code adaptability issues fixed in http.go

* fix: code adaptability issues

Author: MadsRC

.idea/inspectionProfiles/Project_Default.xml

@@ -5,6 +5,12 @@
 [![Code Smells](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=code_smells)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
 [![Coverage](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=coverage)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
 [![Duplicated Lines (%)](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=duplicated_lines_density)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Lines of Code](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=ncloc)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Reliability Rating](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=reliability_rating)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Security Rating](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=security_rating)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Technical Debt](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=sqale_index)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Maintainability Rating](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=sqale_rating)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
+[![Vulnerabilities](https://sonarcloud.io/api/project_badges/measure?project=MadsRC_sophrosyne&metric=vulnerabilities)](https://sonarcloud.io/summary/new_code?id=MadsRC_sophrosyne)
 [![OpenSSF Scorecard](https://api.securityscorecards.dev/projects/github.com/MadsRC/sophrosyne/badge)](https://securityscorecards.dev/viewer/?uri=github.com/MadsRC/sophrosyne)
 [![OpenSSF Best Practices](https://www.bestpractices.dev/projects/8804/badge)](https://www.bestpractices.dev/projects/8804)
 [![CodeQL](https://github.com/MadsRC/sophrosyne/actions/workflows/github-code-scanning/codeql/badge.svg)](https://github.com/MadsRC/sophrosyne/actions/workflows/github-code-scanning/codeql)

README.md

@@ -32,10 +32,9 @@ import (
 )
 
 type Server struct {
-	appConfig      *sophrosyne.Config   `validate:"required"`
-	mux            *http.ServeMux       `validate:"required"`
-	validator      sophrosyne.Validator `validate:"required"`
-	middleware     []func(http.Handler) http.Handler
+	appConfig      *sophrosyne.Config        `validate:"required"`
+	mux            *http.ServeMux            `validate:"required"`
+	validator      sophrosyne.Validator      `validate:"required"`
 	logger         *slog.Logger              `validate:"required"`
 	http           *http.Server              `validate:"required"`
 	tracingService sophrosyne.TracingService `validate:"required"`
@@ -82,30 +81,37 @@ func (s *Server) Handle(path string, handler http.Handler) {
 	s.mux.Handle(path, handler)
 }
 
+const JSONContentType = "application/json"
+const PlainTextContentType = "text/plain"
+
 func RPCHandler(logger *slog.Logger, rpcService sophrosyne.RPCServer) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 
 		body, err := io.ReadAll(r.Body) // Find a way to implement a limit on the body size
+		if err != nil {
+			logger.ErrorContext(r.Context(), "failed to read request body", "error", err)
+			WriteInternalServerError(r.Context(), w, logger)
+			return
+		}
 		b, err := rpcService.HandleRPCRequest(r.Context(), body)
 		if err != nil {
 			logger.ErrorContext(r.Context(), "error handling rpc request", "error", err)
 			WriteInternalServerError(r.Context(), w, logger)
 			return
 		}
-		WriteResponse(r.Context(), w, http.StatusOK, "application/json", b, logger)
+		WriteResponse(r.Context(), w, http.StatusOK, JSONContentType, b, logger)
 	})
 }
 
 func HealthcheckHandler(logger *slog.Logger, healthcheckService sophrosyne.HealthCheckService) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		ok := healthcheckService.UnauthenticatedHealthcheck(r.Context())
 		if ok {
-			WriteResponse(r.Context(), w, http.StatusOK, "application/json", nil, logger)
+			WriteResponse(r.Context(), w, http.StatusOK, JSONContentType, nil, logger)
 			return
 		}
 		w.Header().Set("Retry-After", "5")
-		WriteResponse(r.Context(), w, http.StatusServiceUnavailable, "application/json", nil, logger)
-		return
+		WriteResponse(r.Context(), w, http.StatusServiceUnavailable, JSONContentType, nil, logger)
 	})
 }
 

internal/http/http.go

@@ -84,7 +84,7 @@ func Authentication(exceptions []string, config *sophrosyne.Config, userService
 		if !strings.HasPrefix(authHeader, "Bearer ") {
 			logger.DebugContext(r.Context(), "unable to extract token from Authorization header", "header", authHeader)
 			logger.InfoContext(r.Context(), "authentication", "result", "failed")
-			ownHttp.WriteResponse(r.Context(), w, http.StatusUnauthorized, "text/plain", nil, logger)
+			ownHttp.WriteResponse(r.Context(), w, http.StatusUnauthorized, ownHttp.PlainTextContentType, nil, logger)
 			return
 		}
 
@@ -93,7 +93,7 @@ func Authentication(exceptions []string, config *sophrosyne.Config, userService
 		if err != nil {
 			logger.DebugContext(r.Context(), "unable to decode token", "token", token, "error", err)
 			logger.InfoContext(r.Context(), "authentication", "result", "failed")
-			ownHttp.WriteResponse(r.Context(), w, http.StatusUnauthorized, "text/plain", nil, logger)
+			ownHttp.WriteResponse(r.Context(), w, http.StatusUnauthorized, ownHttp.PlainTextContentType, nil, logger)
 			return
 		}
 
@@ -135,8 +135,6 @@ func (w *responseWrapper) WriteHeader(status int) {
 	w.status = status
 	w.ResponseWriter.WriteHeader(status)
 	w.wroteHeader = true
-
-	return
 }
 
 func (w *responseWrapper) Status() int {

internal/http/middleware/middleware.go

@@ -227,11 +227,14 @@ func NewTLSServerConfig(config *sophrosyne.Config, randSource io.Reader) (*tls.C
 
 	return &tls.Config{
 		Certificates: []tls.Certificate{cert},
+		MinVersion:   tls.VersionTLS13,
 	}, nil
 }
 
 func NewTLSClientConfig(config *sophrosyne.Config) (*tls.Config, error) {
-	c := &tls.Config{}
+	c := &tls.Config{
+		MinVersion: tls.VersionTLS13,
+	}
 	if config.Security.TLS.InsecureSkipVerify {
 		c.InsecureSkipVerify = true
 	}

internal/tls/tls.go

@@ -215,7 +215,7 @@ func newHTTPClient(t *testing.T) *http.Client {
 	return &http.Client{
 		Transport: &http.Transport{
 			TLSClientConfig: &tls.Config{
-				InsecureSkipVerify: true,
+				InsecureSkipVerify: true, //nolint:gosec
 			},
 		},
 	}
@@ -232,7 +232,7 @@ func TestStartup(t *testing.T) {
 
 	t.Run("API served via TLS", func(t *testing.T) {
 		conf := &tls.Config{
-			InsecureSkipVerify: true,
+			InsecureSkipVerify: true, //nolint:gosec
 		}
 		tlsConn, err := tls.Dial("tcp", te.endpoint, conf)
 		require.NoError(t, err)
@@ -263,7 +263,13 @@ func TestStartup(t *testing.T) {
 	// of the LogConsumer added to setupEnv, if this log cannot be unmarshalled as JSON, it fails. Thus this test
 	// ensures that it is logged as JSON.
 	t.Run("client remote error logged as non-json", func(t *testing.T) {
-		tlsConn, err := tls.Dial("tcp", te.endpoint, &tls.Config{})
+		tlsConn, err := tls.Dial("tcp", te.endpoint, &tls.Config{MinVersion: tls.VersionTLS13})
+		require.Error(t, err)
+		require.Nil(t, tlsConn)
+	})
+
+	t.Run("TLS1.3 or better required", func(t *testing.T) {
+		tlsConn, err := tls.Dial("tcp", te.endpoint, &tls.Config{MinVersion: tls.VersionTLS12, MaxVersion: tls.VersionTLS12}) //nolint:gosec
 		require.Error(t, err)
 		require.Nil(t, tlsConn)
 	})