DELETE for /mfa-providers/id

[#151224810] https://www.pivotaltracker.com/story/show/151224810 Signed-off-by: Bharath Sekar <bharath.sekar@ge.com>
cloudfoundry · Oct 13, 2017 · b8b56e3 · b8b56e3
1 parent b2946fc
commit b8b56e3
Show file tree

Hide file tree

Showing 11 changed files with 213 additions and 30 deletions.
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/audit/event/SystemDeletable.java b/server/src/main/java/org/cloudfoundry/identity/uaa/audit/event/SystemDeletable.java
@@ -17,6 +17,7 @@
 
 import org.apache.commons.logging.Log;
 import org.cloudfoundry.identity.uaa.constants.OriginKeys;
+import org.cloudfoundry.identity.uaa.mfa_provider.MfaProvider;
 import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
 import org.cloudfoundry.identity.uaa.user.UaaUser;
@@ -61,6 +62,10 @@ default void onApplicationEvent(EntityDeletedEvent<?> event) {
             String zoneId = ((ScimUser) event.getDeleted()).getZoneId();
             getLogger().debug(String.format("Received SCIM user deletion event for zone_id:%s and user:%s", zoneId, userId));
             deleteByUser(userId, zoneId);
+        } else if (event.getDeleted() instanceof MfaProvider<?>) {
+            String providerId = ((MfaProvider) event.getDeleted()).getId();
+            String zoneId = IdentityZoneHolder.get().getId();
+            deleteByMfaProvider(providerId, zoneId);
         } else {
             getLogger().debug("Unsupported deleted event for deletion of object:"+event.getDeleted());
         }
@@ -76,13 +81,25 @@ default boolean isUaaZone(String zoneId) {
         return IdentityZone.getUaa().getId().equals(zoneId);
     }
 
-    int deleteByIdentityZone(String zoneId);
+    default int deleteByIdentityZone(String zoneId) {
+        return 0;
+    }
 
-    int deleteByOrigin(String origin, String zoneId);
+    default int deleteByOrigin(String origin, String zoneId) {
+        return 0;
+    }
 
-    int deleteByClient(String clientId, String zoneId);
+    default int deleteByClient(String clientId, String zoneId) {
+        return 0;
+    }
 
-    int deleteByUser(String userId, String zoneId);
+    default int deleteByUser(String userId, String zoneId) {
+        return 0;
+    }
+
+    default int deleteByMfaProvider(String id, String zoneId) {
+        return 0;
+    }
 
     Log getLogger();
 }
diff --git a/...src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/JdbcMfaProviderProvisioning.java b/...src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/JdbcMfaProviderProvisioning.java
@@ -32,6 +32,7 @@ public class JdbcMfaProviderProvisioning implements MfaProviderProvisioning, Sys
 
     public static final String MFA_PROVIDER_BY_ID_QUERY = "select " + MFA_PROVIDER_FIELDS + " from " + TABLE_NAME + " where id=? and identity_zone_id=?";
     public static final String MFA_PROVIDERS_QUERY = "select " + MFA_PROVIDER_FIELDS + " from " + TABLE_NAME + " where identity_zone_id=?";
+    public static final String MFA_PROVIDER_DELETE_BY_ID = "delete from " + TABLE_NAME + " where id =? and identity_zone_id=?";
 
     protected final JdbcTemplate jdbcTemplate;
     private MfaProviderValidator mfaProviderValidator;
@@ -101,23 +102,8 @@ public List<MfaProvider> retrieveAll(String zoneId) {
     }
 
     @Override
-    public int deleteByIdentityZone(String zoneId) {
-        return 0;
-    }
-
-    @Override
-    public int deleteByOrigin(String origin, String zoneId) {
-        return 0;
-    }
-
-    @Override
-    public int deleteByClient(String clientId, String zoneId) {
-        return 0;
-    }
-
-    @Override
-    public int deleteByUser(String userId, String zoneId) {
-        return 0;
+    public int deleteByMfaProvider(String providerId, String zoneId) {
+        return jdbcTemplate.update(MFA_PROVIDER_DELETE_BY_ID, providerId, zoneId);
     }
 
     @Override

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderEndpoints.java
@@ -2,19 +2,24 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.context.ApplicationEventPublisherAware;
 import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.http.HttpStatus;
 import org.springframework.http.ResponseEntity;
+import org.springframework.security.core.context.SecurityContextHolder;
 import org.springframework.util.StringUtils;
 import org.springframework.web.bind.annotation.ExceptionHandler;
 import org.springframework.web.bind.annotation.PathVariable;
-import org.springframework.web.bind.annotation.*;
+import org.springframework.web.bind.annotation.RequestBody;
+import org.springframework.web.bind.annotation.RequestMapping;
+import org.springframework.web.bind.annotation.RestController;
 
 import java.util.List;
 
+import static org.springframework.web.bind.annotation.RequestMethod.DELETE;
 import static org.springframework.web.bind.annotation.RequestMethod.GET;
 import static org.springframework.web.bind.annotation.RequestMethod.POST;
 import static org.springframework.web.bind.annotation.RequestMethod.PUT;
@@ -28,8 +33,8 @@ public class MfaProviderEndpoints implements ApplicationEventPublisherAware{
     private MfaProviderValidator mfaProviderValidator;
 
     @Override
-    public void setApplicationEventPublisher(ApplicationEventPublisher applicationEventPublisher) {
-        this.publisher = applicationEventPublisher;
+    public void setApplicationEventPublisher(ApplicationEventPublisher publisher) {
+        this.publisher = publisher;
     }
 
     @RequestMapping(method = POST)
@@ -69,6 +74,13 @@ public ResponseEntity<MfaProvider> retrieveMfaProviderById(@PathVariable String
         return new ResponseEntity<>(provider, HttpStatus.OK);
     }
 
+    @RequestMapping(value = "{id}", method = DELETE)
+    public ResponseEntity<MfaProvider> deleteMfaProviderById(@PathVariable String id) {
+        MfaProvider existing = mfaProviderProvisioning.retrieve(id, IdentityZoneHolder.get().getId());
+        publisher.publishEvent(new EntityDeletedEvent<>(existing, SecurityContextHolder.getContext().getAuthentication()));
+        return new ResponseEntity<>(existing, HttpStatus.OK);
+    }
+
     @ExceptionHandler(InvalidMfaProviderException.class)
     public ResponseEntity<InvalidMfaProviderException> handleInvalidMfaProviderException(InvalidMfaProviderException e) {
         return new ResponseEntity<>(e, HttpStatus.UNPROCESSABLE_ENTITY);
@@ -90,5 +102,4 @@ public void setMfaProviderProvisioning(MfaProviderProvisioning mfaProviderProvis
     public void setMfaProviderValidator(MfaProviderValidator mfaProviderValidator) {
         this.mfaProviderValidator = mfaProviderValidator;
     }
-
 }
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderProvisioning.java
@@ -10,5 +10,4 @@ public interface MfaProviderProvisioning {
     MfaProvider retrieve(String id, String zoneId);
 
     List<MfaProvider> retrieveAll(String zoneId);
-
 }
diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/audit/event/SystemDeletableTest.java b/server/src/test/java/org/cloudfoundry/identity/uaa/audit/event/SystemDeletableTest.java
@@ -16,6 +16,8 @@
 package org.cloudfoundry.identity.uaa.audit.event;
 
 import org.apache.commons.logging.Log;
+import org.cloudfoundry.identity.uaa.mfa_provider.GoogleMfaProviderConfig;
+import org.cloudfoundry.identity.uaa.mfa_provider.MfaProvider;
 import org.cloudfoundry.identity.uaa.provider.IdentityProvider;
 import org.cloudfoundry.identity.uaa.scim.ScimUser;
 import org.cloudfoundry.identity.uaa.user.UaaUser;
@@ -32,6 +34,7 @@
 
 import java.util.Arrays;
 
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doCallRealMethod;
 import static org.mockito.Mockito.mock;
@@ -145,6 +148,18 @@ public void user_event_received() throws Exception {
         }
     }
 
+    @Test
+    public void mfa_event_received() throws Exception {
+        MfaProvider<GoogleMfaProviderConfig> mfaProvider = new MfaProvider<GoogleMfaProviderConfig>().setId("provider1");
+        EntityDeletedEvent<MfaProvider> event = new EntityDeletedEvent<>(mfaProvider, authentication);
+        deletable.onApplicationEvent(event);
+        verify(deletable, never()).deleteByIdentityZone(any());
+        verify(deletable, never()).deleteByOrigin(any(), any());
+        verify(deletable, never()).deleteByClient(any(), any());
+        verify(deletable, never()).deleteByUser(any(), any());
+        verify(deletable, times(1)).deleteByMfaProvider(eq("provider1"), any());
+    }
+
     public void resetDeletable() {
         reset(deletable);
         doCallRealMethod().when(deletable).onApplicationEvent(any(EntityDeletedEvent.class));

diff --git a/...test/java/org/cloudfoundry/identity/uaa/mfa_provider/JdbcMfaProviderProvisioningTest.java b/...test/java/org/cloudfoundry/identity/uaa/mfa_provider/JdbcMfaProviderProvisioningTest.java
@@ -3,7 +3,10 @@
 import org.cloudfoundry.identity.uaa.test.JdbcTestBase;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.springframework.dao.EmptyResultDataAccessException;
 import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
 
 import java.util.List;
@@ -17,6 +20,9 @@ public class JdbcMfaProviderProvisioningTest extends JdbcTestBase {
     JdbcMfaProviderProvisioning mfaProviderProvisioning;
     private MfaProviderValidator mfaProviderValidator;
 
+    @Rule
+    public ExpectedException expection = ExpectedException.none();
+
     @Before
     public void setup() {
         mfaProviderValidator = mock(GeneralMfaProviderValidator.class);
@@ -76,7 +82,6 @@ public void testRetrieveAll() {
         assertEquals(1, afterCount-beforeCount);
     }
 
-
     @Test
     public void testRetrieve() {
         MfaProvider mfaProvider = constructGoogleProvider();
@@ -88,6 +93,19 @@ public void testRetrieve() {
         assertNotNull(created.getId());
     }
 
+    @Test
+    public void testDelete() {
+        String zoneId = IdentityZoneHolder.get().getId();
+        doNothing().when(mfaProviderValidator);
+        MfaProvider mfaProvider = mfaProviderProvisioning.create(constructGoogleProvider(), zoneId);
+        assertNotNull(mfaProviderProvisioning.retrieve(mfaProvider.getId(), zoneId));
+
+        mfaProviderProvisioning.deleteByMfaProvider(mfaProvider.getId(), zoneId);
+
+        expection.expect(EmptyResultDataAccessException.class);
+        mfaProviderProvisioning.retrieve(mfaProvider.getId(), zoneId);
+    }
+
     private MfaProvider<GoogleMfaProviderConfig> constructGoogleProvider() {
         return new MfaProvider<GoogleMfaProviderConfig>()
                 .setName(new RandomValueStringGenerator(10).generate())

diff --git a/...er/src/test/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderEndpointsTest.java b/...er/src/test/java/org/cloudfoundry/identity/uaa/mfa_provider/MfaProviderEndpointsTest.java
@@ -1,11 +1,12 @@
 package org.cloudfoundry.identity.uaa.mfa_provider;
 
+import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent;
 import org.cloudfoundry.identity.uaa.zone.IdentityZoneHolder;
 import org.junit.Before;
-import org.junit.Rule;
 import org.junit.Test;
-import org.junit.rules.ExpectedException;
+import org.mockito.ArgumentCaptor;
 import org.mockito.Mockito;
+import org.springframework.context.ApplicationEventPublisher;
 import org.springframework.http.ResponseEntity;
 import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
 
@@ -90,6 +91,22 @@ public void testGetMfaProviderById() {
 
     }
 
+    @Test
+    public void testDeleteMFaProvider() {
+        ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class);
+        endpoint.setApplicationEventPublisher(publisher);
+        MfaProvider<GoogleMfaProviderConfig> providerToDelete = constructGoogleProvider();
+        String id = new RandomValueStringGenerator(5).generate();
+        when(provisioning.retrieve(eq(id), anyString())).thenReturn(providerToDelete);
+
+        ResponseEntity<MfaProvider> mfaDeleteResponse = endpoint.deleteMfaProviderById(id);
+        assertEquals(providerToDelete, mfaDeleteResponse.getBody());
+        ArgumentCaptor<EntityDeletedEvent> entityDeletedCaptor = ArgumentCaptor.forClass(EntityDeletedEvent.class);
+        verify(provisioning, times(1)).retrieve(id, IdentityZoneHolder.get().getId());
+        verify(publisher, times(1)).publishEvent(entityDeletedCaptor.capture());
+        assertEquals(providerToDelete.getId(), ((MfaProvider)(entityDeletedCaptor.getAllValues().get(0)).getDeleted()).getId());
+    }
+
     private MfaProvider<GoogleMfaProviderConfig> constructGoogleProvider() {
         return new MfaProvider()
                 .setName(new RandomValueStringGenerator(5).generate())

diff --git a/uaa/slateCustomizations/source/index.html.md.erb b/uaa/slateCustomizations/source/index.html.md.erb
@@ -1353,6 +1353,7 @@ _Error Codes_
 | 401        | Unauthorized - Invalid token                                                                           |
 | 403        | Forbidden - Insufficient scope (`uaa.admin` or `zones.<zoneId>.admin` is required to create a MFA provider)|
 | 422        | Unprocessable Entity - Some values in the MFA configuration are invalid                                |
+| 404        | Not Found - Provider id not found                                                                      |
 
 
 ## Get
@@ -1382,6 +1383,32 @@ _Error Codes_
 | 404        | Not Found - Provider id not found                                                                      |
 
 
+## Delete
+
+<aside class="warning">
+  MFA support is in active development and is not ready for production use.
+</aside>
+
+<%= ERB.new(File.read("../../build/generated-snippets/MfaProviderEndpointsDocs/testDeleteMfaProvider/curl-request.md")).result(binding) %>
+<%= ERB.new(File.read("../../build/generated-snippets/MfaProviderEndpointsDocs/testDeleteMfaProvider/http-request.md")).result(binding) %>
+<%= ERB.new(File.read("../../build/generated-snippets/MfaProviderEndpointsDocs/testDeleteMfaProvider/http-response.md")).result(binding) %>
+
+_Request Headers_
+
+<%= ERB.new(File.read("../../build/generated-snippets/MfaProviderEndpointsDocs/testDeleteMfaProvider/request-headers.md")).result(binding) %>
+
+_Response Fields_
+
+<%= ERB.new(File.read("../../build/generated-snippets/MfaProviderEndpointsDocs/testDeleteMfaProvider/response-fields.md")).result(binding) %>
+
+_Error Codes_
+
+| Error Code | Description                                                                                            |
+|------------|--------------------------------------------------------------------------------------------------------|
+| 401        | Unauthorized - Invalid token                                                                           |
+| 403        | Forbidden - Insufficient scope (`uaa.admin` or `zones.<zoneId>.admin` is required to create a MFA provider)|
+| 404        | Not Found - Provider id not found                                                                      |
+
 ## List
 
 <aside class="warning">

diff --git a/uaa/src/main/webapp/WEB-INF/spring/multitenant-endpoints.xml b/uaa/src/main/webapp/WEB-INF/spring/multitenant-endpoints.xml
@@ -116,6 +116,9 @@
         <intercept-url pattern="/mfa-providers/*"
                        access="#oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')"
                        method="GET"/>
+        <intercept-url pattern="/mfa-providers/*"
+                       access="#oauth2.hasScopeInAuthZone('zones.{zone.id}.admin')"
+                       method="DELETE"/>
         <intercept-url pattern="/**" access="denyAll"/>
         <csrf disabled="true"/>
         <custom-filter ref="resourceAgnosticAuthenticationFilter" before="PRE_AUTH_FILTER"/>

diff --git a/...c/test/java/org/cloudfoundry/identity/uaa/mock/mfa_provider/MfaProviderEndpointsDocs.java b/...c/test/java/org/cloudfoundry/identity/uaa/mock/mfa_provider/MfaProviderEndpointsDocs.java
@@ -16,7 +16,6 @@
 import org.springframework.restdocs.mockmvc.RestDocumentationRequestBuilders;
 import org.springframework.restdocs.payload.FieldDescriptor;
 import org.springframework.restdocs.snippet.Snippet;
-import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
 import org.springframework.test.web.servlet.ResultActions;
 
 import static org.cloudfoundry.identity.uaa.mfa_provider.MfaProvider.MfaProviderType.GOOGLE_AUTHENTICATOR;
@@ -202,6 +201,30 @@ public void testListMfaProviders() throws Exception{
         ));
     }
 
+    @Test
+    public void testDeleteMfaProvider() throws Exception {
+        MfaProvider<GoogleMfaProviderConfig> mfaProvider = getGoogleMfaProvider();
+        mfaProvider = createMfaProviderHelper(mfaProvider);
+
+        Snippet responseFields = responseFields(getMfaProviderResponseFields(getGoogleMfaProviderFields()));
+
+        ResultActions getMFaResultAction = getMockMvc().perform(
+                RestDocumentationRequestBuilders.delete("/mfa-providers/{id}", mfaProvider.getId())
+                        .header("Authorization", "Bearer " + adminToken)
+                        .accept(APPLICATION_JSON));
+
+        getMFaResultAction.andDo(document(
+                "{ClassName}/{methodName}",
+                preprocessResponse(prettyPrint()),
+                pathParameters(parameterWithName("id").required().description(ID_DESC)),
+                requestHeaders(
+                        MFA_AUTHORIZATION_HEADER,
+                        IDENTITY_ZONE_ID_HEADER
+                ),
+                responseFields
+        ));
+    }
+
     private MfaProvider createMfaProviderHelper(MfaProvider<GoogleMfaProviderConfig> mfaProvider) throws Exception{
         MockHttpServletResponse createResponse = getMockMvc().perform(
                 post("/mfa-providers")