Skip to content

Commit 00204dd

Browse files
olkkotiTimo Olkkonenwillyborankin
authored
Log request body to audit logs (#5071)
Signed-off-by: Timo Olkkonen <timo.olkkonen@reaktor.com> Co-authored-by: Timo Olkkonen <timo.olkkonen@reaktor.com> Co-authored-by: Andrey Pleskach <ples@aiven.io>
1 parent bd7985d commit 00204dd

File tree

6 files changed

+79
-21
lines changed

6 files changed

+79
-21
lines changed

‎src/integrationTest/java/org/opensearch/security/InternalAuditLogTest.java‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
*/
1010
package org.opensearch.security;
1111

12-
import java.io.IOException;
12+
import java.time.Duration;
1313

1414
import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
1515
import org.apache.logging.log4j.LogManager;
@@ -48,13 +48,24 @@ public class InternalAuditLogTest {
4848
.build();
4949

5050
@Test
51-
public void testAuditLogShouldBeGreenInSingleNodeCluster() throws IOException {
51+
public void testAuditLogShouldBeGreenInSingleNodeCluster() throws InterruptedException {
5252
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
5353
client.get(""); // demo request for insuring audit-log index is created beforehand
54-
TestRestClient.HttpResponse indicesResponse = client.get("_cat/indices");
55-
56-
assertThat(indicesResponse.getBody(), containsString("security-auditlog"));
57-
assertThat(indicesResponse.getBody(), containsString("green"));
54+
int retriesLeft = 5;
55+
while (retriesLeft > 0) {
56+
retriesLeft = retriesLeft - 1;
57+
try {
58+
TestRestClient.HttpResponse indicesResponse = client.get("_cat/indices");
59+
assertThat(indicesResponse.getBody(), containsString("security-auditlog"));
60+
assertThat(indicesResponse.getBody(), containsString("green"));
61+
break;
62+
} catch (AssertionError e) {
63+
if (retriesLeft == 0) {
64+
throw e;
65+
}
66+
Thread.sleep(Duration.ofSeconds(1));
67+
}
68+
}
5869
}
5970
}
6071
}

‎src/integrationTest/java/org/opensearch/security/rest/AuthZinRestLayerTests.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import static org.hamcrest.Matchers.equalTo;
3535
import static org.opensearch.rest.RestRequest.Method.GET;
3636
import static org.opensearch.rest.RestRequest.Method.POST;
37+
import static org.opensearch.security.auditlog.impl.AuditCategory.AUTHENTICATED;
3738
import static org.opensearch.security.auditlog.impl.AuditCategory.FAILED_LOGIN;
3839
import static org.opensearch.security.auditlog.impl.AuditCategory.GRANTED_PRIVILEGES;
3940
import static org.opensearch.security.auditlog.impl.AuditCategory.MISSING_PRIVILEGES;
@@ -140,6 +141,17 @@ public void testShouldAllowAtRestAndBlockAtTransport() {
140141
}
141142
}
142143

144+
@Test
145+
public void testRequestBodyIsAuditLogged() {
146+
try (TestRestClient client = cluster.getRestClient(REST_PLUS_TRANSPORT)) {
147+
String dummyBody = "{\"hello\": \"world\"}";
148+
client.postJson(PROTECTED_API, dummyBody);
149+
auditLogsRule.assertExactlyOne(
150+
privilegePredicateRESTLayer(AUTHENTICATED, REST_PLUS_TRANSPORT, POST, "/" + PROTECTED_API).withRequestBody(dummyBody)
151+
);
152+
}
153+
}
154+
143155
@Test
144156
public void testShouldAllowAtRestAndTransport() {
145157
try (TestRestClient client = cluster.getRestClient(REST_PLUS_TRANSPORT)) {

‎src/integrationTest/java/org/opensearch/test/framework/audit/AuditMessagePredicate.java‎

Lines changed: 41 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ public class AuditMessagePredicate implements Predicate<AuditMessage> {
4444
private final String effectiveUser;
4545
private final String index;
4646
private final String privilege;
47+
private final String requestBody;
4748

4849
private AuditMessagePredicate(
4950
AuditCategory category,
@@ -55,7 +56,8 @@ private AuditMessagePredicate(
5556
String transportRequestType,
5657
String effectiveUser,
5758
String index,
58-
String privilege
59+
String privilege,
60+
String requestBody
5961
) {
6062
this.category = category;
6163
this.requestLayer = requestLayer;
@@ -67,10 +69,11 @@ private AuditMessagePredicate(
6769
this.effectiveUser = effectiveUser;
6870
this.index = index;
6971
this.privilege = privilege;
72+
this.requestBody = requestBody;
7073
}
7174

7275
private AuditMessagePredicate(AuditCategory category) {
73-
this(category, null, null, null, null, null, null, null, null, null);
76+
this(category, null, null, null, null, null, null, null, null, null, null);
7477
}
7578

7679
public static AuditMessagePredicate auditPredicate(AuditCategory category) {
@@ -120,7 +123,8 @@ public AuditMessagePredicate withLayer(Origin layer) {
120123
transportRequestType,
121124
effectiveUser,
122125
index,
123-
privilege
126+
privilege,
127+
requestBody
124128
);
125129
}
126130

@@ -135,7 +139,8 @@ public AuditMessagePredicate withRequestPath(String path) {
135139
transportRequestType,
136140
effectiveUser,
137141
index,
138-
privilege
142+
privilege,
143+
requestBody
139144
);
140145
}
141146

@@ -150,7 +155,8 @@ public AuditMessagePredicate withRestParams(Map<String, String> params) {
150155
transportRequestType,
151156
effectiveUser,
152157
index,
153-
privilege
158+
privilege,
159+
requestBody
154160
);
155161
}
156162

@@ -165,7 +171,8 @@ public AuditMessagePredicate withInitiatingUser(String user) {
165171
transportRequestType,
166172
effectiveUser,
167173
index,
168-
privilege
174+
privilege,
175+
requestBody
169176
);
170177
}
171178

@@ -184,7 +191,8 @@ public AuditMessagePredicate withRestMethod(Method method) {
184191
transportRequestType,
185192
effectiveUser,
186193
index,
187-
privilege
194+
privilege,
195+
requestBody
188196
);
189197
}
190198

@@ -199,7 +207,8 @@ public AuditMessagePredicate withTransportRequestType(String type) {
199207
type,
200208
effectiveUser,
201209
index,
202-
privilege
210+
privilege,
211+
requestBody
203212
);
204213
}
205214

@@ -214,7 +223,8 @@ public AuditMessagePredicate withEffectiveUser(String user) {
214223
transportRequestType,
215224
user,
216225
index,
217-
privilege
226+
privilege,
227+
requestBody
218228
);
219229
}
220230

@@ -237,7 +247,8 @@ public AuditMessagePredicate withIndex(String indexName) {
237247
transportRequestType,
238248
effectiveUser,
239249
indexName,
240-
privilege
250+
privilege,
251+
requestBody
241252
);
242253
}
243254

@@ -252,7 +263,24 @@ public AuditMessagePredicate withPrivilege(String privilegeAction) {
252263
transportRequestType,
253264
effectiveUser,
254265
index,
255-
privilegeAction
266+
privilegeAction,
267+
requestBody
268+
);
269+
}
270+
271+
public Predicate<AuditMessage> withRequestBody(String body) {
272+
return new AuditMessagePredicate(
273+
category,
274+
requestLayer,
275+
restRequestPath,
276+
restParams,
277+
initiatingUser,
278+
requestMethod,
279+
transportRequestType,
280+
effectiveUser,
281+
index,
282+
privilege,
283+
body
256284
);
257285
}
258286

@@ -269,6 +297,7 @@ public boolean test(AuditMessage auditMessage) {
269297
predicates.add(audit -> Objects.isNull(effectiveUser) || effectiveUser.equals(audit.getEffectiveUser()));
270298
predicates.add(audit -> Objects.isNull(index) || containIndex(audit, index));
271299
predicates.add(audit -> Objects.isNull(privilege) || privilege.equals(audit.getPrivilege()));
300+
predicates.add(audit -> Objects.isNull(requestBody) || requestBody.equals(audit.getRequestBody()));
272301
return predicates.stream().reduce(Predicate::and).orElseThrow().test(auditMessage);
273302
}
274303

@@ -303,4 +332,5 @@ public String toString() {
303332
+ '\''
304333
+ '}';
305334
}
335+
306336
}

‎src/main/java/org/opensearch/security/auth/BackendRegistry.java‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
227227
UserSubject subject = new UserSubjectImpl(threadPool, superuser);
228228
threadContext.putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
229229
threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, superuser);
230-
auditLog.logSucceededLogin(sslPrincipal, true, null, request);
231230
return true;
232231
}
233232

@@ -392,9 +391,9 @@ public boolean authenticate(final SecurityRequestChannel request) {
392391
final User impersonatedUser = impersonate(request, authenticatedUser);
393392
final User effectiveUser = impersonatedUser == null ? authenticatedUser : impersonatedUser;
394393
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, effectiveUser);
394+
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INITIATING_USER, authenticatedUser.getName());
395395
UserSubject subject = new UserSubjectImpl(threadPool, effectiveUser);
396396
threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
397-
auditLog.logSucceededLogin(effectiveUser.getName(), false, authenticatedUser.getName(), request);
398397
} else {
399398
if (isDebugEnabled) {
400399
log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size());
@@ -425,7 +424,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
425424

426425
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, anonymousUser);
427426
threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
428-
auditLog.logSucceededLogin(anonymousUser.getName(), false, null, request);
429427
if (isDebugEnabled) {
430428
log.debug("Anonymous User is authenticated");
431429
}

‎src/main/java/org/opensearch/security/filter/SecurityRestFilter.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@
7272

7373
import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX;
7474
import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
75+
import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_INITIATING_USER;
7576

7677
public class SecurityRestFilter {
7778

@@ -168,12 +169,16 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c
168169

169170
// Authorize Request
170171
final User user = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
172+
String intiatingUser = threadContext.getTransient(OPENDISTRO_SECURITY_INITIATING_USER);
171173
if (userIsSuperAdmin(user, adminDNs)) {
172174
// Super admins are always authorized
175+
auditLog.logSucceededLogin(user.getName(), true, intiatingUser, requestChannel);
173176
delegate.handleRequest(request, channel, client);
174177
return;
175178
}
176-
179+
if (user != null) {
180+
auditLog.logSucceededLogin(user.getName(), false, intiatingUser, requestChannel);
181+
}
177182
final Optional<SecurityResponse> deniedResponse = whitelistingSettings.checkRequestIsAllowed(requestChannel)
178183
.or(() -> allowlistingSettings.checkRequestIsAllowed(requestChannel));
179184

‎src/main/java/org/opensearch/security/support/ConfigConstants.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,8 @@ public class ConfigConstants {
120120

121121
public static final String OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT = OPENDISTRO_SECURITY_CONFIG_PREFIX + "user_info";
122122

123+
public static final String OPENDISTRO_SECURITY_INITIATING_USER = OPENDISTRO_SECURITY_CONFIG_PREFIX + "_initiating_user";
124+
123125
public static final String OPENDISTRO_SECURITY_INJECTED_USER = "injected_user";
124126
public static final String OPENDISTRO_SECURITY_INJECTED_USER_HEADER = "injected_user_header";
125127

0 commit comments

Comments
 (0)