Improve scim event UI/UX

Signed-off-by: Bryan Frimin <bryan@getprobo.com>

apps/console/src/pages/iam/organizations/settings/_components/SCIMEventList.tsx

@@ -50,16 +50,13 @@ export function SCIMEventList(props: { fKey: SCIMEventListFragment$key }) {
           <Th>{__("Time")}</Th>
           <Th>{__("Method")}</Th>
           <Th>{__("Path")}</Th>
-          <Th>{__("Status")}</Th>
+          <Th>{__("Result")}</Th>
-          <Th>{__("User")}</Th>
-          <Th>{__("IP Address")}</Th>
-          <Th>{__("Error")}</Th>
         </Tr>
       </Thead>
       <Tbody>
         {eventsPagination.data.events.edges.length === 0 ? (
           <Tr>
-            <Td colSpan={7} className="text-center text-txt-secondary">
+            <Td colSpan={4} className="text-center text-txt-secondary">
               {__("No SCIM events recorded yet.")}
             </Td>
           </Tr>

apps/console/src/pages/iam/organizations/settings/_components/SCIMEventListItem.tsx

@@ -1,7 +1,8 @@
+import { useState } from "react";
 import { useFragment } from "react-relay";
 import { graphql } from "relay-runtime";
 import type { SCIMEventListItemFragment$key } from "/__generated__/iam/SCIMEventListItemFragment.graphql";
-import { Td, Tr, Badge } from "@probo/ui";
+import { Td, Tr, Badge, IconChevronDown, IconChevronRight } from "@probo/ui";
 import { formatDate } from "@probo/helpers";
 
 const SCIMEventListItemFragment = graphql`
@@ -22,15 +23,11 @@ const SCIMEventListItemFragment = graphql`
   }
 `;
 
-const getStatusBadge = (statusCode: number) => {
+const getResultBadge = (statusCode: number) => {
   if (statusCode >= 200 && statusCode < 300) {
-    return <Badge variant="success">{statusCode}</Badge>;
+    return <Badge variant="success">Info</Badge>;
-  } else if (statusCode >= 400 && statusCode < 500) {
-    return <Badge variant="warning">{statusCode}</Badge>;
-  } else if (statusCode >= 500) {
-    return <Badge variant="danger">{statusCode}</Badge>;
   }
-  return <Badge variant="neutral">{statusCode}</Badge>;
+  return <Badge variant="danger">Error</Badge>;
 };
 
 const getMethodBadge = (method: string) => {
@@ -45,27 +42,77 @@ const getMethodBadge = (method: string) => {
   return <Badge variant={variants[method] || "neutral"}>{method}</Badge>;
 };
 
+const decodePath = (path: string): string => {
+  try {
+    return decodeURIComponent(path);
+  } catch {
+    return path;
+  }
+};
+
 export function SCIMEventListItem(props: {
   fKey: SCIMEventListItemFragment$key;
 }) {
   const { fKey } = props;
+  const [isExpanded, setIsExpanded] = useState(false);
 
   const event = useFragment<SCIMEventListItemFragment$key>(
     SCIMEventListItemFragment,
     fKey
   );
 
+  const hasError = !!event.errorMessage;
+
   return (
-    <Tr>
+    <>
-      <Td className="whitespace-nowrap">{formatDate(event.createdAt)}</Td>
+      <Tr
+        className="cursor-pointer hover:bg-bg-hover"
+        onClick={() => setIsExpanded(!isExpanded)}
+      >
+        <Td className="whitespace-nowrap">
+          <div className="flex items-center gap-2">
+            {isExpanded ? (
+              <IconChevronDown size={16} className="text-txt-secondary" />
+            ) : (
+              <IconChevronRight size={16} className="text-txt-secondary" />
+            )}
+            {formatDate(event.createdAt)}
+          </div>
+        </Td>
         <Td>{getMethodBadge(event.method)}</Td>
-      <Td className="font-mono text-xs">{event.path}</Td>
+        <Td className="font-mono text-xs">{decodePath(event.path)}</Td>
-      <Td>{getStatusBadge(event.statusCode)}</Td>
+        <Td>{getResultBadge(event.statusCode)}</Td>
-      <Td>{event.membership?.profile?.fullName || "-"}</Td>
+      </Tr>
-      <Td className="font-mono text-xs">{event.ipAddress}</Td>
+      {isExpanded && (
-      <Td className="max-w-xs truncate text-txt-danger">
+        <Tr>
-        {event.errorMessage || "-"}
+          <Td colSpan={4} className="bg-bg-subtle">
+            <div className="py-2 pl-6 space-y-2">
+              <div className="flex gap-8 text-sm">
+                <div>
+                  <span className="text-txt-secondary">User: </span>
+                  <span>{event.membership?.profile?.fullName || "-"}</span>
+                </div>
+                <div>
+                  <span className="text-txt-secondary">IP Address: </span>
+                  <span className="font-mono text-xs">{event.ipAddress}</span>
+                </div>
+                <div>
+                  <span className="text-txt-secondary">Status Code: </span>
+                  <span className="font-mono text-xs">{event.statusCode}</span>
+                </div>
+              </div>
+              {hasError && (
+                <div>
+                  <div className="text-sm text-txt-secondary">Error</div>
+                  <pre className="mt-1 whitespace-pre-wrap break-all font-mono text-xs text-txt-danger">
+                    {event.errorMessage}
+                  </pre>
+                </div>
+              )}
+            </div>
           </Td>
         </Tr>
+      )}
+    </>
   );
 }

pkg/iam/scim/service.go

@@ -94,7 +94,6 @@ func (s *Service) CreateUser(
 	ctx context.Context,
 	config *coredata.SCIMConfiguration,
 	attributes scim.ResourceAttributes,
-	ipAddress net.IP,
 ) (scim.Resource, error) {
 	email, fullName := ParseUserFromAttributes(attributes)
 	if email == "" {
@@ -183,13 +182,6 @@ func (s *Service) CreateUser(
 			}
 		}
 
-		// Log SCIM event
-		event := s.createEvent(config, "POST", "/Users", membership.ID, ipAddress, 201, nil)
-		err = event.Insert(ctx, tx, scope)
-		if err != nil {
-			s.logger.ErrorCtx(ctx, "cannot log SCIM event", log.Error(err))
-		}
-
 		return nil
 	})
 
@@ -205,7 +197,6 @@ func (s *Service) GetUser(
 	ctx context.Context,
 	config *coredata.SCIMConfiguration,
 	membershipID gid.GID,
-	ipAddress net.IP,
 ) (scim.Resource, error) {
 	scope := coredata.NewScopeFromObjectID(config.OrganizationID)
 
@@ -246,7 +237,6 @@ func (s *Service) ListUsers(
 	filterExpr scimfilter.Expression,
 	startIndex int,
 	count int,
-	ipAddress net.IP,
 ) ([]scim.Resource, int, error) {
 	filter, err := ParseUserFilter(filterExpr)
 	if err != nil {
@@ -297,10 +287,9 @@ func (s *Service) ReplaceUser(
 	config *coredata.SCIMConfiguration,
 	membershipID gid.GID,
 	attributes scim.ResourceAttributes,
-	ipAddress net.IP,
 ) (scim.Resource, error) {
 	fullName, active := ParseUserFromReplaceAttributes(attributes)
-	membership, deactivated, err := s.updateUser(ctx, config, membershipID, fullName, active, "PUT", ipAddress)
+	membership, deactivated, err := s.updateUser(ctx, config, membershipID, fullName, active)
 	if err != nil {
 		return scim.Resource{}, err
 	}
@@ -313,10 +302,9 @@ func (s *Service) PatchUser(
 	config *coredata.SCIMConfiguration,
 	membershipID gid.GID,
 	operations []scim.PatchOperation,
-	ipAddress net.IP,
 ) (scim.Resource, error) {
 	fullName, active := ParseUserFromPatchOperations(operations)
-	membership, deactivated, err := s.updateUser(ctx, config, membershipID, fullName, active, "PATCH", ipAddress)
+	membership, deactivated, err := s.updateUser(ctx, config, membershipID, fullName, active)
 	if err != nil {
 		return scim.Resource{}, err
 	}
@@ -329,8 +317,6 @@ func (s *Service) updateUser(
 	membershipID gid.GID,
 	fullName string,
 	active *bool,
-	method string,
-	ipAddress net.IP,
 ) (*coredata.Membership, bool, error) {
 	scope := coredata.NewScopeFromObjectID(config.OrganizationID)
 	now := time.Now()
@@ -362,13 +348,6 @@ func (s *Service) updateUser(
 
 			deactivated = true
 
-			// Log SCIM event for deactivation
-			event := s.createEvent(config, method, fmt.Sprintf("/Users/%s", membershipID), membershipID, ipAddress, 200, nil)
-			err = event.Insert(ctx, tx, scope)
-			if err != nil {
-				s.logger.ErrorCtx(ctx, "cannot log SCIM event", log.Error(err))
-			}
-
 			return nil
 		}
 
@@ -398,13 +377,6 @@ func (s *Service) updateUser(
 			}
 		}
 
-		// Log SCIM event
-		event := s.createEvent(config, method, fmt.Sprintf("/Users/%s", membershipID), membership.ID, ipAddress, 200, nil)
-		err = event.Insert(ctx, tx, scope)
-		if err != nil {
-			s.logger.ErrorCtx(ctx, "cannot log SCIM event", log.Error(err))
-		}
-
 		return nil
 	})
 
@@ -420,7 +392,6 @@ func (s *Service) DeleteUser(
 	ctx context.Context,
 	config *coredata.SCIMConfiguration,
 	membershipID gid.GID,
-	ipAddress net.IP,
 ) error {
 	scope := coredata.NewScopeFromObjectID(config.OrganizationID)
 
@@ -444,13 +415,6 @@ func (s *Service) DeleteUser(
 			return fmt.Errorf("cannot delete membership: %w", err)
 		}
 
-		// Log SCIM event
-		event := s.createEvent(config, "DELETE", fmt.Sprintf("/Users/%s", membershipID), membershipID, ipAddress, 204, nil)
-		err = event.Insert(ctx, tx, scope)
-		if err != nil {
-			s.logger.ErrorCtx(ctx, "cannot log SCIM event", log.Error(err))
-		}
-
 		return nil
 	})
 }

pkg/server/api/connect/v1/scim_handler.go

@@ -32,20 +32,35 @@ import (
 	scimservice "go.probo.inc/probo/pkg/iam/scim"
 )
 
-type SCIMHandler struct {
+type (
+	SCIMHandler struct {
 		iam    *iam.Service
 		logger *log.Logger
-}
+	}
+
+	scimResourceHandler struct {
+		handler *SCIMHandler
+	}
+
+	scimRequestContext struct {
+		ctx          context.Context
+		config       *coredata.SCIMConfiguration
+		ipAddress    net.IP
+		method       string
+		path         string
+		membershipID *gid.GID
+		handler      *scimResourceHandler
+	}
+)
+
+var (
+	scimConfigCtxKey = &ctxKey{name: "scim_config"}
+)
 
 func NewSCIMHandler(iam *iam.Service, logger *log.Logger) *SCIMHandler {
 	return &SCIMHandler{iam: iam, logger: logger}
 }
 
-// Context key for SCIM configuration
-type scimCtxKey struct{ name string }
-
-var scimConfigCtxKey = &scimCtxKey{name: "scim_config"}
-
 func scimConfigFromContext(ctx context.Context) *coredata.SCIMConfiguration {
 	config, _ := ctx.Value(scimConfigCtxKey).(*coredata.SCIMConfiguration)
 	return config
@@ -122,68 +137,101 @@ func (h *SCIMHandler) BearerTokenMiddleware(next http.Handler) http.Handler {
 	})
 }
 
-// scimResourceHandler implements the elimity-com/scim ResourceHandler interface
+func (rc *scimRequestContext) logAndWrapError(err error, logMsg string) error {
-type scimResourceHandler struct {
+	var scimErr scimerrors.ScimError
-	handler *SCIMHandler
+	if errors.As(err, &scimErr) {
+		errMsg := scimErr.Detail
+
+		// Don't reference membershipID for 404 errors - the resource doesn't exist
+		membershipID := rc.membershipID
+		if scimErr.Status == http.StatusNotFound {
+			membershipID = nil
+		}
+		rc.handler.handler.iam.SCIMService.LogEvent(rc.ctx, rc.config, rc.method, rc.path, membershipID, rc.ipAddress, scimErr.Status, &errMsg)
+		return err
+	}
+
+	rc.handler.handler.logger.ErrorCtx(rc.ctx, logMsg, log.Error(err))
+	errMsg := "internal server error"
+	rc.handler.handler.iam.SCIMService.LogEvent(rc.ctx, rc.config, rc.method, rc.path, rc.membershipID, rc.ipAddress, 500, &errMsg)
+	return scimerrors.ScimErrorInternal
+}
+
+func (rc *scimRequestContext) logSuccess(statusCode int) {
+	rc.handler.handler.iam.SCIMService.LogEvent(rc.ctx, rc.config, rc.method, rc.path, rc.membershipID, rc.ipAddress, statusCode, nil)
 }
 
 func (h *scimResourceHandler) Create(r *http.Request, attributes scim.ResourceAttributes) (scim.Resource, error) {
-	ctx := r.Context()
+	rc := &scimRequestContext{
-	config := scimConfigFromContext(ctx)
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "POST",
+		path:      "/Users",
+		handler:   h,
+	}
 
-	resource, err := h.handler.iam.SCIMService.CreateUser(ctx, config, attributes, getIPAddress(r))
+	resource, err := h.handler.iam.SCIMService.CreateUser(rc.ctx, rc.config, attributes)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return scim.Resource{}, rc.logAndWrapError(err, "cannot create user")
-		if errors.As(err, &scimErr) {
-			return scim.Resource{}, err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot create user", log.Error(err))
-		return scim.Resource{}, scimerrors.ScimErrorInternal
 	}
 
+	membershipID, _ := gid.ParseGID(resource.ID)
+	rc.membershipID = &membershipID
+	rc.logSuccess(201)
 	return resource, nil
 }
 
 func (h *scimResourceHandler) Get(r *http.Request, id string) (scim.Resource, error) {
-	ctx := r.Context()
+	rc := &scimRequestContext{
-	config := scimConfigFromContext(ctx)
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "GET",
+		path:      "/Users/" + id,
+		handler:   h,
+	}
 
 	membershipID, err := gid.ParseGID(id)
 	if err != nil {
-		return scim.Resource{}, scimerrors.ScimErrorResourceNotFound(id)
+		return scim.Resource{}, rc.logAndWrapError(scimerrors.ScimErrorResourceNotFound(id), "invalid membership ID")
 	}
+	rc.membershipID = &membershipID
 
-	resource, err := h.handler.iam.SCIMService.GetUser(ctx, config, membershipID, getIPAddress(r))
+	resource, err := h.handler.iam.SCIMService.GetUser(rc.ctx, rc.config, membershipID)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return scim.Resource{}, rc.logAndWrapError(err, "cannot get user")
-		if errors.As(err, &scimErr) {
-			return scim.Resource{}, err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot get user", log.Error(err))
-		return scim.Resource{}, scimerrors.ScimErrorInternal
 	}
 
+	rc.logSuccess(200)
 	return resource, nil
 }
 
 func (h *scimResourceHandler) GetAll(r *http.Request, params scim.ListRequestParams) (scim.Page, error) {
-	ctx := r.Context()
+	path := "/Users"
-	config := scimConfigFromContext(ctx)
+	if r.URL.RawQuery != "" {
+		path += "?" + r.URL.RawQuery
+	}
+
+	rc := &scimRequestContext{
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "GET",
+		path:      path,
+		handler:   h,
+	}
 
 	if err := params.FilterValidator.Validate(); err != nil {
-		return scim.Page{}, scimerrors.ScimErrorBadRequest(err.Error())
+		return scim.Page{}, rc.logAndWrapError(scimerrors.ScimErrorBadRequest(err.Error()), "invalid filter")
 	}
 
-	resources, totalCount, err := h.handler.iam.SCIMService.ListUsers(ctx, config, params.FilterValidator.GetFilter(), params.StartIndex, params.Count, getIPAddress(r))
+	resources, totalCount, err := h.handler.iam.SCIMService.ListUsers(rc.ctx, rc.config, params.FilterValidator.GetFilter(), params.StartIndex, params.Count)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return scim.Page{}, rc.logAndWrapError(err, "cannot list users")
-		if errors.As(err, &scimErr) {
-			return scim.Page{}, err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot list users", log.Error(err))
-		return scim.Page{}, scimerrors.ScimErrorInternal
 	}
 
+	rc.logSuccess(200)
 	return scim.Page{
 		TotalResults: totalCount,
 		Resources:    resources,
@@ -191,68 +239,78 @@ func (h *scimResourceHandler) GetAll(r *http.Request, params scim.ListRequestPar
 }
 
 func (h *scimResourceHandler) Replace(r *http.Request, id string, attributes scim.ResourceAttributes) (scim.Resource, error) {
-	ctx := r.Context()
+	rc := &scimRequestContext{
-	config := scimConfigFromContext(ctx)
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "PUT",
+		path:      "/Users/" + id,
+		handler:   h,
+	}
 
 	membershipID, err := gid.ParseGID(id)
 	if err != nil {
-		return scim.Resource{}, scimerrors.ScimErrorResourceNotFound(id)
+		return scim.Resource{}, rc.logAndWrapError(scimerrors.ScimErrorResourceNotFound(id), "invalid membership ID")
 	}
+	rc.membershipID = &membershipID
 
-	resource, err := h.handler.iam.SCIMService.ReplaceUser(ctx, config, membershipID, attributes, getIPAddress(r))
+	resource, err := h.handler.iam.SCIMService.ReplaceUser(rc.ctx, rc.config, membershipID, attributes)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return scim.Resource{}, rc.logAndWrapError(err, "cannot update user")
-		if errors.As(err, &scimErr) {
-			return scim.Resource{}, err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot update user", log.Error(err))
-		return scim.Resource{}, scimerrors.ScimErrorInternal
 	}
 
+	rc.logSuccess(200)
 	return resource, nil
 }
 
 func (h *scimResourceHandler) Patch(r *http.Request, id string, operations []scim.PatchOperation) (scim.Resource, error) {
-	ctx := r.Context()
+	rc := &scimRequestContext{
-	config := scimConfigFromContext(ctx)
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "PATCH",
+		path:      "/Users/" + id,
+		handler:   h,
+	}
 
 	membershipID, err := gid.ParseGID(id)
 	if err != nil {
-		return scim.Resource{}, scimerrors.ScimErrorResourceNotFound(id)
+		return scim.Resource{}, rc.logAndWrapError(scimerrors.ScimErrorResourceNotFound(id), "invalid membership ID")
 	}
+	rc.membershipID = &membershipID
 
-	resource, err := h.handler.iam.SCIMService.PatchUser(ctx, config, membershipID, operations, getIPAddress(r))
+	resource, err := h.handler.iam.SCIMService.PatchUser(rc.ctx, rc.config, membershipID, operations)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return scim.Resource{}, rc.logAndWrapError(err, "cannot patch user")
-		if errors.As(err, &scimErr) {
-			return scim.Resource{}, err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot patch user", log.Error(err))
-		return scim.Resource{}, scimerrors.ScimErrorInternal
 	}
 
+	rc.logSuccess(200)
 	return resource, nil
 }
 
 func (h *scimResourceHandler) Delete(r *http.Request, id string) error {
-	ctx := r.Context()
+	rc := &scimRequestContext{
-	config := scimConfigFromContext(ctx)
+		ctx:       r.Context(),
+		config:    scimConfigFromContext(r.Context()),
+		ipAddress: getIPAddress(r),
+		method:    "DELETE",
+		path:      "/Users/" + id,
+		handler:   h,
+	}
 
 	membershipID, err := gid.ParseGID(id)
 	if err != nil {
-		return scimerrors.ScimErrorResourceNotFound(id)
+		return rc.logAndWrapError(scimerrors.ScimErrorResourceNotFound(id), "invalid membership ID")
 	}
+	rc.membershipID = &membershipID
 
-	err = h.handler.iam.SCIMService.DeleteUser(ctx, config, membershipID, getIPAddress(r))
+	err = h.handler.iam.SCIMService.DeleteUser(rc.ctx, rc.config, membershipID)
 	if err != nil {
-		var scimErr scimerrors.ScimError
+		return rc.logAndWrapError(err, "cannot delete user")
-		if errors.As(err, &scimErr) {
-			return err
-		}
-		h.handler.logger.ErrorCtx(ctx, "cannot delete user", log.Error(err))
-		return scimerrors.ScimErrorInternal
 	}
 
+	rc.membershipID = nil
+	rc.logSuccess(204)
 	return nil
 }