mirror of
https://github.com/mattermost/mattermost.git
synced 2026-08-27 05:37:15 -05:00
[MM-67140] Added session validation on logout (#34959)
* add authentication status to audit log for logouts * improve audit log testing for other tests --------- Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
Christopher Poile
co-authored by
Mattermost Build
Mattermost Build
parent
121b429b8e
commit
1ac14a9dfb
@@ -5,6 +5,7 @@ package api4
|
||||
|
||||
import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -1395,3 +1396,63 @@ func (th *TestHelper) SetupScheme(tb testing.TB, scope string) *model.Scheme {
|
||||
func (th *TestHelper) Parallel(t *testing.T) {
|
||||
mainHelper.Parallel(t)
|
||||
}
|
||||
|
||||
// AuditEntry represents a parsed audit log entry for testing
|
||||
type AuditEntry struct {
|
||||
EventName string
|
||||
Status string
|
||||
UserID string
|
||||
SessionID string
|
||||
Parameters map[string]any
|
||||
Raw map[string]any
|
||||
}
|
||||
|
||||
// FindAuditEntry searches audit log data for an entry matching the given event name
|
||||
// and optionally a user ID. Returns the first matching entry or nil if not found.
|
||||
func FindAuditEntry(data string, eventName string, userID string) *AuditEntry {
|
||||
for line := range strings.SplitSeq(data, "\n") {
|
||||
if line == "" {
|
||||
continue
|
||||
}
|
||||
var entry map[string]any
|
||||
if err := json.Unmarshal([]byte(line), &entry); err != nil {
|
||||
continue
|
||||
}
|
||||
|
||||
if entry["event_name"] != eventName {
|
||||
continue
|
||||
}
|
||||
|
||||
auditEntry := &AuditEntry{
|
||||
EventName: eventName,
|
||||
Raw: entry,
|
||||
}
|
||||
|
||||
if status, ok := entry["status"].(string); ok {
|
||||
auditEntry.Status = status
|
||||
}
|
||||
|
||||
if actor, ok := entry["actor"].(map[string]any); ok {
|
||||
if uid, ok := actor["user_id"].(string); ok {
|
||||
auditEntry.UserID = uid
|
||||
}
|
||||
if sid, ok := actor["session_id"].(string); ok {
|
||||
auditEntry.SessionID = sid
|
||||
}
|
||||
}
|
||||
|
||||
if event, ok := entry["event"].(map[string]any); ok {
|
||||
if params, ok := event["parameters"].(map[string]any); ok {
|
||||
auditEntry.Parameters = params
|
||||
}
|
||||
}
|
||||
|
||||
// If userID filter is specified, check it matches
|
||||
if userID != "" && auditEntry.UserID != userID {
|
||||
continue
|
||||
}
|
||||
|
||||
return auditEntry
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -593,9 +593,13 @@ func TestUpdateConfigDiffInAuditRecord(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
require.Contains(t, string(data),
|
||||
fmt.Sprintf(`"config_diffs":[{"actual_val":%d,"base_val":%d,"path":"ServiceSettings.ReadTimeout"}`,
|
||||
timeoutVal+1, timeoutVal))
|
||||
entry := FindAuditEntry(string(data), "updateConfig", "")
|
||||
require.NotNil(t, entry, "should find an updateConfig audit entry")
|
||||
// Verify config diffs are in the raw entry
|
||||
require.Contains(t, fmt.Sprintf("%v", entry.Raw),
|
||||
fmt.Sprintf("actual_val:%d", timeoutVal+1))
|
||||
require.Contains(t, fmt.Sprintf("%v", entry.Raw),
|
||||
fmt.Sprintf("base_val:%d", timeoutVal))
|
||||
}
|
||||
|
||||
func TestGetEnvironmentConfig(t *testing.T) {
|
||||
|
||||
@@ -5,6 +5,7 @@ package api4
|
||||
|
||||
import (
|
||||
"context"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"os"
|
||||
@@ -854,11 +855,12 @@ func TestRegisterOAuthClientAudit(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
auditLog := string(data)
|
||||
assert.Contains(t, auditLog, "registerOAuthClient")
|
||||
assert.Contains(t, auditLog, clientName)
|
||||
assert.Contains(t, auditLog, response.ClientID)
|
||||
assert.Contains(t, auditLog, "success")
|
||||
entry := FindAuditEntry(string(data), "registerOAuthClient", "")
|
||||
require.NotNil(t, entry, "should find a registerOAuthClient audit entry")
|
||||
assert.Equal(t, "success", entry.Status)
|
||||
// Verify client details are in the raw entry
|
||||
assert.Contains(t, fmt.Sprintf("%v", entry.Raw), clientName)
|
||||
assert.Contains(t, fmt.Sprintf("%v", entry.Raw), response.ClientID)
|
||||
})
|
||||
|
||||
t.Run("Failed DCR registration is audited", func(t *testing.T) {
|
||||
@@ -886,8 +888,8 @@ func TestRegisterOAuthClientAudit(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
auditLog := string(data)
|
||||
assert.Contains(t, auditLog, "registerOAuthClient")
|
||||
assert.Contains(t, auditLog, "fail")
|
||||
entry := FindAuditEntry(string(data), "registerOAuthClient", "")
|
||||
require.NotNil(t, entry, "should find a registerOAuthClient audit entry")
|
||||
assert.Equal(t, "fail", entry.Status)
|
||||
})
|
||||
}
|
||||
|
||||
@@ -2350,6 +2350,21 @@ func logout(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
func Logout(c *Context, w http.ResponseWriter, r *http.Request) {
|
||||
auditRec := c.MakeAuditRecord(model.AuditEventLogout, model.AuditStatusFail)
|
||||
defer c.LogAuditRec(auditRec)
|
||||
|
||||
// Determine detailed authentication status for audit record (MM-67140)
|
||||
var authStatus string
|
||||
if c.AppContext.Session().UserId != "" {
|
||||
authStatus = "authenticated"
|
||||
} else {
|
||||
_, tokenLocation := app.ParseAuthTokenFromRequest(r)
|
||||
if tokenLocation == app.TokenLocationNotFound {
|
||||
authStatus = "no_token"
|
||||
} else {
|
||||
authStatus = "token_invalid"
|
||||
}
|
||||
}
|
||||
model.AddEventParameterToAuditRec(auditRec, "auth_status", authStatus)
|
||||
|
||||
c.LogAudit("")
|
||||
|
||||
c.RemoveSessionCookie(w, r)
|
||||
|
||||
@@ -375,7 +375,107 @@ func TestUserLoginAudit(t *testing.T) {
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
// ensure we are auditing the user_id and session_id
|
||||
require.Contains(t, string(data), fmt.Sprintf("\"event_name\":\"login\",\"status\":\"success\",\"actor\":{\"user_id\":\"%s\",\"session_id\":\"%s\"", user.Id, sess[0].Id))
|
||||
entry := FindAuditEntry(string(data), "login", user.Id)
|
||||
require.NotNil(t, entry, "should find a login audit entry for user %s", user.Id)
|
||||
assert.Equal(t, "success", entry.Status)
|
||||
assert.Equal(t, user.Id, entry.UserID)
|
||||
assert.Equal(t, sess[0].Id, entry.SessionID)
|
||||
}
|
||||
|
||||
func TestLogoutAuditAuthStatus(t *testing.T) {
|
||||
logFile, err := os.CreateTemp("", "logout_audit.log")
|
||||
require.NoError(t, err)
|
||||
defer os.Remove(logFile.Name())
|
||||
|
||||
os.Setenv("MM_EXPERIMENTALAUDITSETTINGS_FILEENABLED", "true")
|
||||
os.Setenv("MM_EXPERIMENTALAUDITSETTINGS_FILENAME", logFile.Name())
|
||||
defer os.Unsetenv("MM_EXPERIMENTALAUDITSETTINGS_FILEENABLED")
|
||||
defer os.Unsetenv("MM_EXPERIMENTALAUDITSETTINGS_FILENAME")
|
||||
|
||||
options := []app.Option{app.WithLicense(model.NewTestLicense("advanced_logging"))}
|
||||
th := SetupWithServerOptions(t, options)
|
||||
|
||||
t.Run("authenticated logout has auth_status=authenticated and user_id", func(t *testing.T) {
|
||||
require.NoError(t, logFile.Truncate(0))
|
||||
_, err := logFile.Seek(0, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Login first to get a valid session
|
||||
user, resp, err := th.Client.Login(context.Background(), th.BasicUser.Email, th.BasicUser.Password)
|
||||
require.NoError(t, err)
|
||||
CheckOKStatus(t, resp)
|
||||
|
||||
// Logout with valid token
|
||||
_, err = th.Client.Logout(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
err = th.Server.Audit.Flush()
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, logFile.Sync())
|
||||
|
||||
data, err := io.ReadAll(logFile)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
// Find the logout event for this specific user
|
||||
entry := FindAuditEntry(string(data), "logout", user.Id)
|
||||
require.NotNil(t, entry, "should find a logout audit entry for user %s", user.Id)
|
||||
assert.Equal(t, "authenticated", entry.Parameters["auth_status"],
|
||||
"logout event for user %s should have auth_status=authenticated", user.Id)
|
||||
})
|
||||
|
||||
t.Run("invalid token logout has auth_status=token_invalid", func(t *testing.T) {
|
||||
require.NoError(t, logFile.Truncate(0))
|
||||
_, err := logFile.Seek(0, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a client with an invalid token
|
||||
invalidClient := model.NewAPIv4Client(th.Client.URL)
|
||||
invalidClient.SetToken("invalid_token_12345")
|
||||
|
||||
// Logout with invalid token - should still return OK (idempotent)
|
||||
_, err = invalidClient.Logout(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
err = th.Server.Audit.Flush()
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, logFile.Sync())
|
||||
|
||||
data, err := io.ReadAll(logFile)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
// Find the logout event (no user ID for invalid token)
|
||||
entry := FindAuditEntry(string(data), "logout", "")
|
||||
require.NotNil(t, entry, "should find a logout audit entry")
|
||||
assert.Equal(t, "token_invalid", entry.Parameters["auth_status"])
|
||||
})
|
||||
|
||||
t.Run("no token logout has auth_status=no_token", func(t *testing.T) {
|
||||
require.NoError(t, logFile.Truncate(0))
|
||||
_, err := logFile.Seek(0, 0)
|
||||
require.NoError(t, err)
|
||||
|
||||
// Create a client with no token
|
||||
noTokenClient := model.NewAPIv4Client(th.Client.URL)
|
||||
|
||||
// Logout with no token - should still return OK (idempotent)
|
||||
_, err = noTokenClient.Logout(context.Background())
|
||||
require.NoError(t, err)
|
||||
|
||||
err = th.Server.Audit.Flush()
|
||||
require.NoError(t, err)
|
||||
require.NoError(t, logFile.Sync())
|
||||
|
||||
data, err := io.ReadAll(logFile)
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, data)
|
||||
|
||||
// Find the logout event (no user ID for no token)
|
||||
entry := FindAuditEntry(string(data), "logout", "")
|
||||
require.NotNil(t, entry, "should find a logout audit entry")
|
||||
assert.Equal(t, "no_token", entry.Parameters["auth_status"])
|
||||
})
|
||||
}
|
||||
|
||||
func TestCreateUserInputFilter(t *testing.T) {
|
||||
|
||||
Reference in New Issue
Block a user