Skip to content

Commit

Permalink
Fix AuthFilter crash if trusted network not configured (openhab#3110)
Browse files Browse the repository at this point in the history
* Fix AuthFilter crash if trusted network not configured

Signed-off-by: Jan N. Klug <github@klug.nrw>
GitOrigin-RevId: ef3b13f
  • Loading branch information
J-N-K authored and splatch committed Jul 12, 2023
1 parent 4a6ec5b commit ffb8b9f
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -143,19 +145,16 @@ protected void activate(Map<String, Object> config) {
@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();
}
Expand Down Expand Up @@ -295,7 +294,9 @@ private List<CIDR> parseTrustedNetworks(String value) {
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);
}
Expand All @@ -310,13 +311,17 @@ private String getClientIp(ContainerRequestContext requestContext) throws Unknow
}

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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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());
}
}

0 comments on commit ffb8b9f

Please sign in to comment.