CDAP-21256 : Support preserving client credentials when deleting an OAuth provider

Author: sahusanket

cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthStore.java

@@ -159,17 +159,20 @@ public Optional<OAuthProvider> getProvider(String name) throws OAuthStoreExcepti
   /**
    * Remove an OAuth provider.
    *
-   * @param name name of {@link OAuthProvider} to read
-   * @throws OAuthStoreException if the read fails
+   * @param name name of {@link OAuthProvider} to delete
+   * @param preserveClientCredentials whether to preserve existing secrets in secure store
+   * @throws OAuthStoreException if the delete fails
    */
-  public void deleteProvider(String name) throws OAuthStoreException {
-    // Delete associated Client Credentials with given provider.
-    try {
-      secureStoreManager.delete(NamespaceId.SYSTEM.getNamespace(), getClientCredsKey(name));
-    } catch (Exception e) {
-      // If key is not found, then we can safely delete provider. For any other exception, throw it.
-      if (!e.getClass().getName().contains("NotFoundException")) {
-        throw new OAuthStoreException("Failed to delete client credential from OAuth provider secure storage", e);
+  public void deleteProvider(String name, boolean preserveClientCredentials) throws OAuthStoreException {
+    // Delete associated Client Credentials with given provider if not preserved.
+    if (!preserveClientCredentials) {
+      try {
+        secureStoreManager.delete(NamespaceId.SYSTEM.getNamespace(), getClientCredsKey(name));
+      } catch (Exception e) {
+        // If key is not found, then we can safely delete provider. For any other exception, throw it.
+        if (!e.getClass().getName().contains("NotFoundException")) {
+          throw new OAuthStoreException("Failed to delete client credential from OAuth provider secure storage", e);
+        }
       }
     }
 

cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java

@@ -161,10 +161,12 @@ public void putOAuthProvider(HttpServiceRequest request, HttpServiceResponder re
   @DELETE
   @Path(API_VERSION + "/oauth/provider/{provider}")
   public void deleteOAuthProvider(HttpServiceRequest request, HttpServiceResponder responder,
-                               @PathParam("provider") String oauthProvider) {
+                               @PathParam("provider") String oauthProvider,
+                               @QueryParam("preserve_client_credentials") @DefaultValue("false")
+                               boolean preserveClientCredentials) {
     try {
       try {
-        oauthStore.deleteProvider(oauthProvider);
+        oauthStore.deleteProvider(oauthProvider, preserveClientCredentials);
         responder.sendStatus(HttpURLConnection.HTTP_OK);
       } catch (NullPointerException e) {
         throw new OAuthServiceException(HttpURLConnection.HTTP_BAD_REQUEST, "Invalid provider: " + e.getMessage(), e);

cdap-app-templates/cdap-etl/cdap-data-pipeline-base/src/test/java/io/cdap/cdap/datapipeline/OAuthStoreTest.java

@@ -138,10 +138,27 @@ public void testDeleteProvider() throws Exception {
 
     doNothing().when(mockTable).delete(any());
 
-    oauthStore.deleteProvider(PROVIDER_NAME);
+    oauthStore.deleteProvider(PROVIDER_NAME, false);
     verify(mockSecureStoreManager, times(1)).delete(any(), any());
   }
 
+  @Test
+  public void testDeleteProviderPreserveCredentials() throws Exception {
+    doAnswer(invocation -> {
+      TxRunnable runnable = invocation.getArgument(0);
+      StructuredTableContext mockContext = mock(StructuredTableContext.class);
+      when(mockContext.getTable(any())).thenReturn(mockTable);
+      runnable.run(mockContext);
+      return null;
+    }).when(mockTransactionRunner).run(any(TxRunnable.class));
+
+    doNothing().when(mockTable).delete(any());
+
+    oauthStore.deleteProvider(PROVIDER_NAME, true);
+    verify(mockSecureStoreManager, times(0)).delete(any(), any());
+    verify(mockTable, times(1)).delete(any());
+  }
+
   @Test
   public void testDeleteProviderWhenSecureKeyNotFound() throws Exception {
     class NotFoundException extends Exception {
@@ -161,7 +178,7 @@ public NotFoundException(String message) {
 
     // CASE 1 :  When Secure keys not found, the provider should be deleted.
     doThrow(new NotFoundException("Keys not found.")).when(mockSecureStoreManager).delete(any(), any());
-    oauthStore.deleteProvider(PROVIDER_NAME);
+    oauthStore.deleteProvider(PROVIDER_NAME, false);
     verify(mockSecureStoreManager, times(1)).delete(any(), any());
     verify(mockTable, times(1)).delete(any());
 
@@ -170,7 +187,7 @@ public NotFoundException(String message) {
     org.mockito.Mockito.clearInvocations(mockTable);
     doThrow(new Exception("Unable to delete secure key")).when(mockSecureStoreManager).delete(any(), any());
     try {
-      oauthStore.deleteProvider(PROVIDER_NAME);
+      oauthStore.deleteProvider(PROVIDER_NAME, false);
     } catch (Exception e) {
       assertEquals(e.getClass(), OAuthStoreException.class);
     }