Skip to content

Commit e6a47f5

Browse files
DarshitChanpuraDarshitChanpura
committed
Merge remote-tracking branch 'upstream/main' into resource-sharing-spi
2 parents 2d19530 + 00204dd commit e6a47f5

File tree

7 files changed

+80
-22
lines changed

7 files changed

+80
-22
lines changed

‎sample-resource-plugin/build.gradle

+1-1
Original file line numberDiff line numberDiff line change
@@ -117,4 +117,4 @@ tasks.named("integrationTest").configure {
117117
tasks.named("integrationTest") {
118118
minHeapSize = "512m"
119119
maxHeapSize = "2g"
120-
}
120+
}

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

+17-6
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

+12
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

+41-11
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

+1-3
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
228228
UserSubject subject = new UserSubjectImpl(threadPool, new org.opensearch.security.common.user.User(sslPrincipal));
229229
threadContext.putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
230230
threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, superuser);
231-
auditLog.logSucceededLogin(sslPrincipal, true, null, request);
232231
return true;
233232
}
234233

@@ -393,6 +392,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
393392
final User impersonatedUser = impersonate(request, authenticatedUser);
394393
final User effectiveUser = impersonatedUser == null ? authenticatedUser : impersonatedUser;
395394
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, effectiveUser);
395+
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_INITIATING_USER, authenticatedUser.getName());
396396

397397
// TODO: The following artistry must be reverted when User class is completely moved to :opensearch-security-common
398398
org.opensearch.security.common.user.User effUser = new org.opensearch.security.common.user.User(
@@ -403,7 +403,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
403403
effUser.setAttributes(effectiveUser.getCustomAttributesMap());
404404
UserSubject subject = new UserSubjectImpl(threadPool, effUser);
405405
threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
406-
auditLog.logSucceededLogin(effectiveUser.getName(), false, authenticatedUser.getName(), request);
407406
} else {
408407
if (isDebugEnabled) {
409408
log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size());
@@ -441,7 +440,6 @@ public boolean authenticate(final SecurityRequestChannel request) {
441440

442441
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, anonymousUser);
443442
threadPool.getThreadContext().putPersistent(ConfigConstants.OPENDISTRO_SECURITY_AUTHENTICATED_USER, subject);
444-
auditLog.logSucceededLogin(anonymousUser.getName(), false, null, request);
445443
if (isDebugEnabled) {
446444
log.debug("Anonymous User is authenticated");
447445
}

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

+6-1
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

+2
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,8 @@ public class ConfigConstants {
121121

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

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

0 commit comments

Comments
 (0)