Fix AuthFilter crash if trusted network not configured (#3110)

* Fix AuthFilter crash if trusted network not configured

Signed-off-by: Jan N. Klug <github@klug.nrw>
pull/3113/head
J-N-K 2022-10-13 08:09:23 +02:00 committed by GitHub
parent 3ba0b8cf6d
commit ef3b13fce8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 143 additions and 21 deletions

View File

@ -25,6 +25,8 @@ import java.util.List;
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 @@ import org.slf4j.LoggerFactory;
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 @@ public class AuthFilter implements ContainerRequestFilter {
@Modified
protected void modified(@Nullable Map<String, Object> 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 @@ public class AuthFilter implements ContainerRequestFilter {
var cidrList = new ArrayList<CIDR>();
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 @@ public class AuthFilter implements ContainerRequestFilter {
}
private static class CIDR {
private static final Pattern CIDR_PATTERN = Pattern.compile("(?<networkAddress>.*?)/(?<prefixLength>\\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) {

View File

@ -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());
}
}