StaticMapExclusionFilterFactory and StaticListExclusionFilterFactory will not work via AccessControlSettingOperation.factory

[arderyp]

ComplexAccessPoint.xml lists an example of how to apply an exclusion access control to requests made from outside a specified IP range: https://github.com/iipc/openwayback/blob/master/wayback-webapp/src/main/webapp/WEB-INF/ComplexAccessPoint.xml#L65-88

Historically, we have used something very similar at the Library of Congress, but instead of using OracleExclusionFilterFactory, we use StaticMapExclusionFilterFactory, and follow the exact format outlined in the example that I linked above. So, our code might look like:

<property name="authentication">
    <bean class="org.archive.wayback.authenticationcontrol.AccessControlSettingOperation">
        <property name="operator">
            <bean class="org.archive.wayback.util.operator.NotBooleanOperator">
                <property name="operand">
                    <bean class="org.archive.wayback.authenticationcontrol.IPMatchesBooleanOperator">
                        <property name="allowedRanges">
                            <list>
                                <value>192.168.1.16/24</value>
                           </list>
                       </property> 
                    </bean>
                </property>
            </bean>
        </property>
        <property name="factory">
            <bean class="org.archive.wayback.accesscontrol.staticmap.StaticMapExclusionFilterFactory" init-method="init">
                <property name="file" value="${wayback.basedir}/exclude.txt" />
                <property name="checkInterval" value="600" />
            </bean>
        </property>
    </bean>
</property>

This works for us in Wayback 1.6 and 1.8. However, on the Openwayback 2.1.0 instance that we just installed, this same logic does not work. Using the StaticMapExclusionFilterFactory as the exclusionFactory property of the AccessPoint itself works, something like:

<bean name="8080:accesspoint" class="org.archive.wayback.webapp.AccessPoint">
    ...
    <property name="exclusionFactory">
        <bean class="org.archive.wayback.accesscontrol.staticmap.StaticListExclusionFilterFactory" init-method="init">
            <property name="file" value="${wayback.basedir}/exclude.txt" />
            <property name="checkInterval" value="600" />
        </bean>
    </property>
</bean>

Likewise, using the StaticListExclusionFilterFactory as the AccessPoint's exclusionFactory property works. Tomcat is able to restart without errors when using StaticMapExclusionFilterFactory / StaticListExclusionFilterFactory with AccessControllSetting.factory, but the application does not exclude URLs provided in the defined exclude.txt file.

I've reviewed some of the recent-ish commits to the code involved in this process, and nothing is jumping out to me as an immediate red flag. After that, and doing loads of testing to no avail, I'm left scratching my head on this one. I could definitely be overlooking something more obvious to the folks familiar with this code base, so if that is the case, please let me know.

I am not seeing any helpful information in my Tomcat logs, but if I can provide any further environmental/testing info to assist in the investigation of this issue, just let me know.

Thanks in advance for the assistance.
Phil / pard@loc.gov

EDIT: I believe Gina brought up this same issue in 2014 in the following google group exhange: https://groups.google.com/forum/#!topic/openwayback-dev/2nANsg14QJE

[dmolesUC]

Do we happen to know if the example in the docs still works? I.e., do we know this is specific to StaticMapExclusionFilterFactory?

[arderyp]

Are you referring to the following example in the ComplexAccessPoint.xml? I have not tested it. That is a database driven functionality, no? That was my assumption, which is why I didn't test, as we don't have the database back-end. If not, I could test it out, but am not sure what else would need to be configured for the Oracle features to work.

Also, we do know that this is not specific to StaticMapExclusionFilterFactory, as it is also an issue with StaticListExclusionFilterFactory, at least that's what my testing suggested.

[dmolesUC]

I was. I haven't gotten into it very far, but I suspect the problem is most likely to be in IPMatchesBooleanOperator just because that's where the complexity is -- I'm guessing it never gets to the factory, in which case it would also be a problem with the OracleExclusionFilterFactory configuration. There haven't been a lot of changes in IPMatchesBooleanOperator betwen 1.6 and now, as far as I can tell, but tomorrow I'll write some unit tests and see what I can figure out.

[arderyp]

Awesome, thanks. I will give IPMatchesBooleanOperator another look as well.

[arderyp]

I should note, the IPMatchedBooleanOperator itself works for me when applied to the authentication property without the exclusion list/bean. For example, IP exclusion with the following access point property works as expected:

<property name="authentication">
      <bean class="org.archive.wayback.authenticationcontrol.IPMatchesBooleanOperator">
        <property name="allowedRanges">
          <list>
            <value>(IP RANGE)</value>
          </list>
        </property>
      </bean>
    </property>

This will request anyone from outside of (IP RANGE) to authenticate to access the AccessPoint. However, the following does not work to achieve the opposite effect (require anyone on IP RANGE to authenticate and allow anyone outside of IP RANGE to access freely):

<property name="authentication">
      <bean class="org.archive.wayback.authenticationcontrol.AccessControlSettingOperation">
        <property name="operator">
          <bean class="org.archive.wayback.util.operator.NotBooleanOperator">
            <property name="operand">
              <bean class="org.archive.wayback.authenticationcontrol.IPMatchesBooleanOperator">
                <property name="allowedRanges">
                  <list>
                    <value>(IP RANGE)</value>
                  </list>
                </property>
              </bean>
            </property>
          </bean>
        </property>
      </bean>
    </property>

When I use the above, I am still able to access the AccessPoint from within IP RANGE. When I am outside of that range, however, I no longer get a request to authenticate, but rather get a blank screen.

EDIT: the reverse IP list exclusion does work, just not as written above (which you'd probably expect, the AccessControlSettingOperation was not needed):

<property name="authentication">
      <bean class="org.archive.wayback.util.operator.NotBooleanOperator">
        <property name="operand">
          <bean class="org.archive.wayback.authenticationcontrol.IPMatchesBooleanOperator">
            <property name="allowedRanges">
              <list>
                <value>(IP RANGE)</value>
              </list>
            </property>
          </bean>
        </property>
      </bean>
    </property>

[dmolesUC]

AccessControlSettingOperation always returns true, regardless of the operators -- what the operators do is determine whether it sets a filter on the request. So that makes sense, though it took me a while to figure out. :)

[dmolesUC]

Let me make sure I understand the use case:

1. If someone's in the specified IP address range (e.g. 192.168.1.16/24), return all results unconditionally.
2. If someone's outside that address range, return only those results whose URIs are not listed in exclude.txt.

Is that correct?

[arderyp]

Yup, that's it exactly. I was coming to the same conclusions as you re: AccessControlSettingOperation. As far as I can tell:

1. the ACSO operator is indeed set
2. the operator logic works of its own accord (outside of this use case)
3. the ACSO factory is indeed set
4. the factory works of it's own accord (outside of this use case)

Thus, I am not sure why this wouldn't kick in and limit my access when I'm off the IP RANGE. Maybe I'm misinterpreting it.

[dmolesUC]

So, what we don't have is a proven case of ACSO + filter factory working (with any filter factory).

Here's my current theory: I have unit tests that show that the ACSO + factory setup should work, in isolation, but it's not working in the real world. So maybe tthere's something different between the code path when you set the exclusionFactory property directly on the AccessPoint, and the code path when you set it only indirectly via the ACSO. Either the filters don't get applied or, once applied, they don't get called, for some reason?

But it's going to take some digging into AccessPoint to examine that theory, and it's a monster, as AccessPointTest points out:

 * TODO: this unit test is too complex. it is because AccessPoint class has too
 * much responsibility and many execution paths. some good refactoring of
 * AccessPoint class would help.

[arderyp]

@dmolesUC3, that seems like a reasonable enough theory, and AccessPoint seems like a logical place to check next, particularly because:

1. the logic seems to be there in the other pieces (ACSO, IPMBO, SLEFF, etc)
2. testing indicates that this functionality worked in wayback 1.8 and 1.6
3. there seem to have been more changes to AccessPoint over time than the other pieces

I will continue to poke around.

[ldko]

As is, AccessControlSettingOperation sets the exclusionFilter on a WebRequest directly. In AccessPoint, it used to be that exclusionFilter would only be set on the WebRequest instance if the AccessPoint.exclusionFactory was not null. When that logic in AccessPoint moved into createExclusionFilter() between OpenWayback 2.0.0 and 2.1.0 (1a350bb#diff-e00aad5f52c8452f5e58d3edc6a45479R292) the code changed so the exclusionFilter would always be set by AccessPoint, and if the exclusionFactory is null (like in LOC use case where the exclusionFactory is on AccessControlSettingOperation not the AccessPoint), the exclusionFilter is set to null.

I am proposing a fix that just has AccessPoint check if the WebRequest instance already has exclusionFilter set (not to null) to determine if it should call createExclusionFilter(). This way, if AccessControlSettingOperation sets an exclusionFilter, it will not get overwritten.

@ptrourke

[dmolesUC]

Makes sense to me. Thanks for looking into this, Lauren.

[kngenie]

Ouch, clearly my oversight. Now I fully understand the use case of authentication.
As the intent of 1a350bb was to (eventually) eliminate this kind of confusing way of configuring access-control / exclusion, will you let me think about a slightly different change + test case...?

(if both authentication and exclusionFilterFactory are set, the latter can be silently ignored. that sounds confusing.)

[kngenie]

OK, I tried and found it takes too much effort to clean it up. I just added a test case to detect this issue.

Toward 3.0, I'd propose redesigning AccessControlSettingOperation as ContextExclusionFilterFactory and have it set up through AccessPoint.exclusionFactory. It'll require adding WaybackRequest (or narrower interface) to its 'getExclusionFiltler' method.

Until then, exclusion filter set up through authentication does not work with CDXServer (sorry, I broke it in 1a350bb.)

[ldko]

I agree that AccessControlSettingOperation should not remain as it is and redesigning it using ContextExclusionFilterFactory makes sense.

[kris-sigur]

Is there an issue open for such a redesign? If not, could someone please write it up.

Also, is everyone happy with the fix @ldko proposed in #287 and @kngenie addition in #289 ? If no one objects to those, I'll merge them tomorrow and close this issue.

[gmj2053]

This is our ticket, I don't think we have any objections, Lauren, do we (LC) need to write up the issue for the redesign?

[ldko]

I opened issue #290. It should be put on the 3.0.0 Major Release milestone.

[ptrourke]

Yes, thanks, @ldko, for looking at this!