Repository: zeppelin Updated Branches: refs/heads/master b6cd47996 -> 0e087455a
[ZEPPELIN-1615] - Zeppelin should be able to run without Shiro ### What is this PR for? Right now, Zeppelin use Shiro by default even if you dont need it. (It will use shiro.ini.template file if it doenst find shiro.ini), this behaviors is a little flacky and we should start zeppelin without shiro context if user doenst want to use it. ### What type of PR is it? [Bug Fix | Improvement ] ### Todos * [x] - Update configuration - Return empty if shiro file not found * [x] - refactor Rest Api handler, if shiro.ini not found start a handler without shiro context * [x] - refactor SecurityUtils to handle the case of shiro is disabled. ### What is the Jira issue? * [ZEPPELIN-1615](https://issues.apache.org/jira/browse/ZEPPELIN-1615) ### How should this be tested? Start zeppelin without shiro.ini ### Screenshots (if appropriate) ### Questions: * Does the licenses files need update? No * Is there breaking changes for older versions? No * Does this needs documentation? No Author: Anthony Corbacho <[email protected]> Closes #1595 from anthonycorbacho/fix/ShiroTempleteLoginRequired and squashes the following commits: c163bd8 [Anthony Corbacho] Handle the case of user already have a shiro ini :: backup current ini and perform tests then restaure 608e718 [Anthony Corbacho] Fix AuthenticationIT Test by creating shiro.ini file in conf. 73ce69a [Anthony Corbacho] Fix SecurityRestApiTest test 4c67e8f [Anthony Corbacho] Handle SecurityUtils, if shiro is disabled then script all the getPrincipla and shiro check and return anon or empty collections in certain case f67f82e [Anthony Corbacho] Handle the case of user want to start zeppelin without Shiro - Refactor handler, if not shiro ini foundm start zeppelin wihtout shiro 09387ce [Anthony Corbacho] Rework getShiroIni in zeppelinConfiguration, if shiro.ini file is not found, then return empty instead of shiro.ini.template path Project: http://git-wip-us.apache.org/repos/asf/zeppelin/repo Commit: http://git-wip-us.apache.org/repos/asf/zeppelin/commit/0e087455 Tree: http://git-wip-us.apache.org/repos/asf/zeppelin/tree/0e087455 Diff: http://git-wip-us.apache.org/repos/asf/zeppelin/diff/0e087455 Branch: refs/heads/master Commit: 0e087455af491a1c4b0fd393ceed39489c790793 Parents: b6cd479 Author: Anthony Corbacho <[email protected]> Authored: Sat Nov 5 09:57:20 2016 +0900 Committer: Mina Lee <[email protected]> Committed: Sat Nov 5 12:48:10 2016 +0900 ---------------------------------------------------------------------- .../apache/zeppelin/server/ZeppelinServer.java | 22 ++++++----- .../apache/zeppelin/utils/SecurityUtils.java | 39 ++++++++++++++++---- .../zeppelin/integration/AuthenticationIT.java | 34 ++++++++++------- .../zeppelin/rest/SecurityRestApiTest.java | 15 ++++---- .../zeppelin/conf/ZeppelinConfiguration.java | 6 +-- 5 files changed, 75 insertions(+), 41 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0e087455/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java index 9d6b813..534e8d4 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/server/ZeppelinServer.java @@ -26,7 +26,10 @@ import java.util.Set; import javax.servlet.DispatcherType; import javax.ws.rs.core.Application; +import org.apache.commons.lang.StringUtils; import org.apache.cxf.jaxrs.servlet.CXFNonSpringJaxrsServlet; +import org.apache.shiro.web.env.EnvironmentLoaderListener; +import org.apache.shiro.web.servlet.ShiroFilter; import org.apache.zeppelin.conf.ZeppelinConfiguration; import org.apache.zeppelin.conf.ZeppelinConfiguration.ConfVars; import org.apache.zeppelin.dep.DependencyResolver; @@ -259,15 +262,16 @@ public class ZeppelinServer extends Application { webapp.setSessionHandler(new SessionHandler()); webapp.addServlet(cxfServletHolder, "/api/*"); - webapp.setInitParameter("shiroConfigLocations", - new File(conf.getShiroPath()).toURI().toString()); - - SecurityUtils.initSecurityManager(conf.getShiroPath()); - webapp.addFilter(org.apache.shiro.web.servlet.ShiroFilter.class, "/api/*", - EnumSet.allOf(DispatcherType.class)); - - webapp.addEventListener(new org.apache.shiro.web.env.EnvironmentLoaderListener()); - + String shiroIniPath = conf.getShiroPath(); + if (!StringUtils.isBlank(shiroIniPath)) { + webapp.setInitParameter("shiroConfigLocations", new File(shiroIniPath).toURI().toString()); + SecurityUtils.initSecurityManager(shiroIniPath); + webapp.addFilter(ShiroFilter.class, "/api/*", EnumSet.allOf(DispatcherType.class)); + webapp.addEventListener(new EnvironmentLoaderListener()); + } else { + webapp.addFilter(new FilterHolder(CorsFilter.class), + "/api/*", EnumSet.allOf(DispatcherType.class)); + } } private static WebAppContext setupWebAppContext(ContextHandlerCollection contexts, http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0e087455/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java index d81d2e6..186a324 100644 --- a/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java +++ b/zeppelin-server/src/main/java/org/apache/zeppelin/utils/SecurityUtils.java @@ -16,30 +16,41 @@ */ package org.apache.zeppelin.utils; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.UnknownHostException; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Iterator; +import java.util.Map; + +import org.apache.shiro.config.IniSecurityManagerFactory; +import org.apache.shiro.mgt.SecurityManager; import org.apache.shiro.realm.Realm; import org.apache.shiro.realm.text.IniRealm; import org.apache.shiro.subject.Subject; import org.apache.shiro.util.ThreadContext; import org.apache.shiro.web.mgt.DefaultWebSecurityManager; -import org.apache.shiro.mgt.SecurityManager; -import org.apache.shiro.config.IniSecurityManagerFactory; import org.apache.zeppelin.conf.ZeppelinConfiguration; -import java.net.InetAddress; -import java.net.URI; -import java.net.URISyntaxException; -import java.net.UnknownHostException; -import java.util.*; +import com.google.common.collect.Sets; /** * Tools for securing Zeppelin */ public class SecurityUtils { + private static final String ANONYMOUS = "anonymous"; + private static final HashSet<String> EMPTY_HASHSET = Sets.newHashSet(); + private static boolean isEnabled = false; + public static void initSecurityManager(String shiroPath) { IniSecurityManagerFactory factory = new IniSecurityManagerFactory("file:" + shiroPath); SecurityManager securityManager = factory.getInstance(); org.apache.shiro.SecurityUtils.setSecurityManager(securityManager); + isEnabled = true; } public static Boolean isValidOrigin(String sourceHost, ZeppelinConfiguration conf) @@ -65,18 +76,24 @@ public class SecurityUtils { * @return shiro principal */ public static String getPrincipal() { + if (!isEnabled) { + return ANONYMOUS; + } Subject subject = org.apache.shiro.SecurityUtils.getSubject(); String principal; if (subject.isAuthenticated()) { principal = subject.getPrincipal().toString(); } else { - principal = "anonymous"; + principal = ANONYMOUS; } return principal; } public static Collection getRealmsList() { + if (!isEnabled) { + return Collections.emptyList(); + } DefaultWebSecurityManager defaultWebSecurityManager; String key = ThreadContext.SECURITY_MANAGER_KEY; defaultWebSecurityManager = (DefaultWebSecurityManager) ThreadContext.get(key); @@ -91,6 +108,9 @@ public class SecurityUtils { * @return shiro roles */ public static HashSet<String> getRoles() { + if (!isEnabled) { + return EMPTY_HASHSET; + } Subject subject = org.apache.shiro.SecurityUtils.getSubject(); HashSet<String> roles = new HashSet<>(); Map allRoles = null; @@ -123,6 +143,9 @@ public class SecurityUtils { * Checked if shiro enabled or not */ public static boolean isAuthenticated() { + if (!isEnabled) { + return false; + } return org.apache.shiro.SecurityUtils.getSubject().isAuthenticated(); } } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0e087455/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java index decd713..635f5f1 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/integration/AuthenticationIT.java @@ -16,6 +16,13 @@ */ package org.apache.zeppelin.integration; +import static org.junit.Assert.assertTrue; + +import java.io.File; +import java.io.IOException; +import java.net.URI; +import java.util.List; + import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.StringUtils; import org.apache.zeppelin.AbstractZeppelinIT; @@ -34,13 +41,6 @@ import org.openqa.selenium.WebElement; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import java.io.File; -import java.io.IOException; -import java.net.URI; -import java.util.List; - -import static org.junit.Assert.assertTrue; - /** * Created for org.apache.zeppelin.integration on 13/06/16. @@ -50,7 +50,7 @@ public class AuthenticationIT extends AbstractZeppelinIT { @Rule public ErrorCollector collector = new ErrorCollector(); - + static String shiroPath; static String authShiro = "[users]\n" + "admin = password1, admin\n" + "finance1 = finance1, finance\n" + @@ -82,8 +82,11 @@ public class AuthenticationIT extends AbstractZeppelinIT { try { System.setProperty(ZeppelinConfiguration.ConfVars.ZEPPELIN_HOME.getVarName(), "../"); ZeppelinConfiguration conf = ZeppelinConfiguration.create(); - File file = new File(conf.getShiroPath()); - originalShiro = StringUtils.join(FileUtils.readLines(file, "UTF-8"), "\n"); + shiroPath = conf.getRelativeDir(String.format("%s/shiro.ini", conf.getConfDir())); + File file = new File(shiroPath); + if (file.exists()) { + originalShiro = StringUtils.join(FileUtils.readLines(file, "UTF-8"), "\n"); + } FileUtils.write(file, authShiro, "UTF-8"); } catch (IOException e) { LOG.error("Error in AuthenticationIT startUp::", e); @@ -99,9 +102,14 @@ public class AuthenticationIT extends AbstractZeppelinIT { return; } try { - ZeppelinConfiguration conf = ZeppelinConfiguration.create(); - File file = new File(conf.getShiroPath()); - FileUtils.write(file, originalShiro, "UTF-8"); + if (!StringUtils.isBlank(shiroPath)) { + File file = new File(shiroPath); + if (StringUtils.isBlank(originalShiro)) { + FileUtils.deleteQuietly(file); + } else { + FileUtils.write(file, originalShiro, "UTF-8"); + } + } } catch (IOException e) { LOG.error("Error in AuthenticationIT tearDown::", e); } http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0e087455/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java ---------------------------------------------------------------------- diff --git a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java index b4ecd97..b56763a 100644 --- a/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java +++ b/zeppelin-server/src/test/java/org/apache/zeppelin/rest/SecurityRestApiTest.java @@ -17,8 +17,10 @@ package org.apache.zeppelin.rest; -import com.google.gson.Gson; -import com.google.gson.reflect.TypeToken; +import java.io.IOException; +import java.util.List; +import java.util.Map; + import org.apache.commons.httpclient.methods.GetMethod; import org.hamcrest.CoreMatchers; import org.junit.AfterClass; @@ -27,11 +29,8 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ErrorCollector; -import java.io.IOException; -import java.util.List; -import java.util.Map; - -import static org.junit.Assert.*; +import com.google.gson.Gson; +import com.google.gson.reflect.TypeToken; public class SecurityRestApiTest extends AbstractTestRestApi { Gson gson = new Gson(); @@ -41,7 +40,7 @@ public class SecurityRestApiTest extends AbstractTestRestApi { @BeforeClass public static void init() throws Exception { - AbstractTestRestApi.startUp(); + AbstractTestRestApi.startUpWithAuthenticationEnable();; } @AfterClass http://git-wip-us.apache.org/repos/asf/zeppelin/blob/0e087455/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java ---------------------------------------------------------------------- diff --git a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java index b972fff..b9a3bfc 100644 --- a/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java +++ b/zeppelin-zengine/src/main/java/org/apache/zeppelin/conf/ZeppelinConfiguration.java @@ -27,6 +27,7 @@ import java.util.Map; import org.apache.commons.configuration.ConfigurationException; import org.apache.commons.configuration.XMLConfiguration; import org.apache.commons.configuration.tree.ConfigurationNode; +import org.apache.commons.lang.StringUtils; import org.apache.zeppelin.notebook.repo.VFSNotebookRepo; import org.apache.zeppelin.util.Util; import org.slf4j.Logger; @@ -402,9 +403,8 @@ public class ZeppelinConfiguration extends XMLConfiguration { } public String getShiroPath() { - String shiroPath = getRelativeDir(String.format("%s/shiro.ini", getConfDir())); - return new File(shiroPath).exists() ? shiroPath - : getRelativeDir(String.format("%s/shiro.ini.template", getConfDir())); + String shiroPath = getRelativeDir(String.format("%s/shiro.ini", getConfDir())); + return new File(shiroPath).exists() ? shiroPath : StringUtils.EMPTY; } public String getInterpreterRemoteRunnerPath() {
