Skip to content

Commit 655962d

Browse files
author
Vladimir Kotal
committed
support OPTIONAL verb in authorization framework
fixes #3289
1 parent ee4d3c8 commit 655962d

File tree

4 files changed

+108
-15
lines changed

4 files changed

+108
-15
lines changed

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

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,13 +48,17 @@ public enum AuthControlFlag {
4848
*/
4949
REQUISITE("requisite"),
5050
/**
51-
* If such a plugin succeeds and no prior required plugin has failed the
52-
* authorization framework returns success to the application immediately
53-
* without calling any further plugins in the stack. A failure of a
54-
* sufficient plugin is ignored and processing of the plugin list continues
55-
* unaffected.
51+
* If such a plugin succeeds and no prior required plugin has failed,
52+
* the authorization framework returns success immediately
53+
* without calling any further plugins in the stack.
54+
* A failure of a sufficient plugin is ignored and processing of the stack continues unaffected.
5655
*/
57-
SUFFICIENT("sufficient");
56+
SUFFICIENT("sufficient"),
57+
/**
58+
* The failure is returned only if stack traversal finished and
59+
* there was no prior result recorded.
60+
*/
61+
OPTIONAL("optional");
5862

5963
private final String flag;
6064

@@ -79,6 +83,10 @@ public boolean isSufficient() {
7983
return SUFFICIENT.equals(this);
8084
}
8185

86+
public boolean isOptional() {
87+
return OPTIONAL.equals(this);
88+
}
89+
8290
/**
8391
* Get the enum value for the string parameter.
8492
*

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -523,6 +523,15 @@ public boolean isRequisite() {
523523
return getFlag().isRequisite();
524524
}
525525

526+
/**
527+
* Check if this plugin is marked as optional.
528+
*
529+
* @return true if is optional; false otherwise
530+
*/
531+
public boolean isOptional() {
532+
return getFlag().isOptional();
533+
}
534+
526535
/**
527536
* Print the entity hierarchy.
528537
*

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

Lines changed: 32 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,13 @@ protected boolean processStack(Nameable entity,
219219
PluginDecisionPredicate pluginPredicate,
220220
PluginSkippingPredicate skippingPredicate) {
221221

222-
boolean overallDecision = true;
222+
Boolean overallDecision = null;
223+
boolean optionalFailure = false;
224+
225+
if (getStack().isEmpty()) {
226+
return true;
227+
}
228+
223229
for (AuthorizationEntity authEntity : getStack()) {
224230

225231
if (skippingPredicate.shouldSkip(authEntity)) {
@@ -232,24 +238,29 @@ protected boolean processStack(Nameable entity,
232238
LOGGER.log(Level.FINEST, "AuthEntity \"{0}\" [{1}] testing a name \"{2}\"",
233239
new Object[]{authEntity.getName(), authEntity.getFlag(), entity.getName()});
234240

235-
boolean pluginDecision = authEntity.isAllowed(entity, pluginPredicate, skippingPredicate);
241+
boolean entityDecision = authEntity.isAllowed(entity, pluginPredicate, skippingPredicate);
236242

237243
LOGGER.log(Level.FINEST, "AuthEntity \"{0}\" [{1}] testing a name \"{2}\" => {3}",
238244
new Object[]{authEntity.getName(), authEntity.getFlag(), entity.getName(),
239-
pluginDecision ? "true" : "false"});
245+
entityDecision ? "true" : "false"});
240246

241-
if (!pluginDecision && authEntity.isRequired()) {
247+
if (!entityDecision && authEntity.isRequired()) {
242248
// required sets a failure but still invokes all other plugins
243249
overallDecision = false;
244-
continue;
245-
} else if (!pluginDecision && authEntity.isRequisite()) {
250+
} else if (!entityDecision && authEntity.isRequisite()) {
246251
// requisite sets a failure and immediately returns the failure
247252
overallDecision = false;
248253
break;
249-
} else if (overallDecision && pluginDecision && authEntity.isSufficient()) {
254+
} else if (!entityDecision && authEntity.isOptional()) {
255+
optionalFailure = true;
256+
} else if (entityDecision && authEntity.isSufficient()) {
250257
// sufficient immediately returns the success
258+
if ((overallDecision == null) || overallDecision) {
259+
overallDecision = true;
260+
break;
261+
}
262+
} else if (overallDecision == null && entityDecision) {
251263
overallDecision = true;
252-
break;
253264
}
254265
} catch (AuthorizationException ex) {
255266
// Propagate up so that proper HTTP error can be given.
@@ -264,8 +275,12 @@ protected boolean processStack(Nameable entity,
264275

265276
LOGGER.log(Level.FINEST, "AuthEntity \"{0}\" [{1}] testing a name \"{2}\" => {3}",
266277
new Object[]{authEntity.getName(), authEntity.getFlag(), entity.getName(),
267-
"false (failed)"});
278+
"false (failed)"});
268279

280+
if (authEntity.isOptional()) {
281+
optionalFailure = true;
282+
continue;
283+
}
269284
// set the return value to false for this faulty plugin
270285
if (!authEntity.isSufficient()) {
271286
overallDecision = false;
@@ -277,6 +292,14 @@ protected boolean processStack(Nameable entity,
277292
}
278293
}
279294

295+
if (overallDecision == null && optionalFailure) {
296+
return false;
297+
}
298+
299+
if (overallDecision == null) {
300+
return true;
301+
}
302+
280303
return overallDecision;
281304
}
282305

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

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -654,6 +654,57 @@ public static StackSetup[][] params() {
654654
NewTest(false, createAllowedProject()),
655655
NewTest(false, createAllowedGroup()))
656656
},
657+
{
658+
new StackSetup(
659+
NewStack(AuthControlFlag.REQUIRED,
660+
new AuthorizationPlugin(AuthControlFlag.OPTIONAL, createTestFailingPlugin()),
661+
new AuthorizationPlugin(AuthControlFlag.REQUIRED, createAllowedPrefixPlugin())),
662+
// optional plugin returns false
663+
// required plugin returns false => false
664+
NewTest(false, createUnallowedProject()),
665+
NewTest(false, createUnallowedGroup()),
666+
// optional plugin returns false
667+
// required plugin returns true => true
668+
NewTest(true, createAllowedProject()),
669+
NewTest(true, createAllowedGroup()))
670+
},
671+
{
672+
new StackSetup(
673+
NewStack(AuthControlFlag.REQUIRED,
674+
new AuthorizationPlugin(AuthControlFlag.OPTIONAL, createAllowedPrefixPlugin()),
675+
new AuthorizationPlugin(AuthControlFlag.REQUIRED, createNotAllowedPrefixPlugin())),
676+
// optional plugin returns false
677+
// required plugin returns true => true
678+
NewTest(true, createUnallowedProject()),
679+
NewTest(true, createUnallowedGroup()),
680+
// optional plugin returns true
681+
// required plugin returns false => false
682+
NewTest(false, createAllowedProject()),
683+
NewTest(false, createAllowedGroup()))
684+
},
685+
{
686+
new StackSetup(
687+
NewStack(AuthControlFlag.REQUIRED,
688+
new AuthorizationPlugin(AuthControlFlag.OPTIONAL, createTestFailingPlugin())
689+
),
690+
// optional plugin returns false => false
691+
NewTest(false, createUnallowedProject()),
692+
NewTest(false, createUnallowedGroup()),
693+
NewTest(false, createAllowedProject()),
694+
NewTest(false, createAllowedGroup()))
695+
},
696+
{
697+
new StackSetup(
698+
NewStack(AuthControlFlag.REQUIRED,
699+
new AuthorizationPlugin(AuthControlFlag.OPTIONAL, createAllowedPrefixPlugin())
700+
),
701+
// optional plugin returns false => false
702+
NewTest(false, createUnallowedProject()),
703+
NewTest(false, createUnallowedGroup()),
704+
// optional plugin returns true => true
705+
NewTest(true, createAllowedProject()),
706+
NewTest(true, createAllowedGroup()))
707+
},
657708
};
658709
}
659710

@@ -668,11 +719,13 @@ public void testPluginsGeneric() {
668719
for (TestCase innerSetup : setup.setup) {
669720
String entityName = (innerSetup.entity == null ? "null" : innerSetup.entity.getName());
670721
try {
722+
// group test
671723
actual = framework.isAllowed(innerSetup.request, (Group) innerSetup.entity);
672724
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, entityName),
673725
innerSetup.expected,
674726
actual);
675727
} catch (ClassCastException ex) {
728+
// project test
676729
actual = framework.isAllowed(innerSetup.request, (Project) innerSetup.entity);
677730
Assert.assertEquals(String.format(format, setup.toString(), innerSetup.expected, actual, entityName),
678731
innerSetup.expected,

0 commit comments

Comments
 (0)