This is an automated email from the ASF dual-hosted git repository. billyliu pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 94a197849e9fb46b52e898adb01cd81a7bc12d78 Author: Jiatao Tao <245915...@qq.com> AuthorDate: Sun Feb 4 21:06:21 2018 +0800 minor refactor about ACL. minor. --- ...eAclController.java => TableACLController.java} | 22 +++- .../apache/kylin/rest/service/QueryService.java | 146 ++++++--------------- .../org/apache/kylin/rest/util/ValidateUtil.java | 32 +++-- 3 files changed, 74 insertions(+), 126 deletions(-) diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java similarity index 85% rename from server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java rename to server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java index 198930c..1a97ec5 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/controller/TableAclController.java +++ b/server-base/src/main/java/org/apache/kylin/rest/controller/TableACLController.java @@ -22,7 +22,11 @@ import java.io.IOException; import java.util.List; import java.util.Set; +import static org.apache.kylin.metadata.MetadataConstants.TYPE_USER; + +import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.service.TableACLService; +import org.apache.kylin.rest.service.UserService; import org.apache.kylin.rest.util.ValidateUtil; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; @@ -34,7 +38,7 @@ import org.springframework.web.bind.annotation.ResponseBody; @Controller @RequestMapping(value = "/acl") -public class TableAclController extends BasicController { +public class TableACLController extends BasicController { @Autowired @Qualifier("TableAclService") @@ -44,12 +48,22 @@ public class TableAclController extends BasicController { @Qualifier("validateUtil") private ValidateUtil validateUtil; + @Autowired + @Qualifier("userService") + private UserService userService; + @RequestMapping(value = "/table/{project}/{type}/{table:.+}", method = {RequestMethod.GET}, produces = {"application/json"}) @ResponseBody public List<String> getUsersCanQueryTheTbl(@PathVariable String project, @PathVariable String type, @PathVariable String table) throws IOException { validateUtil.validateArgs(project, table); validateUtil.validateTable(project, table); - Set<String> allIdentifiers = validateUtil.getAllIdentifiers(project, type); + Set<String> allIdentifiers = validateUtil.getAllIdentifiersInPrj(project, type); + // add global admins + if (type.equals(TYPE_USER)) { + allIdentifiers.addAll(userService.listAdminUsers()); + } else { + allIdentifiers.add(Constant.ROLE_ADMIN); + } return tableACLService.getCanAccessList(project, table, allIdentifiers, type); } @@ -70,7 +84,7 @@ public class TableAclController extends BasicController { @PathVariable String table, @PathVariable String name) throws IOException { validateUtil.validateArgs(project, table, name); - validateUtil.validateIdentifiers(name, type); + validateUtil.validateIdentifiers(project, name, type); validateUtil.validateTable(project, table); tableACLService.addToTableACL(project, name, table, type); } @@ -84,7 +98,7 @@ public class TableAclController extends BasicController { @PathVariable String table, @PathVariable String name) throws IOException { validateUtil.validateArgs(project, table, name); - validateUtil.validateIdentifiers(name, type); + validateUtil.validateIdentifiers(project, name, type); validateUtil.validateTable(project, table); tableACLService.deleteFromTableACL(project, name, table, type); } diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java index 5f67aa1..375b069 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java +++ b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java @@ -18,15 +18,35 @@ package org.apache.kylin.rest.service; -import com.fasterxml.jackson.annotation.JsonProperty; -import com.google.common.base.CharMatcher; -import com.google.common.base.Preconditions; -import com.google.common.base.Splitter; -import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; -import net.sf.ehcache.Cache; -import net.sf.ehcache.CacheManager; -import net.sf.ehcache.Element; +import static org.apache.kylin.common.util.CheckUtil.checkCondition; + +import java.io.DataInputStream; +import java.io.DataOutputStream; +import java.io.IOException; +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.Statement; +import java.sql.Time; +import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import javax.annotation.PostConstruct; + import org.apache.calcite.avatica.ColumnMetaData.Rep; import org.apache.calcite.config.CalciteConnectionConfig; import org.apache.calcite.jdbc.CalcitePrepare; @@ -49,7 +69,6 @@ import org.apache.kylin.common.util.DBUtils; import org.apache.kylin.common.util.JsonUtil; import org.apache.kylin.common.util.Pair; import org.apache.kylin.common.util.SetThreadName; -import org.apache.kylin.cube.CubeInstance; import org.apache.kylin.cube.CubeManager; import org.apache.kylin.cube.cuboid.Cuboid; import org.apache.kylin.metadata.badquery.BadQueryEntry; @@ -59,13 +78,11 @@ import org.apache.kylin.metadata.model.JoinTableDesc; import org.apache.kylin.metadata.model.ModelDimensionDesc; import org.apache.kylin.metadata.model.TableRef; import org.apache.kylin.metadata.project.ProjectInstance; -import org.apache.kylin.metadata.project.RealizationEntry; import org.apache.kylin.metadata.querymeta.ColumnMeta; import org.apache.kylin.metadata.querymeta.ColumnMetaWithType; import org.apache.kylin.metadata.querymeta.SelectedColumnMeta; import org.apache.kylin.metadata.querymeta.TableMeta; import org.apache.kylin.metadata.querymeta.TableMetaWithType; -import org.apache.kylin.metadata.realization.RealizationType; import org.apache.kylin.query.QueryConnection; import org.apache.kylin.query.relnode.OLAPContext; import org.apache.kylin.query.util.PushDownUtil; @@ -89,43 +106,21 @@ import org.apache.kylin.rest.util.TableauInterceptor; import org.apache.kylin.shaded.htrace.org.apache.htrace.Sampler; import org.apache.kylin.shaded.htrace.org.apache.htrace.Trace; import org.apache.kylin.shaded.htrace.org.apache.htrace.TraceScope; -import org.apache.kylin.storage.hybrid.HybridInstance; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.security.access.AccessDeniedException; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; -import javax.annotation.PostConstruct; -import java.io.DataInputStream; -import java.io.DataOutputStream; -import java.io.IOException; -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.sql.Connection; -import java.sql.DatabaseMetaData; -import java.sql.PreparedStatement; -import java.sql.ResultSet; -import java.sql.ResultSetMetaData; -import java.sql.SQLException; -import java.sql.Statement; -import java.sql.Time; -import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.LinkedList; -import java.util.List; -import java.util.Map; -import java.util.Set; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.google.common.base.Preconditions; +import com.google.common.collect.Lists; -import static org.apache.kylin.common.util.CheckUtil.checkCondition; +import net.sf.ehcache.Cache; +import net.sf.ehcache.CacheManager; +import net.sf.ehcache.Element; /** * @author xduo @@ -328,65 +323,10 @@ public class QueryService extends BasicService { logger.info(stringBuilder.toString()); } - public void checkAuthorization(SQLResponse sqlResponse, String project) throws AccessDeniedException { - - //project - ProjectInstance projectInstance = getProjectManager().getProject(project); - try { - if (aclEvaluate.hasProjectReadPermission(projectInstance)) { - return; - } - } catch (AccessDeniedException e) { - logger.warn( - "Current user {} has no READ permission on current project {}, please ask Administrator for permission granting."); - //just continue - } - - String realizationsStr = sqlResponse.getCube();//CUBE[name=abc],HYBRID[name=xyz] - - if (StringUtils.isEmpty(realizationsStr)) { - throw new AccessDeniedException( - "Query pushdown requires having READ permission on project, please ask Administrator to grant you permissions"); - } - - String[] splits = StringUtils.split(realizationsStr, ","); - - for (String split : splits) { - - Iterable<String> parts = Splitter.on(CharMatcher.anyOf("[]=,")).split(split); - String[] partsStr = Iterables.toArray(parts, String.class); - - if (RealizationType.HYBRID.toString().equals(partsStr[0])) { - // special care for hybrid - HybridInstance hybridInstance = getHybridManager().getHybridInstance(partsStr[2]); - Preconditions.checkNotNull(hybridInstance); - checkHybridAuthorization(hybridInstance); - } else { - CubeInstance cubeInstance = getCubeManager().getCube(partsStr[2]); - checkCubeAuthorization(cubeInstance); - } - } - } - - private void checkCubeAuthorization(CubeInstance cube) throws AccessDeniedException { - Preconditions.checkState(aclEvaluate.hasCubeReadPermission(cube)); - } - - private void checkHybridAuthorization(HybridInstance hybridInstance) throws AccessDeniedException { - List<RealizationEntry> realizationEntries = hybridInstance.getRealizationEntries(); - for (RealizationEntry realizationEntry : realizationEntries) { - String reName = realizationEntry.getRealization(); - if (RealizationType.CUBE == realizationEntry.getType()) { - CubeInstance cubeInstance = getCubeManager().getCube(reName); - checkCubeAuthorization(cubeInstance); - } else if (RealizationType.HYBRID == realizationEntry.getType()) { - HybridInstance innerHybridInstance = getHybridManager().getHybridInstance(reName); - checkHybridAuthorization(innerHybridInstance); - } - } - } - public SQLResponse doQueryWithCache(SQLRequest sqlRequest) { + long t = System.currentTimeMillis(); + aclEvaluate.checkProjectReadPermission(sqlRequest.getProject()); + logger.info("Check query permission in " + (System.currentTimeMillis() - t) + " ms."); return doQueryWithCache(sqlRequest, false); } @@ -458,10 +398,6 @@ public class QueryService extends BasicService { Trace.addTimelineAnnotation("response without real execution"); } - // check authorization before return, since the response may come from cache - if (!sqlResponse.getIsException()) - checkQueryAuth(sqlResponse, project); - sqlResponse.setDuration(queryContext.getAccumulatedMillis()); sqlResponse.setTraceUrl(traceUrl); logQuery(queryContext.getQueryId(), sqlRequest, sqlResponse); @@ -611,12 +547,6 @@ public class QueryService extends BasicService { return response; } - protected void checkQueryAuth(SQLResponse sqlResponse, String project) throws AccessDeniedException { - if (!sqlResponse.getIsException() && KylinConfig.getInstanceFromEnv().isQuerySecureEnabled()) { - checkAuthorization(sqlResponse, project); - } - } - private SQLResponse queryWithSqlMassage(SQLRequest sqlRequest) throws Exception { Connection conn = null; diff --git a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java index 90d41ca..c250da7 100644 --- a/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java +++ b/server-base/src/main/java/org/apache/kylin/rest/util/ValidateUtil.java @@ -32,7 +32,6 @@ import org.apache.kylin.metadata.MetadataConstants; import org.apache.kylin.metadata.model.ColumnDesc; import org.apache.kylin.metadata.model.TableDesc; import org.apache.kylin.metadata.project.ProjectInstance; -import org.apache.kylin.rest.constant.Constant; import org.apache.kylin.rest.service.AccessService; import org.apache.kylin.rest.service.IUserGroupService; import org.apache.kylin.rest.service.ProjectService; @@ -54,10 +53,6 @@ public class ValidateUtil { private final static Pattern alphaNumUnderscorePattren = Pattern.compile("[a-zA-Z0-9_]+"); @Autowired - @Qualifier("userService") - private UserService userService; - - @Autowired @Qualifier("tableService") private TableService tableService; @@ -70,6 +65,10 @@ public class ValidateUtil { private AccessService accessService; @Autowired + @Qualifier("userService") + private UserService userService; + + @Autowired @Qualifier("userGroupService") private IUserGroupService userGroupService; @@ -77,18 +76,25 @@ public class ValidateUtil { return toCheck == null ? false : alphaNumUnderscorePattren.matcher(toCheck).matches(); } - //Identifiers may be user or user authority(you may call role or group) - public void validateIdentifiers(String name, String type) throws IOException { - if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER) && !userService.userExists(name)) { - throw new RuntimeException("Operation failed, user:" + name + " not exists"); + public void checkIdentifiersExists(String name, boolean isPrincipal) throws IOException { + if (isPrincipal && !userService.userExists(name)) { + throw new RuntimeException("Operation failed, user:" + name + " not exists, please add first."); + } + if (!isPrincipal && !userGroupService.exists(name)) { + throw new RuntimeException("Operation failed, group:" + name + " not exists, please add first."); } - if (type.equalsIgnoreCase(MetadataConstants.TYPE_GROUP) && !userGroupService.exists(name)) { - throw new RuntimeException("Operation failed, group:" + name + " not exists"); + } + + //Identifiers may be user or user authority(you may call role or group) + public void validateIdentifiers(String prj, String name, String type) throws IOException { + Set<String> allIdentifiers = getAllIdentifiersInPrj(prj, type); + if (!allIdentifiers.contains(name)) { + throw new RuntimeException("Operation failed, identifiers:" + name + " not exists"); } } //get all users/user authorities that has permission in the project - public Set<String> getAllIdentifiers(String project, String type) throws IOException { + public Set<String> getAllIdentifiersInPrj(String project, String type) throws IOException { List<Sid> allSids = getAllSids(project); if (type.equalsIgnoreCase(MetadataConstants.TYPE_USER)) { return getUsersInPrj(allSids); @@ -99,7 +105,6 @@ public class ValidateUtil { private Set<String> getAuthoritiesInPrj(List<Sid> allSids) { Set<String> allAuthorities = new TreeSet<>(); - allAuthorities.add(Constant.ROLE_ADMIN); for (Sid sid : allSids) { if (sid instanceof GrantedAuthoritySid) { allAuthorities.add(((GrantedAuthoritySid) sid).getGrantedAuthority()); @@ -110,7 +115,6 @@ public class ValidateUtil { private Set<String> getUsersInPrj(List<Sid> allSids) throws IOException { Set<String> allUsers = new TreeSet<>(); - allUsers.addAll(userService.listAdminUsers()); for (Sid sid : allSids) { if (sid instanceof PrincipalSid) { allUsers.add(((PrincipalSid) sid).getPrincipal()); -- To stop receiving notification emails like this one, please contact billy...@apache.org.