Directly inject CredentialAuth dependency

In today's blueprint-oriented OSGi DI, it is more efficient to
express the dependency on AAA's CredentialAuth API rather than to
spin waiting for a ServiceReference using a ServiceTracker.  This
change specifies direct injection of AAA's CredentialAuth Service
through blueprint wiring.

Change-Id: Iec02f8acc4763922505881a772ff128f1f9d9986
Signed-off-by: Ryan Goulding <ryandgoulding@gmail.com>

diff --git a/netconf/aaa-authn-odl-plugin/src/main/java/org/opendaylight/aaa/odl/CredentialServiceAuthProvider.java b/netconf/aaa-authn-odl-plugin/src/main/java/org/opendaylight/aaa/odl/CredentialServiceAuthProvider.java
index 3d86ee1751adfd5037c8c369a42124e4a3f0f442..30a2149456d252d61dc181a95aeda667b6243eb9 100644 (file)
--- a/netconf/aaa-authn-odl-plugin/src/main/java/org/opendaylight/aaa/odl/CredentialServiceAuthProvider.java
+++ b/netconf/aaa-authn-odl-plugin/src/main/java/org/opendaylight/aaa/odl/CredentialServiceAuthProvider.java
@@ -12,10 +12,6 @@ import org.opendaylight.aaa.api.Claim;
 import org.opendaylight.aaa.api.CredentialAuth;
 import org.opendaylight.aaa.api.PasswordCredentials;
 import org.opendaylight.netconf.auth.AuthProvider;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceReference;
-import org.osgi.util.tracker.ServiceTracker;
-import org.osgi.util.tracker.ServiceTrackerCustomizer;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -23,59 +19,27 @@ import org.slf4j.LoggerFactory;
 /**
  * AuthProvider implementation delegating to AAA CredentialAuth<PasswordCredentials> instance.
  */
-public final class CredentialServiceAuthProvider implements AuthProvider, AutoCloseable {
+public final class CredentialServiceAuthProvider implements AuthProvider {
     private static final Logger LOG = LoggerFactory.getLogger(CredentialServiceAuthProvider.class);
 
     // FIXME CredentialAuth is generic and it causes warnings during compilation
     // Maybe there should be a PasswordCredentialAuth implements CredentialAuth<PasswordCredentials>
-    private volatile CredentialAuth<PasswordCredentials> nullableCredService;
-    private final ServiceTracker<CredentialAuth, CredentialAuth> listenerTracker;
+    private final CredentialAuth<PasswordCredentials> credService;
 
-    public CredentialServiceAuthProvider(final BundleContext bundleContext) {
-
-        final ServiceTrackerCustomizer<CredentialAuth, CredentialAuth> customizer =
-                new ServiceTrackerCustomizer<CredentialAuth, CredentialAuth>() {
-            @Override
-            public CredentialAuth addingService(final ServiceReference<CredentialAuth> reference) {
-                LOG.trace("Credential service {} added", reference);
-                nullableCredService = bundleContext.getService(reference);
-                return nullableCredService;
-            }
-
-            @Override
-            public void modifiedService(final ServiceReference<CredentialAuth> reference,
-                                        final CredentialAuth service) {
-                LOG.trace("Replacing modified Credential service {}", reference);
-                nullableCredService = service;
-            }
-
-            @Override
-            public void removedService(final ServiceReference<CredentialAuth> reference, final CredentialAuth service) {
-                LOG.trace("Removing Credential service {}. "
-                        + "This AuthProvider will fail to authenticate every time", reference);
-                synchronized (CredentialServiceAuthProvider.this) {
-                    nullableCredService = null;
-                }
-            }
-        };
-        listenerTracker = new ServiceTracker<>(bundleContext, CredentialAuth.class, customizer);
-        listenerTracker.open();
+    public CredentialServiceAuthProvider(final CredentialAuth<PasswordCredentials> credService) {
+        this.credService = credService;
     }
 
     /**
      * Authenticate user. This implementation tracks CredentialAuth&lt;PasswordCredentials&gt;
-     * and delegates the decision to it. If the service is not available, IllegalStateException is thrown.
+     * and delegates the decision to it.
      */
     @Override
-    public synchronized boolean authenticated(final String username, final String password) {
-        if (nullableCredService == null) {
-            LOG.warn("Cannot authenticate user '{}', Credential service is missing", username);
-            throw new IllegalStateException("Credential service is not available");
-        }
+    public boolean authenticated(final String username, final String password) {
 
         Claim claim;
         try {
-            claim = nullableCredService.authenticate(new PasswordCredentialsWrapper(username, password));
+            claim = credService.authenticate(new PasswordCredentialsWrapper(username, password));
         } catch (AuthenticationException e) {
             LOG.debug("Authentication failed for user '{}' : {}", username, e);
             return false;
@@ -85,15 +49,6 @@ public final class CredentialServiceAuthProvider implements AuthProvider, AutoCl
         return true;
     }
 
-    /**
-     * Invoked by blueprint.
-     */
-    @Override
-    public void close() {
-        listenerTracker.close();
-        nullableCredService = null;
-    }
-
     private static final class PasswordCredentialsWrapper implements PasswordCredentials {
         private final String username;
         private final String password;

diff --git a/netconf/aaa-authn-odl-plugin/src/main/resources/org/opendaylight/blueprint/aaa-authn-netconf.xml b/netconf/aaa-authn-odl-plugin/src/main/resources/org/opendaylight/blueprint/aaa-authn-netconf.xml
index 6e553714d5fdeab900d80157ac738c38c123e385..7ee51423e955381380d5d9f4b6cc00ed0e48e651 100644 (file)
--- a/netconf/aaa-authn-odl-plugin/src/main/resources/org/opendaylight/blueprint/aaa-authn-netconf.xml
+++ b/netconf/aaa-authn-odl-plugin/src/main/resources/org/opendaylight/blueprint/aaa-authn-netconf.xml
@@ -10,12 +10,12 @@
            xmlns:odl="http://opendaylight.org/xmlns/blueprint/v1.0.0"
            odl:use-default-for-reference-types="true">
 
-    <bean id="credentialServiceAuthProvider"
-          class="org.opendaylight.aaa.odl.CredentialServiceAuthProvider"
-          destroy-method="close">
-        <argument ref="blueprintBundleContext"/>
+    <reference id="credentialAuth" interface="org.opendaylight.aaa.api.CredentialAuth" odl:type="default" />
+
+    <bean id="credentialServiceAuthProvider" class="org.opendaylight.aaa.odl.CredentialServiceAuthProvider">
+        <argument ref="credentialAuth"/>
     </bean>
     <service ref="credentialServiceAuthProvider" interface="org.opendaylight.netconf.auth.AuthProvider"
              odl:type="netconf-auth-provider"/>
 
-</blueprint>
\ No newline at end of file
+</blueprint>

diff --git a/netconf/aaa-authn-odl-plugin/src/test/java/org/opendaylight/aaa/odl/CredentialServiceAuthProviderTest.java b/netconf/aaa-authn-odl-plugin/src/test/java/org/opendaylight/aaa/odl/CredentialServiceAuthProviderTest.java
index 08456118c3bc2fe95604df3cc161749392c35110..ea7e1e93fc2f29cbe177d65d13265c2ec89fcd93 100644 (file)
--- a/netconf/aaa-authn-odl-plugin/src/test/java/org/opendaylight/aaa/odl/CredentialServiceAuthProviderTest.java
+++ b/netconf/aaa-authn-odl-plugin/src/test/java/org/opendaylight/aaa/odl/CredentialServiceAuthProviderTest.java
@@ -8,11 +8,8 @@
 package org.opendaylight.aaa.odl;
 
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Matchers.any;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
@@ -27,9 +24,6 @@ import org.opendaylight.aaa.api.AuthenticationException;
 import org.opendaylight.aaa.api.Claim;
 import org.opendaylight.aaa.api.CredentialAuth;
 import org.opendaylight.aaa.api.PasswordCredentials;
-import org.osgi.framework.BundleContext;
-import org.osgi.framework.Filter;
-import org.osgi.framework.ServiceEvent;
 import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceReference;
 
@@ -37,56 +31,31 @@ public class CredentialServiceAuthProviderTest {
 
     @Mock
     private CredentialAuth<PasswordCredentials> credAuth;
-    @Mock
-    private BundleContext ctx;
 
     @Before
     public void setUp() throws Exception {
         MockitoAnnotations.initMocks(this);
-        doReturn(mock(Filter.class)).when(ctx).createFilter(anyString());
     }
 
-    @Test(expected = IllegalStateException.class)
-    public void testAuthenticatedNoDelegate() throws Exception {
-        CredentialServiceAuthProvider credentialServiceAuthProvider = new CredentialServiceAuthProvider(ctx);
-        credentialServiceAuthProvider.authenticated("user", "pwd");
-    }
 
     @Test
     public void testAuthenticatedTrue() throws Exception {
         ServiceReference serviceRef = mock(ServiceReference.class);
 
         ServiceListenerAnswer answer = new ServiceListenerAnswer();
-        doAnswer(answer).when(ctx).addServiceListener(any(ServiceListener.class), anyString());
 
         Claim claim = mock(Claim.class);
         doReturn("domain").when(claim).domain();
         doReturn(claim).when(credAuth).authenticate(any(PasswordCredentials.class));
 
-        doReturn(credAuth).when(ctx).getService(serviceRef);
-        CredentialServiceAuthProvider credentialServiceAuthProvider = new CredentialServiceAuthProvider(ctx);
-
-        answer.serviceListener.serviceChanged(new ServiceEvent(ServiceEvent.REGISTERED, serviceRef));
-        assertNotNull(answer.serviceListener);
-
+        CredentialServiceAuthProvider credentialServiceAuthProvider = new CredentialServiceAuthProvider(credAuth);
         assertTrue(credentialServiceAuthProvider.authenticated("user", "pwd"));
     }
 
     @Test
     public void testAuthenticatedFalse() throws Exception {
-        ServiceReference serviceRef = mock(ServiceReference.class);
-
-        ServiceListenerAnswer answer = new ServiceListenerAnswer();
-        doAnswer(answer).when(ctx).addServiceListener(any(ServiceListener.class), anyString());
-
         doThrow(AuthenticationException.class).when(credAuth).authenticate(any(PasswordCredentials.class));
-
-        doReturn(credAuth).when(ctx).getService(serviceRef);
-        CredentialServiceAuthProvider credentialServiceAuthProvider = new CredentialServiceAuthProvider(ctx);
-
-        answer.serviceListener.serviceChanged(new ServiceEvent(ServiceEvent.REGISTERED, serviceRef));
-        assertNotNull(answer.serviceListener);
-
+        CredentialServiceAuthProvider credentialServiceAuthProvider = new CredentialServiceAuthProvider(credAuth);
         assertFalse(credentialServiceAuthProvider.authenticated("user", "pwd"));
     }