Skip to content

Commit

Permalink
update Unthrotized error code to 401 (opensearch-project#2076)
Browse files Browse the repository at this point in the history 
Signed-off-by: Xun Zhang <xunzh@amazon.com>
  • Loading branch information
@Zhangxunmt @austintlee
Zhangxunmt authored and austintlee committed Mar 18, 2024
1 parent fea050b commit 41407e7
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 24 deletions.
17 changes: 13 additions & 4 deletions memory/src/main/java/org/opensearch/ml/memory/index/ConversationMetaIndex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -279,7 +279,10 @@ public void deleteConversation(String conversationId, ActionListener<Boolean> 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); }));
}
Expand Down Expand Up @@ -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); }));
}
Expand Down Expand Up @@ -435,7 +441,10 @@ public void getConversation(String conversationId, ActionListener<ConversationMe
// Otherwise you don't have permission
internalListener
.onFailure(
new OpenSearchSecurityException("User [" + user + "] does not have access to conversation " + conversationId)
new OpenSearchStatusException(
"User [" + user + "] does not have access to conversation " + conversationId,
RestStatus.UNAUTHORIZED
)
);
}, e -> { internalListener.onFailure(e); });
client.admin().indices().refresh(Requests.refreshRequest(META_INDEX_NAME), ActionListener.wrap(refreshResponse -> {
Expand Down
37 changes: 29 additions & 8 deletions memory/src/main/java/org/opensearch/ml/memory/index/InteractionsIndex.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -485,7 +494,10 @@ public void deleteConversation(String conversationId, ActionListener<Boolean> 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);
Expand Down Expand Up @@ -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); }));
}
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
20 changes: 12 additions & 8 deletions memory/src/test/java/org/opensearch/ml/memory/ConversationalMemoryHandlerITTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
};
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand All @@ -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 {
Expand All @@ -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);
Expand Down Expand Up @@ -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(
Expand All @@ -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();
Expand Down
10 changes: 6 additions & 4 deletions memory/src/test/java/org/opensearch/ml/memory/index/ConversationMetaIndexITTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand All @@ -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();
Expand Down

0 comments on commit 41407e7

Please sign in to comment.