This repository was archived by the owner on Jun 23, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 10
DB Fix bug#67471678. Include ACLs in the db.singleDocContentSql #110
Open
srinicodebytes
wants to merge
11
commits into
master
Choose a base branch
from
sri-acls
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
f28dade
DB Fix bug#67471678. Include ACLs in the db.singleDocContentSql
srinicodebytes a8df1f1
Changes as per review.
srinicodebytes 16f8673
Changes as suggested.
srinicodebytes ec20849
Changes as per review.
srinicodebytes bdbe6c5
Restructured code around getAcl()
srinicodebytes 9f2edd6
Restored getAclFromDb(). Removed AllPublic.
srinicodebytes ad41538
Modified getAcl(), isUserAuthorized() and added tests.
srinicodebytes e07e129
Changes as per review.
srinicodebytes aed6dfa
Changes as per review:
srinicodebytes 61b6b9f
Changes as per review.
srinicodebytes bcf643c
Merge branch 'master' into sri-acls
srinicodebytes File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -274,9 +274,11 @@ public void init(AdaptorContext context) throws Exception { | |
| if (!isNullOrEmptyString(cfg.getValue("db.aclSql"))) { | ||
| aclSql = cfg.getValue("db.aclSql"); | ||
| log.config("acl sql: " + aclSql); | ||
| aclPrincipalDelimiter = cfg.getValue("db.aclPrincipalDelimiter"); | ||
| log.config("aclPrincipalDelimiter: '" + aclPrincipalDelimiter + "'"); | ||
| } | ||
| // May have delimiter and no aclSql as ACLs might come from | ||
| // singleDocContentSql rather than aclSql. | ||
| aclPrincipalDelimiter = cfg.getValue("db.aclPrincipalDelimiter"); | ||
| log.config("aclPrincipalDelimiter: '" + aclPrincipalDelimiter + "'"); | ||
|
|
||
| disableStreaming = | ||
| Boolean.parseBoolean(cfg.getValue("db.disableStreaming")); | ||
|
|
@@ -291,11 +293,7 @@ public void init(AdaptorContext context) throws Exception { | |
| ignored.add("db.updateTimestampTimezone"); | ||
| } | ||
|
|
||
| if (aclSql == null) { | ||
| context.setAuthzAuthority(new AllPublic()); | ||
| } else { | ||
| context.setAuthzAuthority(new AccessChecker()); | ||
| } | ||
| context.setAuthzAuthority(new AccessChecker()); | ||
|
|
||
| aclNamespace = cfg.getValue("adaptor.namespace"); | ||
| log.config("namespace: " + aclNamespace); | ||
|
|
@@ -675,10 +673,8 @@ public void getDocContent(Request req, Response resp) throws IOException { | |
| } | ||
| // Generate response metadata first. | ||
| addMetadataToResponse(resp, rs); | ||
| // Generate Acl if aclSql is provided. | ||
| if (aclSql != null) { | ||
| resp.setAcl(getAcl(conn, id.getUniqueId())); | ||
| } | ||
| // Generate Acl. | ||
| resp.setAcl(getAcl(conn, rs, id.getUniqueId())); | ||
| // Generate response body. | ||
| // In database adaptor's case, we almost never want to follow the URLs. | ||
| // One record means one document. | ||
|
|
@@ -693,22 +689,34 @@ public static void main(String[] args) { | |
| AbstractAdaptor.main(new DatabaseAdaptor(), args); | ||
| } | ||
|
|
||
| private Acl getAcl(Connection conn, String uniqueId) throws SQLException { | ||
| try (PreparedStatement stmt = getAclFromDb(conn, uniqueId); | ||
| ResultSet rs = stmt.executeQuery()) { | ||
| log.finer("got acl"); | ||
| return buildAcl(rs, aclPrincipalDelimiter, aclNamespace); | ||
| private Acl getAcl(Connection conn, ResultSet contentRs, String uniqueId) | ||
| throws SQLException { | ||
| if (aclSql != null || contentRs == null) { | ||
| try (PreparedStatement stmt = getAclFromDb(conn, uniqueId); | ||
| ResultSet rs = stmt.executeQuery()) { | ||
| log.finer("got acl"); | ||
| boolean hasResult = rs.next(); | ||
| if (!hasResult) { | ||
| // empty Acl ensures adaptor will mark this document as secure | ||
| return Acl.EMPTY; | ||
| } else if (aclSql != null) { | ||
| return buildAcl(rs, aclPrincipalDelimiter, aclNamespace, | ||
| ResultSetNext.PROCESS_ALL_ROWS, Acl.EMPTY); | ||
| } else { | ||
| return buildAcl(rs, aclPrincipalDelimiter, aclNamespace, | ||
| ResultSetNext.PROCESS_ONE_ROW, null); | ||
| } | ||
| } | ||
| } else { | ||
| // Generate ACL if it came as part of result to singleDocContentSql. | ||
| return buildAcl(contentRs, aclPrincipalDelimiter, aclNamespace, | ||
| ResultSetNext.PROCESS_ONE_ROW, null); | ||
| } | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
| static Acl buildAcl(ResultSet rs, String delim, String namespace) | ||
| throws SQLException { | ||
| boolean hasResult = rs.next(); | ||
| if (!hasResult) { | ||
| // empty Acl ensures adaptor will mark this document as secure | ||
| return Acl.EMPTY; | ||
| } | ||
| static Acl buildAcl(ResultSet rs, String delim, String namespace, | ||
| ResultSetNext rsNext, Acl defaultAcl) throws SQLException { | ||
| ResultSetMetaData metadata = rs.getMetaData(); | ||
| Acl.Builder builder = new Acl.Builder(); | ||
| ArrayList<UserPrincipal> permitUsers = new ArrayList<UserPrincipal>(); | ||
|
|
@@ -723,6 +731,12 @@ static Acl buildAcl(ResultSet rs, String delim, String namespace) | |
| hasColumn(metadata, GsaSpecialColumns.GSA_PERMIT_GROUPS); | ||
| boolean hasDenyGroups = | ||
| hasColumn(metadata, GsaSpecialColumns.GSA_DENY_GROUPS); | ||
|
|
||
| if (!hasPermitUsers && !hasDenyUsers | ||
| && !hasPermitGroups && !hasDenyGroups) { | ||
| return defaultAcl; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why do we need default acl parameter?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we have aclSql but no ACL entries, we want an empty ACL to mark the document secure. But if we have no aclSql and no ACL entries, then we want a null ACL to mark the document public. |
||
| } | ||
|
|
||
| do { | ||
| if (hasPermitUsers) { | ||
| permitUsers.addAll(getUserPrincipalsFromResultSet(rs, | ||
|
|
@@ -740,7 +754,7 @@ static Acl buildAcl(ResultSet rs, String delim, String namespace) | |
| denyGroups.addAll(getGroupPrincipalsFromResultSet(rs, | ||
| GsaSpecialColumns.GSA_DENY_GROUPS, delim, namespace)); | ||
| } | ||
| } while (rs.next()); | ||
| } while (rsNext == ResultSetNext.PROCESS_ALL_ROWS && rs.next()); | ||
| return builder | ||
| .setPermitUsers(permitUsers) | ||
| .setDenyUsers(denyUsers) | ||
|
|
@@ -817,8 +831,15 @@ private PreparedStatement getDocFromDb(Connection conn, | |
|
|
||
| private PreparedStatement getAclFromDb(Connection conn, | ||
| String uniqueId) throws SQLException { | ||
| PreparedStatement st = conn.prepareStatement(aclSql); | ||
| uniqueKey.setAclSqlValues(st, uniqueId); | ||
| PreparedStatement st; | ||
| // aclSql will only be null when called from isUserAuthorized. | ||
| if (aclSql == null) { | ||
| st = conn.prepareStatement(singleDocContentSql); | ||
| uniqueKey.setContentSqlValues(st, uniqueId); | ||
| } else { | ||
| st = conn.prepareStatement(aclSql); | ||
| uniqueKey.setAclSqlValues(st, uniqueId); | ||
| } | ||
| log.log(Level.FINER, "about to get acl: {0}", uniqueId); | ||
| return st; | ||
| } | ||
|
|
@@ -1085,16 +1106,9 @@ enum GsaSpecialColumns { | |
| GSA_TIMESTAMP; | ||
| } | ||
|
|
||
| private static class AllPublic implements AuthzAuthority { | ||
| public Map<DocId, AuthzStatus> isUserAuthorized(AuthnIdentity userIdentity, | ||
| Collection<DocId> ids) throws IOException { | ||
| Map<DocId, AuthzStatus> result = | ||
| new HashMap<DocId, AuthzStatus>(ids.size() * 2); | ||
| for (DocId docId : ids) { | ||
| result.put(docId, AuthzStatus.PERMIT); | ||
| } | ||
| return Collections.unmodifiableMap(result); | ||
| } | ||
| enum ResultSetNext { | ||
| PROCESS_ALL_ROWS, | ||
| PROCESS_ONE_ROW, | ||
| } | ||
|
|
||
| private static Map<DocId, AuthzStatus> allDeny(Collection<DocId> ids) { | ||
|
|
@@ -1109,32 +1123,29 @@ private static Map<DocId, AuthzStatus> allDeny(Collection<DocId> ids) { | |
| private class AccessChecker implements AuthzAuthority { | ||
| public Map<DocId, AuthzStatus> isUserAuthorized(AuthnIdentity userIdentity, | ||
| Collection<DocId> ids) throws IOException { | ||
| if (null == userIdentity) { | ||
| log.info("null identity to authorize"); | ||
| return allDeny(ids); // TODO: consider way to permit public | ||
| } | ||
| UserPrincipal user = userIdentity.getUser(); | ||
| if (null == user) { | ||
| log.info("null user to authorize"); | ||
| return allDeny(ids); // TODO: consider way to permit public | ||
| } | ||
| log.log(Level.INFO, "about to authorize {0} {1}", | ||
| new Object[]{user, userIdentity.getGroups()}); | ||
| Map<DocId, AuthzStatus> result | ||
| = new HashMap<DocId, AuthzStatus>(ids.size() * 2); | ||
| try (Connection conn = makeNewConnection()) { | ||
| for (DocId id : ids) { | ||
| log.log(Level.FINE, "about to get acl of doc {0}", id); | ||
| Acl acl = getAcl(conn, id.getUniqueId()); | ||
| List<Acl> aclChain = Arrays.asList(acl); | ||
| log.log(Level.FINE, | ||
| "about to authorize user {0} for doc {1} and acl {2}", | ||
| new Object[]{user, id, acl}); | ||
| AuthzStatus decision = Acl.isAuthorized(userIdentity, aclChain); | ||
| log.log(Level.FINE, | ||
| "authorization decision {0} for user {1} and doc {2}", | ||
| new Object[]{decision, user, id}); | ||
| result.put(id, decision); | ||
| Acl acl = getAcl(conn, null, id.getUniqueId()); | ||
| if (acl == null) { | ||
| result.put(id, AuthzStatus.PERMIT); | ||
| } else if (userIdentity == null || userIdentity.getUser() == null) { | ||
| result.put(id, AuthzStatus.DENY); | ||
| } else { | ||
| log.log(Level.FINE, "about to authorize {0} {1}", | ||
| new Object[] {user, userIdentity.getGroups()}); | ||
| List<Acl> aclChain = Arrays.asList(acl); | ||
| log.log(Level.FINE, | ||
| "about to authorize user {0} for doc {1} and acl {2}", | ||
| new Object[] {user, id, acl}); | ||
| AuthzStatus decision = Acl.isAuthorized(userIdentity, aclChain); | ||
| log.log(Level.FINE, | ||
| "authorization decision {0} for user {1} and doc {2}", | ||
| new Object[] {decision, user, id}); | ||
| result.put(id, decision); | ||
| } | ||
| } | ||
| } catch (SQLException ex) { | ||
| throw new IOException("authz retrieval error", ex); | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AllPublic is unused now.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes missed it, removed it.