Repository: gravitee-io/graviteeio-access-management
Pull request #373 (2018-10-14 12:35:53)

fix(scope): Delete scope approvals when a scope is being deleted


Patch:
diff --git a/gravitee-am-management-api/gravitee-am-management-api-repository/src/main/java/io/gravitee/am/management/repository/proxy/ScopeApprovalRepositoryProxy.java b/gravitee-am-management-api/gravitee-am-management-api-repository/src/main/java/io/gravitee/am/management/repository/proxy/ScopeApprovalRepositoryProxy.java
new file mode 100644
index 0000000000..aa4851d93d
--- /dev/null
+++ b/gravitee-am-management-api/gravitee-am-management-api-repository/src/main/java/io/gravitee/am/management/repository/proxy/ScopeApprovalRepositoryProxy.java
@@ -0,0 +1,61 @@
+/**
+ * Copyright (C) 2015 The Gravitee team (http://gravitee.io)
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *         http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package io.gravitee.am.management.repository.proxy;
+
+import io.gravitee.am.model.oauth2.ScopeApproval;
+import io.gravitee.am.repository.oauth2.api.ScopeApprovalRepository;
+import io.reactivex.Completable;
+import io.reactivex.Maybe;
+import io.reactivex.Single;
+import org.springframework.stereotype.Component;
+
+import java.util.Set;
+
+/**
+ * @author David BRASSELY (david.brassely at graviteesource.com)
+ * @author GraviteeSource Team
+ */
+@Component
+public class ScopeApprovalRepositoryProxy extends AbstractProxy<ScopeApprovalRepository> implements ScopeApprovalRepository {
+
+    public Maybe<ScopeApproval> findById(String s) {
+        return target.findById(s);
+    }
+
+    public Single<ScopeApproval> create(ScopeApproval item) {
+        return target.create(item);
+    }
+
+    public Single<ScopeApproval> update(ScopeApproval item) {
+        return target.update(item);
+    }
+
+    public Completable delete(String s) {
+        return target.delete(s);
+    }
+
+    public Single<Set<ScopeApproval>> findByDomainAndUserAndClient(String domain, String userId, String clientId) {
+        return target.findByDomainAndUserAndClient(domain, userId, clientId);
+    }
+
+    public Single<ScopeApproval> upsert(ScopeApproval scopeApproval) {
+        return target.upsert(scopeApproval);
+    }
+
+    public Completable delete(String domain, String scope) {
+        return target.delete(domain, scope);
+    }
+}
diff --git a/gravitee-am-repository/gravitee-am-repository-api/src/main/java/io/gravitee/am/repository/oauth2/api/ScopeApprovalRepository.java b/gravitee-am-repository/gravitee-am-repository-api/src/main/java/io/gravitee/am/repository/oauth2/api/ScopeApprovalRepository.java
index 96a5b8a198..65f4968c44 100644
--- a/gravitee-am-repository/gravitee-am-repository-api/src/main/java/io/gravitee/am/repository/oauth2/api/ScopeApprovalRepository.java
+++ b/gravitee-am-repository/gravitee-am-repository-api/src/main/java/io/gravitee/am/repository/oauth2/api/ScopeApprovalRepository.java
@@ -17,6 +17,7 @@ package io.gravitee.am.repository.oauth2.api;
 
 import io.gravitee.am.model.oauth2.ScopeApproval;
 import io.gravitee.am.repository.common.CrudRepository;
+import io.reactivex.Completable;
 import io.reactivex.Single;
 
 import java.util.Set;
@@ -30,4 +31,6 @@ public interface ScopeApprovalRepository extends CrudRepository<ScopeApproval, S
     Single<Set<ScopeApproval>> findByDomainAndUserAndClient(String domain, String userId, String clientId);
 
     Single<ScopeApproval> upsert(ScopeApproval scopeApproval);
+
+    Completable delete(String domain, String scope);
 }
diff --git a/gravitee-am-repository/gravitee-am-repository-mongodb/src/main/java/io/gravitee/am/repository/mongodb/oauth2/MongoScopeApprovalRepository.java b/gravitee-am-repository/gravitee-am-repository-mongodb/src/main/java/io/gravitee/am/repository/mongodb/oauth2/MongoScopeApprovalRepository.java
index eeaaafcf10..f6cabba1a3 100644
--- a/gravitee-am-repository/gravitee-am-repository-mongodb/src/main/java/io/gravitee/am/repository/mongodb/oauth2/MongoScopeApprovalRepository.java
+++ b/gravitee-am-repository/gravitee-am-repository-mongodb/src/main/java/io/gravitee/am/repository/mongodb/oauth2/MongoScopeApprovalRepository.java
@@ -112,6 +112,12 @@ public class MongoScopeApprovalRepository extends AbstractOAuth2MongoRepository
                 });
     }
 
+    @Override
+    public Completable delete(String domain, String scope) {
+        return Completable.fromPublisher(scopeApprovalsCollection.deleteMany(
+                and(eq(FIELD_DOMAIN, domain), eq(FIELD_SCOPE, scope))));
+    }
+
     @Override
     public Completable delete(String id) {
         return Completable.fromPublisher(scopeApprovalsCollection.deleteOne(eq(FIELD_ID, id)));
diff --git a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ScopeServiceImpl.java b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ScopeServiceImpl.java
index e5c0a314b5..74b84d865a 100644
--- a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ScopeServiceImpl.java
+++ b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ScopeServiceImpl.java
@@ -15,10 +15,9 @@
  */
 package io.gravitee.am.service.impl;
 
-import io.gravitee.am.model.Client;
-import io.gravitee.am.model.Role;
 import io.gravitee.am.model.oauth2.Scope;
 import io.gravitee.am.repository.management.api.ScopeRepository;
+import io.gravitee.am.repository.oauth2.api.ScopeApprovalRepository;
 import io.gravitee.am.service.ClientService;
 import io.gravitee.am.service.RoleService;
 import io.gravitee.am.service.ScopeService;
@@ -41,7 +40,6 @@ import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.stereotype.Component;
 
 import java.util.Date;
-import java.util.List;
 import java.util.Set;
 import java.util.stream.Collectors;
 
@@ -61,6 +59,9 @@ public class ScopeServiceImpl implements ScopeService {
     @Autowired
     private ScopeRepository scopeRepository;
 
+    @Autowired
+    private ScopeApprovalRepository scopeApprovalRepository;
+
     @Autowired
     private RoleService roleService;
 
@@ -137,62 +138,64 @@ public class ScopeServiceImpl implements ScopeService {
         LOGGER.debug("Delete scope {}", scopeId);
         return scopeRepository.findById(scopeId)
                 .switchIfEmpty(Maybe.error(new ScopeNotFoundException(scopeId)))
-                .flatMapSingle(scope -> {
-                    // 1_ Remove permissions from role
-                    Single<List<Role>> removePermissionsFromRole = roleService.findByDomain(scope.getDomain())
-                            .flatMapObservable(roles -> Observable.fromIterable(roles.stream()
-                                    .filter(role -> role.getPermissions().contains(scope.getKey()))
-                                    .collect(Collectors.toList())))
-                            .flatMapSingle(role -> {
-                                role.getPermissions().remove(scope.getKey());
-                                UpdateRole updatedRole = new UpdateRole();
-                                updatedRole.setName(role.getName());
-                                updatedRole.setDescription(role.getDescription());
-                                updatedRole.setPermissions(role.getPermissions());
-                                // Save role
-                                return roleService.update(scope.getDomain(), role.getId(), updatedRole);
-                            }).toList();
-
-                    // 2_ Remove scopes from client
-                    Single<List<Client>> removeScopesFromClient = clientService.findByDomain(scope.getDomain())
-                            .flatMapObservable(clients -> Observable.fromIterable(clients.stream()
-                                    .filter(client -> client.getScopes().contains(scope.getKey()))
-                                    .collect(Collectors.toList())))
-                            .flatMapSingle(client -> {
-                                // Remove scope from client
-                                client.getScopes().remove(scope.getKey());
-
-                                UpdateClient updateClient = new UpdateClient();
-                                updateClient.setAutoApproveScopes(client.getAutoApproveScopes());
-                                updateClient.setScopes(client.getScopes());
-                                updateClient.setRefreshTokenValiditySeconds(client.getRefreshTokenValiditySeconds());
-                                updateClient.setRedirectUris(client.getRedirectUris());
-                                updateClient.setAccessTokenValiditySeconds(client.getAccessTokenValiditySeconds());
-                                updateClient.setAuthorizedGrantTypes(client.getAuthorizedGrantTypes());
-                                updateClient.setCertificate(client.getCertificate());
-                                updateClient.setEnabled(client.isEnabled());
-                                updateClient.setEnhanceScopesWithUserPermissions(client.isEnhanceScopesWithUserPermissions());
-                                updateClient.setGenerateNewTokenPerRequest(client.isGenerateNewTokenPerRequest());
-                                updateClient.setIdentities(client.getIdentities());
-                                updateClient.setIdTokenCustomClaims(client.getIdTokenCustomClaims());
-                                updateClient.setIdTokenValiditySeconds(client.getIdTokenValiditySeconds());
-
-                                // Save client
-                                return clientService.update(scope.getDomain(), client.getId(), updateClient);
-                            }).toList();
-
-                    return Single.merge(removePermissionsFromRole, removeScopesFromClient).toList();
-                })
-                .flatMapCompletable(irrelevant -> scopeRepository.delete(scopeId))
-                .onErrorResumeNext(ex -> {
-                    if (ex instanceof AbstractManagementException) {
-                        return Completable.error(ex);
-                    }
-
-                    LOGGER.error("An error occurs while trying to delete scope: {}", scopeId, ex);
-                    return Completable.error(new TechnicalManagementException(
-                            String.format("An error occurs while trying to delete scope: %s", scopeId), ex));
-                });
+                .flatMapCompletable(scope ->
+
+                        Completable.fromSingle(
+                                // 1_ Remove permissions from role
+                                roleService.findByDomain(scope.getDomain())
+                                        .flatMapObservable(roles -> Observable.fromIterable(roles.stream()
+                                                .filter(role -> role.getPermissions() != null && role.getPermissions().contains(scope.getKey()))
+                                                .collect(Collectors.toList())))
+                                        .flatMapSingle(role -> {
+                                            role.getPermissions().remove(scope.getKey());
+                                            UpdateRole updatedRole = new UpdateRole();
+                                            updatedRole.setName(role.getName());
+                                            updatedRole.setDescription(role.getDescription());
+                                            updatedRole.setPermissions(role.getPermissions());
+                                            // Save role
+                                            return roleService.update(scope.getDomain(), role.getId(), updatedRole);
+                                        }).toList())
+                                .andThen(
+                                        // 2_ Remove scopes from client
+                                        clientService.findByDomain(scope.getDomain())
+                                                .flatMapObservable(clients -> Observable.fromIterable(clients.stream()
+                                                        .filter(client -> client.getScopes().contains(scope.getKey()))
+                                                        .collect(Collectors.toList())))
+                                                .flatMapSingle(client -> {
+                                                    // Remove scope from client
+                                                    client.getScopes().remove(scope.getKey());
+
+                                                    UpdateClient updateClient = new UpdateClient();
+                                                    updateClient.setAutoApproveScopes(client.getAutoApproveScopes());
+                                                    updateClient.setScopes(client.getScopes());
+                                                    updateClient.setRefreshTokenValiditySeconds(client.getRefreshTokenValiditySeconds());
+                                                    updateClient.setRedirectUris(client.getRedirectUris());
+                                                    updateClient.setAccessTokenValiditySeconds(client.getAccessTokenValiditySeconds());
+                                                    updateClient.setAuthorizedGrantTypes(client.getAuthorizedGrantTypes());
+                                                    updateClient.setCertificate(client.getCertificate());
+                                                    updateClient.setEnabled(client.isEnabled());
+                                                    updateClient.setEnhanceScopesWithUserPermissions(client.isEnhanceScopesWithUserPermissions());
+                                                    updateClient.setGenerateNewTokenPerRequest(client.isGenerateNewTokenPerRequest());
+                                                    updateClient.setIdentities(client.getIdentities());
+                                                    updateClient.setIdTokenCustomClaims(client.getIdTokenCustomClaims());
+                                                    updateClient.setIdTokenValiditySeconds(client.getIdTokenValiditySeconds());
+
+                                                    // Save client
+                                                    return clientService.update(scope.getDomain(), client.getId(), updateClient);
+                                                }).toList()).toCompletable()
+                                // 3_ Remove scopes from scope_approvals
+                                .andThen(scopeApprovalRepository.delete(scope.getDomain(), scope.getKey()))
+                                // 4_ Delete scope
+                                .andThen(scopeRepository.delete(scopeId)))
+                                .onErrorResumeNext(ex -> {
+                                    if (ex instanceof AbstractManagementException) {
+                                        return Completable.error(ex);
+                                    }
+
+                                    LOGGER.error("An error occurs while trying to delete scope: {}", scopeId, ex);
+                                    return Completable.error(new TechnicalManagementException(
+                                            String.format("An error occurs while trying to delete scope: %s", scopeId), ex));
+                                });
     }
 
     @Override


Test patch:
diff --git a/gravitee-am-service/src/test/java/io/gravitee/am/service/ScopeServiceTest.java b/gravitee-am-service/src/test/java/io/gravitee/am/service/ScopeServiceTest.java
index f4f646bfc6..5dfd521bbd 100644
--- a/gravitee-am-service/src/test/java/io/gravitee/am/service/ScopeServiceTest.java
+++ b/gravitee-am-service/src/test/java/io/gravitee/am/service/ScopeServiceTest.java
@@ -20,6 +20,7 @@ import io.gravitee.am.model.Role;
 import io.gravitee.am.model.oauth2.Scope;
 import io.gravitee.am.repository.exceptions.TechnicalException;
 import io.gravitee.am.repository.management.api.ScopeRepository;
+import io.gravitee.am.repository.oauth2.api.ScopeApprovalRepository;
 import io.gravitee.am.service.exception.ScopeAlreadyExistsException;
 import io.gravitee.am.service.exception.ScopeNotFoundException;
 import io.gravitee.am.service.exception.TechnicalManagementException;
@@ -67,6 +68,9 @@ public class ScopeServiceTest {
     @Mock
     private ScopeRepository scopeRepository;
 
+    @Mock
+    private ScopeApprovalRepository scopeApprovalRepository;
+
     private final static String DOMAIN = "domain1";
 
     @Test
@@ -246,6 +250,7 @@ public class ScopeServiceTest {
         when(clientService.findByDomain(DOMAIN)).thenReturn(Single.just(Collections.emptySet()));
         when(scopeRepository.findById("my-scope")).thenReturn(Maybe.just(scope));
         when(scopeRepository.delete("my-scope")).thenReturn(Completable.complete());
+        when(scopeApprovalRepository.delete(scope.getDomain(), scope.getKey())).thenReturn(Completable.complete());
 
         TestObserver testObserver = scopeService.delete("my-scope").test();
         testObserver.awaitTerminalEvent();
@@ -278,6 +283,7 @@ public class ScopeServiceTest {
         when(clientService.update(anyString(), anyString(), any(UpdateClient.class))).thenReturn(Single.just(new Client()));
         when(scopeRepository.findById("my-scope")).thenReturn(Maybe.just(scope));
         when(scopeRepository.delete("my-scope")).thenReturn(Completable.complete());
+        when(scopeApprovalRepository.delete(scope.getDomain(), scope.getKey())).thenReturn(Completable.complete());
 
         TestObserver testObserver = scopeService.delete("my-scope").test();
         testObserver.awaitTerminalEvent();