Repository: Azure/ARO-RP
Pull request #2663 (2023-01-27 07:52:23)

replaced deprecated gorilla with standard library
### Which issue this PR addresses:
this is part of the effort to get rid of the deprecated gorilla library
https://issues.redhat.com/browse/ARO-1731
<!--
Please include a link to the ADO work item as well as any GitHub issues.

Usage: `Fixes #<GitHub issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes

### What this PR does / why we need it:

<!--
Include a brief summary of what the PR is intended to accomplish and how the PR
does it. (2-3 sentences)
-->

### Test plan for issue:
It should pass existing tests (bar the one that I modified because there is no segregation on methods in servemux)
<!--
How did you test that this PR works?

- Are there unit tests?
- Are there integration/e2e tests?
- If it is not possible to write automated tests, explain why and document how
  to manually test and verify the feature.
-->

### Is there any documentation that needs to be updated for this PR?

<!--
- If yes and the docs are in GitHub, include doc updates in the PR.
- If yes and the docs are not in GitHub (i.e. ADO wiki), include a link to the
  docs.
- If no, explain why (e.g. "tech debt cleanup, N/A").
-->



Patch:
diff --git a/pkg/dbtoken/server.go b/pkg/dbtoken/server.go
index 6caff7f3e..2b3a32da7 100644
--- a/pkg/dbtoken/server.go
+++ b/pkg/dbtoken/server.go
@@ -16,7 +16,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/gorilla/mux"
 	"github.com/sirupsen/logrus"
 
 	"github.com/Azure/ARO-RP/pkg/database/cosmosdb"
@@ -96,23 +95,31 @@ func NewServer(
 	}, nil
 }
 
+var healthProbe http.Handler = http.HandlerFunc(healthCheck)
+
+func healthCheck(w http.ResponseWriter, r *http.Request) {
+	if r.Method != http.MethodGet {
+		http.NotFound(w, r)
+		return
+	}
+	// empty 200
+}
+
 func (s *server) Run(ctx context.Context) error {
 	go heartbeat.EmitHeartbeat(s.log, s.m, "dbtoken.heartbeat", nil, func() bool { return true })
 
-	r := mux.NewRouter()
-	r.Use(middleware.Panic(s.log))
+	logHandler := Log(s.accessLog)
+	panicMiddleware := middleware.Panic(s.log)
+
+	muxRouter := http.NewServeMux()
 
-	unauthenticatedRouter := r.NewRoute().Subrouter()
-	unauthenticatedRouter.Use(Log(s.accessLog))
-	s.unauthenticatedRoutes(unauthenticatedRouter)
+	muxRouter.Handle("/healthz/ready", panicMiddleware(logHandler(healthProbe)))
 
-	authenticatedRouter := r.NewRoute().Subrouter()
-	authenticatedRouter.Use(s.authenticate)
-	authenticatedRouter.Use(Log(s.accessLog))
-	s.authenticatedRoutes(authenticatedRouter)
+	tokenRefresh := panicMiddleware(s.authenticate(logHandler(http.HandlerFunc(s.token))))
+	muxRouter.Handle("/token", tokenRefresh)
 
 	srv := &http.Server{
-		Handler:     r,
+		Handler:     muxRouter,
 		ReadTimeout: 10 * time.Second,
 		IdleTimeout: 2 * time.Minute,
 		ErrorLog:    log.New(s.log.Writer(), "", 0),
@@ -122,14 +129,6 @@ func (s *server) Run(ctx context.Context) error {
 	return srv.Serve(s.l)
 }
 
-func (s *server) unauthenticatedRoutes(r *mux.Router) {
-	r.NewRoute().Methods(http.MethodGet).Path("/healthz/ready").HandlerFunc(func(http.ResponseWriter, *http.Request) {})
-}
-
-func (s *server) authenticatedRoutes(r *mux.Router) {
-	r.NewRoute().Methods(http.MethodPost).Path("/token").HandlerFunc(s.token)
-}
-
 func (s *server) authenticate(h http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		ctx := r.Context()
@@ -158,6 +157,10 @@ func (s *server) authenticate(h http.Handler) http.Handler {
 }
 
 func (s *server) token(w http.ResponseWriter, r *http.Request) {
+	if r.Method != http.MethodPost {
+		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
+		return
+	}
 	ctx := r.Context()
 
 	permission := r.URL.Query().Get("permission")


Test patch:
diff --git a/pkg/dbtoken/server_test.go b/pkg/dbtoken/server_test.go
index 2e2e6b75f..4a6ac9e76 100644
--- a/pkg/dbtoken/server_test.go
+++ b/pkg/dbtoken/server_test.go
@@ -55,7 +55,7 @@ func TestServer(t *testing.T) {
 			wantStatusCode: http.StatusOK,
 		},
 		{
-			name: "GET /token returns 405",
+			name: "GET unauthorized /token returns 403",
 			req: &http.Request{
 				Method: http.MethodGet,
 				URL: &url.URL{
@@ -64,7 +64,7 @@ func TestServer(t *testing.T) {
 					Path:   "/token",
 				},
 			},
-			wantStatusCode: http.StatusMethodNotAllowed,
+			wantStatusCode: http.StatusForbidden,
 		},
 		{
 			name: "POST /token?permission=good returns 403 (no auth)",
@@ -188,6 +188,31 @@ func TestServer(t *testing.T) {
 			},
 			wantStatusCode: http.StatusInternalServerError,
 		},
+		{
+			name: "get /token?permission=perm returns 405",
+			permissionClientFactory: func(controller *gomock.Controller) func(userid string) cosmosdb.PermissionClient {
+				return func(userid string) cosmosdb.PermissionClient {
+					permc := mock_cosmosdb.NewMockPermissionClient(controller)
+					permc.EXPECT().Get(gomock.Any(), "perm").Return(&cosmosdb.Permission{
+						Token: "token",
+					}, nil)
+					return permc
+				}
+			},
+			req: &http.Request{
+				Method: http.MethodGet,
+				URL: &url.URL{
+					Scheme:   "http",
+					Host:     "localhost",
+					Path:     "/token",
+					RawQuery: "permission=perm",
+				},
+				Header: http.Header{
+					"Authorization": []string{`Bearer {"sub": "00000000-0000-0000-0000-000000000000"}`},
+				},
+			},
+			wantStatusCode: http.StatusMethodNotAllowed,
+		},
 		{
 			name: "POST /token?permission=perm returns 200",
 			permissionClientFactory: func(controller *gomock.Controller) func(userid string) cosmosdb.PermissionClient {