feat(audit): enrich device sign-out audit logs

internal/services/login_session.go

@@ -203,6 +203,23 @@ func (s *LoginSessionService) IsSessionOwnedByUser(id, userID string) (bool, err
 // and writes an audit event. The revoked device's next authenticated request is
 // bounced to /login by the auth middleware.
 func (s *LoginSessionService) RevokeByID(ctx context.Context, id, userID string) error {
+	// Best-effort pre-revoke fetch so the audit record names the device
+	// (browser/OS/IP) instead of a bare UUID. Derive the labels via the same
+	// ParseUserAgent path the Active Sessions list uses (the stored Browser/OS
+	// columns are forensic only); the audit keeps just the bare name, not the
+	// version the UI additionally renders. Fail open: a fetch error must not
+	// prevent the revoke or the audit entry — we just log without the device
+	// Details.
+	var details models.AuditDetails
+	if ls, err := s.store.GetLoginSessionByID(id); err == nil {
+		ua := ParseUserAgent(ls.UserAgent)
+		details = models.AuditDetails{
+			"browser": ua.Browser,
+			"os":      ua.OS,
+			"ip":      ls.IP,
+		}
+	}
+
 	if err := s.store.RevokeLoginSessionByID(id); err != nil {
 		return err
 	}
@@ -213,6 +230,7 @@ func (s *LoginSessionService) RevokeByID(ctx context.Context, id, userID string)
 		ResourceType: models.ResourceLoginSession,
 		ResourceID:   id,
 		Action:       "Revoked browser login session",
+		Details:      details,
 		Success:      true,
 	})
 	return nil
@@ -227,8 +245,19 @@ func (s *LoginSessionService) RevokeOthers(
 ) (int64, error) {
 	// Without a current SID we cannot identify the device to preserve, and
 	// hashSID("") matches no real row — so the "except" filter would exclude
-	// nothing and revoke the caller's own session too. Refuse instead.
+	// nothing and revoke the caller's own session too. Refuse instead. A security
+	// action that silently did nothing is worth surfacing, so audit the refusal.
 	if currentRawSID == "" {
+		s.auditService.Log(ctx, core.AuditLogEntry{
+			EventType:    models.EventSessionRevokedAll,
+			Severity:     models.SeverityWarning,
+			ActorUserID:  userID,
+			ResourceType: models.ResourceLoginSession,
+			ResourceID:   userID,
+			Action:       "Refused sign-out of other browser sessions: missing current session id",
+			Details:      models.AuditDetails{"reason": "missing_current_sid"},
+			Success:      false,
+		})
 		return 0, nil
 	}
 	count, err := s.store.RevokeLoginSessionsByUserExceptSIDHash(userID, hashSID(currentRawSID))

internal/services/login_session_test.go

@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/go-authgate/authgate/internal/config"
+	"github.com/go-authgate/authgate/internal/models"
 	"github.com/go-authgate/authgate/internal/store"
 
 	"github.com/google/uuid"
@@ -15,9 +16,10 @@ import (
 
 func setupLoginSessionService(t *testing.T) (*store.Store, *LoginSessionService) {
 	t.Helper()
-	s, err := store.New(context.Background(), "sqlite", ":memory:", &config.Config{})
-	require.NoError(t, err)
-	svc := NewLoginSessionService(s, NewNoopAuditService(), time.Hour, 30*24*time.Hour)
+	// Reuse the recording variant and discard the recorder: a recordingAuditLogger
+	// is a superset of the no-op service (it embeds it), so tests that ignore audit
+	// behave identically while the store/TTL bootstrap lives in one place.
+	s, svc, _ := setupRecordingLoginSessionService(t)
 	return s, svc
 }
 
@@ -62,6 +64,75 @@ func TestLoginSession_GetActiveBySID_EmptyAndUnknown(t *testing.T) {
 	assert.Nil(t, ls)
 }
 
+// setupRecordingLoginSessionService is like setupLoginSessionService but wires a
+// recordingAuditLogger so tests can assert on the captured audit entries.
+func setupRecordingLoginSessionService(
+	t *testing.T,
+) (*store.Store, *LoginSessionService, *recordingAuditLogger) {
+	t.Helper()
+	s, err := store.New(context.Background(), "sqlite", ":memory:", &config.Config{})
+	require.NoError(t, err)
+	rec := &recordingAuditLogger{NoopAuditService: NewNoopAuditService()}
+	svc := NewLoginSessionService(s, rec, time.Hour, 30*24*time.Hour)
+	return s, svc, rec
+}
+
+// TestLoginSession_RevokeByID_AuditEnriched proves the single-device sign-out
+// records the revoked device's browser/OS/IP in the audit Details (not just a
+// bare session UUID), so a reviewer sees "Chrome / macOS @ 192.0.2.10".
+func TestLoginSession_RevokeByID_AuditEnriched(t *testing.T) {
+	_, svc, rec := setupRecordingLoginSessionService(t)
+	ctx := context.Background()
+	userID := uuid.New().String()
+
+	sid, err := svc.Establish(ctx, EstablishParams{
+		UserID:    userID,
+		IP:        "192.0.2.10",
+		UserAgent: "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) Chrome/120.0 Safari/537.36",
+	})
+	require.NoError(t, err)
+	ls, err := svc.GetActiveBySID(ctx, sid)
+	require.NoError(t, err)
+	require.NotNil(t, ls)
+
+	require.NoError(t, svc.RevokeByID(ctx, ls.ID, userID))
+
+	require.Len(t, rec.entries, 1)
+	entry := rec.entries[0]
+	assert.Equal(t, models.EventSessionRevoked, entry.EventType)
+	assert.True(t, entry.Success)
+	require.NotNil(t, entry.Details)
+	assert.Equal(t, "Chrome", entry.Details["browser"])
+	assert.Equal(t, "macOS", entry.Details["os"])
+	assert.Equal(t, "192.0.2.10", entry.Details["ip"])
+
+	// The revoke still took effect.
+	got, err := svc.GetActiveBySID(ctx, sid)
+	require.NoError(t, err)
+	assert.Nil(t, got)
+}
+
+// TestLoginSession_RevokeByID_MissingRowDegradesGracefully proves a pre-revoke
+// fetch failure (non-existent id) does not break the revoke or the audit entry:
+// it still returns nil, still emits a SESSION_REVOKED event, and simply omits
+// the device Details rather than panicking.
+func TestLoginSession_RevokeByID_MissingRowDegradesGracefully(t *testing.T) {
+	_, svc, rec := setupRecordingLoginSessionService(t)
+	ctx := context.Background()
+	userID := uuid.New().String()
+
+	require.NoError(t, svc.RevokeByID(ctx, "does-not-exist", userID))
+
+	require.Len(t, rec.entries, 1)
+	entry := rec.entries[0]
+	assert.Equal(t, models.EventSessionRevoked, entry.EventType)
+	assert.True(t, entry.Success)
+	// No device row was found, so no browser/os/ip Details are recorded.
+	assert.NotContains(t, entry.Details, "browser")
+	assert.NotContains(t, entry.Details, "os")
+	assert.NotContains(t, entry.Details, "ip")
+}
+
 func TestLoginSession_RevokeByID_NoResurrection(t *testing.T) {
 	_, svc := setupLoginSessionService(t)
 	ctx := context.Background()
@@ -109,7 +180,7 @@ func TestLoginSession_RevokeOthers(t *testing.T) {
 // empty current SID (legacy cookie whose backfill save failed) would otherwise
 // revoke ALL the user's rows — including the caller's own session.
 func TestLoginSession_RevokeOthers_EmptySIDIsNoop(t *testing.T) {
-	_, svc := setupLoginSessionService(t)
+	_, svc, rec := setupRecordingLoginSessionService(t)
 	ctx := context.Background()
 	userID := uuid.New().String()
 
@@ -126,6 +197,14 @@ func TestLoginSession_RevokeOthers_EmptySIDIsNoop(t *testing.T) {
 	remaining, err := svc.ListActiveByUser(ctx, userID)
 	require.NoError(t, err)
 	assert.Len(t, remaining, 2)
+
+	// The refused (no-op) attempt is now surfaced as a WARNING audit entry.
+	require.Len(t, rec.entries, 1)
+	entry := rec.entries[0]
+	assert.Equal(t, models.EventSessionRevokedAll, entry.EventType)
+	assert.Equal(t, models.SeverityWarning, entry.Severity)
+	assert.False(t, entry.Success)
+	assert.Equal(t, "missing_current_sid", entry.Details["reason"])
 }
 
 func TestLoginSession_TouchIfStale_Throttled(t *testing.T) {