From 954b52dc0a5014626d94d80afb453531259caea6 Mon Sep 17 00:00:00 2001 From: NMerz Date: Sat, 14 Nov 2020 14:29:48 -0500 Subject: [PATCH] Stricter access checking Properly restrict access to list actions to only authorized users. --- Lambdas/Lists/List/src/ListAdder.java | 3 +- Lambdas/Lists/List/src/ListDeleter.java | 5 +++ Lambdas/Lists/List/src/ListGetter.java | 10 ++++- Lambdas/Lists/List/src/ListPermissions.java | 41 +++++++++++++++++++ .../Lists/ListEntry/src/ListEntryAdder.java | 23 ++++++++--- .../Lists/ListEntry/src/ListEntryDeleter.java | 24 ++++++++++- Lambdas/Lists/ListShare/src/ListSharer.java | 10 +++-- Lambdas/Lists/User/src/UserGetter.java | 23 ++++++++--- .../java/com/example/listify/ListPage.java | 2 +- .../ShoppingListsSwipeableAdapter.java | 2 +- .../com/example/listify/data/ListShare.java | 16 ++++---- 11 files changed, 132 insertions(+), 27 deletions(-) create mode 100644 Lambdas/Lists/List/src/ListPermissions.java diff --git a/Lambdas/Lists/List/src/ListAdder.java b/Lambdas/Lists/List/src/ListAdder.java index 03fd809..b94af67 100644 --- a/Lambdas/Lists/List/src/ListAdder.java +++ b/Lambdas/Lists/List/src/ListAdder.java @@ -9,7 +9,7 @@ public class ListAdder implements CallHandler { private String cognitoID; private final String LIST_CREATE = "INSERT INTO List (name, owner, lastUpdated) VALUES (?, ?, ?);"; - private final String LIST_ACCESS_GRANT = "INSERT INTO ListSharee(listID, userID) VALUES(?, ?);"; + private final String LIST_ACCESS_GRANT = "INSERT INTO ListSharee(listID, userID, permissionLevel) VALUES(?, ?, ?);"; public ListAdder(Connection connection, String cognitoID) { this.connection = connection; @@ -31,6 +31,7 @@ public class ListAdder implements CallHandler { PreparedStatement accessGrant = connection.prepareStatement(LIST_ACCESS_GRANT); accessGrant.setInt(1, newID); accessGrant.setString(2, cognitoID); + accessGrant.setInt(3, ListPermissions.getAll()); System.out.println(accessGrant); accessGrant.executeUpdate(); connection.commit(); diff --git a/Lambdas/Lists/List/src/ListDeleter.java b/Lambdas/Lists/List/src/ListDeleter.java index 820f65f..f3761e9 100644 --- a/Lambdas/Lists/List/src/ListDeleter.java +++ b/Lambdas/Lists/List/src/ListDeleter.java @@ -37,6 +37,11 @@ public class ListDeleter implements CallHandler { ResultSet userLists = accessCheck.executeQuery(); if (!userLists.next()) { throw new AccessControlException("User does not have access to list"); + } else { + Integer permissionLevel = userLists.getInt("permissionLevel"); + if (!ListPermissions.hasPermission(permissionLevel, "Delete")) { + throw new AccessControlException("User " + cognitoID + " does not have permission to delete list " + listID); + } } PreparedStatement cleanAccess = connection.prepareStatement(DELETE_LIST_ACCESS); cleanAccess.setInt(1, listID); diff --git a/Lambdas/Lists/List/src/ListGetter.java b/Lambdas/Lists/List/src/ListGetter.java index 22bea8d..de3fe5f 100644 --- a/Lambdas/Lists/List/src/ListGetter.java +++ b/Lambdas/Lists/List/src/ListGetter.java @@ -1,3 +1,4 @@ +import java.security.AccessControlException; import java.sql.Connection; import java.sql.PreparedStatement; import java.sql.ResultSet; @@ -40,7 +41,14 @@ public class ListGetter implements CallHandler{ System.out.println(checkAccess); ResultSet accessResults = checkAccess.executeQuery(); int sharees = 0; - while (sharees < 2 && accessResults.next()) { + boolean verifiedAccess = false; + while ((sharees < 2 && accessResults.next()) || !verifiedAccess) { + if (accessResults.getString("userID").equals(cognitoID)) { + verifiedAccess = true; + if (!ListPermissions.hasPermission(accessResults.getInt("permissionLevel"), "Read")) { + throw new AccessControlException("User " + cognitoID + " does not have permission to read list " + id); + } + } sharees++; } boolean shared = false; diff --git a/Lambdas/Lists/List/src/ListPermissions.java b/Lambdas/Lists/List/src/ListPermissions.java new file mode 100644 index 0000000..7405191 --- /dev/null +++ b/Lambdas/Lists/List/src/ListPermissions.java @@ -0,0 +1,41 @@ +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + +public class ListPermissions { + private static final Map keysToPerms; + static { + //All keys should be a prime number > 1 + //All keys need to be maintained here and in ListShare object in data on the client side + HashMap keysToPermsTemp = new HashMap<>(); + keysToPermsTemp.put(2, "read"); + keysToPermsTemp.put(3, "write"); + keysToPermsTemp.put(5, "delete"); + keysToPermsTemp.put(7, "share"); + keysToPerms = Collections.unmodifiableMap(keysToPermsTemp); + } + + public static Integer getAll() { + Integer toReturn = 1; + for (Integer key : keysToPerms.keySet()) { + toReturn *= key; + } + return toReturn; + } + + public static boolean hasPermission(Integer level, String permission) { + return level % getKeyForPermission(permission) == 0; + } + + public static Integer getKeyForPermission(String permissionRaw) { + String permission = permissionRaw.toLowerCase(); + for (Map.Entry entry : keysToPerms.entrySet()) { + if (entry.getValue().equals(permission)) { + return entry.getKey(); + } + } + System.out.println("Tried to get key for invalid permission: " + permission); + return -1; + } + +} diff --git a/Lambdas/Lists/ListEntry/src/ListEntryAdder.java b/Lambdas/Lists/ListEntry/src/ListEntryAdder.java index 6f5dd33..0ed50a7 100644 --- a/Lambdas/Lists/ListEntry/src/ListEntryAdder.java +++ b/Lambdas/Lists/ListEntry/src/ListEntryAdder.java @@ -1,3 +1,4 @@ +import java.security.AccessControlException; import java.sql.*; import java.time.Instant; import java.util.HashMap; @@ -8,7 +9,7 @@ public class ListEntryAdder implements CallHandler { private Connection connection; private String cognitoID; - + private final String ACCESS_CHECK = "SELECT * from ListSharee WHERE userID = ? and listID = ?;"; private final String CHECK_ITEM_IN_LIST = "SELECT quantity from ListProduct WHERE productID = ? AND listID = ?;"; private final String CLEAR_PAIRING = "DELETE from ListProduct WHERE productID = ? AND listID = ?;"; private final String ITEM_TO_LIST = "INSERT INTO ListProduct (productID, listID, quantity, addedDate, purchased) VALUES (?, ?, ?, ?, ?)"; @@ -19,12 +20,24 @@ public class ListEntryAdder implements CallHandler { } public Object conductAction(Map bodyMap, HashMap queryString, String cognitoID) throws SQLException { - PreparedStatement quantitiyStatement = connection.prepareStatement(CHECK_ITEM_IN_LIST); Integer productID = (Integer) bodyMap.get("productID"); Integer listID = (Integer) bodyMap.get("listID"); - quantitiyStatement.setInt(1, productID); - quantitiyStatement.setInt(2, listID); - ResultSet quanitityRS = quantitiyStatement.executeQuery(); + PreparedStatement accessCheck = connection.prepareStatement(ACCESS_CHECK); + accessCheck.setString(1, cognitoID); + accessCheck.setInt(2, listID); + ResultSet access = accessCheck.executeQuery(); + if (access.next()) { + if (!ListPermissions.hasPermission(access.getInt("permissionLevel"), "Write")) { + throw new AccessControlException("User " + cognitoID + " does not have write permissions for list " + listID); + } + } else { + throw new AccessControlException("User " + cognitoID + " does not have any permissions to access list " + listID); + } + + PreparedStatement quantityStatement = connection.prepareStatement(CHECK_ITEM_IN_LIST); + quantityStatement.setInt(1, productID); + quantityStatement.setInt(2, listID); + ResultSet quanitityRS = quantityStatement.executeQuery(); int priorQuanity = 0; if (quanitityRS.next()) { priorQuanity = quanitityRS.getInt(1); diff --git a/Lambdas/Lists/ListEntry/src/ListEntryDeleter.java b/Lambdas/Lists/ListEntry/src/ListEntryDeleter.java index 2cb63cd..fbb063b 100644 --- a/Lambdas/Lists/ListEntry/src/ListEntryDeleter.java +++ b/Lambdas/Lists/ListEntry/src/ListEntryDeleter.java @@ -1,5 +1,7 @@ +import java.security.AccessControlException; import java.sql.Connection; import java.sql.PreparedStatement; +import java.sql.ResultSet; import java.sql.SQLException; import java.util.HashMap; import java.util.Map; @@ -10,6 +12,7 @@ public class ListEntryDeleter implements CallHandler { private String cognitoID; private final String REMOVE_FROM_LIST = "DELETE FROM ListProduct WHERE (ProductID = ? AND ListID = ?);"; + private final String ACCESS_CHECK = "SELECT * from ListSharee WHERE userID = ? and listID = ?;"; public ListEntryDeleter(Connection connection, String cognitoID) { this.connection = connection; @@ -17,9 +20,26 @@ public class ListEntryDeleter implements CallHandler { } public Object conductAction(Map bodyMap, HashMap queryString, String cognitoID) throws SQLException { + Integer productID = (Integer) bodyMap.get("productID"); + Integer listID = (Integer) bodyMap.get("listID"); + + PreparedStatement accessCheck = connection.prepareStatement(ACCESS_CHECK); + accessCheck.setString(1, cognitoID); + accessCheck.setInt(2, listID); + ResultSet access = accessCheck.executeQuery(); + if (access.next()) { + if (!ListPermissions.hasPermission(access.getInt("permissionLevel"), "Write")) { + throw new AccessControlException("User " + cognitoID + " does not have write permissions for list " + listID); + } + } else { + throw new AccessControlException("User " + cognitoID + " does not have any permissions to access list " + listID); + } + + PreparedStatement statement = connection.prepareStatement(REMOVE_FROM_LIST); - statement.setInt(1, (Integer) bodyMap.get("productID")); - statement.setInt(2, (Integer) bodyMap.get("listID")); + statement.setInt(1, productID); + statement.setInt(2, listID); + System.out.println(statement); statement.executeUpdate(); connection.commit(); diff --git a/Lambdas/Lists/ListShare/src/ListSharer.java b/Lambdas/Lists/ListShare/src/ListSharer.java index f5488a7..c3a3e10 100644 --- a/Lambdas/Lists/ListShare/src/ListSharer.java +++ b/Lambdas/Lists/ListShare/src/ListSharer.java @@ -22,7 +22,7 @@ public class ListSharer implements CallHandler { } final private String CHECK_ACCESS = "SELECT * from ListSharee WHERE listID = ? AND userID = ?;"; - final private String SHARE_LIST = "INSERT INTO ListSharee(listID, userID, permissionLevel) VALUES(?, ?, ?);"; + final private String SHARE_LIST = "REPLACE INTO ListSharee(listID, userID, permissionLevel) VALUES(?, ?, ?);"; public Object conductAction(Map bodyMap, HashMap queryString, String cognitoID) throws SQLException { PreparedStatement checkAccess = connection.prepareStatement(CHECK_ACCESS); @@ -30,8 +30,12 @@ public class ListSharer implements CallHandler { checkAccess.setInt(1, listID); checkAccess.setString(2, cognitoID); ResultSet checkAccessRS = checkAccess.executeQuery(); - if (!checkAccessRS.next()) { - throw new AccessControlException("The requesting user does not have access to the requested list"); + if (checkAccessRS.next()) { + if (!ListPermissions.hasPermission(checkAccessRS.getInt("permissionLevel"), "Share")) { + throw new AccessControlException("User " + cognitoID + " does not have share permissions for list " + listID); + } + } else { + throw new AccessControlException("User " + cognitoID + " does not have any permissions to access list " + listID); } InvokeRequest invokeRequest = new InvokeRequest(); invokeRequest.setFunctionName("UserGET"); diff --git a/Lambdas/Lists/User/src/UserGetter.java b/Lambdas/Lists/User/src/UserGetter.java index 043a0c2..42d7422 100644 --- a/Lambdas/Lists/User/src/UserGetter.java +++ b/Lambdas/Lists/User/src/UserGetter.java @@ -28,11 +28,22 @@ public class UserGetter implements CallHandler { System.out.println(userPoolId); ListUsersRequest checkRequest = new ListUsersRequest().withUserPoolId(userPoolId); Object emailObject = bodyMap.get("emailToCheck"); + String attributeToGet = "sub"; if (emailObject != null) { checkRequest.setFilter("email=\"" + emailObject.toString() +"\""); } else { - // checkRequest.setFilter("sub=\"" + cognitoID + "\""); - return cognitoID; + try { + String id = queryMap.get("id"); + if ((id != null) && (!id.equals(""))) { + attributeToGet = "email"; + checkRequest.setFilter("sub=\"" + cognitoID + "\""); + } else { + return cognitoID; + } + } catch (Exception e) { + System.out.println(e); + return cognitoID; + } } System.out.println(checkRequest); AWSCognitoIdentityProvider awsCognitoIdentityProvider = AWSCognitoIdentityProviderClientBuilder.defaultClient(); @@ -47,14 +58,14 @@ public class UserGetter implements CallHandler { } UserType foundUser = foundUsers.get(0); System.out.println(foundUser.getAttributes()); - String sub = ""; + String attributeToReturn = ""; for (AttributeType attribute : foundUser.getAttributes()) { - if (attribute.getName().equals("sub")) { - sub = attribute.getValue(); + if (attribute.getName().equals(attributeToGet)) { + attributeToReturn = attribute.getValue(); break; } System.out.println(attribute.getName() + ": " + attribute.getValue()); } - return sub; + return attributeToReturn; } } diff --git a/Listify/app/src/main/java/com/example/listify/ListPage.java b/Listify/app/src/main/java/com/example/listify/ListPage.java index 454d32a..589d947 100644 --- a/Listify/app/src/main/java/com/example/listify/ListPage.java +++ b/Listify/app/src/main/java/com/example/listify/ListPage.java @@ -114,7 +114,7 @@ public class ListPage extends AppCompatActivity implements Requestor.Receiver { public void onClick(DialogInterface dialog, int which) { EditText sharedEmailText = (EditText) codeView.findViewById(R.id.editTextTextSharedEmail); String sharedEmail = sharedEmailText.getText().toString(); - ListShare listShare = new ListShare(listID, sharedEmail, "Read, Edit, Delete"); + ListShare listShare = new ListShare(listID, sharedEmail, "Read, Edit, Delete, Share"); try { requestor.postObject(listShare); } diff --git a/Listify/app/src/main/java/com/example/listify/adapter/ShoppingListsSwipeableAdapter.java b/Listify/app/src/main/java/com/example/listify/adapter/ShoppingListsSwipeableAdapter.java index 9ad540a..24163de 100644 --- a/Listify/app/src/main/java/com/example/listify/adapter/ShoppingListsSwipeableAdapter.java +++ b/Listify/app/src/main/java/com/example/listify/adapter/ShoppingListsSwipeableAdapter.java @@ -129,7 +129,7 @@ public class ShoppingListsSwipeableAdapter extends BaseAdapter { public void onClick(DialogInterface dialog, int which) { EditText sharedEmailText = (EditText) codeView.findViewById(R.id.editTextTextSharedEmail); String sharedEmail = sharedEmailText.getText().toString(); - ListShare listShare = new ListShare(curList.getItemID(), sharedEmail, "Read, Edit, Delete"); + ListShare listShare = new ListShare(curList.getItemID(), sharedEmail, "Read, Edit, Delete, Share"); try { requestor.postObject(listShare); } diff --git a/Listify/app/src/main/java/com/example/listify/data/ListShare.java b/Listify/app/src/main/java/com/example/listify/data/ListShare.java index bcc5226..36abd8a 100644 --- a/Listify/app/src/main/java/com/example/listify/data/ListShare.java +++ b/Listify/app/src/main/java/com/example/listify/data/ListShare.java @@ -3,7 +3,6 @@ package com.example.listify.data; import com.example.listify.BuildConfig; import java.util.Collections; -import java.util.Comparator; import java.util.HashMap; import java.util.Map; @@ -15,10 +14,12 @@ public class ListShare { private static final Map keysToPerms; static { //All keys should be a prime number > 1 + //All keys need to be maintained here and in List module->ListPermissions class on the Lambda side HashMap keysToPermsTemp = new HashMap<>(); - keysToPermsTemp.put(2, "Read"); - keysToPermsTemp.put(3, "Edit"); - keysToPermsTemp.put(5, "Delete"); + keysToPermsTemp.put(2, "read"); + keysToPermsTemp.put(3, "write"); + keysToPermsTemp.put(5, "delete"); + keysToPermsTemp.put(7, "share"); keysToPerms = Collections.unmodifiableMap(keysToPermsTemp); } @@ -28,7 +29,8 @@ public class ListShare { this.permissionLevel = permissionLevel; } - public ListShare(Integer listID, String shareWithEmail, String permissions) { + public ListShare(Integer listID, String shareWithEmail, String permissionsRaw) { + String permissions = permissionsRaw.toLowerCase(); this.listID = listID; this.shareWithEmail = shareWithEmail; permissionLevel = 1; @@ -48,8 +50,8 @@ public class ListShare { " [Permissions: "); int permissionLevelCopy = permissionLevel; - for (Object permissionObject : keysToPerms.keySet().stream().sorted(Comparator.reverseOrder()).toArray()) { - Integer permissionInteger = (Integer) permissionObject; + for (Integer permissionObject : keysToPerms.keySet()) { + Integer permissionInteger = permissionObject; if (permissionLevelCopy % permissionInteger == 0) { permissionLevelCopy /= permissionInteger; toReturn.append(keysToPerms.get(permissionInteger)).append(",");