Show changes between old an new values in audit log

changelog/unreleased/audit-events.md

@@ -0,0 +1,6 @@
+Enhancement: Add old & new values to audit logs
+
+We have added old & new values to the audit logs
+We have added the missing events for role changes
+
+https://github.com/owncloud/ocis/pull/6537

go.mod

@@ -13,7 +13,7 @@ require (
 	github.com/coreos/go-oidc v2.2.1+incompatible
 	github.com/coreos/go-oidc/v3 v3.6.0
 	github.com/cs3org/go-cs3apis v0.0.0-20230516150832-730ac860c71d
-	github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161
+	github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a
 	github.com/disintegration/imaging v1.6.2
 	github.com/dutchcoders/go-clamd v0.0.0-20170520113014-b970184f4d9e
 	github.com/egirna/icap-client v0.1.1

go.sum

@@ -625,8 +625,8 @@ github.com/crewjam/httperr v0.2.0 h1:b2BfXR8U3AlIHwNeFFvZ+BV1LFvKLlzMjzaTnZMybNo
 github.com/crewjam/httperr v0.2.0/go.mod h1:Jlz+Sg/XqBQhyMjdDiC+GNNRzZTD7x39Gu3pglZ5oH4=
 github.com/crewjam/saml v0.4.13 h1:TYHggH/hwP7eArqiXSJUvtOPNzQDyQ7vwmwEqlFWhMc=
 github.com/crewjam/saml v0.4.13/go.mod h1:igEejV+fihTIlHXYP8zOec3V5A8y3lws5bQBFsTm4gA=
-github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161 h1:rtitZ/GDtdkP+R56mK7DXzrFuNhi5JGdC0SO3JV7ZJU=
-github.com/cs3org/reva/v2 v2.14.1-0.20230614134159-186dd7ce3161/go.mod h1:E32krZG159YflDSjDWfx/QGIC2529PS5LiPnGNHu3d0=
+github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a h1:T+504loI0dvksiutkzF4ciLfIJD/LJtJFdFJ6d7I7dA=
+github.com/cs3org/reva/v2 v2.14.1-0.20230615141102-1906a729ee5a/go.mod h1:E32krZG159YflDSjDWfx/QGIC2529PS5LiPnGNHu3d0=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8 h1:Z9lwXumT5ACSmJ7WGnFl+OMLLjpz5uR2fyz7dC255FI=
 github.com/cubewise-code/go-mime v0.0.0-20200519001935-8c5762b177d8/go.mod h1:4abs/jPXcmJzYoYGF91JF9Uq9s/KL5n1jvFDix8KcqY=
 github.com/cyberdelia/templates v0.0.0-20141128023046-ca7fffd4298c/go.mod h1:GyV+0YP4qX0UQ7r2MoYZ+AvYDp12OF5yg4q8rGnyNh4=

services/graph/pkg/service/v0/approleassignments.go

@@ -4,6 +4,9 @@ import (
 	"fmt"
 	"net/http"
 
+	revactx "github.com/cs3org/reva/v2/pkg/ctx"
+	"github.com/cs3org/reva/v2/pkg/events"
+	"github.com/cs3org/reva/v2/pkg/utils"
 	"github.com/go-chi/chi/v5"
 	"github.com/go-chi/render"
 	libregraph "github.com/owncloud/libre-graph-api-go"
@@ -54,6 +57,11 @@ func (g Graph) CreateAppRoleAssignment(w http.ResponseWriter, r *http.Request) {
 
 	userID := chi.URLParam(r, "userID")
 
+	// We can ignore the error, in the worst case the old role will be empty
+	oldRoles, _ := g.roleService.ListRoleAssignments(r.Context(), &settingssvc.ListRoleAssignmentsRequest{
+		AccountUuid: userID,
+	})
+
 	if appRoleAssignment.GetPrincipalId() != userID {
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, fmt.Sprintf("user id %s does not match principal id %v", userID, appRoleAssignment.GetPrincipalId()))
 		return
@@ -76,6 +84,27 @@ func (g Graph) CreateAppRoleAssignment(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	var role string
+	roles := oldRoles.GetAssignments()
+	if len(roles) > 0 {
+		role = roles[0].GetRoleId()
+	}
+
+	e := events.UserFeatureChanged{
+		UserID: userID,
+		Features: []events.UserFeature{
+			{
+				Name:     "roleChanged",
+				Value:    appRoleAssignment.AppRoleId,
+				OldValue: &role,
+			},
+		},
+		Timestamp: utils.TSNow(),
+	}
+	if currentUser, ok := revactx.ContextGetUser(r.Context()); ok {
+		e.Executant = currentUser.GetId()
+	}
+	g.publishEvent(e)
 	render.Status(r, http.StatusCreated)
 	render.JSON(w, r, g.assignmentToAppRoleAssignment(artur.GetAssignment()))
 }

services/graph/pkg/service/v0/approleassignments_test.go

@@ -139,6 +139,12 @@ var _ = Describe("AppRoleAssignments", func() {
 				AccountUuid: user.GetId(),
 				RoleId:      "some-appRole-ID",
 			}
+			roleService.On("ListRoleAssignments", mock.Anything, mock.Anything, mock.Anything).Return(&settings.ListRoleAssignmentsResponse{
+				Assignments: []*settingsmsg.UserRoleAssignment{
+					userRoleAssignment,
+				},
+			}, nil)
+
 			roleService.On("AssignRoleToUser", mock.Anything, mock.Anything, mock.Anything).Return(&settings.AssignRoleToUserResponse{Assignment: userRoleAssignment}, nil)
 
 			ara := libregraph.NewAppRoleAssignmentWithDefaults()

services/graph/pkg/service/v0/educationuser.go

@@ -361,8 +361,9 @@ func (g Graph) PatchEducationUser(w http.ResponseWriter, r *http.Request) {
 	}
 
 	e := events.UserFeatureChanged{
-		UserID:   nameOrID,
-		Features: features,
+		UserID:    nameOrID,
+		Features:  features,
+		Timestamp: utils.TSNow(),
 	}
 	if currentUser, ok := revactx.ContextGetUser(r.Context()); ok {
 		e.Executant = currentUser.GetId()

services/graph/pkg/service/v0/users.go

@@ -9,6 +9,7 @@ import (
 	"reflect"
 	"regexp"
 	"sort"
+	"strconv"
 	"strings"
 
 	"github.com/CiscoM31/godata"
@@ -612,6 +613,22 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	sanitizedPath := strings.TrimPrefix(r.URL.Path, "/graph/v1.0/")
+
+	odataReq, err := godata.ParseRequest(r.Context(), sanitizedPath, r.URL.Query())
+	if err != nil {
+		logger.Debug().Err(err).Interface("query", r.URL.Query()).Msg("could not get users: query error")
+		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, err.Error())
+		return
+	}
+
+	oldUserValues, err := g.identityBackend.GetUser(r.Context(), nameOrID, odataReq)
+	if err != nil {
+		logger.Debug().Err(err).Str("id", nameOrID).Msg("could not get user: backend error")
+		errorcode.RenderError(w, r, err)
+		return
+	}
+
 	if nameOrID == "" {
 		logger.Debug().Msg("could not update user: missing user id")
 		errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "missing user id")
@@ -627,7 +644,7 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 	}
 
 	if reflect.ValueOf(*changes).IsZero() {
-		logger.Debug().Interface("body", r.Body).Msg("ignoring empyt request body")
+		logger.Debug().Interface("body", r.Body).Msg("ignoring empty request body")
 		render.Status(r, http.StatusNoContent)
 		render.NoContent(w, r)
 		return
@@ -649,19 +666,47 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 				fmt.Sprintf("'%s' is not a valid email address", *mail))
 			return
 		}
-		features = append(features, events.UserFeature{Name: "email", Value: *mail})
+		features = append(features, events.UserFeature{
+			Name:     "email",
+			Value:    *mail,
+			OldValue: oldUserValues.Mail,
+		})
 	}
 
 	if name, ok := changes.GetDisplayNameOk(); ok {
-		features = append(features, events.UserFeature{Name: "displayname", Value: *name})
+		features = append(features, events.UserFeature{
+			Name:     "displayname",
+			Value:    *name,
+			OldValue: oldUserValues.DisplayName,
+		})
 	}
 
-	if changes.HasUserType() {
+	if userType, ok := changes.GetUserTypeOk(); ok {
 		if !isValidUserType(*changes.UserType) {
 			logger.Debug().Interface("user", changes).Msg("invalid userType attribute")
 			errorcode.InvalidRequest.Render(w, r, http.StatusBadRequest, "invalid userType attribute, valid options are 'Member' or 'Guest'")
 			return
 		}
+		features = append(features, events.UserFeature{
+			Name:     "changeUserType",
+			Value:    *userType,
+			OldValue: oldUserValues.UserType,
+		})
+	}
+
+	if accEnabled, ok := changes.GetAccountEnabledOk(); ok {
+		oldAccVal := strconv.FormatBool(oldUserValues.GetAccountEnabled())
+		features = append(features, events.UserFeature{
+			Name:     "accountEnabled",
+			Value:    strconv.FormatBool(*accEnabled),
+			OldValue: &oldAccVal,
+		})
+	}
+
+	if changes.HasPasswordProfile() {
+		features = append(features, events.UserFeature{
+			Name: "passwordChanged",
+		})
 	}
 
 	logger.Debug().Str("nameid", nameOrID).Interface("changes", *changes).Msg("calling update user on backend")
@@ -673,8 +718,9 @@ func (g Graph) PatchUser(w http.ResponseWriter, r *http.Request) {
 	}
 
 	e := events.UserFeatureChanged{
-		UserID:   nameOrID,
-		Features: features,
+		UserID:    nameOrID,
+		Features:  features,
+		Timestamp: utils.TSNow(),
 	}
 	if currentUser, ok := revactx.ContextGetUser(r.Context()); ok {
 		e.Executant = currentUser.GetId()

services/graph/pkg/service/v0/users_test.go

@@ -809,7 +809,7 @@ var _ = Describe("Users", func() {
 			user.SetMail("[email protected]")
 			user.SetId("/users/user")
 
-			identityBackend.On("GetUser", mock.Anything, mock.Anything, mock.Anything).Return(&user, nil)
+			identityBackend.On("GetUser", mock.Anything, mock.Anything, mock.Anything).Return(user, nil)
 		})
 
 		It("handles missing userids", func() {
@@ -866,7 +866,7 @@ var _ = Describe("Users", func() {
 			data, err := json.Marshal(user)
 			Expect(err).ToNot(HaveOccurred())
 
-			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users?$invalid=true", bytes.NewBuffer(data))
+			r := httptest.NewRequest(http.MethodPost, "/graph/v1.0/users", bytes.NewBuffer(data))
 			rctx := chi.NewRouteContext()
 			rctx.URLParams.Add("userID", user.GetId())
 			r = r.WithContext(context.WithValue(revactx.ContextSetUser(ctx, currentUser), chi.RouteCtxKey, rctx))