From d61d63b66f4ef4bb96d39bc0b2c4d425ab116408 Mon Sep 17 00:00:00 2001
From: Roman Perekhod <rperekhod@owncloud.com>
Date: Tue, 12 Nov 2024 11:46:22 +0100
Subject: [PATCH] fix impersonated request user mismatch

---
 changelog/unreleased/fix-auth-app.md     |  6 ++++++
 services/auth-app/pkg/service/service.go | 23 ++++++++++++++++++-----
 2 files changed, 24 insertions(+), 5 deletions(-)
 create mode 100644 changelog/unreleased/fix-auth-app.md

diff --git a/changelog/unreleased/fix-auth-app.md b/changelog/unreleased/fix-auth-app.md
new file mode 100644
index 000000000..13675ef98
--- /dev/null
+++ b/changelog/unreleased/fix-auth-app.md
@@ -0,0 +1,6 @@
+Bugfix: Fix impersonated request user mismatch
+
+We fixed a user id and name mismatch in the impersonated auth-app API request
+
+https://github.com/owncloud/ocis/pull/10548
+https://github.com/owncloud/ocis/issues/10292
diff --git a/services/auth-app/pkg/service/service.go b/services/auth-app/pkg/service/service.go
index 53de039fe..c4515a575 100644
--- a/services/auth-app/pkg/service/service.go
+++ b/services/auth-app/pkg/service/service.go
@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"net/http"
 	"time"
 
@@ -25,6 +26,8 @@ import (
 	"google.golang.org/grpc/metadata"
 )
 
+var ErrBadRequest = errors.New("bad request")
+
 // AuthAppToken represents an app token.
 type AuthAppToken struct {
 	Token          string    `json:"token"`
@@ -97,8 +100,10 @@ func (a *AuthAppService) HandleCreate(w http.ResponseWriter, r *http.Request) {
 	}
 
 	label := "Generated via API"
-	cid := buildClientID(q.Get("userID"), q.Get("userName"))
-	if cid != "" {
+
+	// Impersonated request
+	userID, userName := q.Get("userID"), q.Get("userName")
+	if userID != "" || userName != "" {
 		if !a.cfg.AllowImpersonation {
 			sublog.Error().Msg("impersonation is not allowed")
 			http.Error(w, "impersonation is not allowed", http.StatusForbidden)
@@ -115,9 +120,13 @@ func (a *AuthAppService) HandleCreate(w http.ResponseWriter, r *http.Request) {
 			w.WriteHeader(http.StatusForbidden)
 			return
 		}
-		ctx, err = a.authenticateUser(cid, gwc)
+		ctx, err = a.authenticateUser(userID, userName, gwc)
 		if err != nil {
 			sublog.Error().Err(err).Msg("error authenticating user")
+			if errors.Is(err, ErrBadRequest) {
+				http.Error(w, err.Error(), http.StatusBadRequest)
+				return
+			}
 			w.WriteHeader(http.StatusInternalServerError)
 			return
 		}
@@ -241,11 +250,11 @@ func (a *AuthAppService) HandleDelete(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 }
 
-func (a *AuthAppService) authenticateUser(clientID string, gwc gateway.GatewayAPIClient) (context.Context, error) {
+func (a *AuthAppService) authenticateUser(userID, userName string, gwc gateway.GatewayAPIClient) (context.Context, error) {
 	ctx := context.Background()
 	authRes, err := gwc.Authenticate(ctx, &gateway.AuthenticateRequest{
 		Type:         "machine",
-		ClientId:     clientID,
+		ClientId:     buildClientID(userID, userName),
 		ClientSecret: a.cfg.MachineAuthAPIKey,
 	})
 	if err != nil {
@@ -256,6 +265,10 @@ func (a *AuthAppService) authenticateUser(clientID string, gwc gateway.GatewayAP
 		return nil, errors.New("error authenticating user: " + authRes.GetStatus().GetMessage())
 	}
 
+	if (userID != "" && authRes.GetUser().GetId().GetOpaqueId() != userID) || (userName != "" && authRes.GetUser().GetUsername() != userName) {
+		return nil, fmt.Errorf("requested user does not match authenticated user: userID:%s, userName:%s, %w", authRes.GetUser().GetId().GetOpaqueId(), authRes.GetUser().GetUsername(), ErrBadRequest)
+	}
+
 	ctx = ctxpkg.ContextSetUser(ctx, &userpb.User{Id: authRes.GetUser().GetId()})
 	return metadata.AppendToOutgoingContext(ctx, ctxpkg.TokenHeader, authRes.GetToken()), nil
 }