Skip to content

Commit

Permalink
SAK-32699 IP whitelist for internal user authentication (sakaiproject…
Browse files Browse the repository at this point in the history
…#4633)

* SAK-32699 IP whitelisting for internal user authentication

* SAK-32699 Change authentication for SakaiLogin so that whitelist is effective

* SAK-32699 Update entitybroker session provider to use authenticationManager
  • Loading branch information
smarquard authored and ottenhoff committed Aug 3, 2017
1 parent 34ef01e commit 16fc606
Show file tree
Hide file tree
Showing 19 changed files with 292 additions and 30 deletions.
2 changes: 1 addition & 1 deletion dav/dav/src/java/org/sakaiproject/dav/DavServlet.java
Original file line number Diff line number Diff line change
Expand Up @@ -1076,7 +1076,7 @@ protected void service(HttpServletRequest req, HttpServletResponse res) throws S
{
String eid = prin.getName();
String pw = ((DavPrincipal) prin).getPassword();
Evidence e = new IdPwEvidence(eid, pw);
Evidence e = new IdPwEvidence(eid, pw, req.getRemoteAddr());

// in older versions of this code, we didn't authenticate
// if there was a session for this user. Unfortunately the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,14 @@
import org.sakaiproject.tool.api.SessionManager;
import org.sakaiproject.authz.api.AuthzGroupService;
import org.sakaiproject.authz.api.SecurityService;
import org.sakaiproject.user.api.Authentication;
import org.sakaiproject.user.api.AuthenticationException;
import org.sakaiproject.user.api.AuthenticationManager;
import org.sakaiproject.user.api.Evidence;
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.sakaiproject.util.IdPwEvidence;

/**
* Entity provider for Sakai Sessions
Expand All @@ -72,6 +77,11 @@ public class SessionEntityProvider extends AbstractEntityProvider implements Cor
private static Logger log = LoggerFactory.getLogger(SessionEntityProvider.class);
protected static final String SU_WS_BECOME_USER = "su.ws.become";

public AuthenticationManager authenticationManager;
public void setAuthenticationManager(AuthenticationManager authenticationManager) {
this.authenticationManager = authenticationManager;
}

public SessionManager sessionManager;
public void setSessionManager(SessionManager sessionManager) {
this.sessionManager = sessionManager;
Expand Down Expand Up @@ -199,18 +209,21 @@ public String createEntity(EntityReference ref, Object entity, Map<String, Objec
+ "the username must be provided as '_username' and the password as '_password' in the POST");
}
// now we auth
User u = userDirectoryService.authenticate(username, password);
if (u == null) {
try {
Evidence evidence = new IdPwEvidence(username, password, req.getRemoteAddr());
Authentication auth = authenticationManager.authenticate(evidence);

// create session or update existing one
currentSession = sessionManager.getCurrentSession();
if (currentSession == null) {
// start a session if none is found
currentSession = sessionManager.startSession();
}
currentSession.setUserId(auth.getUid());
currentSession.setUserEid(auth.getEid());
} catch (AuthenticationException ae) {
throw new SecurityException("The username or password provided were invalid, could not authenticate user ("+username+") to create a session");
}
// create session or update existing one
currentSession = sessionManager.getCurrentSession();
if (currentSession == null) {
// start a session if none is found
currentSession = sessionManager.startSession();
}
currentSession.setUserId(u.getId());
currentSession.setUserEid(u.getEid());
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
<bean parent="org.sakaiproject.entitybroker.entityprovider.AbstractEntityProvider"
class="org.sakaiproject.entitybroker.providers.SessionEntityProvider">
<property name="sessionManager" ref="org.sakaiproject.tool.api.SessionManager" />
<property name="authenticationManager" ref="org.sakaiproject.user.api.AuthenticationManager"/>
<property name="userDirectoryService" ref="org.sakaiproject.user.api.UserDirectoryService" />
<property name="securityService" ref="org.sakaiproject.authz.api.SecurityService" />
<property name="authzGroupService" ref="org.sakaiproject.authz.api.AuthzGroupService"/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@

/**
* <p>
* IdPwEvidence is Authetication evidence made up of a user identifier and a password. Note the "id" used here is something the user offers for authentication purposes, and is *not* the user's Sakai user object UUID.
* IdPwEvidence is Authetication evidence made up of a user identifier, a password and the remote address from where the user's
* request originates. Note the "id" used here is something the user offers for authentication purposes, and is *not* the user's
* Sakai user object UUID.
* </p>
*/
public interface IdPwEvidence extends Evidence
Expand All @@ -41,4 +43,11 @@ public interface IdPwEvidence extends Evidence
* @return The password.
*/
String getPassword();

/**
* Access the remote address
*
* @return The remote address
*/
String getRemoteAddr();
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.sakaiproject.user.api.User;
import org.sakaiproject.user.api.UserDirectoryService;
import org.sakaiproject.user.api.UserNotDefinedException;
import org.sakaiproject.util.IPAddrUtil;

/**
* <p>
Expand Down Expand Up @@ -111,10 +112,18 @@ public Authentication authenticate(Evidence e) throws AuthenticationException
authenticationCache().putAuthenticationFailure(evidence.getIdentifier(), evidence.getPassword());
throw new AuthenticationException("Invalid Login: Either user not found or password incorrect.");
}

// Check to see if the user account is disabled
String disabled = user.getProperties().getProperty("disabled");
if (disabled != null && "true".equals(disabled))
{
throw new AuthenticationException("Account Disabled: The users authentication has been disabled");
throw new AuthenticationException("Account Disabled: The user's authentication has been disabled");
}

// Check optional whitelist for this account
String whitelist = user.getProperties().getProperty("ip-whitelist");
if (whitelist != null && !whitelist.isEmpty() && !IPAddrUtil.matchIPList(whitelist, evidence.getRemoteAddr())) {
throw new AuthenticationException("Authentication refused: The user may only authenticate from whitelisted addresses");
}

rv = new org.sakaiproject.util.Authentication(user.getId(), user.getEid());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
public class AuthenticationCacheTest extends SakaiKernelTestBase {
private static Logger log = LoggerFactory.getLogger(AuthenticationCacheTest.class);
private static String[] USER_DATA_1 = {"localonly1user", null, "First", "Last1", "local1@edu", "local1password"};
private static IdPwEvidence USER_EVIDENCE_1 = new IdPwEvidence(USER_DATA_1[0], USER_DATA_1[5]);
private static IdPwEvidence USER_EVIDENCE_1 = new IdPwEvidence(USER_DATA_1[0], USER_DATA_1[5], null);
private static String[] USER_DATA_2 = {"localonly2user", null, "First", "Last2", "local2@edu", "local2password"};
private AuthenticationManager authenticationManager;
private AuthenticationCache authenticationCache;
Expand Down Expand Up @@ -98,7 +98,7 @@ public void testAuthenticationCache() throws Exception {
Assert.assertTrue(authentication.getEid().equals(USER_DATA_1[0]));

// Test authentication failure throttle.
IdPwEvidence badEvidence = new IdPwEvidence(USER_DATA_1[0], "Not the password");
IdPwEvidence badEvidence = new IdPwEvidence(USER_DATA_1[0], "Not the password", null);
try {
authenticationManager.authenticate(badEvidence);
Assert.fail();
Expand Down
5 changes: 5 additions & 0 deletions kernel/kernel-util/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@
<groupId>commons-io</groupId>
<artifactId>commons-io</artifactId>
</dependency>
<dependency>
<groupId>commons-net</groupId>
<artifactId>commons-net</artifactId>
<version>3.6</version>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ public boolean doLogin(HttpServletRequest req) throws IOException {
String eid = auth.substring(0, colon);
String pw = auth.substring(colon + 1);
if (eid.length() > 0 && pw.length() > 0) {
e = new IdPwEvidence(eid, pw);
e = new IdPwEvidence(eid, pw, req.getRemoteAddr());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
/**********************************************************************************
* $URL$
* $Id$
***********************************************************************************
*
* Copyright (c) 2017 Apereo Foundation
*
* Licensed under the Educational Community License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.opensource.org/licenses/ECL-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
**********************************************************************************/

package org.sakaiproject.util;

import java.util.Arrays;
import java.util.List;

import org.apache.commons.lang.StringUtils;
import org.apache.commons.net.util.SubnetUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* <p>
* IPAddrUtil contains utility methods for working with IP addresses.
* </p>
*/
public class IPAddrUtil
{
private static final Logger log = LoggerFactory.getLogger(IPAddrUtil.class);

/**
* Match an address against a list of IP CIDR addresses
*
* @param addrlist
* The comma-separated list of addresses
* @param addr
* The IP address to match
* @return true if address is contained in one or more of the CIDR network blocks listed in addrlist, false if not
*/
public static boolean matchIPList(String addrlist, String addr)
{
log.info("Checking login IP '" + addr + "' is contained in whitelist '" + addrlist + "'");

// TODO Support IPv6

if (StringUtils.isBlank(addrlist) || StringUtils.isBlank(addr))
return false;

boolean match = false;

for (String netaddr : Arrays.asList(addrlist.split(","))) {
if (netaddr.contains("/")) {
// Contained in subnet?
try {
SubnetUtils.SubnetInfo subnet = new SubnetUtils(netaddr.trim()).getInfo();
if (subnet.isInRange(addr)) {
log.debug("IP Address " + addr + " is in network range " + subnet.getCidrSignature());
match = true;
break;
}
} catch (IllegalArgumentException e) {
log.warn("IP network address '" + netaddr + "' is not a valid CIDR format");
}
} else {
// Exact match?
if (netaddr.trim().equals(addr)) {
match = true;
break;
}
}
}
return match;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ public class IdPwEvidence implements org.sakaiproject.user.api.IdPwEvidence
/** The password string. */
protected String m_password = null;

/** The remote address. */
protected String m_remoteAddr = null;

/**
* Construct, with identifier and password.
*
Expand All @@ -42,10 +45,11 @@ public class IdPwEvidence implements org.sakaiproject.user.api.IdPwEvidence
* @param password
* The password string.
*/
public IdPwEvidence(String identifier, String password)
public IdPwEvidence(String identifier, String password, String remoteAddr)
{
m_identifier = identifier;
m_password = password;
m_remoteAddr = remoteAddr;
}

/**
Expand All @@ -63,4 +67,35 @@ public String getPassword()
{
return m_password;
}

/**
* @inheritDoc
*/
public String getRemoteAddr()
{
return m_remoteAddr;
}

@Override
public boolean equals(Object o) {

if (o == this) return true;

if (!(o instanceof IdPwEvidence)) return false;

IdPwEvidence e = (IdPwEvidence) o;

return e.getIdentifier().equals(m_identifier) &&
e.getPassword().equals(m_password) &&
e.getRemoteAddr().equals(m_remoteAddr);
}

@Override
public int hashCode() {
int result = m_identifier.hashCode();
result = 31 * result + m_password.hashCode();
result = 31 * result + m_remoteAddr.hashCode();
return result;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
/**********************************************************************************
* $URL:$
* $Id:$
***********************************************************************************
*
* Copyright (c) 2007, 2008 Sakai Foundation
*
* Licensed under the Educational Community License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.opensource.org/licenses/ECL-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*
**********************************************************************************/

package org.sakaiproject.util;

import org.junit.Assert;
import org.junit.Ignore;
import org.junit.Test;
import org.sakaiproject.util.IPAddrUtil;


/**
* Testing the IPAddrUtil
*/
public class IPAddrUtilTest {

/**
* Test method for {@link org.sakaiproject.content.util.IPAddrUtil#matchIPList()}.
*
*/
@Test
public void testMatchIPList() {

String privateRanges = "10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16 , 198.51.100.0/24, 127.0.0.0/8";

// null or empty list never matches
Assert.assertFalse(IPAddrUtil.matchIPList("", "1.2.3.4"));
Assert.assertFalse(IPAddrUtil.matchIPList(null, "1.2.3.4"));

// Inside the range
Assert.assertTrue(IPAddrUtil.matchIPList(privateRanges, "10.0.3.1"));
Assert.assertTrue(IPAddrUtil.matchIPList(privateRanges, "172.25.3.250"));
Assert.assertTrue(IPAddrUtil.matchIPList(privateRanges, "192.168.4.10"));
Assert.assertTrue(IPAddrUtil.matchIPList(privateRanges, "127.0.0.1"));

// Outside the range
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "11.0.3.1"));
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "172.32.0.0"));
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "192.169.0.1"));
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "128.3.2.1"));

// Invalid address format
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "301.3.2.1"));
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "10.0.3"));
Assert.assertFalse(IPAddrUtil.matchIPList(privateRanges, "address"));

// Invalid format inside the list
Assert.assertTrue(IPAddrUtil.matchIPList("10.0.0.0/8,address,127.0.0.0/8", "10.0.0.1"));
Assert.assertFalse(IPAddrUtil.matchIPList("10.0.0.0:8,address,127.0.0.0/8", "10.0.0.1"));

// Single address
Assert.assertTrue(IPAddrUtil.matchIPList("10.0.0.33,address,127.0.0.0/8", "10.0.0.33"));
Assert.assertFalse(IPAddrUtil.matchIPList("10.0.0.33,address,127.0.0.0/8", "10.0.0.32"));

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void authenticate(LoginCredentials credentials) throws LoginException {
// Do NOT trim the password, since many authentication systems allow whitespace.
eid = eid.trim();

Evidence e = new IdPwEvidence(eid, pw);
Evidence e = new IdPwEvidence(eid, pw, credentials.getRemoteAddr());

Authentication a = authenticationManager().authenticate(e);

Expand Down
Loading

0 comments on commit 16fc606

Please sign in to comment.