From 302db362d5ad1034eaf20593a0786c4bb476b5cf Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Mon, 11 Jul 2011 12:32:54 +0200
Subject: [PATCH 1/7] Added configuration option for accepting external
auth(applicationContext.xml), default is OFF
---
.../se/su/dsv/scipro/ApplicationSettings.java | 9 ++++++
.../se/su/dsv/scipro/SciProApplication.java | 8 ++---
.../ExternalAuthenticationRequestHelper.java | 30 +++++++++++++------
src/main/resources/applicationContext.xml | 2 ++
4 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/src/main/java/se/su/dsv/scipro/ApplicationSettings.java b/src/main/java/se/su/dsv/scipro/ApplicationSettings.java
index c018687bee..39f25b473d 100644
--- a/src/main/java/se/su/dsv/scipro/ApplicationSettings.java
+++ b/src/main/java/se/su/dsv/scipro/ApplicationSettings.java
@@ -11,6 +11,7 @@ public class ApplicationSettings {
private boolean enableRemoteUserLookup;
private String remoteLookupUrl;
+ private boolean acceptExternalAuthentication;
public boolean isEnableRemoteUserLookup() {
return enableRemoteUserLookup;
@@ -28,4 +29,12 @@ public class ApplicationSettings {
return remoteLookupUrl;
}
+ public boolean isAcceptExternalAuthentication(){
+ return acceptExternalAuthentication;
+ }
+
+ public void setAcceptExternalAuthentication(boolean pAcceptExternalAuthentication){
+ acceptExternalAuthentication = pAcceptExternalAuthentication;
+ }
+
}
diff --git a/src/main/java/se/su/dsv/scipro/SciProApplication.java b/src/main/java/se/su/dsv/scipro/SciProApplication.java
index d1fd6babce..ae8d3df0f9 100644
--- a/src/main/java/se/su/dsv/scipro/SciProApplication.java
+++ b/src/main/java/se/su/dsv/scipro/SciProApplication.java
@@ -310,7 +310,7 @@ public class SciProApplication extends RepositoryApplication implements IThemabl
public WebRequest newWebRequest(final HttpServletRequest request){
final WebRequest webRequest = super.newWebRequest(request);
if(attemptExternalAuthentication(webRequest)){
- logger.debug("External authentication used");
+ logger.debug("External authentication used successfully");
}
return webRequest;
}
@@ -331,19 +331,19 @@ public class SciProApplication extends RepositoryApplication implements IThemabl
if(session != null){
if(session.isLoggedIn()){
if(!helper.isRemoteUserValid(session.getUser())){//This check may not be needed and may hinder performance, but better safe than sorry for now.
- logger.warn("User is logged in, but conflicting info is supplied via external authentication protocols.");
+ logger.debug("User is logged in as '"+session.getUser().getEmailAddress()+"', but conflicting info ('"+helper.getExternalAuthRemoteUser()+"') is supplied via external authentication protocols.");
}
}else{
//logger.info("Attempting sign in with external auth data");
if(!helper.signIn(session)){
- logger.error("User passes external authentication but cannot be signed in.");
+ logger.error("User '"+helper.getExternalAuthRemoteUser()+"' passes external authentication but cannot be signed in.");
}else{
logger.debug("Signed in user '"+helper.getExternalAuthRemoteUser()+"' via external authentication");
return true;
}
}
}else{
- throw new IllegalStateException("External authentication was attempted, but no session was available.");
+ throw new IllegalStateException("External authentication was attempted, but no session was available for sign in.");
}
}
return false;
diff --git a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
index b092537ae1..a4fc208926 100644
--- a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
+++ b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
@@ -1,6 +1,5 @@
package se.su.dsv.scipro.security.auth;
-import java.security.Policy.Parameters;
import java.util.Enumeration;
import java.util.Map;
import java.util.Set;
@@ -8,7 +7,10 @@ import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import org.apache.log4j.Logger;
+import org.apache.wicket.injection.web.InjectorHolder;
+import org.apache.wicket.spring.injection.annot.SpringBean;
+import se.su.dsv.scipro.ApplicationSettings;
import se.su.dsv.scipro.SciProSession;
import se.su.dsv.scipro.data.dataobjects.User;
import se.su.dsv.scipro.data.dataobjects.Username;
@@ -30,18 +32,24 @@ public final class ExternalAuthenticationRequestHelper{
//Wrapped request
private final HttpServletRequest req;
//remote user attribute
- private String remoteUser;
+ private String remoteUser=null;
//if remote user is on the username@realm form, this attribute holds the username
- private String remoteUserId;
+ private String remoteUserId=null;
//if remote user is on the username@realm form, this attribute holds the realm
- private String remoteUserRealm;
+ private String remoteUserRealm=null;
//logger instance
private Logger logger = Logger.getLogger(this.getClass());
+ @SpringBean
+ private ApplicationSettings appSettings;
/**
* Construct a utility wrapper from a servlet request.
+ * Throws IllegalStateException if the request is null.
* @param request
*/
- public ExternalAuthenticationRequestHelper(final HttpServletRequest request){
+ public ExternalAuthenticationRequestHelper(final HttpServletRequest request) throws IllegalStateException{
+ if(request==null)
+ throw new IllegalStateException("Request is null, this is considered illegal.");
+ InjectorHolder.getInjector().inject(this);
req = request;
formatUserString();
}
@@ -54,7 +62,7 @@ public final class ExternalAuthenticationRequestHelper{
}
/**
* Exposed query method.
- * @return If remote user is on the username@realm form, this attribute holds the userid, else getExternalAuthRemoteUser().
+ * @return If remote user is on the username@realm form, this attribute holds the username, else getExternalAuthRemoteUser().
*/
public String getExternalAuthRemoteUserId(){
return remoteUserId;
@@ -83,7 +91,11 @@ public final class ExternalAuthenticationRequestHelper{
* @return true if the application is configured to accept external authentication and the needed information is available on the request, else false.
*/
public boolean isExternalAuthSupported(){
- return (true && isExternalAuthInfoOnRequest());
+ if(appSettings.isAcceptExternalAuthentication() && !isExternalAuthInfoOnRequest())
+ logger.error("External authentication support is ON, but REMOTE_USER is not populated");
+ if(!appSettings.isAcceptExternalAuthentication() && isExternalAuthInfoOnRequest())
+ logger.error("External authentication support is OFF, but REMOTE_USER is populated");
+ return (appSettings.isAcceptExternalAuthentication() && isExternalAuthInfoOnRequest());
}
/**
* Private utility method for dumping headers.
@@ -165,8 +177,8 @@ public final class ExternalAuthenticationRequestHelper{
}
}
/**
- * Signs the stored remote user in on the given SciProSession.
- * @param session
+ * Signs the stored remote user in on the supplied SciProSession.
+ * @param session If null, method fails gracefully by returning false.
* @return true on success, else false.
*/
public boolean signIn(final SciProSession session){
diff --git a/src/main/resources/applicationContext.xml b/src/main/resources/applicationContext.xml
index ca4ef4ae28..a49ad5e785 100644
--- a/src/main/resources/applicationContext.xml
+++ b/src/main/resources/applicationContext.xml
@@ -83,6 +83,8 @@
<property name="enableRemoteUserLookup" value="true"></property>
<!-- This property points to the location of the daisy json search -->
<property name="remoteLookupUrl" value="https://thesis.dsv.su.se/projectplan/json" />
+ <!-- External auth support (via J2EE standard mechanism REMOTE_USER) -->
+ <property name="acceptExternalAuthentication" value="false"/>
</bean>
<!-- Defines the class used for lookup in username against a remote server NOW AUTOWIRED AND DEPRECATED, NOT MAINTAINED-->
From a0e5201b4debb3e65b465d20e7c1f0876f528ac4 Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Mon, 11 Jul 2011 17:39:21 +0200
Subject: [PATCH 2/7] Added initial unit tests for authentication routines,
very experimental so far.
Also, external auth now defaults to true which will generate warnings in non-REMOTE_USER enabled environments
---
.../java/se/su/dsv/scipro/SciProSession.java | 2 +-
.../ExternalAuthenticationRequestHelper.java | 8 +-
src/main/resources/applicationContext.xml | 2 +-
.../security/auth/TestAuthRoutines.java | 166 ++++++++++++++++++
4 files changed, 172 insertions(+), 6 deletions(-)
create mode 100644 src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
diff --git a/src/main/java/se/su/dsv/scipro/SciProSession.java b/src/main/java/se/su/dsv/scipro/SciProSession.java
index 2bc15ed2b1..82d21f1e39 100644
--- a/src/main/java/se/su/dsv/scipro/SciProSession.java
+++ b/src/main/java/se/su/dsv/scipro/SciProSession.java
@@ -207,7 +207,7 @@ public class SciProSession extends WebSession {
* @return true if the switch was successful, else false.
*/
public boolean switchAuthenticatedUser(final String suUser, final String suRealm){
- logger.info("Currently logged in user: '"+user.getEmailAddress()+"' attempting switch to '"+suUser+"'");
+ logger.info("Currently logged in user: '"+user.getEmailAddress()+"' attempting switch to '"+suUser+"@"+suRealm+"'");
if(suUser != null && roleDao.isSysadmin(user)){
iRoles.clear();
return signInAuthenticatedUser(suUser, suRealm);
diff --git a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
index a4fc208926..983d72614c 100644
--- a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
+++ b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
@@ -78,7 +78,7 @@ public final class ExternalAuthenticationRequestHelper{
* Internal query method
*/
private boolean isExternalAuthInfoOnRequest(){
- return (req.getRemoteUser()!=null);
+ return (remoteUser!=null);
}
/**
* Internal query method
@@ -92,9 +92,9 @@ public final class ExternalAuthenticationRequestHelper{
*/
public boolean isExternalAuthSupported(){
if(appSettings.isAcceptExternalAuthentication() && !isExternalAuthInfoOnRequest())
- logger.error("External authentication support is ON, but REMOTE_USER is not populated");
+ logger.warn("External authentication support is ON, but REMOTE_USER is not populated");
if(!appSettings.isAcceptExternalAuthentication() && isExternalAuthInfoOnRequest())
- logger.error("External authentication support is OFF, but REMOTE_USER is populated");
+ logger.warn("External authentication support is OFF, but REMOTE_USER is populated");
return (appSettings.isAcceptExternalAuthentication() && isExternalAuthInfoOnRequest());
}
/**
@@ -182,7 +182,7 @@ public final class ExternalAuthenticationRequestHelper{
* @return true on success, else false.
*/
public boolean signIn(final SciProSession session){
- if(session != null){
+ if(session != null && isExternalAuthSupported()){
//dumpAuthInfo();
return session.signInAuthenticatedUser(getExternalAuthRemoteUserId(), getExternalAuthRemoteUserRealm());
}
diff --git a/src/main/resources/applicationContext.xml b/src/main/resources/applicationContext.xml
index a49ad5e785..3c6fef0b8c 100644
--- a/src/main/resources/applicationContext.xml
+++ b/src/main/resources/applicationContext.xml
@@ -84,7 +84,7 @@
<!-- This property points to the location of the daisy json search -->
<property name="remoteLookupUrl" value="https://thesis.dsv.su.se/projectplan/json" />
<!-- External auth support (via J2EE standard mechanism REMOTE_USER) -->
- <property name="acceptExternalAuthentication" value="false"/>
+ <property name="acceptExternalAuthentication" value="true"/>
</bean>
<!-- Defines the class used for lookup in username against a remote server NOW AUTOWIRED AND DEPRECATED, NOT MAINTAINED-->
diff --git a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
new file mode 100644
index 0000000000..e21ede311e
--- /dev/null
+++ b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
@@ -0,0 +1,166 @@
+package se.su.dsv.scipro.security.auth;
+
+import static org.junit.Assert.*;
+
+import java.util.Date;
+import java.util.HashSet;
+import java.util.Set;
+
+import javax.servlet.http.HttpServletRequestWrapper;
+
+import org.apache.wicket.Request;
+import org.apache.wicket.protocol.http.HttpSessionStore;
+import org.apache.wicket.session.ISessionStore;
+import org.apache.wicket.spring.injection.annot.SpringBean;
+import org.apache.wicket.spring.injection.annot.SpringComponentInjector;
+import org.apache.wicket.spring.test.ApplicationContextMock;
+import org.apache.wicket.util.tester.WicketTester;
+import org.junit.Assert;
+import org.junit.Before;
+import org.junit.Test;
+import org.mockito.Mockito;
+import org.mockito.invocation.InvocationOnMock;
+import org.mockito.stubbing.Answer;
+import org.springframework.orm.jpa.LocalEntityManagerFactoryBean;
+
+import se.su.dsv.scipro.ApplicationSettings;
+import se.su.dsv.scipro.SciProApplication;
+import se.su.dsv.scipro.SciProSession;
+import se.su.dsv.scipro.data.dao.interfaces.ProjectDao;
+import se.su.dsv.scipro.data.dao.interfaces.RoleDao;
+import se.su.dsv.scipro.data.dao.interfaces.StringResourceDao;
+import se.su.dsv.scipro.data.dao.interfaces.UserDao;
+import se.su.dsv.scipro.data.dao.interfaces.UserSettingsDao;
+import se.su.dsv.scipro.data.dataobjects.Role;
+import se.su.dsv.scipro.data.dataobjects.Student;
+import se.su.dsv.scipro.data.dataobjects.SysAdmin;
+import se.su.dsv.scipro.data.dataobjects.User;
+import se.su.dsv.scipro.data.dataobjects.Username;
+import se.su.dsv.scipro.json.IUserLookup;
+import se.su.dsv.scipro.repository.util.RepositoryManager;
+
+public class TestAuthRoutines {
+ SciProSession session;
+ WicketTester wt;
+ User user;
+ Role sysAdm;
+ IUserLookup fixedLookup;
+ @Before
+ public void init(){
+ final ApplicationContextMock ac = new ApplicationContextMock();
+ //Create mock object in need of config
+ ApplicationSettings appSettings = new ApplicationSettings();
+ appSettings.setAcceptExternalAuthentication(true);
+ appSettings.setEnableRemoteUserLookup(true);
+ appSettings.setRemoteLookupUrl("https://thesis.dsv.su.se/projectplan/json");
+ //Fake a lookup mechanism
+ fixedLookup = new IUserLookup(){
+ @Override
+ public User lookup(String username) throws Exception{
+ return user;
+ }
+ };
+ //Create mock user and associated data
+ user = new User();
+ user.setDateCreated(new Date());
+ user.setEmailAddress("kalle-kula@dsv.su.se");
+ user.setFirstName("Kalle");
+ user.setLastName("Kula");
+ user.setIdentifier(new Long(666));
+ user.setLastModified(new Date());
+ Set<Role> roles = new HashSet<Role>();
+ //Faked student
+ Role role = new Student();
+ role.setId(new Long(555));
+ role.setUser(user);
+ roles.add(new Student());
+ user.setRoles(roles);
+ //Faked sysadm, added later
+ sysAdm = new SysAdmin();
+ sysAdm.setId(new Long(444));
+ Set<Username> usernames = new HashSet<Username>();
+ Username username = new Username();
+ username.setUserName("kalle-kula");
+ username.setRealm("DSV.SU.SE");
+ usernames.add(username);
+ user.setUserNames(usernames);
+ //Put stuff on bean context
+ ac.putBean("entityManagerFactory",Mockito.mock(LocalEntityManagerFactoryBean.class));
+ ac.putBean("repositoryManager",Mockito.mock(RepositoryManager.class));
+ ac.putBean("applicationSettings",appSettings);
+ ac.putBean("userDao",Mockito.mock(UserDao.class));
+ ac.putBean("stringResourceDao",Mockito.mock(StringResourceDao.class));
+ RoleDao mockedRoleDao = Mockito.mock(RoleDao.class);
+ Mockito.when(mockedRoleDao.isSysadmin(user)).thenAnswer(new Answer<Boolean>() {
+ @Override
+ public Boolean answer(InvocationOnMock invocation) throws Throwable {
+ return user.getRoles().contains(sysAdm);
+ }
+ });
+ ac.putBean("roleDao",mockedRoleDao);
+ ac.putBean("projectDao",Mockito.mock(ProjectDao.class));
+ ac.putBean("userSettingsDao",Mockito.mock(UserSettingsDao.class));
+ ac.putBean("userFullLookup",fixedLookup);
+ //Create tester
+ wt = new WicketTester(new SciProApplication(){
+ @Override
+ protected ISessionStore newSessionStore(){
+ return new HttpSessionStore(this);
+ }
+ @Override
+ protected SpringComponentInjector getSpringInjector() {
+ return new SpringComponentInjector(this, ac, true);
+ }
+ });
+ wt.setupRequestAndResponse();
+ session = (SciProSession)wt.getWicketSession();
+ }
+ @Test(expected=IllegalStateException.class)
+ public void testAuthenticationHelper() throws IllegalStateException {
+ //Try with faulty request
+ ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(wt.getServletRequest());
+ Assert.assertFalse(helper.isExternalAuthSupported());
+ Assert.assertTrue(helper.getExternalAuthRemoteUser()==null);
+ Assert.assertTrue(helper.getExternalAuthRemoteUserId()==null);
+ Assert.assertTrue(helper.getExternalAuthRemoteUserRealm().equals("DSV.SU.SE"));
+ Assert.assertFalse(helper.isRemoteUserValid(user));
+ Assert.assertFalse(helper.signIn(null));
+ //Try with conforming request
+ helper = new ExternalAuthenticationRequestHelper(new HttpServletRequestWrapper(wt.getServletRequest()){
+ @Override
+ public String getRemoteUser(){
+ return "kalle-kula@dsv.su.se";
+ }
+ });
+ Assert.assertTrue(helper.isExternalAuthSupported());
+ Assert.assertTrue(helper.getExternalAuthRemoteUser().equals("kalle-kula@dsv.su.se"));
+ Assert.assertTrue(helper.getExternalAuthRemoteUserId().equals("kalle-kula"));
+ Assert.assertTrue(helper.getExternalAuthRemoteUserRealm().equals("DSV.SU.SE"));
+ Assert.assertTrue(helper.isRemoteUserValid(user));
+ //At this point, an exception should be thrown
+ new ExternalAuthenticationRequestHelper(null);
+ }
+ @Test
+ public void testSignInAndSu(){
+ ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(new HttpServletRequestWrapper(wt.getServletRequest()){
+ @Override
+ public String getRemoteUser(){
+ return "kalle-kula@dsv.su.se";
+ }
+ });
+ Assert.assertTrue(helper.isExternalAuthSupported());
+ Assert.assertTrue(helper.signIn(session));
+ Assert.assertTrue(helper.isRemoteUserValid(user));
+ Assert.assertTrue(session.isLoggedIn());
+ Assert.assertTrue(session.getUser().getIdentifier().equals(user.getIdentifier()));
+ //User not authorized to switch, this should fail
+ Assert.assertFalse(session.switchAuthenticatedUser("kalle-kula", "dsv.su.se"));
+ //Change his roles and try again
+ Set<Role> roles = user.getRoles();
+ sysAdm.setUser(user);
+ roles.add(sysAdm);
+ user.setRoles(roles);
+ Assert.assertTrue(session.switchAuthenticatedUser("kalle-kula", "dsv.su.se"));
+ Assert.assertTrue(session.getUser().getIdentifier().equals(user.getIdentifier()));
+ }
+}
From f8421f77f7c274ce016008174595f409623d9fbb Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Mon, 11 Jul 2011 17:41:22 +0200
Subject: [PATCH 3/7] Removed unused imports
---
.../java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java | 4 ----
1 file changed, 4 deletions(-)
diff --git a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
index e21ede311e..07c5aef6c3 100644
--- a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
+++ b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
@@ -1,17 +1,13 @@
package se.su.dsv.scipro.security.auth;
-import static org.junit.Assert.*;
-
import java.util.Date;
import java.util.HashSet;
import java.util.Set;
import javax.servlet.http.HttpServletRequestWrapper;
-import org.apache.wicket.Request;
import org.apache.wicket.protocol.http.HttpSessionStore;
import org.apache.wicket.session.ISessionStore;
-import org.apache.wicket.spring.injection.annot.SpringBean;
import org.apache.wicket.spring.injection.annot.SpringComponentInjector;
import org.apache.wicket.spring.test.ApplicationContextMock;
import org.apache.wicket.util.tester.WicketTester;
From 58ef8a3ff755ac4adb64472ae72d9c6334e17a84 Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Tue, 12 Jul 2011 14:55:54 +0200
Subject: [PATCH 4/7] Added some more testing procedures, more is likely to be
added after review of the sign-in process
---
.../se/su/dsv/scipro/SciProApplication.java | 3 +-
.../java/se/su/dsv/scipro/SciProSession.java | 2 ++
src/main/resources/applicationContext.xml | 2 +-
.../security/auth/TestAuthRoutines.java | 33 ++++++++++++++++---
4 files changed, 33 insertions(+), 7 deletions(-)
diff --git a/src/main/java/se/su/dsv/scipro/SciProApplication.java b/src/main/java/se/su/dsv/scipro/SciProApplication.java
index ae8d3df0f9..0c652139c3 100644
--- a/src/main/java/se/su/dsv/scipro/SciProApplication.java
+++ b/src/main/java/se/su/dsv/scipro/SciProApplication.java
@@ -104,9 +104,8 @@ public class SciProApplication extends RepositoryApplication implements IThemabl
/**
* Logger instance.
- * @TODO Inject
*/
- private Logger logger = Logger.getLogger(this.getClass());
+ private Logger logger = Logger.getLogger(SciProApplication.class);
/**
* Constructor
*/
diff --git a/src/main/java/se/su/dsv/scipro/SciProSession.java b/src/main/java/se/su/dsv/scipro/SciProSession.java
index 82d21f1e39..3d9f1245e4 100644
--- a/src/main/java/se/su/dsv/scipro/SciProSession.java
+++ b/src/main/java/se/su/dsv/scipro/SciProSession.java
@@ -207,6 +207,8 @@ public class SciProSession extends WebSession {
* @return true if the switch was successful, else false.
*/
public boolean switchAuthenticatedUser(final String suUser, final String suRealm){
+ if(!isLoggedIn() || user == null)//Terminate early
+ return false;
logger.info("Currently logged in user: '"+user.getEmailAddress()+"' attempting switch to '"+suUser+"@"+suRealm+"'");
if(suUser != null && roleDao.isSysadmin(user)){
iRoles.clear();
diff --git a/src/main/resources/applicationContext.xml b/src/main/resources/applicationContext.xml
index 3c6fef0b8c..016c637877 100644
--- a/src/main/resources/applicationContext.xml
+++ b/src/main/resources/applicationContext.xml
@@ -83,7 +83,7 @@
<property name="enableRemoteUserLookup" value="true"></property>
<!-- This property points to the location of the daisy json search -->
<property name="remoteLookupUrl" value="https://thesis.dsv.su.se/projectplan/json" />
- <!-- External auth support (via J2EE standard mechanism REMOTE_USER) -->
+ <!-- External auth support (via J2EE standard mechanism REMOTE_USER), if true: other authentication mechanics will be bypassed.-->
<property name="acceptExternalAuthentication" value="true"/>
</bean>
diff --git a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
index 07c5aef6c3..f9b19506e8 100644
--- a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
+++ b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
@@ -53,7 +53,10 @@ public class TestAuthRoutines {
fixedLookup = new IUserLookup(){
@Override
public User lookup(String username) throws Exception{
- return user;
+ if(username.equals("kalle-kula"))
+ return user;
+ else
+ return null;
}
};
//Create mock user and associated data
@@ -111,7 +114,7 @@ public class TestAuthRoutines {
wt.setupRequestAndResponse();
session = (SciProSession)wt.getWicketSession();
}
- @Test(expected=IllegalStateException.class)
+ @Test
public void testAuthenticationHelper() throws IllegalStateException {
//Try with faulty request
ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(wt.getServletRequest());
@@ -133,11 +136,14 @@ public class TestAuthRoutines {
Assert.assertTrue(helper.getExternalAuthRemoteUserId().equals("kalle-kula"));
Assert.assertTrue(helper.getExternalAuthRemoteUserRealm().equals("DSV.SU.SE"));
Assert.assertTrue(helper.isRemoteUserValid(user));
+ }
+ @Test(expected=IllegalStateException.class)
+ public void testNullRequest(){
//At this point, an exception should be thrown
new ExternalAuthenticationRequestHelper(null);
}
- @Test
- public void testSignInAndSu(){
+ @Test(expected=NullPointerException.class)
+ public void testSessionSignInAndSu(){
ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(new HttpServletRequestWrapper(wt.getServletRequest()){
@Override
public String getRemoteUser(){
@@ -158,5 +164,24 @@ public class TestAuthRoutines {
user.setRoles(roles);
Assert.assertTrue(session.switchAuthenticatedUser("kalle-kula", "dsv.su.se"));
Assert.assertTrue(session.getUser().getIdentifier().equals(user.getIdentifier()));
+ //This should fail with an exception, there is no such user
+ session.switchAuthenticatedUser("somebody","somewhere.se");
+ }
+ @Test(expected=NullPointerException.class)
+ public void testFailedAuthenticatedSignIn(){
+ ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(new HttpServletRequestWrapper(wt.getServletRequest()){
+ @Override
+ public String getRemoteUser(){
+ return "some-dude@ki.se";
+ }
+ });
+ //This should throw exceptions, not sure about this interface (throwing exceptions when authentication passes but no user can be located).
+ helper.signIn(session);
+ }
+ @Test
+ public void testFailedSwitchAuthentitedUser(){
+ Assert.assertFalse(session.isLoggedIn());
+ Assert.assertFalse(session.switchAuthenticatedUser("some-dude-who-is-not-real","someplace.se"));
+ Assert.assertFalse(session.switchAuthenticatedUser("some-dude","someplace.se"));
}
}
From b28b37cda2eb9361c67c6e366b9dde29a4a65151 Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Tue, 12 Jul 2011 14:56:57 +0200
Subject: [PATCH 5/7] Another test to ensure that external auth is configurable
---
.../security/auth/TestAuthRoutines.java | 21 ++++++++++++-------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
index f9b19506e8..d1acc68e08 100644
--- a/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
+++ b/src/test/java/se/su/dsv/scipro/security/auth/TestAuthRoutines.java
@@ -36,19 +36,18 @@ import se.su.dsv.scipro.json.IUserLookup;
import se.su.dsv.scipro.repository.util.RepositoryManager;
public class TestAuthRoutines {
- SciProSession session;
- WicketTester wt;
- User user;
- Role sysAdm;
- IUserLookup fixedLookup;
+ private SciProSession session;
+ private WicketTester wt;
+ private User user;
+ private Role sysAdm;
+ private IUserLookup fixedLookup;
+ private ApplicationSettings appSettings;
@Before
public void init(){
final ApplicationContextMock ac = new ApplicationContextMock();
//Create mock object in need of config
- ApplicationSettings appSettings = new ApplicationSettings();
+ appSettings = new ApplicationSettings();
appSettings.setAcceptExternalAuthentication(true);
- appSettings.setEnableRemoteUserLookup(true);
- appSettings.setRemoteLookupUrl("https://thesis.dsv.su.se/projectplan/json");
//Fake a lookup mechanism
fixedLookup = new IUserLookup(){
@Override
@@ -137,6 +136,12 @@ public class TestAuthRoutines {
Assert.assertTrue(helper.getExternalAuthRemoteUserRealm().equals("DSV.SU.SE"));
Assert.assertTrue(helper.isRemoteUserValid(user));
}
+ @Test
+ public void testTurnOffViaSettings(){
+ appSettings.setAcceptExternalAuthentication(false);
+ ExternalAuthenticationRequestHelper helper = new ExternalAuthenticationRequestHelper(wt.getServletRequest());
+ Assert.assertFalse(helper.isExternalAuthSupported());
+ }
@Test(expected=IllegalStateException.class)
public void testNullRequest(){
//At this point, an exception should be thrown
From ef88a11790d9679f353c16da26509aa5c58afd39 Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Tue, 12 Jul 2011 19:23:43 +0200
Subject: [PATCH 6/7] Added display page for server/request parameters, this
will be used to monitor what data is sent via the AJP-proxy.
Refactoring/documentation is next.
---
.../se/su/dsv/scipro/SciProApplication.java | 2 +
.../pages/AbstractAdminSettingsPage.java | 2 +
.../AdminServerEnvironmentSettingsPage.html | 17 ++++
.../AdminServerEnvironmentSettingsPage.java | 87 +++++++++++++++++++
.../ExternalAuthenticationRequestHelper.java | 2 +-
5 files changed, 109 insertions(+), 1 deletion(-)
create mode 100644 src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.html
create mode 100644 src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
diff --git a/src/main/java/se/su/dsv/scipro/SciProApplication.java b/src/main/java/se/su/dsv/scipro/SciProApplication.java
index 0c652139c3..2b3c943ee1 100644
--- a/src/main/java/se/su/dsv/scipro/SciProApplication.java
+++ b/src/main/java/se/su/dsv/scipro/SciProApplication.java
@@ -28,6 +28,7 @@ import se.su.dsv.scipro.admin.pages.settings.AdminFinalSeminarSettingsPage;
import se.su.dsv.scipro.admin.pages.settings.AdminFinalSeminarSettingsPerProjectClassPage;
import se.su.dsv.scipro.admin.pages.settings.AdminGeneralSettingsPage;
import se.su.dsv.scipro.admin.pages.settings.AdminPeerSettingsPage;
+import se.su.dsv.scipro.admin.pages.settings.AdminServerEnvironmentSettingsPage;
import se.su.dsv.scipro.basepages.DemoPage;
import se.su.dsv.scipro.basepages.errorpages.AccessDeniedPage;
import se.su.dsv.scipro.basepages.errorpages.NotFoundPage;
@@ -190,6 +191,7 @@ public class SciProApplication extends RepositoryApplication implements IThemabl
mountBookmarkablePage("admin/files", SysAdminFilePage.class);
mountBookmarkablePage("admin/allfinalseminars", AdminFinalSeminarPage.class);
mountBookmarkablePage("admin/settings", AdminGeneralSettingsPage.class);
+ mountBookmarkablePage("admin/settings/serverenvironment", AdminServerEnvironmentSettingsPage.class);
mountBookmarkablePage("admin/settings/finalseminargeneralsettings", AdminFinalSeminarSettingsPage.class);
mountBookmarkablePage("admin/settings/finalseminarprojectlevel", AdminFinalSeminarSettingsPerProjectClassPage.class);
mountBookmarkablePage("admin/settings/peer", AdminPeerSettingsPage.class);
diff --git a/src/main/java/se/su/dsv/scipro/admin/pages/AbstractAdminSettingsPage.java b/src/main/java/se/su/dsv/scipro/admin/pages/AbstractAdminSettingsPage.java
index e097dd2c61..4a44cf3a49 100644
--- a/src/main/java/se/su/dsv/scipro/admin/pages/AbstractAdminSettingsPage.java
+++ b/src/main/java/se/su/dsv/scipro/admin/pages/AbstractAdminSettingsPage.java
@@ -10,6 +10,7 @@ import se.su.dsv.scipro.admin.pages.settings.AdminFinalSeminarSettingsPage;
import se.su.dsv.scipro.admin.pages.settings.AdminFinalSeminarSettingsPerProjectClassPage;
import se.su.dsv.scipro.admin.pages.settings.AdminGeneralSettingsPage;
import se.su.dsv.scipro.admin.pages.settings.AdminPeerSettingsPage;
+import se.su.dsv.scipro.admin.pages.settings.AdminServerEnvironmentSettingsPage;
import se.su.dsv.scipro.components.AbstractMenuPanel;
import se.su.dsv.scipro.icons.ImageIcon;
import se.su.dsv.scipro.security.auth.Authorization;
@@ -29,6 +30,7 @@ public abstract class AbstractAdminSettingsPage extends AbstractAdminPage {
protected List<MenuItem> getItemList() {
final List<MenuItem> items = new ArrayList<MenuItem>();
items.add(new MenuItem("General settings", AdminGeneralSettingsPage.class));
+ items.add(new MenuItem("Server Environment", AdminServerEnvironmentSettingsPage.class));
items.add(new MenuItem("Final seminar general settings", AdminFinalSeminarSettingsPage.class));
items.add(new MenuItem("Final seminar project level settings", AdminFinalSeminarSettingsPerProjectClassPage.class));
items.add(new MenuItem("Peer settings", AdminPeerSettingsPage.class, ImageIcon.ICON_SETTINGS));
diff --git a/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.html b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.html
new file mode 100644
index 0000000000..71a7e5df94
--- /dev/null
+++ b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.html
@@ -0,0 +1,17 @@
+<!DOCTYPE html>
+<html
+ xmlns:wicket="http://wicket.apache.org/dtds.data/wicket-xhtml1.4-strict.dtd">
+<body>
+<wicket:extend>
+ <h3 class="section">Server/Request environment</h3>
+ <div style="overflow:auto;height:50%;">
+ <ul>
+ <li wicket:id="requestAttributes">
+ <span wicket:id="name">[name]</span>:
+ <span wicket:id="value">[value]</span>
+ </li>
+ </ul>
+ </div>
+</wicket:extend>
+</body>
+</html>
\ No newline at end of file
diff --git a/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
new file mode 100644
index 0000000000..3d989606bf
--- /dev/null
+++ b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
@@ -0,0 +1,87 @@
+package se.su.dsv.scipro.admin.pages.settings;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Enumeration;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.wicket.PageParameters;
+import org.apache.wicket.markup.html.basic.Label;
+import org.apache.wicket.markup.html.list.ListItem;
+import org.apache.wicket.markup.html.list.ListView;
+
+import se.su.dsv.scipro.admin.pages.AbstractAdminSettingsPage;
+
+public class AdminServerEnvironmentSettingsPage extends AbstractAdminSettingsPage {
+ public AdminServerEnvironmentSettingsPage(final PageParameters pp) {
+ super(pp);
+ add(new ListView<KeyValuePair<String>>("requestAttributes",getRequestAttributes()){
+ private static final long serialVersionUID = 1L;
+ @Override
+ protected void populateItem(ListItem<KeyValuePair<String>> item){
+ KeyValuePair<String> pair = item.getModelObject();
+ item.add(new Label("name",pair.getKey()));
+ item.add(new Label("value","'"+pair.getValue()+"'"));
+ }
+ });
+ }
+ private final class KeyValuePair<V> implements Serializable{
+ private static final long serialVersionUID = 1L;
+ private final String key;
+ private final V value;
+ public KeyValuePair(final String key, final V value){
+ this.key = key;
+ this.value = value;
+ }
+ public String getKey(){
+ return key;
+ }
+ public V getValue(){
+ return value;
+ }
+ @Override
+ public boolean equals(final Object o){
+ if(o==this)
+ return true;
+ if(!(o instanceof KeyValuePair))
+ return false;
+ @SuppressWarnings("unchecked")
+ final KeyValuePair<V> other = (KeyValuePair<V>)o;
+ return ((key==null?other.key==null:key.equals(other.key)) &&
+ (value==null?other.value==null:value.equals(other.value)));
+ }
+ @Override
+ public int hashCode(){
+ final int w = 31;
+ int result = 17;//Why hello there Mr Bloch, how are we feeling today ?
+ result = w * result + (key==null?0:key.hashCode());
+ result = w * result + (value==null?0:value.hashCode());
+ return result;
+ }
+ @Override
+ public String toString(){
+ return ("{"+key+":"+value+"}");
+ }
+ }
+ private List<KeyValuePair<String>> getRequestAttributes(){
+ final HttpServletRequest rawRequest = getWebRequestCycle().getWebRequest().getHttpServletRequest();
+ List<KeyValuePair<String>> list = new ArrayList<KeyValuePair<String>>();
+ list.add(new KeyValuePair<String>("[CALL] getRemoteUser",rawRequest.getRemoteUser()));
+ list.add(new KeyValuePair<String>("[CALL] getAuthType",rawRequest.getAuthType()));
+ @SuppressWarnings("rawtypes") Enumeration attributes = rawRequest.getAttributeNames();
+ while(attributes.hasMoreElements()){
+ final String key = (String)attributes.nextElement();
+ final String value = rawRequest.getAttribute(key).toString();
+ list.add(new KeyValuePair<String>("[ATTR] "+key,value));
+ }
+ Map<String,String> envs = System.getenv();
+ for(String key : envs.keySet()){
+ list.add(new KeyValuePair<String>("[ENV] "+key,envs.get(key)));
+ }
+ return list;
+ }
+}
+
diff --git a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
index 983d72614c..c70c556893 100644
--- a/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
+++ b/src/main/java/se/su/dsv/scipro/security/auth/ExternalAuthenticationRequestHelper.java
@@ -100,7 +100,7 @@ public final class ExternalAuthenticationRequestHelper{
/**
* Private utility method for dumping headers.
*/
- public void dumpAuthInfo(){
+ private void dumpAuthInfo(){
logger.debug("---Standard methods---");
logger.debug("Request implementation:" + req.getClass().getName());
logger.debug("getAuthType = '" + getExternalAuthType() +"'");
From 134953400758132a78554ba46fbd50df12715662 Mon Sep 17 00:00:00 2001
From: Robin Eklund <robi-ekl@dsv.su.se>
Date: Wed, 13 Jul 2011 15:52:25 +0200
Subject: [PATCH 7/7] Refactored out small utility class, feel free to use it
when you need immutable kvp's for whatever reason.
---
.../AdminServerEnvironmentSettingsPage.java | 40 +---------
.../se/su/dsv/scipro/util/KeyValuePair.java | 79 +++++++++++++++++++
.../su/dsv/scipro/util/TestKeyValuePair.java | 22 ++++++
3 files changed, 102 insertions(+), 39 deletions(-)
create mode 100644 src/main/java/se/su/dsv/scipro/util/KeyValuePair.java
create mode 100644 src/test/java/se/su/dsv/scipro/util/TestKeyValuePair.java
diff --git a/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
index 3d989606bf..8ede9d512a 100644
--- a/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
+++ b/src/main/java/se/su/dsv/scipro/admin/pages/settings/AdminServerEnvironmentSettingsPage.java
@@ -1,6 +1,5 @@
package se.su.dsv.scipro.admin.pages.settings;
-import java.io.Serializable;
import java.util.ArrayList;
import java.util.Enumeration;
import java.util.List;
@@ -14,6 +13,7 @@ import org.apache.wicket.markup.html.list.ListItem;
import org.apache.wicket.markup.html.list.ListView;
import se.su.dsv.scipro.admin.pages.AbstractAdminSettingsPage;
+import se.su.dsv.scipro.util.KeyValuePair;
public class AdminServerEnvironmentSettingsPage extends AbstractAdminSettingsPage {
public AdminServerEnvironmentSettingsPage(final PageParameters pp) {
@@ -28,44 +28,6 @@ public class AdminServerEnvironmentSettingsPage extends AbstractAdminSettingsPag
}
});
}
- private final class KeyValuePair<V> implements Serializable{
- private static final long serialVersionUID = 1L;
- private final String key;
- private final V value;
- public KeyValuePair(final String key, final V value){
- this.key = key;
- this.value = value;
- }
- public String getKey(){
- return key;
- }
- public V getValue(){
- return value;
- }
- @Override
- public boolean equals(final Object o){
- if(o==this)
- return true;
- if(!(o instanceof KeyValuePair))
- return false;
- @SuppressWarnings("unchecked")
- final KeyValuePair<V> other = (KeyValuePair<V>)o;
- return ((key==null?other.key==null:key.equals(other.key)) &&
- (value==null?other.value==null:value.equals(other.value)));
- }
- @Override
- public int hashCode(){
- final int w = 31;
- int result = 17;//Why hello there Mr Bloch, how are we feeling today ?
- result = w * result + (key==null?0:key.hashCode());
- result = w * result + (value==null?0:value.hashCode());
- return result;
- }
- @Override
- public String toString(){
- return ("{"+key+":"+value+"}");
- }
- }
private List<KeyValuePair<String>> getRequestAttributes(){
final HttpServletRequest rawRequest = getWebRequestCycle().getWebRequest().getHttpServletRequest();
List<KeyValuePair<String>> list = new ArrayList<KeyValuePair<String>>();
diff --git a/src/main/java/se/su/dsv/scipro/util/KeyValuePair.java b/src/main/java/se/su/dsv/scipro/util/KeyValuePair.java
new file mode 100644
index 0000000000..0b7958d70e
--- /dev/null
+++ b/src/main/java/se/su/dsv/scipro/util/KeyValuePair.java
@@ -0,0 +1,79 @@
+package se.su.dsv.scipro.util;
+
+import java.io.Serializable;
+
+/**
+ * Small utility class for immutable (yes it's immutable and no it's never going to be otherwise) key/value pairs.
+ * The "value" field is parameterized, but the key is always intended to be a simple String.
+ * The Serializable properties of this depends on those of the parameterized type.
+ * Typical usage:
+ * <code>
+ * KeyValuePair<SomeType> pair = new KeyValuePair<SomeType>("thisIsAKey",someObject);
+ * pair.getKey();
+ * pair.getValue();
+ * </code>
+ * @param <V>
+ */
+public final class KeyValuePair<V> implements Serializable{
+ private static final long serialVersionUID = 1L;
+ private final String key;
+ private final V value;
+ /**
+ * Default and only constructor, provide key and value.
+ * null values are accepted for both key and value, and should be safe to use (yet nonsensical).
+ * @param key
+ * @param value
+ */
+ public KeyValuePair(final String key, final V value){
+ this.key = key;
+ this.value = value;
+ }
+ /**
+ * Getter for wrapped key String
+ * @return The key
+ */
+ public String getKey(){
+ return key;
+ }
+ /**
+ * Getter for wrapped Value
+ * @return The value
+ */
+ public V getValue(){
+ return value;
+ }
+ /**
+ * Equals override, behaves in a logical fashion (two objects are equal if both key and value matches).
+ * Null values are accounted for, ie: two objects with null key+value are considered equal.
+ */
+ @Override
+ public boolean equals(final Object o){
+ if(o==this)
+ return true;
+ if(!(o instanceof KeyValuePair))
+ return false;
+ @SuppressWarnings("unchecked")
+ final KeyValuePair<V> other = (KeyValuePair<V>)o;
+ return ((key==null?other.key==null:key.equals(other.key)) &&
+ (value==null?other.value==null:value.equals(other.value)));
+ }
+ /**
+ * Hash-code override, uses both components in a standard fashion.
+ */
+ @Override
+ public int hashCode(){
+ final int w = 31;
+ int result = 17;//Why hello there Mr Bloch, how are we feeling today ?
+ result = w * result + (key==null?0:key.hashCode());
+ result = w * result + (value==null?0:value.hashCode());
+ return result;
+ }
+ /**
+ * Standard override, return String format not exposed and subject to change.
+ * Any parsing of this representation is strongly advised against and done at your own risk.
+ */
+ @Override
+ public String toString(){
+ return ("{"+key+":"+value+"}");
+ }
+}
\ No newline at end of file
diff --git a/src/test/java/se/su/dsv/scipro/util/TestKeyValuePair.java b/src/test/java/se/su/dsv/scipro/util/TestKeyValuePair.java
new file mode 100644
index 0000000000..ac2c9fd764
--- /dev/null
+++ b/src/test/java/se/su/dsv/scipro/util/TestKeyValuePair.java
@@ -0,0 +1,22 @@
+package se.su.dsv.scipro.util;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestKeyValuePair {
+ @Test
+ public void testEqualsOperations() {
+ KeyValuePair<String> kvpOne = new KeyValuePair<String>("key","value");
+ KeyValuePair<String> kvpTwo = new KeyValuePair<String>("key","value");
+ KeyValuePair<String> kvpThree = new KeyValuePair<String>("key3","value");
+ KeyValuePair<String> kvpFour = new KeyValuePair<String>(null,null);
+ KeyValuePair<TestKeyValuePair> kvpFive = new KeyValuePair<TestKeyValuePair>("key",this);
+
+ Assert.assertTrue(kvpOne.equals(kvpOne));
+ Assert.assertTrue(kvpFour.equals(kvpFour));
+ Assert.assertTrue(kvpOne.equals(kvpTwo) && kvpTwo.equals(kvpOne));
+ Assert.assertFalse(kvpOne.equals(kvpThree) && kvpThree.equals(kvpOne));
+ Assert.assertFalse(kvpFour.equals(kvpThree) && kvpThree.equals(kvpFour));
+ Assert.assertFalse(kvpOne.equals(kvpFive) && kvpFive.equals(kvpOne));
+ }
+}