Make BasePage.onInitialize() final.

Add security check for GitFilter

gitop.core/src/main/java/com/pmease/gitop/core/GitFilter.java

@@ -14,6 +14,8 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.lang3.StringUtils;
+import org.apache.shiro.SecurityUtils;
+import org.apache.shiro.authz.UnauthorizedException;
 import org.eclipse.jgit.http.server.ServletUtils;
 import org.eclipse.jgit.transport.PacketLineOut;
 import org.slf4j.Logger;
@@ -22,6 +24,7 @@ import org.slf4j.LoggerFactory;
 import com.pmease.commons.git.Git;
 import com.pmease.gitop.core.manager.ProjectManager;
 import com.pmease.gitop.core.model.Project;
+import com.pmease.gitop.core.permission.ObjectPermission;
 
 @Singleton
 public class GitFilter implements Filter {
@@ -37,7 +40,7 @@ public class GitFilter implements Filter {
 		this.projectManager = projectManager;
 	}
 	
-	private Project getProject(HttpServletRequest request, HttpServletResponse response, String repoInfo) 
+	private Project getProject(HttpServletRequest request, HttpServletResponse response, String pathInfo, String repoInfo) 
 			throws IOException {
 		
 		repoInfo = StringUtils.stripStart(StringUtils.stripEnd(repoInfo, "/"), "/");
@@ -47,8 +50,8 @@ public class GitFilter implements Filter {
 
 		if (StringUtils.isBlank(ownerName) || StringUtils.isBlank(projectName)) {
 			String url = request.getRequestURL().toString();
-			String urlRoot = url.substring(0, url.length()-request.getPathInfo().length());
+			String urlRoot = url.substring(0, url.length()-pathInfo.length());
-			String message = "Expecting url of format " + urlRoot + "/<owner name>/<project name>";
+			String message = "Expecting url of format " + urlRoot + "<owner name>/<project name>";
 			logger.error("Error serving git request: " + message);
 			response.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
 			return null;
@@ -61,7 +64,7 @@ public class GitFilter implements Filter {
 		if (project == null) {
 			String message = "Unable to find project '" + projectName + "' owned by '" + ownerName + "'.";
 			logger.error("Error serving git request: " + message);
-			response.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
+			response.sendError(HttpServletResponse.SC_NOT_FOUND, message);
 			return null;
 		}
 		
@@ -78,7 +81,7 @@ public class GitFilter implements Filter {
 		String service = StringUtils.substringAfterLast(pathInfo, "/");
 
 		String repoInfo = StringUtils.substringBeforeLast(pathInfo, "/");
-		Project project = getProject(req, resp, repoInfo);
+		Project project = getProject(req, resp, pathInfo, repoInfo);
 		
 		if (project != null) {
 			doNotCache(resp);
@@ -86,23 +89,34 @@ public class GitFilter implements Filter {
 
 			Git git = new Git(projectManager.locateStorage(project).ofCode());
 
-			try {
+			if (service.contains("upload")) {
-				if (service.contains("upload")) {
+				if (!SecurityUtils.getSubject().isPermitted(ObjectPermission.ofProjectRead(project))) {
-					git.upload().input(ServletUtils.getInputStream(req)).output(resp.getOutputStream()).call();
+					throw new UnauthorizedException("You do not have permission to pull from this project.");
-				} else if (service.contains("receive")) {
-					git.receive().input(ServletUtils.getInputStream(req)).output(resp.getOutputStream()).call();
-				} else {
-					String message = "Invalid service name '" + service + "'.";
-					logger.error("Error serving git request: " + message);
-					resp.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
 				}
-			} catch (Exception e) {
+				git.upload().input(ServletUtils.getInputStream(req)).output(resp.getOutputStream()).call();
-				logger.error("Error serving git request.", e);
+			} else if (service.contains("receive")) {
-				resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage());
+				if (!SecurityUtils.getSubject().isPermitted(ObjectPermission.ofProjectWrite(project))) {
+					throw new UnauthorizedException("You do not have permission to push to this project.");
+				}
+				git.receive().input(ServletUtils.getInputStream(req)).output(resp.getOutputStream()).call();
+			} else {
+				String message = "Invalid service name '" + service + "'.";
+				logger.error("Error serving git request: " + message);
+				resp.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
 			}
 		}
 	}
 	
+	private void writeInitial(HttpServletResponse resp, String service) throws IOException {
+		doNotCache(resp);
+		resp.setHeader("Content-Type", "application/x-" + service + "-advertisement");			
+		
+		PacketLineOut pack = new PacketLineOut(resp.getOutputStream());
+		pack.setFlushOnEnd(false);
+		pack.writeString("# service=" + service + "\n");
+		pack.end();
+	}
+	
 	protected void processRefs(HttpServletRequest req, HttpServletResponse resp, String pathInfo) throws ServletException, IOException {
 		if (!pathInfo.endsWith(INFO_REFS)) {
 			String message = "Invalid refs request url: " + req.getRequestURL();
@@ -112,32 +126,27 @@ public class GitFilter implements Filter {
 		}
 		
 		String repoInfo = pathInfo.substring(0, pathInfo.length() - INFO_REFS.length());
-		Project project = getProject(req, resp, repoInfo);
+		Project project = getProject(req, resp, pathInfo, repoInfo);
 		if (project != null) {
-			doNotCache(resp);
 			String service = req.getParameter("service");
-			resp.setHeader("Content-Type", "application/x-" + service + "-advertisement");			
-			
-			PacketLineOut pack = new PacketLineOut(resp.getOutputStream());
-			pack.setFlushOnEnd(false);
-			pack.writeString("# service=" + service + "\n");
-			pack.end();
-			
 			Git git = new Git(projectManager.locateStorage(project).ofCode());
 
-			try {
+			if (service.contains("upload")) {
-				if (service.contains("upload")) {
+				if (!SecurityUtils.getSubject().isPermitted(ObjectPermission.ofProjectRead(project))) {
-					git.advertiseUploadRefs().output(resp.getOutputStream()).call();
+					throw new UnauthorizedException("You do not have permission to pull from this project.");
-				} else if (service.contains("receive")) {
-					git.advertiseReceiveRefs().output(resp.getOutputStream()).call();
-				} else {
-					String message = "Invalid service name '" + service + "'.";
-					logger.error("Error serving git request: " + message);
-					resp.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
 				}
-			} catch (Exception e) {
+				writeInitial(resp, service);
-				logger.error("Error serving git request.", e);
+				git.advertiseUploadRefs().output(resp.getOutputStream()).call();
-				resp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR, e.getMessage());
+			} else if (service.contains("receive")) {
+				if (!SecurityUtils.getSubject().isPermitted(ObjectPermission.ofProjectWrite(project))) {
+					throw new UnauthorizedException("You do not have permission to push to this project.");
+				}
+				writeInitial(resp, service);
+				git.advertiseReceiveRefs().output(resp.getOutputStream()).call();
+			} else {
+				String message = "Invalid service name '" + service + "'.";
+				logger.error("Error serving git request: " + message);
+				resp.sendError(HttpServletResponse.SC_BAD_REQUEST, message);
 			}
 		}
 	}
@@ -151,7 +160,6 @@ public class GitFilter implements Filter {
 			FilterChain chain) throws IOException, ServletException {
 		HttpServletRequest httpRequest = (HttpServletRequest) request;
 		HttpServletResponse httpResponse = (HttpServletResponse) response;
-		
 		String pathInfo = httpRequest.getRequestURI().substring(httpRequest.getContextPath().length());
 		pathInfo = StringUtils.stripStart(pathInfo, "/");
 		

gitop.web/src/main/java/com/pmease/gitop/web/TestPage.java

@@ -14,10 +14,7 @@ import com.pmease.gitop.web.page.BasePage;
 @SuppressWarnings("serial")
 public class TestPage extends BasePage {
 
-	@Override
+	public TestPage() {
-	protected void onInitialize() {
-		super.onInitialize();
-		
 		final EditContext editContext = EditableUtils.getContext(new Project());
 		
 		Form<?> form = new Form<Void>("form") {

gitop.web/src/main/java/com/pmease/gitop/web/WebUserNameReservation.java

@@ -42,6 +42,7 @@ public class WebUserNameReservation implements UserNameReservation {
 		}
 		
 		reserved.add("wicket");
+		reserved.add("rest");
 		
 		for (IRequestMapper mapper: webApp.getRequestMappers()) {
 			if (mapper instanceof MountedMapper || mapper instanceof ResourceMapper) {

gitop.web/src/main/java/com/pmease/gitop/web/page/AbstractLayoutPage.java

@@ -1,6 +1,8 @@
 package com.pmease.gitop.web.page;
 
 import org.apache.shiro.SecurityUtils;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
 
 import com.google.common.base.Optional;
 import com.pmease.gitop.core.model.User;
@@ -8,9 +10,21 @@ import com.pmease.gitop.core.model.User;
 @SuppressWarnings("serial")
 public abstract class AbstractLayoutPage extends BasePage {
 
-	@Override
+	public AbstractLayoutPage() {
-	protected void onInitialize() {
+		commonInit();
-		super.onInitialize();
+	}
+	
+	public AbstractLayoutPage(PageParameters params) {
+		super(params);
+		commonInit();
+	}
+	
+	public AbstractLayoutPage(IModel<?> model) {
+		super(model);
+		commonInit();
+	}
+	
+	private void commonInit() {
 		add(new GlobalHeaderPanel("header"));
 	}
 	

gitop.web/src/main/java/com/pmease/gitop/web/page/BasePage.java

@@ -49,23 +49,9 @@ public abstract class BasePage extends WebPage {
 			}
 		}));
 
-		if (!Gitop.getInstance().isReady() && getClass() != ServerInitPage.class) {
+		if (!isServerReady() && getClass() != ServerInitPage.class) {
 			throw new RestartResponseAtInterceptPageException(ServerInitPage.class);
 		}
-	}
-	
-	protected String getPageCssClass() {
-		String name = getClass().getSimpleName();
-		return StringUtils.camelCaseToLowerCaseWithHyphen(name);
-	}
-
-	protected boolean isPermitted() {
-		return true;
-	}
-	
-	@Override
-	protected void onInitialize() {
-		super.onInitialize();
 		
 		if (!isPermitted()) {
 			throw new AccessDeniedException();
@@ -100,6 +86,31 @@ public abstract class BasePage extends WebPage {
 		add(new WebMarkupContainer("globalResourceBinder").add(new BaseResourceBehavior()));
 	}
 	
+	protected String getPageCssClass() {
+		String name = getClass().getSimpleName();
+		return StringUtils.camelCaseToLowerCaseWithHyphen(name);
+	}
+
+	protected boolean isPermitted() {
+		return true;
+	}
+	
+	protected boolean isServerReady() {
+		return Gitop.getInstance().isReady();
+	}
+	
+	/*
+	 * For pages, we make this final to prevent sub classes from putting page initialization 
+	 * logics here. Instead, one should put all page initialization logic in page 
+	 * constructor to avoid the situation that if page constructor throws an exception 
+	 * intentionally (such as RestartResponseException) to by pass initialization logic 
+	 * but onInitialize will still be called to cause undesired behavior.  
+	 */
+	@Override
+	protected final void onInitialize() {
+		super.onInitialize();
+	}
+	
 	protected abstract String getPageTitle();
 	
 	protected int getPageRefreshInterval() {

gitop.web/src/main/java/com/pmease/gitop/web/page/account/AccountHomePage.java

@@ -36,11 +36,6 @@ public class AccountHomePage extends AbstractLayoutPage {
 			}
 			
 		};
-	}
-	
-	@Override
-	protected void onInitialize() {
-		super.onInitialize();
 
 		add(new Label("accountName", getAccount().getName()));
 		

gitop.web/src/main/java/com/pmease/gitop/web/page/account/RegisterPage.java

@@ -41,10 +41,7 @@ public class RegisterPage extends AbstractLayoutPage {
 		return "Gitop - Sign Up";
 	}
 	
-	@Override
+	public RegisterPage() {
-	protected void onInitialize() {
-		super.onInitialize();
-
 		final IModel<User> model = Model.<User>of(new User());
 		Form<User> form = new Form<User>("form", model);
 		add(form);

gitop.web/src/main/java/com/pmease/gitop/web/page/account/setting/AccountSettingPage.java

@@ -10,6 +10,8 @@ import org.apache.wicket.markup.html.link.Link;
 import org.apache.wicket.markup.html.list.ListItem;
 import org.apache.wicket.markup.html.list.ListView;
 import org.apache.wicket.model.AbstractReadOnlyModel;
+import org.apache.wicket.model.IModel;
+import org.apache.wicket.request.mapper.parameter.PageParameters;
 
 import com.google.common.collect.ImmutableList;
 import com.pmease.gitop.core.model.User;
@@ -48,10 +50,21 @@ public abstract class AccountSettingPage extends AbstractLayoutPage {
 		}
 	}
 
-	@Override
+	public AccountSettingPage() {
-	protected void onInitialize() {
+		commonInit();
-		super.onInitialize();
+	}
 	
+	public AccountSettingPage(PageParameters params) {
+		super(params);
+		commonInit();
+	}
+	
+	public AccountSettingPage(IModel<?> model) {
+		super(model);
+		commonInit();
+	}
+
+	private void commonInit() {
 		add(new UserAvatarLink("userlink", new UserModel(getAccount())));
 		
 		add(new ListView<Category>("setting", ImmutableList.<Category>copyOf(Category.values())) {

gitop.web/src/main/java/com/pmease/gitop/web/page/account/setting/password/AccountPasswordPage.java

@@ -34,10 +34,7 @@ public class AccountPasswordPage extends AccountSettingPage {
 		return Category.PASSWORD;
 	}
 
-	@Override
+	public AccountPasswordPage() {
-	protected void onInitialize() {
-		super.onInitialize();
-		
 		Form<User> form = new Form<User>("form", new UserModel(getAccount()));
 		add(form);
 		form.add(new PasswordFieldElement("oldPass", "Current Password", 

gitop.web/src/main/java/com/pmease/gitop/web/page/account/setting/profile/AccountProfilePage.java

@@ -28,10 +28,7 @@ public class AccountProfilePage extends AccountSettingPage {
 		return Category.PROFILE;
 	}
 	
-	@Override
+	public AccountProfilePage() {
-	protected void onInitialize() {
-		super.onInitialize();
-		
 		add(new ProfileForm("form", new UserModel(getAccount())));
 	}
 

gitop.web/src/main/java/com/pmease/gitop/web/page/home/HomePage.html

@@ -6,6 +6,5 @@
 		<div wicket:id="upload"></div>
 	</div>
 	<a wicket:id="accountLink">account</a>
-	<a wicket:id="projectLink">project</a>
 </wicket:extend>
 </html>

gitop.web/src/main/java/com/pmease/gitop/web/page/home/HomePage.java

@@ -3,29 +3,24 @@ package com.pmease.gitop.web.page.home;
 import org.apache.wicket.markup.html.link.BookmarkablePageLink;
 
 import com.pmease.gitop.core.Gitop;
-import com.pmease.gitop.core.manager.ProjectManager;
 import com.pmease.gitop.core.manager.UserManager;
 import com.pmease.gitop.web.common.component.fileupload.FileUploadBar;
 import com.pmease.gitop.web.page.AbstractLayoutPage;
 import com.pmease.gitop.web.page.account.AccountHomePage;
-import com.pmease.gitop.web.page.project.ProjectHomePage;
 
 public class HomePage extends AbstractLayoutPage {
 
 	private static final long serialVersionUID = 1L;
 	
+	public HomePage() {
+		add(new FileUploadBar("upload"));
+		
+		add(new BookmarkablePageLink<>("accountLink", AccountHomePage.class, AccountHomePage.paramsOf(Gitop.getInstance(UserManager.class).getRootUser())));
+	}
+
 	@Override
 	protected String getPageTitle() {
 		return "Gitop - Home";
 	}
 
-	@Override
-	protected void onInitialize() {
-		super.onInitialize();
-		
-		add(new FileUploadBar("upload"));
-		
-		add(new BookmarkablePageLink<>("accountLink", AccountHomePage.class, AccountHomePage.paramsOf(Gitop.getInstance(UserManager.class).getRootUser())));
-		add(new BookmarkablePageLink<>("projectLink", ProjectHomePage.class, ProjectHomePage.paramsOf(Gitop.getInstance(ProjectManager.class).load(1L))));
-	}
 }

gitop.web/src/main/java/com/pmease/gitop/web/page/init/ServerInitPage.java

@@ -25,30 +25,23 @@ public class ServerInitPage extends BasePage {
 			continueToOriginalDestination();
 			throw new RestartResponseException(getApplication().getHomePage());
 		}
-	}
 
-	@Override
+		add(new Label("message", initStage.getMessage()));
-	protected void onInitialize() {
-		super.onInitialize();
 		
-		if (initStage != null) {
+		if (!initStage.getManualConfigs().isEmpty()) {
-			add(new Label("message", initStage.getMessage()));
+			List<ManualConfigStep> configSteps = new ArrayList<ManualConfigStep>();
+			for (ManualConfig each: initStage.getManualConfigs())
+				configSteps.add(new ManualConfigStep(each));
+			add(new Wizard("wizard", configSteps) {
 
-			if (!initStage.getManualConfigs().isEmpty()) {
+				@Override
-				List<ManualConfigStep> configSteps = new ArrayList<ManualConfigStep>();
+				protected void finished() {
-				for (ManualConfig each: initStage.getManualConfigs())
+					setResponsePage(ServerInitPage.class);
-					configSteps.add(new ManualConfigStep(each));
+				}
-				add(new Wizard("wizard", configSteps) {
 				
-					@Override
+			});
-					protected void finished() {
+		} else {
-						setResponsePage(ServerInitPage.class);
+			add(new WebMarkupContainer("wizard").setVisible(false));
-					}
-					
-				});
-			} else {
-				add(new WebMarkupContainer("wizard").setVisible(false));
-			}
 		}
 	}
 	

gitop.web/src/main/java/com/pmease/gitop/web/page/project/ProjectHomePage.html

@@ -3,7 +3,5 @@
 	<h1>Welcome, Project Home</h1>
 	<div wicket:id="accountName"></div>
 	<div wicket:id="projectName"></div>
-	
-	<a wicket:id="link">link</a>
 </wicket:extend>
 </html>

gitop.web/src/main/java/com/pmease/gitop/web/page/project/ProjectHomePage.java

@@ -1,7 +1,6 @@
 package com.pmease.gitop.web.page.project;
 
 import org.apache.wicket.markup.html.basic.Label;
-import org.apache.wicket.markup.html.link.Link;
 import org.apache.wicket.model.IModel;
 import org.apache.wicket.model.LoadableDetachableModel;
 import org.apache.wicket.request.mapper.parameter.PageParameters;
@@ -45,20 +44,6 @@ public class ProjectHomePage extends AbstractLayoutPage {
 			
 		};
 		
-		add(new Link<Void>("link") {
-
-			@Override
-			public void onClick() {
-				
-			}
-			
-		});
-	}
-	
-	@Override
-	protected void onInitialize() {
-		super.onInitialize();
-		
 		add(new Label("accountName", getProject().getOwner().getName()));
 		add(new Label("projectName", getProject().getName()));
 	}

gitop.web/src/main/java/com/pmease/gitop/web/shiro/LoginPage.java

@@ -26,17 +26,11 @@ public class LoginPage extends AbstractLayoutPage {
 		if (isSignedIn()) {
 			throw new RestartResponseException(getApplication().getHomePage());
 		}
-	}
-	
-	@Override
-	protected void onInitialize() {
-		super.onInitialize();
 		add(new LoginForm("login"));
 		FeedbackPanel feedback = new FeedbackPanel("feedback");
 		add(feedback);
 	}
 	
-	
 	private class LoginForm extends StatelessForm<Void> {
 
 		public LoginForm(String id) {