Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix AuthFilter crash if trusted network not configured #3110

Merged
merged 2 commits into from
Oct 13, 2022
Merged

Fix AuthFilter crash if trusted network not configured #3110

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a6bc3a0
Fix AuthFilter crash if trusted network not configured
J-N-K Oct 11, 2022
aa88769
Update bundles/org.openhab.core.io.rest.auth/src/main/java/org/openha…
J-N-K Oct 12, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 26 additions & 21 deletions ...ab.core.io.rest.auth/src/main/java/org/openhab/core/io/rest/auth/internal/AuthFilter.java
View file
Open in desktop
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 (!value.isBlank()) {
J-N-K marked this conversation as resolved.
Show resolved Hide resolved
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
117 changes: 117 additions & 0 deletions ...ore.io.rest.auth/src/test/java/org/openhab/core/io/rest/auth/internal/AuthFilterTest.java
View file
Open in desktop
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());
}
}