fix: respect requested user in summary compat endpoint (resolve #455)

routes/utils/user_utils.go

@@ -13,34 +13,34 @@ import (
 // CheckEffectiveUser extracts the requested user from a URL (like '/users/{user}'), compares it with the currently authorized user and writes an HTTP error if they differ.
 // Fallback can be used to manually set a value for '{user}' if none is present.
 func CheckEffectiveUser(w http.ResponseWriter, r *http.Request, userService services.IUserService, fallback string) (*models.User, error) {
-	var vars = mux.Vars(r)
-	var authorizedUser, requestedUser *models.User
-
-	if vars["user"] == "" {
-		vars["user"] = fallback
-	}
-
-	authorizedUser = middlewares.GetPrincipal(r)
-	if authorizedUser != nil {
-		if vars["user"] == "current" {
-			vars["user"] = authorizedUser.ID
-		}
-	}
-
-	requestedUser, err := userService.GetUserById(vars["user"])
-	if err != nil {
-		err := errors.New("user not found")
-		w.WriteHeader(http.StatusNotFound)
-		w.Write([]byte(err.Error()))
-		return nil, err
-	}
-
-	if authorizedUser == nil || authorizedUser.ID != requestedUser.ID && !authorizedUser.IsAdmin {
+	respondError := func(code int, text string) (*models.User, error) {
 		err := errors.New(conf.ErrUnauthorized)
 		w.WriteHeader(http.StatusUnauthorized)
 		w.Write([]byte(err.Error()))
 		return nil, err
 	}
 
+	var vars = mux.Vars(r)
+
+	if vars["user"] == "" {
+		vars["user"] = fallback
+	}
+
+	authorizedUser := middlewares.GetPrincipal(r)
+	if authorizedUser == nil {
+		return respondError(http.StatusUnauthorized, conf.ErrUnauthorized)
+	} else if vars["user"] == "current" {
+		return authorizedUser, nil
+	}
+
+	if authorizedUser.ID != vars["user"] && !authorizedUser.IsAdmin {
+		return respondError(http.StatusUnauthorized, conf.ErrUnauthorized)
+	}
+
+	requestedUser, err := userService.GetUserById(vars["user"])
+	if err != nil {
+		return respondError(http.StatusNotFound, "user not found")
+	}
+
 	return requestedUser, nil
 }

routes/utils/user_utils_test.go

@@ -0,0 +1,83 @@
+package utils
+
+import (
+	"context"
+	"fmt"
+	"github.com/gorilla/mux"
+	"github.com/muety/wakapi/middlewares"
+	"github.com/muety/wakapi/mocks"
+	"github.com/muety/wakapi/models"
+	"github.com/stretchr/testify/assert"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+)
+
+func TestCheckEffectiveUser_Current(t *testing.T) {
+	// request current as normal user -> return myself
+	r, w, userServiceMock := mockUserAwareRequest("current", "user1")
+	user, err := CheckEffectiveUser(w, r, userServiceMock, "current")
+	assert.Nil(t, err)
+	assert.Equal(t, "user1", user.ID)
+	userServiceMock.AssertNumberOfCalls(t, "GetUserById", 0)
+}
+
+func TestCheckEffectiveUser_Other(t *testing.T) {
+	// request someone else as admin -> return someone else
+	r, w, userServiceMock := mockUserAwareRequest("user2", "admin")
+	user, err := CheckEffectiveUser(w, r, userServiceMock, "current")
+	assert.Nil(t, err)
+	assert.Equal(t, "user2", user.ID)
+	userServiceMock.AssertCalled(t, "GetUserById", "user2")
+	userServiceMock.AssertNumberOfCalls(t, "GetUserById", 1)
+}
+
+func TestCheckEffectiveUser_FallbackUnauthorized(t *testing.T) {
+	// request someone else as non-admin -> error
+	r, w, userServiceMock := mockUserAwareRequest("user2", "user1")
+	user, err := CheckEffectiveUser(w, r, userServiceMock, "current")
+	assert.NotNil(t, err)
+	assert.Nil(t, user)
+	userServiceMock.AssertNumberOfCalls(t, "GetUserById", 0)
+}
+
+func TestCheckEffectiveUser_FallbackEmpty(t *testing.T) {
+	// request none -> return myself
+	r, w, userServiceMock := mockUserAwareRequest("", "user1")
+	user, err := CheckEffectiveUser(w, r, userServiceMock, "current")
+	assert.Nil(t, err)
+	assert.Equal(t, "user1", user.ID)
+	userServiceMock.AssertNumberOfCalls(t, "GetUserById", 0)
+}
+
+func TestCheckEffectiveUser_FallbackUnauthenticated(t *testing.T) {
+	// request anyone without authentication -> error
+	r, w, userServiceMock := mockUserAwareRequest("user1", "")
+	user, err := CheckEffectiveUser(w, r, userServiceMock, "current")
+	assert.NotNil(t, err)
+	assert.Nil(t, user)
+	userServiceMock.AssertNumberOfCalls(t, "GetUserById", 0)
+}
+
+func mockUserAwareRequest(requestedUser, authorizedUser string) (*http.Request, http.ResponseWriter, *mocks.UserServiceMock) {
+	testUser := models.User{
+		ID:      authorizedUser,
+		IsAdmin: authorizedUser == "admin",
+	}
+
+	testPrincipal := middlewares.PrincipalContainer{}
+	if authorizedUser != "" {
+		testPrincipal.SetPrincipal(&testUser)
+	}
+
+	r := httptest.NewRequest("GET", fmt.Sprintf("http://localhost:3000/api/%s/data", requestedUser), nil)
+	r = mux.SetURLVars(r, map[string]string{"user": requestedUser})
+	r = r.WithContext(context.WithValue(r.Context(), "principal", &testPrincipal))
+
+	userServiceMock := new(mocks.UserServiceMock)
+	userServiceMock.On("GetUserById", "user1").Return(&models.User{ID: "user1"}, nil)
+	userServiceMock.On("GetUserById", "user2").Return(&models.User{ID: "user2"}, nil)
+	userServiceMock.On("GetUserById", "admin").Return(&models.User{ID: "admin"}, nil)
+
+	return r, httptest.NewRecorder(), userServiceMock
+}