diff --git a/lib/utilities/serverAccessLogger.js b/lib/utilities/serverAccessLogger.js index 3832e5ecca..b474cc81ef 100644 --- a/lib/utilities/serverAccessLogger.js +++ b/lib/utilities/serverAccessLogger.js @@ -411,62 +411,61 @@ function logServerAccess(req, res) { pid: process.pid, // Analytics - action: params.analyticsAction === undefined ? null : params.analyticsAction, - accountName: params.analyticsAccountName === undefined ? null : params.analyticsAccountName, - accountDisplayName: authInfo ? authInfo.getAccountDisplayName() : null, - userName: params.analyticsUserName === undefined ? null : params.analyticsUserName, - clientPort: req.socket.remotePort === undefined ? null : req.socket.remotePort, - httpMethod: req.method === undefined ? null : req.method, - bytesDeleted: params.analyticsBytesDeleted === undefined ? null : params.analyticsBytesDeleted, - bytesReceived: req.parsedContentLength === undefined ? null : req.parsedContentLength, - bodyLength: req.headers['content-length'] === undefined ? null : parseInt(req.headers['content-length'], 10), - contentLength: req.parsedContentLength === undefined ? null : req.parsedContentLength, + action: params.analyticsAction ?? undefined, + accountName: params.analyticsAccountName ?? undefined, + userName: params.analyticsUserName ?? undefined, + httpMethod: req.method ?? undefined, + bytesDeleted: params.analyticsBytesDeleted ?? undefined, + bytesReceived: req.parsedContentLength ?? undefined, + bodyLength: req.headers['content-length'] !== undefined + ? parseInt(req.headers['content-length'], 10) + : undefined, + contentLength: req.parsedContentLength ?? undefined, // eslint-disable-next-line camelcase - elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime), - httpURL: req.url === undefined ? null : req.url, + elapsed_ms: calculateElapsedMS(params.startTime, params.onCloseEndTime) ?? undefined, // AWS access server logs fields https://docs.aws.amazon.com/AmazonS3/latest/userguide/LogFormat.html - startTime: timestampToDateTime643(params.startTimeUnixMS), // AWS "Time" field - requester: getRequester(authInfo), + startTime: timestampToDateTime643(params.startTimeUnixMS) ?? undefined, // AWS "Time" field + requester: getRequester(authInfo) ?? undefined, operation: getOperation(req), - requestURI: getURI(req), - errorCode: errorCode === undefined ? null : errorCode, - objectSize: getObjectSize(req, res), - totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime), - turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime), - referer: req.headers.referer === undefined ? null : req.headers.referer, - userAgent: req.headers['user-agent'] === undefined ? null : req.headers['user-agent'], - versionID: !req.query ? null : req.query.versionId === undefined ? null : req.query.versionId, - signatureVersion: authInfo ? authInfo.getAuthVersion() : null, - cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : null, - authenticationType: authInfo ? authInfo.getAuthType() : null, - hostHeader: req.headers.host === undefined ? null : req.headers.host, - tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : null, - aclRequired: null, // TODO: CLDSRV-774 - // hostID: null, // NOT IMPLEMENTED - // accessPointARN: null, // NOT IMPLEMENTED + requestURI: getURI(req) ?? undefined, + errorCode: errorCode ?? undefined, + objectSize: getObjectSize(req, res) ?? undefined, + totalTime: calculateTotalTime(params.startTime, params.onFinishEndTime) ?? undefined, + turnAroundTime: calculateTurnAroundTime(params.startTurnAroundTime, endTurnAroundTime) ?? undefined, + referer: req.headers.referer ?? undefined, + userAgent: req.headers['user-agent'] ?? undefined, + versionID: req.query?.versionId ?? undefined, + signatureVersion: authInfo?.getAuthVersion() ?? undefined, + cipherSuite: req.socket.encrypted ? req.socket.getCipher()['standardName'] : undefined, + authenticationType: authInfo?.getAuthType() ?? undefined, + hostHeader: req.headers.host ?? undefined, + tlsVersion: req.socket.encrypted ? req.socket.getCipher()['version'] : undefined, + aclRequired: undefined, // TODO: CLDSRV-774 + // hostID: undefined, // NOT IMPLEMENTED + // accessPointARN: undefined, // NOT IMPLEMENTED // Shared between AWS access server logs and Analytics logs - bucketOwner: params.bucketOwner === undefined ? null : params.bucketOwner, - bucketName: params.bucketName === undefined ? null : params.bucketName, // AWS "Bucket" field + bucketOwner: params.bucketOwner ?? undefined, + bucketName: params.bucketName ?? undefined, // AWS "Bucket" field // eslint-disable-next-line camelcase - req_id: requestID === undefined ? null : requestID, // AWS "Request ID" field - bytesSent: getBytesSent(res, bytesSent), - clientIP: getRemoteIPFromRequest(req), // AWS 'Remote IP' field - httpCode: res.statusCode === undefined ? null : res.statusCode, // AWS "HTTP Status" field - objectKey: params.objectKey === undefined ? null : params.objectKey, // AWS "Key" field + req_id: requestID ?? undefined, // AWS "Request ID" field + bytesSent: getBytesSent(res, bytesSent) ?? undefined, + clientIP: getRemoteIPFromRequest(req) ?? undefined, // AWS 'Remote IP' field + httpCode: res.statusCode ?? undefined, // AWS "HTTP Status" field + objectKey: params.objectKey ?? undefined, // AWS "Key" field // Scality server access logs extra fields logFormatVersion: SERVER_ACCESS_LOG_FORMAT_VERSION, - loggingEnabled: params.enabled, - loggingTargetBucket: params.loggingEnabled ? params.loggingEnabled.TargetBucket : null, - loggingTargetPrefix: params.loggingEnabled ? params.loggingEnabled.TargetPrefix : null, - awsAccessKeyID: authInfo ? authInfo.getAccessKey() : null, - raftSessionID: params.raftSessionID === undefined ? null : params.raftSessionID, + loggingEnabled: params.enabled ?? undefined, + loggingTargetBucket: params.loggingEnabled?.TargetBucket ?? undefined, + loggingTargetPrefix: params.loggingEnabled?.TargetPrefix ?? undefined, + awsAccessKeyID: authInfo?.getAccessKey() ?? undefined, + raftSessionID: params.raftSessionID ?? undefined, // Rate Limiting fields (only present when rate limited) - rateLimited: params.rateLimited, - rateLimitSource: params.rateLimitSource, + rateLimited: params.rateLimited ?? undefined, + rateLimitSource: params.rateLimitSource ?? undefined, })}\n`); } diff --git a/package.json b/package.json index 3482f4cc11..722bed4eec 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@zenko/cloudserver", - "version": "9.2.13", + "version": "9.2.14", "description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol", "main": "index.js", "engines": { diff --git a/schema/server_access_log.schema.json b/schema/server_access_log.schema.json index b7e803eb24..a103f95551 100644 --- a/schema/server_access_log.schema.json +++ b/schema/server_access_log.schema.json @@ -27,20 +27,10 @@ "description": "Requester account display name.", "type": ["string", "null"] }, - "accountDisplayName": { - "description": "Requester account display name.", - "type": ["string", "null"] - }, "userName": { "description": "Requester IAM display name, or null if the requester is not an IAM user.", "type": ["string", "null"] }, - "clientPort": { - "description": "Requester remote connection port.", - "type": "integer", - "maximum": 65535, - "minimum": 1 - }, "httpMethod": { "description": "Request HTTP method.", "type": "string", @@ -71,10 +61,6 @@ "type": "number", "minimum": 0 }, - "httpURL": { - "description": "Request URL string. This contains only the URL that is present in the actual HTTP request.", - "type": "string" - }, "startTime": { "description": "Timestamp formatted as: 'seconds.milliseconds', recorded when the server first routes the request. Represents the AWS server access log 'Time' field. String type compatible with Clickhouse DateTime64(3) type.", "type": "string" @@ -211,47 +197,16 @@ "time", "hostname", "pid", - "action", - "accountName", - "accountDisplayName", - "userName", - "clientPort", + "operation", + "logFormatVersion", "httpMethod", - "bytesDeleted", - "bytesReceived", - "bodyLength", - "contentLength", - "elapsed_ms", - "httpURL", "startTime", - "requester", - "operation", - "requestURI", - "errorCode", - "objectSize", + "elapsed_ms", "totalTime", - "turnAroundTime", - "referer", - "userAgent", - "versionID", - "signatureVersion", - "cipherSuite", - "authenticationType", + "requestURI", "hostHeader", - "tlsVersion", - "aclRequired", - "bucketOwner", - "bucketName", "req_id", - "bytesSent", - "clientIP", "httpCode", - "objectKey", - "logFormatVersion", - "loggingEnabled", - "loggingTargetBucket", - "loggingTargetPrefix", - "awsAccessKeyID", - "raftSessionID" + "loggingEnabled" ] } diff --git a/tests/functional/aws-node-sdk/test/serverAccessLogs/testServerAccessLogFile.js b/tests/functional/aws-node-sdk/test/serverAccessLogs/testServerAccessLogFile.js index c1d51bedb5..e79133cab5 100644 --- a/tests/functional/aws-node-sdk/test/serverAccessLogs/testServerAccessLogFile.js +++ b/tests/functional/aws-node-sdk/test/serverAccessLogs/testServerAccessLogFile.js @@ -118,7 +118,6 @@ describe('Server Access Logs - File Output', async () => { // 'pid': '', // UNKNOWN 'action': 'REQUIRED', // DYNAMIC 'accountName': 'Bart', // STATIC - 'accountDisplayName': 'Bart', // STATIC 'userName': null, // TODO: Add test with IAM user to get a non null userName. // 'clientPort': '', // UNKNOWN 'httpMethod': 'REQUIRED', // DYNAMIC @@ -2642,6 +2641,12 @@ describe('Server Access Logs - File Output', async () => { const properties = operation.expected[operationIdx]; for (const [key, val] of Object.entries(properties)) { + if (val === null) { + // Verify that null fields are omitted (not present in log) + assert.strictEqual(key in logEntries[logEntryIdx], false, + `Field ${key} should be omitted when null, action ${properties.action}`); + continue; + } assert.strictEqual(logEntries[logEntryIdx][key], val, `Invalid value for ${key}, action ${properties.action}`); } diff --git a/tests/unit/utils/serverAccessLogger.js b/tests/unit/utils/serverAccessLogger.js index e2895101f7..49aa1abae7 100644 --- a/tests/unit/utils/serverAccessLogger.js +++ b/tests/unit/utils/serverAccessLogger.js @@ -14,7 +14,6 @@ const { timestampToDateTime643, } = require('../../../lib/utilities/serverAccessLogger'); - describe('serverAccessLogger utility functions', () => { describe('getRemoteIPFromRequest', () => { it('should return IP from x-forwarded-for header', () => { @@ -804,23 +803,20 @@ describe('serverAccessLogger utility functions', () => { // Verify analytics fields assert.strictEqual(loggedData.action, 'ObjectGet'); assert.strictEqual(loggedData.accountName, 'testAccount'); - assert.strictEqual(loggedData.accountDisplayName, 'testAccount'); assert.strictEqual(loggedData.userName, 'testUser'); - assert.strictEqual(loggedData.clientPort, 54321); assert.strictEqual(loggedData.httpMethod, 'GET'); assert.strictEqual(loggedData.bytesDeleted, 0); assert.strictEqual(loggedData.bytesReceived, 1024); assert.strictEqual(loggedData.bodyLength, 1024); assert.strictEqual(loggedData.contentLength, 1024); assert.strictEqual(loggedData.elapsed_ms, 20.5); - assert.strictEqual(loggedData.httpURL, '/test-bucket/test-key.txt'); // Verify AWS access server log fields assert.strictEqual(loggedData.startTime, '1234567890.000'); assert.strictEqual(loggedData.requester, 'canonical123'); assert.strictEqual(loggedData.operation, 'REST.GET.OBJECT'); assert.strictEqual(loggedData.requestURI, 'GET /test-bucket/test-key.txt HTTP/1.1'); - assert.strictEqual(loggedData.errorCode, null); + assert.strictEqual('errorCode' in loggedData, false); assert.strictEqual(loggedData.objectSize, 2048); assert.strictEqual(loggedData.totalTime, '9'); assert.strictEqual(loggedData.turnAroundTime, '1'); @@ -832,7 +828,7 @@ describe('serverAccessLogger utility functions', () => { assert.strictEqual(loggedData.authenticationType, 'REST-HEADER'); assert.strictEqual(loggedData.hostHeader, 's3.amazonaws.com'); assert.strictEqual(loggedData.tlsVersion, 'TLSv1.3'); - assert.strictEqual(loggedData.aclRequired, null); + assert.strictEqual('aclRequired' in loggedData, false); // Verify shared fields assert.strictEqual(loggedData.bucketOwner, 'bucketOwner123'); @@ -893,11 +889,10 @@ describe('serverAccessLogger utility functions', () => { assert.strictEqual(mockLogger.write.callCount, 1); const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); - assert.strictEqual(loggedData.accountDisplayName, null); - assert.strictEqual(loggedData.requester, null); - assert.strictEqual(loggedData.signatureVersion, null); - assert.strictEqual(loggedData.authenticationType, null); - assert.strictEqual(loggedData.awsAccessKeyID, null); + assert.strictEqual('requester' in loggedData, false); + assert.strictEqual('signatureVersion' in loggedData, false); + assert.strictEqual('authenticationType' in loggedData, false); + assert.strictEqual('awsAccessKeyID' in loggedData, false); }); it('should handle non-encrypted connection', () => { @@ -918,8 +913,8 @@ describe('serverAccessLogger utility functions', () => { assert.strictEqual(mockLogger.write.callCount, 1); const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); - assert.strictEqual(loggedData.cipherSuite, null); - assert.strictEqual(loggedData.tlsVersion, null); + assert.strictEqual('cipherSuite' in loggedData, false); + assert.strictEqual('tlsVersion' in loggedData, false); }); it('should handle missing query parameters', () => { @@ -939,7 +934,7 @@ describe('serverAccessLogger utility functions', () => { assert.strictEqual(mockLogger.write.callCount, 1); const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); - assert.strictEqual(loggedData.versionID, null); + assert.strictEqual('versionID' in loggedData, false); }); it('should handle loggingEnabled without TargetBucket/TargetPrefix', () => { @@ -960,8 +955,8 @@ describe('serverAccessLogger utility functions', () => { assert.strictEqual(mockLogger.write.callCount, 1); const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); - assert.strictEqual(loggedData.loggingTargetBucket, null); - assert.strictEqual(loggedData.loggingTargetPrefix, null); + assert.strictEqual('loggingTargetBucket' in loggedData, false); + assert.strictEqual('loggingTargetPrefix' in loggedData, false); }); it('should handle PUT request with objectPut', () => { @@ -1019,6 +1014,114 @@ describe('serverAccessLogger utility functions', () => { const jsonData = writtenData.trim(); assert.doesNotThrow(() => JSON.parse(jsonData)); }); + + it('should omit null fields from log output', () => { + setServerAccessLogger(mockLogger); + const req = { + serverAccessLog: {}, + headers: {}, + socket: {}, + }; + const res = { + serverAccessLog: {}, + getHeader: () => null, + }; + + logServerAccess(req, res); + + assert.strictEqual(mockLogger.write.callCount, 1); + const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); + + // Verify null fields are omitted + assert.strictEqual('errorCode' in loggedData, false); + assert.strictEqual('bytesDeleted' in loggedData, false); + assert.strictEqual('turnAroundTime' in loggedData, false); + assert.strictEqual('referer' in loggedData, false); + assert.strictEqual('versionID' in loggedData, false); + assert.strictEqual('cipherSuite' in loggedData, false); + assert.strictEqual('tlsVersion' in loggedData, false); + assert.strictEqual('aclRequired' in loggedData, false); + + // Verify non-null fields are present + assert.strictEqual('time' in loggedData, true); + assert.strictEqual('hostname' in loggedData, true); + assert.strictEqual('pid' in loggedData, true); + }); + + it('should preserve zero values in logs', () => { + setServerAccessLogger(mockLogger); + const req = { + serverAccessLog: { + analyticsBytesDeleted: 0, + }, + headers: { 'content-length': '0' }, + parsedContentLength: 0, + socket: {}, + }; + const res = { + serverAccessLog: {}, + statusCode: 0, + getHeader: () => null, + }; + + logServerAccess(req, res); + + assert.strictEqual(mockLogger.write.callCount, 1); + const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); + + // Verify zero values are preserved + assert.strictEqual(loggedData.bytesDeleted, 0); + assert.strictEqual(loggedData.contentLength, 0); + assert.strictEqual(loggedData.httpCode, 0); + }); + + it('should preserve false values in logs', () => { + setServerAccessLogger(mockLogger); + const req = { + serverAccessLog: { + enabled: false, + }, + headers: {}, + socket: {}, + }; + const res = { + serverAccessLog: {}, + getHeader: () => null, + }; + + logServerAccess(req, res); + + assert.strictEqual(mockLogger.write.callCount, 1); + const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); + + // Verify false is preserved + assert.strictEqual(loggedData.loggingEnabled, false); + assert.strictEqual('loggingEnabled' in loggedData, true); + }); + + it('should include error code when present', () => { + setServerAccessLogger(mockLogger); + const req = { + serverAccessLog: {}, + headers: {}, + socket: {}, + }; + const res = { + serverAccessLog: { + errorCode: 'NoSuchKey', + }, + getHeader: () => null, + }; + + logServerAccess(req, res); + + assert.strictEqual(mockLogger.write.callCount, 1); + const loggedData = JSON.parse(mockLogger.write.firstCall.args[0].trim()); + + // Verify error code is included + assert.strictEqual(loggedData.errorCode, 'NoSuchKey'); + assert.strictEqual('errorCode' in loggedData, true); + }); }); });