Merge pull request #6531 from JohannesLks/Remove-requirement-to-set-a-header-value-for-headerBasedSessionManagement

authhelper: allow empty header list

Author: kingthorin

addOns/authhelper/CHANGELOG.md

@@ -14,6 +14,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Changed
 - Send the referer header on verification if set on the original request.
+- Removed requirement to set at least one header in the GUI for Header-Based Session Management.
 
 ### Fixed
 - Do not fail the authentication on diagnostic errors.

addOns/authhelper/src/main/java/org/zaproxy/addon/authhelper/HeaderBasedSessionManagementMethodType.java

@@ -456,17 +456,20 @@ public void importData(Configuration config, SessionManagementMethod sessionMeth
     @Override
     public ApiDynamicActionImplementor getSetMethodForContextApiAction() {
         return new ApiDynamicActionImplementor(
-                API_METHOD_NAME, new String[] {PARAM_HEADERS}, null) {
+                API_METHOD_NAME, new String[0], new String[] {PARAM_HEADERS}) {
 
             @Override
             public void handleAction(JSONObject params) throws ApiException {
                 Context context =
                         ApiUtils.getContextByParamId(params, SessionManagementAPI.PARAM_CONTEXT_ID);
                 HeaderBasedSessionManagementMethod smm =
                         createSessionManagementMethod(context.getId());
-                // Headers are newline separated key: value pairs
-                String[] headerArray = params.getString(PARAM_HEADERS).split("\n");
-                smm.setHeaderConfigs(getHeaderPairs(headerArray));
+                String headersStr = params.optString(PARAM_HEADERS, "");
+                if (!headersStr.isBlank()) {
+                    // Headers are newline separated key: value pairs
+                    String[] headerArray = headersStr.split("\n");
+                    smm.setHeaderConfigs(getHeaderPairs(headerArray));
+                }
                 context.setSessionManagementMethod(smm);
             }
         };
@@ -518,11 +521,6 @@ public void validateFields() throws IllegalStateException {
                                     "authhelper.session.method.header.error.value"));
                 }
             }
-            if (headers.isEmpty()) {
-                throw new IllegalStateException(
-                        Constant.messages.getString(
-                                "authhelper.session.method.header.error.headers"));
-            }
         }
 
         @Override

addOns/authhelper/src/main/resources/org/zaproxy/addon/authhelper/resources/Messages.properties

@@ -150,7 +150,6 @@ authhelper.session-detect.name = Session Management Response Identified
 authhelper.session-detect.soln = This is an informational alert rather than a vulnerability and so there is nothing to fix.
 
 authhelper.session.method.auto.name = Auto-Detect Session Management
-authhelper.session.method.header.error.headers = You must specify at least one header
 authhelper.session.method.header.error.json.parse = Unable to parse authentication response body from {0} as JSON: {1} 
 authhelper.session.method.header.error.value = You must specify both a header and value
 authhelper.session.method.header.label.footer = Any number of headers are supported - a new row is added when any characters are added to the last field.\nThe following tokens can be used in the values:\n* {%json:path.to.data%}\tJSON authentication response data\n* {%env:env_var%}\tenvironmental variable\n* {%script:glob_var%}\tglobal script variable\n* {%header:env_var%}\tauthentication response header\n* {%url:key%}\t\tauthentication URL param

addOns/authhelper/src/test/java/org/zaproxy/addon/authhelper/HeaderBasedSessionManagementMethodTypeUnitTest.java

@@ -22,11 +22,14 @@
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.withSettings;
 
@@ -35,6 +38,7 @@
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import net.sf.json.JSONObject;
 import org.apache.commons.httpclient.Cookie;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
@@ -55,6 +59,7 @@
 import org.zaproxy.zap.model.Context;
 import org.zaproxy.zap.network.HttpRequestBody;
 import org.zaproxy.zap.network.HttpResponseBody;
+import org.zaproxy.zap.session.SessionManagementMethod;
 import org.zaproxy.zap.testutils.TestUtils;
 import org.zaproxy.zap.utils.Pair;
 import org.zaproxy.zap.utils.ZapXmlConfiguration;
@@ -389,4 +394,84 @@ void shouldParseHeaderValuesProperly(String entry, String expectedFirst, String
         assertThat(header.first, is(equalTo(expectedFirst)));
         assertThat(header.second, is(equalTo(expectedSecond)));
     }
+
+    @ParameterizedTest
+    @CsvSource(value = {"'',0", "Header1:Value1,1", "Header:, 1"})
+    void shouldSetHeaderConfigsFromApi(String headers, int expectedSize) throws ApiException {
+        // Given
+        HeaderBasedSessionManagementMethodType type = new HeaderBasedSessionManagementMethodType();
+        Context context = mock(Context.class);
+        JSONObject params = new JSONObject();
+        params.put("contextId", 1);
+        params.put("headers", headers);
+
+        Model model = mock(Model.class, withSettings().strictness(Strictness.LENIENT));
+        Model.setSingletonForTesting(model);
+        Session session = mock(Session.class);
+        given(model.getSession()).willReturn(session);
+        given(session.getContext(1)).willReturn(context);
+
+        // When
+        type.getSetMethodForContextApiAction().handleAction(params);
+
+        // Then
+        ArgumentCaptor<SessionManagementMethod> captor =
+                ArgumentCaptor.forClass(SessionManagementMethod.class);
+        verify(context).setSessionManagementMethod(captor.capture());
+        HeaderBasedSessionManagementMethod savedMethod =
+                (HeaderBasedSessionManagementMethod) captor.getValue();
+        assertThat(savedMethod.getHeaderConfigs(), hasSize(expectedSize));
+    }
+
+    @ParameterizedTest
+    @CsvSource(value = {"'   \\t\\n '", "Header"})
+    void shouldRejectInvalidHeaderConfigsFromApi(String headers) {
+        // Given
+        HeaderBasedSessionManagementMethodType type = new HeaderBasedSessionManagementMethodType();
+        Context context = mock(Context.class);
+        JSONObject params = new JSONObject();
+        params.put("contextId", 1);
+        params.put("headers", headers);
+
+        Model model = mock(Model.class, withSettings().strictness(Strictness.LENIENT));
+        Model.setSingletonForTesting(model);
+        Session session = mock(Session.class);
+        given(model.getSession()).willReturn(session);
+        given(session.getContext(1)).willReturn(context);
+
+        // When / Then
+        ApiException e =
+                assertThrows(
+                        ApiException.class,
+                        () -> type.getSetMethodForContextApiAction().handleAction(params));
+
+        assertThat(e.getType(), is(equalTo(ApiException.Type.ILLEGAL_PARAMETER)));
+        assertThat(e.getMessage(), is(containsString("headers")));
+    }
+
+    @Test
+    void shouldSetHeaderConfigsFromApiWhenParamMissing() throws ApiException {
+        // Given
+        HeaderBasedSessionManagementMethodType type = new HeaderBasedSessionManagementMethodType();
+        Context context = mock(Context.class);
+        JSONObject params = new JSONObject();
+        params.put("contextId", 1);
+
+        Model model = mock(Model.class, withSettings().strictness(Strictness.LENIENT));
+        Model.setSingletonForTesting(model);
+        Session session = mock(Session.class);
+        given(model.getSession()).willReturn(session);
+        given(session.getContext(1)).willReturn(context);
+
+        // When
+        type.getSetMethodForContextApiAction().handleAction(params);
+
+        // Then
+        ArgumentCaptor<SessionManagementMethod> captor =
+                ArgumentCaptor.forClass(SessionManagementMethod.class);
+        verify(context).setSessionManagementMethod(captor.capture());
+        HeaderBasedSessionManagementMethod savedMethod =
+                (HeaderBasedSessionManagementMethod) captor.getValue();
+        assertThat(savedMethod.getHeaderConfigs(), hasSize(0));
+    }
 }