Skip to content

Commit 01cebc9

Browse files
author
Vladimir Kotal
committed
deal with null entities in AuthorizationFramework
fixes #2587
1 parent 98910b0 commit 01cebc9

File tree

2 files changed

+25
-4
lines changed

2 files changed

+25
-4
lines changed

opengrok-indexer/src/main/java/org/opengrok/indexer/authorization/AuthorizationFramework.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -763,6 +763,12 @@ private boolean checkAll(HttpServletRequest request, String cache, Nameable enti
763763
return true;
764764
}
765765

766+
if (entity == null) {
767+
LOGGER.log(Level.WARNING, "entity was null for request with parameters: {}",
768+
request.getParameterMap());
769+
return false;
770+
}
771+
766772
Statistics stats = RuntimeEnvironment.getInstance().getStatistics();
767773

768774
Boolean val;

opengrok-indexer/src/test/java/org/opengrok/indexer/authorization/AuthorizationFrameworkTest.java

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
package org.opengrok.indexer.authorization;
2525

2626
import java.util.Arrays;
27+
import java.util.HashMap;
2728
import java.util.List;
2829
import java.util.Locale;
2930
import java.util.Map;
@@ -69,6 +70,14 @@ public static StackSetup[][] params() {
6970
NewTest(true, createUnallowedProject()),
7071
NewTest(true, createUnallowedGroup()))
7172
},
73+
// Test that null entities will result in denial.
74+
{
75+
new StackSetup(
76+
NewStack(AuthControlFlag.REQUIRED),
77+
// no plugins should return true however we have null entities here
78+
NewTest(false, (Project) null),
79+
NewTest(false, (Group) null))
80+
},
7281
// -------------------------------------------------------------- //
7382
//
7483
// Test authorization flags for plugins. Both plugins do not fail
@@ -657,14 +666,15 @@ public void testPluginsGeneric() {
657666
String format = "%s <%s> was <%s> for entity %s";
658667

659668
for (TestCase innerSetup : setup.setup) {
669+
String entityName = (innerSetup.entity == null ? "null" : innerSetup.entity.getName());
660670
try {
661671
actual = framework.isAllowed(innerSetup.request, (Group) innerSetup.entity);
662-
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, innerSetup.entity.getName()),
672+
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, entityName),
663673
innerSetup.expected,
664674
actual);
665675
} catch (ClassCastException ex) {
666676
actual = framework.isAllowed(innerSetup.request, (Project) innerSetup.entity);
667-
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, innerSetup.entity.getName()),
677+
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, entityName),
668678
innerSetup.expected,
669679
actual);
670680
}
@@ -692,7 +702,12 @@ static private Group createUnallowedGroup() {
692702
}
693703

694704
static private HttpServletRequest createRequest() {
695-
return new DummyHttpServletRequest();
705+
return new DummyHttpServletRequest() {
706+
@Override
707+
public Map<String, String[]> getParameterMap() {
708+
return new HashMap<>();
709+
}
710+
};
696711
}
697712

698713
static private IAuthorizationPlugin createAllowedPrefixPlugin() {
@@ -792,7 +807,7 @@ public TestCase(boolean expected, HttpServletRequest request, Nameable entity) {
792807

793808
@Override
794809
public String toString() {
795-
return "expected <" + expected + "> for entity " + entity.getName();
810+
return "expected <" + expected + "> for entity " + (entity == null ? "null" : entity.getName());
796811
}
797812
}
798813

0 commit comments

Comments
 (0)