Repository: gravitee-io/graviteeio-access-management
Pull request #769 (2020-04-15 14:06:13)

fix(user): revoke user data access of an application
Closes gravitee-io/issues#3546


Patch:
diff --git a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ApplicationServiceImpl.java b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ApplicationServiceImpl.java
index d0671a8eb2..7cc3c32d71 100644
--- a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ApplicationServiceImpl.java
+++ b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ApplicationServiceImpl.java
@@ -192,7 +192,7 @@ public class ApplicationServiceImpl implements ApplicationService {
 
     @Override
     public Maybe<Application> findByDomainAndClientId(String domain, String clientId) {
-        LOGGER.debug("Find application by domain: {} and client_id", domain, clientId);
+        LOGGER.debug("Find application by domain: {} and client_id {}", domain, clientId);
         return applicationRepository.findByDomainAndClientId(domain, clientId)
                 .onErrorResumeNext(ex -> {
                     LOGGER.error("An error occurs while trying to find an application using its domain: {} and client_id : {}", domain, clientId, ex);
diff --git a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ClientServiceImpl.java b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ClientServiceImpl.java
index df1ec559e3..5b8176fd86 100644
--- a/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ClientServiceImpl.java
+++ b/gravitee-am-service/src/main/java/io/gravitee/am/service/impl/ClientServiceImpl.java
@@ -56,6 +56,7 @@ import java.util.stream.Collectors;
 @Component
 public class ClientServiceImpl implements ClientService {
 
+    public static final String DEFAULT_CLIENT_NAME = "Unknown Client";
     private final Logger LOGGER = LoggerFactory.getLogger(ClientServiceImpl.class);
 
     @Autowired
@@ -264,17 +265,26 @@ public class ClientServiceImpl implements ClientService {
             return Single.error(new InvalidClientMetadataException("No domain set on client"));
         }
 
+        boolean clientIdGenerated = false;
+
         /* openid response metadata */
         client.setId(RandomString.generate());
         //client_id & client_secret may be already informed if created through UI
         if(client.getClientId()==null) {
             client.setClientId(SecureRandomString.generate());
+            clientIdGenerated = true;
         }
         if(client.getClientSecret()==null || client.getClientSecret().trim().isEmpty()) {
             client.setClientSecret(SecureRandomString.generate());
         }
         if(client.getClientName()==null || client.getClientName().trim().isEmpty()) {
-            client.setClientName("Unknown Client");
+
+            if(clientIdGenerated) {
+                client.setClientName(DEFAULT_CLIENT_NAME);
+            } else {
+                // ClientId has been provided by user, reuse it as clientName.
+                client.setClientName(client.getClientId());
+            }
         }
 
         /* GRAVITEE.IO custom fields */
@@ -335,7 +345,7 @@ public class ClientServiceImpl implements ClientService {
         application.setCreatedAt(client.getCreatedAt());
         application.setUpdatedAt(client.getUpdatedAt());
         // set application name
-        application.setName(client.getClientName() != null ? client.getClientName() : client.getClientId());
+        application.setName(client.getClientName());
         // set application type
         application.setType(getType(client));
         // set application settings
diff --git a/gravitee-am-ui/src/app/app-routing.module.ts b/gravitee-am-ui/src/app/app-routing.module.ts
index 5abf2b9129..13031ae1fb 100644
--- a/gravitee-am-ui/src/app/app-routing.module.ts
+++ b/gravitee-am-ui/src/app/app-routing.module.ts
@@ -347,10 +347,7 @@ const routes: Routes = [
             },
             children: [
               { path: '', redirectTo: 'profile', pathMatch: 'full' },
-              { path: 'profile', component: UserProfileComponent },
-              { path: 'applications', component: UserApplicationsComponent, resolve: {consents: ConsentsResolver}},
-              { path: 'applications/:clientId', component: UserApplicationComponent, resolve: {consents: ConsentsResolver}},
-              { path: 'roles', component: UserRolesComponent, resolve: { roles : UserRolesResolver}}
+              { path: 'profile', component: UserProfileComponent }
             ]
           },
           { path: 'groups',
@@ -393,8 +390,7 @@ const routes: Routes = [
             children: [
               { path: '', redirectTo: 'settings', pathMatch: 'full' },
               { path: 'settings', component: GroupSettingsComponent },
-              { path: 'members', component: GroupMembersComponent, resolve: { members : GroupMembersResolver}},
-              { path: 'roles', component: GroupRolesComponent, resolve: { roles : GroupRolesResolver}}
+              { path: 'members', component: GroupMembersComponent, resolve: { members : GroupMembersResolver}}
             ]
           },
           { path: 'roles',
@@ -1100,7 +1096,7 @@ const routes: Routes = [
               { path: '', redirectTo: 'profile', pathMatch: 'full' },
               { path: 'profile', component: UserProfileComponent },
               { path: 'applications', component: UserApplicationsComponent, resolve: {consents: ConsentsResolver}},
-              { path: 'applications/:clientId', component: UserApplicationComponent, resolve: {consents: ConsentsResolver}},
+              { path: 'applications/:appId', component: UserApplicationComponent, resolve: {application: ApplicationResolver, consents: ConsentsResolver}},
               { path: 'factors', component: UserFactorsComponent, resolve: {factors: EnrolledFactorsResolver}},
               { path: 'roles', component: UserRolesComponent, resolve: { roles : UserRolesResolver}}
             ]
diff --git a/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.html b/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.html
index 02f1a6c09c..55d4b593e6 100644
--- a/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.html
+++ b/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.html
@@ -19,7 +19,7 @@
   <a [routerLink]="['..']" class="gv-back-link"><small><< Back to authorized apps</small></a>
   <div>
     <div fxLayout="row" style="align-items: center;">
-      <h1>{{clientId}}</h1>
+      <h1>{{application.name}}</h1>
       <div fxFlex fxLayoutAlign="end">
         <button *ngIf="canRevokeAccess() && canRevoke" style="height: 40px;" mat-stroked-button color="warn" (click)="revokeApplication($event)">Revoke access</button>
       </div>
diff --git a/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.ts b/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.ts
index e425ebb6ec..e552b20b2d 100644
--- a/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.ts
+++ b/gravitee-am-ui/src/app/domain/settings/users/user/applications/application/application.component.ts
@@ -20,6 +20,7 @@ import {DialogService} from "../../../../../../services/dialog.service";
 import {UserService} from "../../../../../../services/user.service";
 import * as _ from 'lodash';
 import {AuthService} from "../../../../../../services/auth.service";
+import {BreadcrumbService} from "../../../../../../services/breadcrumb.service";
 
 @Component({
   selector: 'app-user-application',
@@ -27,6 +28,7 @@ import {AuthService} from "../../../../../../services/auth.service";
   styleUrls: ['./application.component.scss']
 })
 export class UserApplicationComponent implements OnInit {
+  private application: any;
   private domainId: string;
   private userId: string;
   clientId: string;
@@ -38,15 +40,19 @@ export class UserApplicationComponent implements OnInit {
               private snackbarService: SnackbarService,
               private dialogService: DialogService,
               private userService: UserService,
+              private breadcrumbService: BreadcrumbService,
               private authService: AuthService) {
   }
 
   ngOnInit() {
+    this.application = this.route.snapshot.data['application'];
     this.domainId = this.route.snapshot.parent.parent.parent.params['domainId'];
     this.userId = this.route.snapshot.parent.params['userId'];
     this.consents = this.route.snapshot.data['consents'];
-    this.clientId = this.consents[0] ? this.consents[0].clientEntity.name : this.route.snapshot.params['clientId'];
+    this.clientId = this.route.snapshot.queryParamMap.get('clientId');
     this.canRevoke = this.authService.hasPermissions(['domain_user_update']);
+
+    this.initBreadcrumb();
   }
 
   revokeApplication(event) {
@@ -56,7 +62,7 @@ export class UserApplicationComponent implements OnInit {
       .subscribe(res => {
         if (res) {
           this.userService.revokeConsents(this.domainId, this.userId, this.clientId).subscribe(response => {
-            this.snackbarService.open('Access for application ' + this.clientId + ' revoked');
+            this.snackbarService.open('Access for application ' + this.application.name + ' revoked');
             this.router.navigate(['/domains', this.domainId, 'settings', 'users', this.userId, 'applications']);
           });
         }
@@ -98,4 +104,7 @@ export class UserApplicationComponent implements OnInit {
     };
   }
 
+  initBreadcrumb() {
+    this.breadcrumbService.addFriendlyNameForRouteRegex('/domains/' + this.domainId + '/settings/users/' + this.userId + '/applications/'+this.application.id+'.*$', this.application.name);
+  }
 }
diff --git a/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.html b/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.html
index 7d5a4b2eb7..74bf1751ff 100644
--- a/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.html
+++ b/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.html
@@ -51,7 +51,7 @@
         <ngx-datatable-column [flexGrow]="1">
           <ng-template let-row="row" ngx-datatable-cell-template>
             <div fxLayout="row" class="gv-table-cell-actions">
-              <a mat-icon-button [routerLink]="[row.clientId]"><mat-icon matTooltip="More info">more_horiz</mat-icon></a>
+              <a mat-icon-button [routerLink]="[row.clientEntity.id]" [queryParams]="{clientId: row.clientId}"><mat-icon matTooltip="More info">more_horiz</mat-icon></a>
               <button *ngIf="canRevokeAccessForClient(row.clientId) && canRevoke" mat-icon-button (click)="revoke($event, row)"><mat-icon style="color: #f44336;" matTooltip="Revoke application">highlight_off</mat-icon></button>
             </div>
           </ng-template>
diff --git a/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.ts b/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.ts
index befd1284da..864f941cb2 100644
--- a/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.ts
+++ b/gravitee-am-ui/src/app/domain/settings/users/user/applications/applications.component.ts
@@ -47,7 +47,7 @@ export class UserApplicationsComponent implements OnInit {
     this.user = this.route.snapshot.parent.data['user'];
     this.consents = _.sortBy(this.route.snapshot.data['consents'], 'updatedAt').reverse();
     this.appConsentsGrouped  = _.groupBy(this.consents, 'clientId');
-    this.appConsents = _.sortedUniqBy(this.consents, 'clientId');
+    this.appConsents = _.uniqBy(this.consents, 'clientId');
     this.canRevoke = this.authService.hasPermissions(['domain_user_update']);
   }
 
@@ -62,7 +62,7 @@ export class UserApplicationsComponent implements OnInit {
       .subscribe(res => {
         if (res) {
           this.userService.revokeConsents(this.domainId, this.user.id, consent.clientId).subscribe(response => {
-            this.snackbarService.open('Access for application '+ consent.clientId + ' revoked');
+            this.snackbarService.open('Access for application '+ consent.clientEntity.name + ' revoked');
             this.loadConsents();
           });
         }
diff --git a/gravitee-am-ui/src/app/resolvers/application.resolver.ts b/gravitee-am-ui/src/app/resolvers/application.resolver.ts
index 631aed5fa7..ecc11fe3b5 100644
--- a/gravitee-am-ui/src/app/resolvers/application.resolver.ts
+++ b/gravitee-am-ui/src/app/resolvers/application.resolver.ts
@@ -25,8 +25,13 @@ export class ApplicationResolver implements Resolve<any> {
 
   resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<any>|Promise<any>|any {
     let domainId = (route.paramMap.get('domainId')) ? route.paramMap.get('domainId') : route.parent.paramMap.get('domainId');
+
+    // try access domainId from higher levels
+    if (!domainId && route.parent.parent) {
+      domainId = route.parent.parent.paramMap.get('domainId') ? route.parent.parent.paramMap.get('domainId') : (route.parent.parent.parent && route.parent.parent.parent.paramMap.get('domainId')) ? route.parent.parent.parent.paramMap.get('domainId') : null;
+    }
+
     let appId: string = route.paramMap.get('appId');
     return this.applicationService.get(domainId, appId);
   }
-
 }
diff --git a/gravitee-am-ui/src/app/resolvers/consents.resolver.ts b/gravitee-am-ui/src/app/resolvers/consents.resolver.ts
index b48ad56383..b164e161df 100644
--- a/gravitee-am-ui/src/app/resolvers/consents.resolver.ts
+++ b/gravitee-am-ui/src/app/resolvers/consents.resolver.ts
@@ -26,7 +26,7 @@ export class ConsentsResolver implements Resolve<any> {
   resolve(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<any>|Promise<any>|any {
     const domainId = route.parent.parent.parent.paramMap.get('domainId');
     const userId = route.parent.paramMap.get('userId');
-    const clientId = route.paramMap.get('clientId');
+    const clientId = route.paramMap.get('clientId') ? route.paramMap.get('clientId') : route.queryParamMap.get('clientId');
     return this.userService.consents(domainId, userId, clientId);
   }
 }


Test patch:
diff --git a/gravitee-am-service/src/test/java/io/gravitee/am/service/ClientServiceTest.java b/gravitee-am-service/src/test/java/io/gravitee/am/service/ClientServiceTest.java
index de78d7ef0c..403d1fb99d 100644
--- a/gravitee-am-service/src/test/java/io/gravitee/am/service/ClientServiceTest.java
+++ b/gravitee-am-service/src/test/java/io/gravitee/am/service/ClientServiceTest.java
@@ -43,6 +43,7 @@ import org.mockito.junit.MockitoJUnitRunner;
 
 import java.util.*;
 
+import static io.gravitee.am.service.impl.ClientServiceImpl.DEFAULT_CLIENT_NAME;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyString;
 import static org.mockito.Mockito.*;
@@ -262,18 +263,57 @@ public class ClientServiceTest {
     @Test
     public void shouldCreate() {
         NewClient newClient = Mockito.mock(NewClient.class);
-        Application createClient = Mockito.mock(Application.class);
+        Application createApplication = Mockito.mock(Application.class);
 
         when(newClient.getClientId()).thenReturn("my-client");
-        when(applicationService.create(any(Application.class))).thenReturn(Single.just(createClient));
+        when(applicationService.create(any(Application.class))).thenReturn(Single.just(createApplication));
 
-        TestObserver testObserver = clientService.create(DOMAIN, newClient).test();
+        TestObserver<Client> testObserver = clientService.create(DOMAIN, newClient).test();
         testObserver.awaitTerminalEvent();
 
         testObserver.assertComplete();
         testObserver.assertNoErrors();
 
-        verify(applicationService, times(1)).create(any(Application.class));
+        verify(applicationService, times(1)).create(argThat(application -> application.getName().equals(newClient.getClientId())));
+    }
+
+    @Test
+    public void shouldCreate_withoutClientId() {
+        NewClient newClient = Mockito.mock(NewClient.class);
+        Application createApplication = Mockito.mock(Application.class);
+
+        when(newClient.getClientId()).thenReturn(null);
+        when(applicationService.create(any(Application.class))).thenReturn(Single.just(createApplication));
+
+        TestObserver<Client> testObserver = clientService.create(DOMAIN, newClient).test();
+        testObserver.awaitTerminalEvent();
+
+        testObserver.assertComplete();
+        testObserver.assertNoErrors();
+
+        // Check the application name is equals to 'Unknown Client' (to avoid having generated uuid as application name).
+        verify(applicationService, times(1)).create(argThat(application -> application.getName().equals(DEFAULT_CLIENT_NAME)));
+    }
+
+    @Test
+    public void shouldCreate_withClientName() {
+        String customClientName = "My custom client name";
+
+        NewClient newClient = Mockito.mock(NewClient.class);
+        Application createApplication = Mockito.mock(Application.class);
+
+        when(newClient.getClientId()).thenReturn(null);
+        when(newClient.getClientName()).thenReturn(customClientName);
+        when(applicationService.create(any(Application.class))).thenReturn(Single.just(createApplication));
+
+        TestObserver<Client> testObserver = clientService.create(DOMAIN, newClient).test();
+        testObserver.awaitTerminalEvent();
+
+        testObserver.assertComplete();
+        testObserver.assertNoErrors();
+
+        // Check the application name is customized when client name has been specified.
+        verify(applicationService, times(1)).create(argThat(application -> application.getName().equals(customClientName)));
     }
 
     @Test
@@ -457,7 +497,7 @@ public class ClientServiceTest {
     }
 
     @Test
-    public void update_clientCredentials_ok() {
+    public void   update_clientCredentials_ok() {
         when(applicationService.update(any(Application.class))).thenReturn(Single.just(new Application()));
 
         Client toUpdate = new Client();