diff --git a/bundles/org.openhab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java b/bundles/org.openhab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java index 9a106a54d99..8d4b07fd5f9 100644 --- a/bundles/org.openhab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java +++ b/bundles/org.openhab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java @@ -25,6 +25,8 @@ import java.util.Map; import java.util.Objects; import java.util.Random; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import javax.annotation.Priority; import javax.servlet.http.HttpServletRequest; @@ -85,13 +87,13 @@ public class AuthFilter implements ContainerRequestFilter { private final Logger logger = LoggerFactory.getLogger(AuthFilter.class); - private static final String ALT_AUTH_HEADER = "X-OPENHAB-TOKEN"; - private static final String API_TOKEN_PREFIX = "oh."; + static final String ALT_AUTH_HEADER = "X-OPENHAB-TOKEN"; + static final String API_TOKEN_PREFIX = "oh."; protected static final String CONFIG_URI = "system:restauth"; - private static final String CONFIG_ALLOW_BASIC_AUTH = "allowBasicAuth"; - private static final String CONFIG_IMPLICIT_USER_ROLE = "implicitUserRole"; - private static final String CONFIG_TRUSTED_NETWORKS = "trustedNetworks"; - private static final String CONFIG_CACHE_EXPIRATION = "cacheExpiration"; + static final String CONFIG_ALLOW_BASIC_AUTH = "allowBasicAuth"; + static final String CONFIG_IMPLICIT_USER_ROLE = "implicitUserRole"; + static final String CONFIG_TRUSTED_NETWORKS = "trustedNetworks"; + static final String CONFIG_CACHE_EXPIRATION = "cacheExpiration"; private boolean allowBasicAuth = false; private boolean implicitUserRole = true; @@ -143,19 +145,16 @@ protected void activate(Map config) { @Modified protected void modified(@Nullable Map properties) { if (properties != null) { - Object value = properties.get(CONFIG_ALLOW_BASIC_AUTH); - allowBasicAuth = value != null && "true".equals(value.toString()); - value = properties.get(CONFIG_IMPLICIT_USER_ROLE); - implicitUserRole = value == null || !"false".equals(value.toString()); + allowBasicAuth = ConfigParser.valueAsOrElse(properties.get(CONFIG_ALLOW_BASIC_AUTH), Boolean.class, false); + implicitUserRole = ConfigParser.valueAsOrElse(properties.get(CONFIG_IMPLICIT_USER_ROLE), Boolean.class, + true); trustedNetworks = parseTrustedNetworks( ConfigParser.valueAsOrElse(properties.get(CONFIG_TRUSTED_NETWORKS), String.class, "")); - value = properties.get(CONFIG_CACHE_EXPIRATION); - if (value != null) { - try { - cacheExpiration = Long.valueOf(value.toString()); - } catch (NumberFormatException e) { - logger.warn("Ignoring invalid configuration value '{}' for cacheExpiration parameter.", value); - } + try { + cacheExpiration = ConfigParser.valueAsOrElse(properties.get(CONFIG_CACHE_EXPIRATION), Long.class, 6L); + } catch (NumberFormatException e) { + logger.warn("Ignoring invalid configuration value '{}' for cacheExpiration parameter.", + properties.get(CONFIG_CACHE_EXPIRATION)); } authCache.clear(); } @@ -295,7 +294,9 @@ private List parseTrustedNetworks(String value) { var cidrList = new ArrayList(); for (var cidrString : value.split(",")) { try { - cidrList.add(new CIDR(cidrString.trim())); + if (!cidrString.isBlank()) { + cidrList.add(new CIDR(cidrString.trim())); + } } catch (UnknownHostException e) { logger.warn("Error parsing trusted network cidr: {}", cidrString); } @@ -310,13 +311,17 @@ private String getClientIp(ContainerRequestContext requestContext) throws Unknow } private static class CIDR { + private static final Pattern CIDR_PATTERN = Pattern.compile("(?.*?)/(?\\d+)"); private final byte[] networkBytes; private final int prefix; public CIDR(String cidr) throws UnknownHostException { - String[] parts = cidr.split("/"); - this.prefix = Integer.parseInt(parts[1]); - this.networkBytes = InetAddress.getByName(parts[0]).getAddress(); + Matcher m = CIDR_PATTERN.matcher(cidr); + if (!m.matches()) { + throw new UnknownHostException(); + } + this.prefix = Integer.parseInt(m.group("prefixLength")); + this.networkBytes = InetAddress.getByName(m.group("networkAddress")).getAddress(); } public boolean isInRange(byte[] address) { diff --git a/bundles/org.openhab.core.io.rest.auth/src/test/java/org/openhab/core/io/rest/auth/internal/AuthFilterTest.java b/bundles/org.openhab.core.io.rest.auth/src/test/java/org/openhab/core/io/rest/auth/internal/AuthFilterTest.java new file mode 100644 index 00000000000..5e5c3824017 --- /dev/null +++ b/bundles/org.openhab.core.io.rest.auth/src/test/java/org/openhab/core/io/rest/auth/internal/AuthFilterTest.java @@ -0,0 +1,117 @@ +/** + * Copyright (c) 2010-2022 Contributors to the openHAB project + * + * See the NOTICE file(s) distributed with this work for additional + * information. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0 + * + * SPDX-License-Identifier: EPL-2.0 + */ +package org.openhab.core.io.rest.auth.internal; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.util.Map; + +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.container.ContainerRequestContext; + +import org.eclipse.jdt.annotation.NonNullByDefault; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.ExtendWith; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import org.mockito.junit.jupiter.MockitoExtension; +import org.mockito.junit.jupiter.MockitoSettings; +import org.mockito.quality.Strictness; +import org.openhab.core.auth.UserRegistry; + +/** + * The {@link AuthFilterTest} is a + * + * @author Jan N. Klug - Initial contribution + */ +@NonNullByDefault +@ExtendWith(MockitoExtension.class) +@MockitoSettings(strictness = Strictness.LENIENT) +public class AuthFilterTest { + + @InjectMocks + private @NonNullByDefault({}) AuthFilter authFilter; + + private @Mock @NonNullByDefault({}) JwtHelper jwtHelperMock; + private @Mock @NonNullByDefault({}) UserRegistry userRegistryMock; + + private @Mock @NonNullByDefault({}) ContainerRequestContext containerRequestContext; + + private @Mock @NonNullByDefault({}) HttpServletRequest servletRequest; + + @BeforeEach + public void setup() { + MockitoAnnotations.initMocks(this); + when(servletRequest.getRemoteAddr()).thenReturn("192.168.0.100"); + } + + @Test + public void implicitUserRoleAllowsAccess() throws IOException { + authFilter.activate(Map.of()); // implicit user role is true by default + authFilter.filter(containerRequestContext); + + verify(containerRequestContext).setSecurityContext(any()); + } + + @Test + public void noImplicitUserRoleDeniesAccess() throws IOException { + authFilter.activate(Map.of(AuthFilter.CONFIG_IMPLICIT_USER_ROLE, false)); + authFilter.filter(containerRequestContext); + + verify(containerRequestContext, never()).setSecurityContext(any()); + } + + @Test + public void trustedNetworkAllowsAccessIfForwardedHeaderMatches() throws IOException { + authFilter.activate(Map.of(AuthFilter.CONFIG_IMPLICIT_USER_ROLE, false, AuthFilter.CONFIG_TRUSTED_NETWORKS, + "192.168.1.0/24")); + when(containerRequestContext.getHeaderString("x-forwarded-for")).thenReturn("192.168.1.100"); + authFilter.filter(containerRequestContext); + + verify(containerRequestContext).setSecurityContext(any()); + } + + @Test + public void trustedNetworkDeniesAccessIfForwardedHeaderDoesNotMatch() throws IOException { + authFilter.activate(Map.of(AuthFilter.CONFIG_IMPLICIT_USER_ROLE, false, AuthFilter.CONFIG_TRUSTED_NETWORKS, + "192.168.1.0/24")); + when(containerRequestContext.getHeaderString("x-forwarded-for")).thenReturn("192.168.2.100"); + authFilter.filter(containerRequestContext); + + verify(containerRequestContext, never()).setSecurityContext(any()); + } + + @Test + public void trustedNetworkAllowsAccessIfRemoteAddressMatches() throws IOException { + authFilter.activate(Map.of(AuthFilter.CONFIG_IMPLICIT_USER_ROLE, false, AuthFilter.CONFIG_TRUSTED_NETWORKS, + "192.168.0.0/24")); + authFilter.filter(containerRequestContext); + + verify(containerRequestContext).setSecurityContext(any()); + } + + @Test + public void trustedNetworkDeniesAccessIfRemoteAddressDoesNotMatch() throws IOException { + authFilter.activate(Map.of(AuthFilter.CONFIG_IMPLICIT_USER_ROLE, false, AuthFilter.CONFIG_TRUSTED_NETWORKS, + "192.168.1.0/24")); + authFilter.filter(containerRequestContext); + + verify(containerRequestContext, never()).setSecurityContext(any()); + } +}