From 41407e71043198c32ba2bc7b30586c1de55c8bc3 Mon Sep 17 00:00:00 2001 From: Xun Zhang Date: Mon, 12 Feb 2024 12:24:05 -0800 Subject: [PATCH] update Unthrotized error code to 401 (#2076) Signed-off-by: Xun Zhang --- .../memory/index/ConversationMetaIndex.java | 17 +++++++-- .../ml/memory/index/InteractionsIndex.java | 37 +++++++++++++++---- .../ConversationalMemoryHandlerITTests.java | 20 ++++++---- .../index/ConversationMetaIndexITTests.java | 10 +++-- 4 files changed, 60 insertions(+), 24 deletions(-) diff --git a/memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java b/memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java index 86a045a35a..04c1e6c9b8 100644 --- a/memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java +++ b/memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java @@ -25,7 +25,7 @@ import java.util.List; import java.util.Map; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.OpenSearchStatusException; import org.opensearch.OpenSearchWrapperException; import org.opensearch.ResourceAlreadyExistsException; import org.opensearch.ResourceNotFoundException; @@ -279,7 +279,10 @@ public void deleteConversation(String conversationId, ActionListener li listener.onFailure(e); } } else { - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); })); } @@ -383,7 +386,10 @@ public void updateConversation(String conversationId, UpdateRequest updateReques .getThreadContext() .getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); })); } @@ -435,7 +441,10 @@ public void getConversation(String conversationId, ActionListener { internalListener.onFailure(e); }); client.admin().indices().refresh(Requests.refreshRequest(META_INDEX_NAME), ActionListener.wrap(refreshResponse -> { diff --git a/memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java b/memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java index fdb8d0c282..3348f45d8b 100644 --- a/memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java +++ b/memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java @@ -26,7 +26,7 @@ import java.util.List; import java.util.Map; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.OpenSearchStatusException; import org.opensearch.OpenSearchWrapperException; import org.opensearch.ResourceAlreadyExistsException; import org.opensearch.ResourceNotFoundException; @@ -195,7 +195,10 @@ public void createInteraction( listener.onFailure(e); } } else { - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); })); } else { @@ -271,7 +274,10 @@ public void getInteractions(String conversationId, int from, int maxResults, Act .getThreadContext() .getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); }); conversationMetaIndex.checkAccess(conversationId, accessListener); @@ -355,7 +361,10 @@ public void getTraces(String interactionId, int from, int maxResults, ActionList String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to interaction " + interactionId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to interaction " + interactionId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); }); conversationMetaIndex.checkAccess(conversationId, accessListener); @@ -485,7 +494,10 @@ public void deleteConversation(String conversationId, ActionListener li if (access) { getAllInteractions(conversationId, resultsAtATime, searchListener); } else { - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); }); conversationMetaIndex.checkAccess(conversationId, accessListener); @@ -531,7 +543,10 @@ public void searchInteractions(String conversationId, SearchRequest request, Act .getThreadContext() .getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to conversation " + conversationId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); })); } @@ -615,7 +630,10 @@ public void updateInteraction(String interactionId, UpdateRequest updateRequest, String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to interaction " + interactionId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to interaction " + interactionId, + RestStatus.UNAUTHORIZED + ); } }, e -> { listener.onFailure(e); }); conversationMetaIndex.checkAccess(conversationId, accessListener); @@ -652,7 +670,10 @@ private void checkInteractionPermission(String interactionId, Interaction intera .getThreadContext() .getTransient(ConfigConstants.OPENSEARCH_SECURITY_USER_INFO_THREAD_CONTEXT); String user = User.parse(userstr) == null ? ActionConstants.DEFAULT_USERNAME_FOR_ERRORS : User.parse(userstr).getName(); - throw new OpenSearchSecurityException("User [" + user + "] does not have access to interaction " + interactionId); + throw new OpenSearchStatusException( + "User [" + user + "] does not have access to interaction " + interactionId, + RestStatus.UNAUTHORIZED + ); } }, e -> { internalListener.onFailure(e); }); conversationMetaIndex.checkAccess(conversationId, accessListener); diff --git a/memory/src/test/java/org/opensearch/ml/memory/ConversationalMemoryHandlerITTests.java b/memory/src/test/java/org/opensearch/ml/memory/ConversationalMemoryHandlerITTests.java index 5449f91e18..4aef6ca900 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/ConversationalMemoryHandlerITTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/ConversationalMemoryHandlerITTests.java @@ -24,7 +24,7 @@ import java.util.function.Consumer; import org.junit.Before; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.OpenSearchStatusException; import org.opensearch.action.LatchedActionListener; import org.opensearch.action.StepListener; import org.opensearch.client.Client; @@ -34,6 +34,7 @@ import org.opensearch.common.util.concurrent.ThreadContext.StoredContext; import org.opensearch.commons.ConfigConstants; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; import org.opensearch.ml.common.conversation.ConversationMeta; import org.opensearch.ml.common.conversation.Interaction; import org.opensearch.ml.memory.index.OpenSearchConversationalMemoryHandler; @@ -350,7 +351,10 @@ public void testDifferentUsers_DifferentConversations() { while (!contextStack.empty()) { contextStack.pop().close(); } - OpenSearchSecurityException e = new OpenSearchSecurityException("User was given inappropriate access controls"); + OpenSearchStatusException e = new OpenSearchStatusException( + "User was given inappropriate access controls", + RestStatus.UNAUTHORIZED + ); log.error(e); throw e; }; @@ -470,7 +474,7 @@ public void testDifferentUsers_DifferentConversations() { }, onFail); failiid1.whenComplete(shouldHaveFailedAsString, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user2 + "] does not have access to conversation ")) { cmHandler .createInteraction( @@ -488,7 +492,7 @@ public void testDifferentUsers_DifferentConversations() { }); failiid2.whenComplete(shouldHaveFailedAsString, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user2 + "] does not have access to conversation ")) { cmHandler.getConversations(10, conversations2); } else { @@ -510,7 +514,7 @@ public void testDifferentUsers_DifferentConversations() { }, onFail); failInter2.whenComplete(shouldHaveFailedAsInterList, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user2 + "] does not have access to conversation ")) { cmHandler.getInteractions(cid1.result(), 0, 10, failInter1); } else { @@ -519,7 +523,7 @@ public void testDifferentUsers_DifferentConversations() { }); failInter1.whenComplete(shouldHaveFailedAsInterList, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user2 + "] does not have access to conversation ")) { contextStack.pop().restore(); cmHandler.getConversations(0, 10, conversations1); @@ -549,7 +553,7 @@ public void testDifferentUsers_DifferentConversations() { }, onFail); failInter3.whenComplete(shouldHaveFailedAsInterList, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user1 + "] does not have access to conversation ")) { cmHandler .createInteraction( @@ -567,7 +571,7 @@ public void testDifferentUsers_DifferentConversations() { }); failiid3.whenComplete(shouldHaveFailedAsString, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user1 + "] does not have access to conversation ")) { contextStack.pop().restore(); cdl.countDown(); diff --git a/memory/src/test/java/org/opensearch/ml/memory/index/ConversationMetaIndexITTests.java b/memory/src/test/java/org/opensearch/ml/memory/index/ConversationMetaIndexITTests.java index fc8e7d0145..734b5466fd 100644 --- a/memory/src/test/java/org/opensearch/ml/memory/index/ConversationMetaIndexITTests.java +++ b/memory/src/test/java/org/opensearch/ml/memory/index/ConversationMetaIndexITTests.java @@ -27,7 +27,7 @@ import org.junit.Before; import org.junit.Ignore; -import org.opensearch.OpenSearchSecurityException; +import org.opensearch.OpenSearchStatusException; import org.opensearch.action.LatchedActionListener; import org.opensearch.action.StepListener; import org.opensearch.action.search.SearchRequest; @@ -39,6 +39,7 @@ import org.opensearch.common.util.concurrent.ThreadContext.StoredContext; import org.opensearch.commons.ConfigConstants; import org.opensearch.core.action.ActionListener; +import org.opensearch.core.rest.RestStatus; import org.opensearch.index.query.QueryBuilders; import org.opensearch.ml.common.conversation.ConversationMeta; import org.opensearch.ml.common.conversation.ConversationalIndexConstants; @@ -388,8 +389,9 @@ public void testDifferentUsersCannotTouchOthersConversations() { }, onFail); delListener.whenComplete(success -> { - Exception e = new OpenSearchSecurityException( - "Incorrect access was given to user [" + user2 + "] for conversation " + cid1.result() + Exception e = new OpenSearchStatusException( + "Incorrect access was given to user [" + user2 + "] for conversation " + cid1.result(), + RestStatus.UNAUTHORIZED ); while (!contextStack.empty()) { contextStack.pop().close(); @@ -398,7 +400,7 @@ public void testDifferentUsersCannotTouchOthersConversations() { log.error(e); assert (false); }, e -> { - if (e instanceof OpenSearchSecurityException + if (e instanceof OpenSearchStatusException && e.getMessage().startsWith("User [" + user2 + "] does not have access to conversation ")) { contextStack.pop().restore(); contextStack.pop().restore();