Address comments, fix tests

Signed-off-by: Stephen Crawford <steecraw@amazon.com>

libs/common/src/main/java/org/opensearch/common/NotNull.java

@@ -0,0 +1,55 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*
+ * Modifications Copyright OpenSearch Contributors. See
+ * GitHub history for details.
+ */
+
+package org.opensearch.common;
+
+import javax.annotation.Nonnull;
+import javax.annotation.meta.TypeQualifierNickname;
+import java.lang.annotation.Documented;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+
+/**
+ * The presence of this annotation above a method indicates that
+ * {@code null} cannot be returned by that method.
+ *
+ * @opensearch.api
+ */
+@Documented
+@TypeQualifierNickname
+@Nonnull
+@Retention(RetentionPolicy.RUNTIME)
+@Target({ ElementType.METHOD })
+public @interface NotNull {
+}

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/OpenSearchRealm.java

@@ -19,7 +19,7 @@
 import org.apache.shiro.realm.AuthenticatingRealm;
 import org.apache.shiro.authc.UsernamePasswordToken;
 
-import org.opensearch.identity.StringPrincipal;
+import org.opensearch.identity.NamedPrincipal;
 
 import java.util.Objects;
 import java.util.Map;
@@ -56,7 +56,7 @@ public Builder(final String name) {
         public OpenSearchRealm build() {
             // TODO: Replace hardcoded admin user / user map with an external provider
             final User adminUser = new User();
-            adminUser.setUsername(new StringPrincipal("admin"));
+            adminUser.setUsername(new NamedPrincipal("admin"));
             adminUser.setBcryptHash("$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG"); // Password 'admin'
             final Map<String, User> internalUsers = Map.of("admin", adminUser);
             return new OpenSearchRealm(name, internalUsers);

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/User.java

@@ -8,7 +8,7 @@
 
 package org.opensearch.identity.shiro.realm;
 
-import org.opensearch.identity.StringPrincipal;
+import org.opensearch.identity.NamedPrincipal;
 
 /**
  * A non-volatile and immutable object in the storage.
@@ -17,14 +17,14 @@
  */
 public class User {
 
-    private StringPrincipal username;
+    private NamedPrincipal username;
     private String bcryptHash;
 
-    public StringPrincipal getUsername() {
+    public NamedPrincipal getUsername() {
         return username;
     }
 
-    public void setUsername(StringPrincipal username) {
+    public void setUsername(NamedPrincipal username) {
         this.username = username;
     }
 

server/src/main/java/org/opensearch/identity/IdentityService.java

@@ -8,6 +8,7 @@
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.opensearch.OpenSearchException;
+import org.opensearch.common.NotNull;
 import org.opensearch.identity.noop.NoopIdentityPlugin;
 import java.util.List;
 import org.opensearch.common.settings.Settings;
@@ -46,6 +47,7 @@ public IdentityService(final Settings settings, final List<IdentityPlugin> ident
     /**
      * Gets the current subject
      */
+    @NotNull
     public Subject getSubject() {
         return identityPlugin.getSubject();
     }

server/src/main/java/org/opensearch/identity/StringPrincipal.java → server/src/main/java/org/opensearch/identity/NamedPrincipal.java

@@ -13,15 +13,15 @@
  *
  * @opensearch.experimental
  */
-public class StringPrincipal implements Principal {
+public class NamedPrincipal implements Principal {
 
     private final String name;
 
     /**
      * Creates a principal for an identity specified as a string
      * @param name A persistent string that represent an identity
      */
-    public StringPrincipal(final String name) {
+    public NamedPrincipal(final String name) {
         this.name = name;
     }
 
@@ -45,6 +45,6 @@ public int hashCode() {
 
     @Override
     public String toString() {
-        return "StringPrincipal(" + "name=" + name + ")";
+        return "NamedPrincipal(" + "name=" + name + ")";
     }
 }

server/src/main/java/org/opensearch/identity/Principals.java → server/src/main/java/org/opensearch/identity/NativePrincipal.java

@@ -12,17 +12,17 @@
  *
  * @opensearch.experimental
  */
-public enum Principals {
+public class NativePrincipal {
 
     /**
      * Represents a principal which has not been authenticated
      */
-    UNAUTHENTICATED(new StringPrincipal("Unauthenticated"));
+    public static final NativePrincipal UNAUTHENTICATED = new NativePrincipal("Unauthenticated");
 
     private final Principal principal;
 
-    Principals(final Principal principal) {
-        this.principal = principal;
+    NativePrincipal(final String principal) {
+        this.principal = new NamedPrincipal(principal);
     }
 
     /**

server/src/main/java/org/opensearch/identity/noop/NoopSubject.java

@@ -11,9 +11,9 @@
 import java.security.Principal;
 import java.util.Objects;
 
+import org.opensearch.identity.NativePrincipal;
 import org.opensearch.identity.tokens.AuthToken;
 import org.opensearch.identity.Subject;
-import org.opensearch.identity.Principals;
 
 /**
  * Implementation of subject that is always authenticated
@@ -26,7 +26,7 @@ public class NoopSubject implements Subject {
 
     @Override
     public Principal getPrincipal() {
-        return Principals.UNAUTHENTICATED.getPrincipal();
+        return NativePrincipal.UNAUTHENTICATED.getPrincipal();
     }
 
     @Override

server/src/main/java/org/opensearch/plugins/IdentityPlugin.java

@@ -8,6 +8,7 @@
 
 package org.opensearch.plugins;
 
+import org.opensearch.common.Nullable;
 import org.opensearch.identity.Subject;
 
 /**
@@ -20,5 +21,6 @@ public interface IdentityPlugin {
     /**
      * Get the current subject
      * */
+    @Nullable
     public Subject getSubject();
 }

server/src/test/java/org/opensearch/rest/RestControllerTests.java

@@ -52,6 +52,7 @@
 import org.opensearch.http.HttpResponse;
 import org.opensearch.http.HttpServerTransport;
 import org.opensearch.http.HttpStats;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.indices.breaker.HierarchyCircuitBreakerService;
 import org.opensearch.rest.action.admin.indices.RestCreateIndexAction;
 import org.opensearch.test.OpenSearchTestCase;
@@ -286,7 +287,7 @@ public void testRestHandlerWrapper() throws Exception {
         final RestController restController = new RestController(Collections.emptySet(), h -> {
             assertSame(handler, h);
             return (RestRequest request, RestChannel channel, NodeClient client) -> wrapperCalled.set(true);
-        }, client, circuitBreakerService, usageServic, identityService);
+        }, client, circuitBreakerService, usageService, identityService);
         restController.registerHandler(RestRequest.Method.GET, "/wrapped", handler);
         RestRequest request = testRestRequest("/wrapped", "{}", XContentType.JSON);
         AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST);

server/src/test/java/org/opensearch/rest/action/admin/indices/RestValidateQueryActionTests.java

@@ -31,6 +31,7 @@
 
 package org.opensearch.rest.action.admin.indices;
 
+import java.util.List;
 import org.opensearch.action.ActionListener;
 import org.opensearch.action.ActionRequest;
 import org.opensearch.action.ActionType;
@@ -43,6 +44,7 @@
 import org.opensearch.common.io.stream.NamedWriteableRegistry;
 import org.opensearch.common.settings.Settings;
 import org.opensearch.common.xcontent.XContentType;
+import org.opensearch.identity.IdentityService;
 import org.opensearch.indices.breaker.NoneCircuitBreakerService;
 import org.opensearch.rest.RestController;
 import org.opensearch.rest.RestRequest;
@@ -73,7 +75,7 @@ public class RestValidateQueryActionTests extends AbstractSearchTestCase {
     private static NodeClient client = new NodeClient(Settings.EMPTY, threadPool);
 
     private static UsageService usageService = new UsageService();
-    private static IdentityService identityService = new IdentityService(settings, List.of());
+    private static IdentityService identityService = new IdentityService(Settings.EMPTY, List.of());
     private static RestController controller = new RestController(
         emptySet(),
         null,