Additional check to ensure read access on the issue container.

issues/src/org/labkey/issue/IssuesController.java

@@ -113,6 +113,7 @@
 import org.labkey.api.security.roles.OwnerRole;
 import org.labkey.api.security.roles.ReaderRole;
 import org.labkey.api.security.roles.RoleManager;
+import org.labkey.api.security.roles.SubmitterRole;
 import org.labkey.api.util.ButtonBuilder;
 import org.labkey.api.util.CSRFUtil;
 import org.labkey.api.util.DOM;
@@ -2564,4 +2565,59 @@ private void deleteProjects()
             _projectA = null;
         }
     }
+
+    /**
+     * GitHub Issue 1317 regression test.
+     */
+    public static class GetIssuePermissionTestCase extends AbstractContainerScopingTest
+    {
+        private static final String ISSUE_TITLE = "getIssue() permission test issue";
+
+        @Test
+        public void testSubmitterCannotReadIssue() throws Exception
+        {
+            Container c = createIssuesContainer("Submitter");
+            int issueId = createIssue(c);
+
+            // A reader should be able to access the issue
+            User reader = createUserInRole(c, ReaderRole.class);
+            IssueObject asReader = IssueManager.getIssue(c, reader, issueId);
+            assertNotNull("Reader should be able to read the issue", asReader);
+            assertEquals(ISSUE_TITLE, asReader.getTitle());
+
+            // A submitter holds InsertPermission but not ReadPermission, so the issue shouldn't be accessible.
+            User submitter = createUserInRole(c, SubmitterRole.class);
+            assertNull("Submitter should not be able to read an issue", IssueManager.getIssue(c, submitter, issueId));
+            assertNull("Submitter should not be able to read an issue in an unspecified container", IssueManager.getIssue(null, submitter, issueId));
+        }
+
+        private Container createIssuesContainer(String name)
+        {
+            Container c = createContainer(name, ModuleLoader.getInstance().getModule(IssuesModule.NAME));
+
+            IssueListDef def = new IssueListDef();
+            def.setName(IssueListDef.DEFAULT_ISSUE_LIST_NAME);
+            def.setLabel(IssueListDef.DEFAULT_ISSUE_LIST_NAME);
+            def.setKind(IssueDefDomainKind.NAME);
+            def.beforeInsert(getAdmin(), c.getId());
+            def.save(getAdmin());
+
+            return c;
+        }
+
+        private int createIssue(Container c)
+        {
+            User admin = getAdmin();
+            IssueObject issue = new IssueObject();
+            issue.open(c, admin);
+            issue.setAssignedTo(admin.getUserId());
+            issue.setTitle(ISSUE_TITLE);
+            issue.setPriority("3");
+            issue.setIssueDefName(IssueListDef.DEFAULT_ISSUE_LIST_NAME);
+            ObjectFactory.Registry.getFactory(IssueObject.class).toMap(issue, issue.getProperties());
+            IssueManager.saveIssue(admin, c, issue);
+
+            return issue.getIssueId();
+        }
+    }
 }

issues/src/org/labkey/issue/IssuesModule.java

@@ -193,7 +193,8 @@ public ActionURL getTabURL(Container c, User user)
         return Set.of(
             org.labkey.issue.model.IssueManager.TestCase.class,
             org.labkey.issue.IssuesController.MoveActionContainerScopingTestCase.class,
-            org.labkey.issue.IssuesController.GetUsersForGroupScopingTestCase.class
+            org.labkey.issue.IssuesController.GetUsersForGroupScopingTestCase.class,
+            org.labkey.issue.IssuesController.GetIssuePermissionTestCase.class
         );
     }
 

issues/src/org/labkey/issue/model/IssueManager.java

@@ -122,9 +122,9 @@
 import java.util.TreeSet;
 import java.util.function.Consumer;
 
-import static org.labkey.api.util.IntegerUtils.asInteger;
 import static org.labkey.api.search.SearchService.PROPERTY.categories;
 import static org.labkey.api.security.UserManager.USER_DISPLAY_NAME_COMPARATOR;
+import static org.labkey.api.util.IntegerUtils.asInteger;
 
 public class IssueManager
 {
@@ -240,14 +240,21 @@ public static IssueObject getIssue(
     private static IssueObject _getIssue(@Nullable Container c, User user, int issueId)
     {
         IssueObject issue = _getRawIssue(c, issueId);
+        if (issue == null)
+            return null;
 
-        if (issue != null && issue.getIssueDefId() != null)
-        {
-            // container may initially be null if we don't care about a specific folder, but we need the
-            // correct domain for the provisioned table properties associated with the issue
-            if (c == null)
-                c = ContainerManager.getForId(issue.getContainerId());
+        // container may initially be null if we don't care about a specific folder, but we need the
+        // correct domain for the provisioned table properties associated with the issue
+        if (c == null)
+            c = ContainerManager.getForId(issue.getContainerId());
 
+        // GitHub Issue 1317: explicitly check for read access on the target container before querying to
+        // avoid Submitter roles from accessing the table and then hitting an exception during the read.
+        if (c == null || !c.hasPermission(user, ReadPermission.class))
+            return null;
+
+        if (issue.getIssueDefId() != null)
+        {
             IssueListDef issueListDef = getIssueListDef(issue.getContainerFromId(), issue.getIssueDefId());
             UserSchema userSchema = QueryService.get().getUserSchema(user, c, IssuesQuerySchema.SCHEMA_NAME);
             TableInfo table = userSchema.getTable(issueListDef.getName());