using authorization stacks instead of a list #1525

tulinkry · 2017-04-10T14:05:00Z

Plugin stack

This change is to introduce the ability to a plugin stack to be a substack of another plugin stack with similar attributes as the plugins have (1):
- required
- requisite
- sufficient
The substack can contain an arbitrary number of
- other stacks
- plugin classes
The meaning of the three keywords in (1) is not changed and is extended for the entire stacks as well.

There is a default required top level stack for the application with a name "default stack"
Plugin parameters

Another friendliness is that the plugin can now accept an arbitrary parameter included in java Map object. This parameters can be configured in configuration. The parameters can be also used for the entire stack and then it is merged with the plugin specific parameters (the plugin overwrites the conflicts).

However note that this is BC break as the IAuthorizationInterface has changed slightly.

A configuration example can be this configuration with:

two standalone plugins (SamplePlugin, RequisitePlugin)
a required substack
- containing two other plugins

  <void property="pluginStack">
   <void method="add">
    <object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
     <void property="name">
      <string>SamplePlugin</string>
     </void>
     <void property="role">
      <string>SUFFICIENT</string>
     </void>
    </object>
   </void>
   <void method="add">
    <object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
     <void property="name">
      <string>tulinkry.my.RequisitePlugin</string>
     </void>
     <void property="role">
      <string>REQUIRED</string>
     </void>
     <void property="setup">
		<void method="put">
			<string>config</string>
			<string>/home/ktulinge/my.xml</string>
		</void>
		<void method="put">
			<string>group</string>
			<object idref="group"/>
		</void>
     </void>
    </object>
   </void>
   <void method="add">
    <object class="org.opensolaris.opengrok.authorization.AuthorizationStack">
     <void property="name">
      <string>stack for group 1</string>
     </void>
     <void property="role">
      <string>REQUIRED</string>
     </void>
     <void method="add">
		<object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
		 <void property="name">
		  <string>org.opensolaris.opengrok.authorization.plugins.GroupPlugin</string>
		 </void>
		 <void property="role">
		  <string>SUFFICIENT</string>
		 </void>
		 <void property="setup">
		 	<void method="put">
		 	  <string>groups</string>
              <array class="java.lang.String" length="2">
               <void index="0">
                 <string>1</string>
               </void>
               <void index="1">
                 <string>2</string>
               </void>
              </array>
		 	</void>
         </void>
		</object>
     </void>
     <void method="add">
		<object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
		 <void property="name">
		  <string>Plugin</string>
		 </void>
		 <void property="role">
		  <string>REQUIRED</string>
		 </void>
		</object>
     </void>
    </object>
   </void>
  </void>

Also note this parts which is how you can set the parameters to the plugin class - property setup which is a Map.

  <void property="groups">
   <void method="add">
    <object class="org.opensolaris.opengrok.configuration.Group"  id="group">
     <void property="name">
      <string>3</string>
     </void>
     <void property="pattern">
      <string>.*\([456]th copy\)</string>
     </void>
    </object>
   </void>
  </void>

  <!-- group with id="group" declared above and the reference can be used later in this file -->

   <void method="add">
    <object class="org.opensolaris.opengrok.authorization.AuthorizationPlugin">
     <void property="name">
      <string>tulinkry.my.RequisitePlugin</string>
     </void>
     <void property="role">
      <string>REQUIRED</string>
     </void>
     <void property="setup">
		<void method="put">
			<string>config</string> <!-- PARAMETER KEY -->
			<string>/home/ktulinge/my.xml</string> <!-- PARAMETER VALUE -->
		</void>
		<void method="put">
			<string>group</string> <!-- PARAMETER KEY -->
			<object idref="group"/> <!-- PARAMETER VALUE => here a group declared in this configuration before this statement -->
		</void>
     </void>
    </object>
   </void>

tulinkry · 2017-04-10T14:07:19Z

src/org/opensolaris/opengrok/authorization/AuthControlFlag.java

@@ -0,0 +1,95 @@
+/*


This file is refactored from the AuthorizationCheck.java class

tulinkry · 2017-04-10T14:08:27Z

src/org/opensolaris/opengrok/authorization/AuthorizationCheck.java

@@ -1,155 +0,0 @@
-/*


Refactored to:

AuthorizationEntity.java

AuthControlFlag.java

tulinkry · 2017-04-10T14:11:23Z

test/org/opensolaris/opengrok/authorization/AuthorizationFrameworkTest.java

 public class AuthorizationFrameworkTest {

    private static String pluginDirectory;
-


This file is heavily refactored and changed to easily form the test cases for the authorization and to support a test cases for substacks. All important test functions in the previous version have been converted to the array of parameters in the new version.

tulinkry · 2017-04-10T14:12:54Z

test/org/opensolaris/opengrok/web/ProjectHelperTestBase.java

-        } catch (Exception ex) {
-            Assert.fail("invokeRemoveAll should not throw an exception");
-        }
+        env.setPluginStack(new AuthorizationStack(AuthControlFlag.REQUIRED, "default-stack"));


This is necessary because the application changes the stack at runtime directly into env and does not have its local copy in the auth. framework.

vladak · 2017-04-21T13:43:04Z

src/org/opensolaris/opengrok/authorization/AuthControlFlag.java

+
+    private final String role;
+
+    private AuthControlFlag(String role) {


there is still a bit of schizophrenia in the naming - role vs. control flag.

so all should be flag as in pam list?

vladak · 2017-04-25T15:09:15Z

src/org/opensolaris/opengrok/authorization/AuthorizationEntity.java

+import org.opensolaris.opengrok.configuration.Nameable;
+
+/**
+ *


should have some description of the purpose

vladak · 2017-04-25T15:13:08Z

src/org/opensolaris/opengrok/authorization/AuthorizationStack.java

+    }
+
+    /**
+     * Load all authorization checks in this stack.


"checks" should probably be replaced with different term.

vladak · 2017-04-26T12:58:05Z

src/org/opensolaris/opengrok/authorization/AuthControlFlag.java

+        return SUFFICIENT.equals(this);
+    }
+
+    public static AuthControlFlag get(String flag) {


vladak · 2017-04-26T13:01:45Z

src/org/opensolaris/opengrok/authorization/AuthControlFlag.java

+        try {
+            return AuthControlFlag.valueOf(flag.toUpperCase());
+        } catch (IllegalArgumentException ex) {
+            // role does not exist -> add some more info about which roles do exist


vladak · 2017-04-26T13:02:02Z

src/org/opensolaris/opengrok/authorization/AuthControlFlag.java

+        } catch (IllegalArgumentException ex) {
+            // role does not exist -> add some more info about which roles do exist
+            throw new IllegalArgumentException(
+                    String.format("No authorization flag \"%s\", available flags are [%s]. %s",


control flag

vladak · 2017-04-26T13:04:44Z

src/org/opensolaris/opengrok/authorization/AuthorizationEntity.java

+     * Test the given entity if it should be allowed with this authorization
+     * check.
+     *
+     * @param entity the given entity


project/group - the terminology clashes a bit here

added a more detailed comment

vladak · 2017-04-26T13:11:07Z

src/org/opensolaris/opengrok/authorization/AuthorizationPlugin.java

+        try {
+            /**
+             * The exception should not happen here as we already have an
+             * instance of IAuthoriazationPlugin. But is is required by the


vladak · 2017-04-26T13:36:18Z

src/org/opensolaris/opengrok/authorization/AuthorizationStack.java

+     * Load all authorization entities in this stack.
+     *
+     * <p>
+     * If the method is unable to load all the entities contained in this stack


does not match the code below + explain the rationale (why this is done)

changed both code and the comment + added the purpose

vladak · 2017-04-26T13:41:46Z

src/org/opensolaris/opengrok/authorization/AuthorizationStack.java

+     *
+     * @param s the entity to remove
+     */
+    public void remove(AuthorizationEntity s) {


how does the unload/remove work in general w.r.t. incoming requests ? If the Configuration is being reloaded and the authorization stack deconstructed what mechanism is used to ensure that the evaluation of the request waits until the stack is fully loaded ?

I added a synchronized block over the stack replacement and changed couple of stuff:

the stack is copied from the configuration (not just the reference)

The reload is made like:

construct a new copy of the configuration stack

load plugins into this new copy

replace the old stack with the new stack in the synchronized block (this should be fast as it's just a reference swap)

free the old stack

vladak · 2017-04-26T13:45:03Z

src/org/opensolaris/opengrok/authorization/AuthorizationStack.java

+    /**
+     * Set the plugin to all classes in this stack which requires this class in
+     * the configuration. This creates a new instance of the plugin for each
+     * class which needs it.


explain why this is necessary

vladak · 2017-04-26T13:52:03Z

Other than the partial comments I had this looks good, feel free to merge.

tulinkry · 2017-04-28T00:08:12Z

Check the new addition, it should be fine by now

vladak · 2017-04-28T08:52:30Z

src/org/opensolaris/opengrok/authorization/AuthorizationFramework.java

-        loadAllPlugins();
+        loadAllPlugins(newStack);
+
+        // replace the stack in a synchronized block


explain why synchonized is used

explained, waiting for the travis to finish

vladak · 2017-04-28T09:19:50Z

Ok, merge then.

tulinkry self-assigned this Apr 10, 2017

tulinkry added the enhancement label Apr 10, 2017

tulinkry commented Apr 10, 2017

View reviewed changes

This was referenced Apr 10, 2017

Easily discover groups in authorization plugins #1527

Merged

Two plugin classes implemented in OpenGrok - groups and projects restrictions #1528

Closed

tulinkry added this to the 1.1 milestone Apr 21, 2017

tulinkry force-pushed the auth-stacks branch from f6055d7 to 4ff9166 Compare April 21, 2017 11:10

tulinkry removed this from the 1.1 milestone Apr 21, 2017

vladak reviewed Apr 21, 2017

View reviewed changes

tulinkry force-pushed the auth-stacks branch 3 times, most recently from a3a5643 to e04b811 Compare April 24, 2017 16:14

tulinkry added 3 commits April 24, 2017 18:17

using authorization stacks instead of a list

328aa85

role -> flag globally

3fe807e

always invoke the set plugin method

6b0b10f

tulinkry force-pushed the auth-stacks branch from e04b811 to 6b0b10f Compare April 24, 2017 16:17

vladak reviewed Apr 25, 2017

View reviewed changes

do not share instances of plugins in the stack

807497f

tulinkry mentioned this pull request Apr 25, 2017

Support inheritance in plugin classes #1551

Merged

vladak reviewed Apr 26, 2017

View reviewed changes

tulinkry force-pushed the auth-stacks branch from 46036d6 to 06169d8 Compare April 26, 2017 13:38

vladak reviewed Apr 26, 2017

View reviewed changes

tulinkry force-pushed the auth-stacks branch from 06169d8 to 108f4ea Compare April 26, 2017 13:45

tulinkry added 2 commits April 26, 2017 15:51

documenting the authorization classes

29b3e55

fixing javadoc

8b1918b

tulinkry force-pushed the auth-stacks branch from 108f4ea to 8b1918b Compare April 26, 2017 13:51

vladak reviewed Apr 28, 2017

View reviewed changes

ensuring thread safe framework reload

18c99e4

tulinkry force-pushed the auth-stacks branch from 7caa012 to 18c99e4 Compare April 28, 2017 09:53

tulinkry merged commit 0770c90 into oracle:master Apr 28, 2017

tulinkry deleted the auth-stacks branch April 28, 2017 09:59

tulinkry added the authorization label May 5, 2017

tulinkry mentioned this pull request May 5, 2017

update doc for authorization #1579

Open

		public class AuthorizationFrameworkTest {

		private static String pluginDirectory;


		private final String role;

		private AuthControlFlag(String role) {

using authorization stacks instead of a list #1525

using authorization stacks instead of a list #1525

Uh oh!

Conversation

tulinkry commented Apr 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladak commented Apr 26, 2017

Uh oh!

tulinkry commented Apr 28, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladak commented Apr 28, 2017

Uh oh!

Uh oh!

tulinkry commented Apr 10, 2017 •

edited

Loading