Plugins cleanup (#4490)

remove unused methods and improve test coverage

Author: vladak

plugins/src/main/java/opengrok/auth/plugin/LdapUserPlugin.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2016, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 package opengrok.auth.plugin;
 
@@ -37,6 +37,7 @@
 import opengrok.auth.plugin.ldap.AbstractLdapProvider;
 import opengrok.auth.plugin.ldap.LdapException;
 import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.VisibleForTesting;
 import org.opengrok.indexer.authorization.AuthorizationException;
 import org.opengrok.indexer.configuration.Group;
 import org.opengrok.indexer.configuration.Project;
@@ -78,6 +79,7 @@ public class LdapUserPlugin extends AbstractLdapPlugin {
     private Integer instanceNum;
 
     // for testing
+    @VisibleForTesting
     void load(Map<String, Object> parameters, AbstractLdapProvider provider) {
         super.load(provider);
 

plugins/src/main/java/opengrok/auth/plugin/decoders/FakeOSSOHeaderDecoder.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2017, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
  * Portions Copyright (c) 2020, Chris Fraire <[email protected]>.
  */
 package opengrok.auth.plugin.decoders;
@@ -43,12 +43,12 @@ public class OSSOHeaderDecoder implements IUserDecoder {
 
     private static final Logger LOGGER = Logger.getLogger(OSSOHeaderDecoder.class.getName());
 
-    protected static String OSSO_COOKIE_TIMESTAMP_HEADER = "osso-cookie-timestamp";
-    protected static String OSSO_TIMEOUT_EXCEEDED_HEADER = "osso-idle-timeout-exceeded";
-    protected static String OSSO_SUBSCRIBER_DN_HEADER = "osso-subscriber-dn";
-    protected static String OSSO_SUBSCRIBER_HEADER = "osso-subscriber";
-    protected static String OSSO_USER_DN_HEADER = "osso-user-dn";
-    protected static String OSSO_USER_GUID_HEADER = "osso-user-guid";
+    protected static final String OSSO_COOKIE_TIMESTAMP_HEADER = "osso-cookie-timestamp";
+    protected static final String OSSO_TIMEOUT_EXCEEDED_HEADER = "osso-idle-timeout-exceeded";
+    protected static final String OSSO_SUBSCRIBER_DN_HEADER = "osso-subscriber-dn";
+    protected static final String OSSO_SUBSCRIBER_HEADER = "osso-subscriber";
+    protected static final String OSSO_USER_DN_HEADER = "osso-user-dn";
+    protected static final String OSSO_USER_GUID_HEADER = "osso-user-guid";
 
     @Override
     public User fromRequest(HttpServletRequest request) {

plugins/src/main/java/opengrok/auth/plugin/decoders/OSSOHeaderDecoder.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 package opengrok.auth.plugin.entity;
 
@@ -65,10 +65,6 @@ public String getId() {
         return id;
     }
 
-    public void setId(String id) {
-        this.id = id;
-    }
-
     public String getUsername() {
         return username;
     }
@@ -81,48 +77,14 @@ public Date getCookieTimestamp() {
         return cookieTimestamp;
     }
 
-    public void setCookieTimestamp(Date cookieTimestamp) {
-        this.cookieTimestamp = cookieTimestamp;
-    }
-
     public boolean getTimeouted() {
         return isTimeouted();
     }
 
-    public void setTimeouted(boolean timeouted) {
-        this.timeouted = timeouted;
-    }
-
     public boolean isTimeouted() {
         return timeouted;
     }
 
-    /**
-     * Implemented for the forced authentication as described in
-     * <a href="https://docs.oracle.com/cd/B28196_01/idmanage.1014/b15997/mod_osso.htm#i1006381">mod_osso documentation</a>.
-     *
-     * @param forcedAuthDate the date of the forced authentication trigger
-     * @param newLoginDate the date of the new login
-     * @return true if login date was before forced auth date or cookie timestamp
-     */
-    public boolean isForcedTimeouted(Date forcedAuthDate, Date newLoginDate) {
-        if (cookieTimestamp == null || forcedAuthDate == null || newLoginDate == null) {
-            return true;
-        }
-
-        return newLoginDate.before(forcedAuthDate) || newLoginDate.before(cookieTimestamp);
-    }
-
-    /**
-     * Get custom user property.
-     *
-     * @param key the key
-     * @return the the value associated with the key
-     */
-    public Object getAttribute(String key) {
-        return attrs.get(key);
-    }
-
     /**
      * Set custom user property.
      *
@@ -134,16 +96,6 @@ public Object setAttribute(String key, Object value) {
         return attrs.put(key, value);
     }
 
-    /**
-     * Remote custom user property.
-     *
-     * @param key the key
-     * @return the value previously associated with the key
-     */
-    public Object removeAttribute(String key) {
-        return attrs.remove(key);
-    }
-
     @Override
     public String toString() {
         return "User{" + "id=" + id + ", username=" + username + ", cookieTimestamp=" + cookieTimestamp +

plugins/src/main/java/opengrok/auth/plugin/entity/User.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 package opengrok.auth.plugin;
 
@@ -133,6 +133,24 @@ void testFillSessionWithDnOff() throws LdapException {
         assertEquals(dn, ((LdapUser) request.getSession().getAttribute(SESSION_ATTR)).getDn());
     }
 
+    /**
+     * Test that supplied LDAP filter is expanded for the LDAP query.
+     */
+    @Test
+    void testFilterExpansion() throws Exception {
+        AbstractLdapProvider mockprovider = mock(LdapFacade.class);
+        HttpServletRequest request = new DummyHttpServletRequestLdap();
+        User user = new User("[email protected]", "id");
+        LdapUserPlugin plugin = new LdapUserPlugin();
+        Map<String, Object> params = getParamsMap();
+        params.put(LdapUserPlugin.ATTRIBUTES, "mail");
+        params.put(LdapUserPlugin.LDAP_FILTER, "%guid%");
+        plugin.load(params, mockprovider);
+        plugin.fillSession(request, user);
+        final String expectedFilter = plugin.expandFilter(user);
+        verify(mockprovider).lookupLdapContent(eq(null), eq(expectedFilter), any(String[].class));
+    }
+
     @Test
     void testNegativeCache() throws LdapException {
         AbstractLdapProvider mockprovider = mock(LdapFacade.class);

plugins/src/test/java/opengrok/auth/plugin/LdapUserPluginTest.java

@@ -18,7 +18,7 @@
  */
 
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 package opengrok.auth.plugin.decoders;
 
@@ -61,6 +61,7 @@ void setUp() {
     /**
      * Test of fromRequest method, of class User.
      */
+    @Test
     void testAll() {
         dummyRequest.setHeader(OSSO_COOKIE_TIMESTAMP_HEADER, "5761172f");
         dummyRequest.setHeader(OSSO_TIMEOUT_EXCEEDED_HEADER, "false");
@@ -99,10 +100,10 @@ void testGetUserId() {
     }
 
     /**
-     * Test of getUserDn method, of class User.
+     * Test {@link User#getUsername()}.
      */
     @Test
-    void testGetUserDn() {
+    void testGetUsername() {
         String[] tests = {
                 "123456",
                 "sd45gfgf5sd4g5ffd54g",
@@ -118,7 +119,7 @@ void testGetUserDn() {
     }
 
     /**
-     * Test of getCookieTimestamp method, of class User.
+     * Test {@link User#getCookieTimestamp()}.
      */
     @Test
     void testGetCookieTimestamp() {
@@ -134,7 +135,7 @@ void testGetCookieTimestamp() {
     }
 
     /**
-     * Test of getCookieTimestamp method, of class User.
+     * Negative test {@link User#getCookieTimestamp()}.
      */
     @Test
     void testInvalidGetCookieTimestamp() {

plugins/src/test/java/opengrok/auth/plugin/decoders/OSSODecoderTest.java

@@ -18,19 +18,28 @@
  */
 
 /*
- * Copyright (c) 2019, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2019, 2023, Oracle and/or its affiliates. All rights reserved.
  */
 package opengrok.auth.plugin.decoders;
 
+import jakarta.servlet.http.HttpServletRequest;
 import opengrok.auth.plugin.entity.User;
 import opengrok.auth.plugin.util.DummyHttpServletRequestUser;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.security.Principal;
+import java.util.Arrays;
+import java.util.Collection;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 class UserPrincipalDecoderTest {
     DummyHttpServletRequestUser dummyRequest;
@@ -53,6 +62,19 @@ void testHttpBasicDecoding() {
         assertFalse(result.isTimeouted());
     }
 
+    private static Collection<Principal> invalidPrincipals() {
+         return Arrays.asList(() -> null, () -> "");
+    }
+
+    @ParameterizedTest
+    @MethodSource("invalidPrincipals")
+    void testInvalidUsername(Principal principal) {
+        HttpServletRequest mockRequest = mock(DummyHttpServletRequestUser.class);
+        when(mockRequest.getUserPrincipal()).thenReturn(principal);
+        User result = decoder.fromRequest(mockRequest);
+        assertNull(result);
+    }
+
     @Test
     void testMissingHeader() {
         assertNull(decoder.fromRequest(new DummyHttpServletRequestUser()));