From 0c70faec609ab5051b104b09b4eccf07ae7278b1 Mon Sep 17 00:00:00 2001 From: Ryan Goulding Date: Mon, 14 Aug 2017 15:48:46 -0400 Subject: [PATCH] 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 --- .../odl/CredentialServiceAuthProvider.java | 59 +++---------------- .../blueprint/aaa-authn-netconf.xml | 10 ++-- .../CredentialServiceAuthProviderTest.java | 35 +---------- 3 files changed, 14 insertions(+), 90 deletions(-) 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 3d86ee1751..30a2149456 100644 --- 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 - private volatile CredentialAuth nullableCredService; - private final ServiceTracker listenerTracker; + private final CredentialAuth credService; - public CredentialServiceAuthProvider(final BundleContext bundleContext) { - - final ServiceTrackerCustomizer customizer = - new ServiceTrackerCustomizer() { - @Override - public CredentialAuth addingService(final ServiceReference reference) { - LOG.trace("Credential service {} added", reference); - nullableCredService = bundleContext.getService(reference); - return nullableCredService; - } - - @Override - public void modifiedService(final ServiceReference reference, - final CredentialAuth service) { - LOG.trace("Replacing modified Credential service {}", reference); - nullableCredService = service; - } - - @Override - public void removedService(final ServiceReference 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 credService) { + this.credService = credService; } /** * Authenticate user. This implementation tracks CredentialAuth<PasswordCredentials> - * 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 6e553714d5..7ee51423e9 100644 --- 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"> - - + + + + - \ No newline at end of file + 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 08456118c3..ea7e1e93fc 100644 --- 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 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")); } -- 2.36.6