Repository: gravitee-io/graviteeio-access-management
Pull request #596 (2019-06-10 12:32:02)

fix(scopes): OAuth 2.0 scopes should be case sensitive
fixes gravitee-io/issues#2343


Patch:
diff --git a/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/java/io/gravitee/am/identityprovider/oauth2/authentication/OAuth2GenericAuthenticationProvider.java b/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/java/io/gravitee/am/identityprovider/oauth2/authentication/OAuth2GenericAuthenticationProvider.java
index d41cc8e1d3..d01125146c 100644
--- a/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/java/io/gravitee/am/identityprovider/oauth2/authentication/OAuth2GenericAuthenticationProvider.java
+++ b/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/java/io/gravitee/am/identityprovider/oauth2/authentication/OAuth2GenericAuthenticationProvider.java
@@ -203,6 +203,13 @@ public class OAuth2GenericAuthenticationProvider implements OAuth2Authentication
     public void afterPropertiesSet() throws Exception {
         OAuth2GenericIdentityProviderConfiguration configuration = (OAuth2GenericIdentityProviderConfiguration) this.configuration;
 
+        // check configuration
+        // a client secret is required if authorization code flow is used
+        if (io.gravitee.am.common.oauth2.ResponseType.CODE.equals(configuration.getResponseType())
+                && (configuration.getClientSecret() == null || configuration.getClientSecret().isEmpty())) {
+            throw new IllegalArgumentException("A client_secret must be supplied in order to use the Authorization Code flow");
+        }
+
         // fetch OpenID Provider information
         getOpenIDProviderConfiguration(configuration);
 
diff --git a/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/resources/schemas/schema-form.json b/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/resources/schemas/schema-form.json
index 3add462e44..f32b7ab17b 100644
--- a/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/resources/schemas/schema-form.json
+++ b/gravitee-am-identityprovider/gravitee-am-identityprovider-oauth2-generic/src/main/resources/schemas/schema-form.json
@@ -108,7 +108,6 @@
   },
   "required": [
     "clientId",
-    "clientSecret",
     "responseType"
   ],
   "oneOf":[
@@ -124,5 +123,11 @@
         "userProfileUri"
       ]
     }
-  ]
+  ],
+  "if": {
+    "properties": {
+      "responseType": { "const": "code" }
+    }
+  },
+  "then": { "required": ["clientSecret"] }
 }
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 f89022f19b..72358c1a7b 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
@@ -91,7 +91,8 @@ public class ScopeServiceImpl implements ScopeService {
     @Override
     public Single<Scope> create(String domain, NewScope newScope, User principal) {
         LOGGER.debug("Create a new scope {} for domain {}", newScope, domain);
-        String scopeKey = newScope.getKey().toLowerCase();
+        // replace all whitespace by an underscore (whitespace is a reserved keyword to separate tokens)
+        String scopeKey = newScope.getKey().replaceAll("\\s+","_");
         return scopeRepository.findByDomainAndKey(domain, scopeKey)
                 .isEmpty()
                 .flatMap(empty -> {
diff --git a/gravitee-am-ui/src/app/domain/settings/providers/provider/settings/settings.component.ts b/gravitee-am-ui/src/app/domain/settings/providers/provider/settings/settings.component.ts
index 1f95dd4bc7..4551239173 100644
--- a/gravitee-am-ui/src/app/domain/settings/providers/provider/settings/settings.component.ts
+++ b/gravitee-am-ui/src/app/domain/settings/providers/provider/settings/settings.component.ts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import { Component, OnInit } from '@angular/core';
+import { Component, OnInit, ViewChild } from '@angular/core';
 import { ProviderService } from "../../../../../services/provider.service";
 import { SnackbarService } from "../../../../../services/snackbar.service";
 import { ActivatedRoute, Router } from "@angular/router";
@@ -21,6 +21,7 @@ import { PlatformService } from "../../../../../services/platform.service";
 import { BreadcrumbService } from "../../../../../../libraries/ng2-breadcrumb/components/breadcrumbService";
 import { DomainService } from "../../../../../services/domain.service";
 import { AppConfig } from "../../../../../../config/app.config";
+import { NgForm } from "@angular/forms";
 
 @Component({
   selector: 'provider-settings',
@@ -28,6 +29,7 @@ import { AppConfig } from "../../../../../../config/app.config";
   styleUrls: ['./settings.component.scss']
 })
 export class ProviderSettingsComponent implements OnInit {
+  @ViewChild('providerForm') public form: NgForm;
   private domainId: string;
   domain: any = {};
   configurationIsValid: boolean = true;
@@ -65,6 +67,8 @@ export class ProviderSettingsComponent implements OnInit {
     this.providerService.update(this.domainId, this.provider.id, this.provider).subscribe(data => {
       this.breadcrumbService.addFriendlyNameForRouteRegex('/domains/'+this.domainId+'/providers/'+this.provider.id+'$', this.provider.name);
       this.snackbarService.open("Provider updated");
+      this.configurationPristine = true;
+      this.form.reset(data);
     });
   }
 
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.html b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.html
index f3f2f27b38..7517a8009e 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.html
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.html
@@ -16,6 +16,7 @@
 
 -->
 <div class="new-scope-container">
+  <a [routerLink]="['..']" class="back-link"><small><< Back to scopes</small></a>
   <h1>New Scope</h1>
   <div class="new-scope-content">
     <form (ngSubmit)="create()" #scopeForm="ngForm" fxLayout="column" fxFlex="70">
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.scss b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.scss
index bec1901409..a4dc5f12a7 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.scss
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.scss
@@ -3,8 +3,15 @@
   h1 {
     font-weight: 400;
   }
+  a.back-link {
+    color: #006CAB;
+    text-decoration: none;
+    &:hover {
+      color: #2196f3;
+      text-decoration: underline;
+    }
+  }
   .new-scope-content {
-    padding: 50px 0 20px;
     form {
       padding: 20px;
       background: #fafafa;
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.ts b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.ts
index 1aafd511ed..944fa2c0a6 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.ts
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/creation/scope-creation.component.ts
@@ -44,8 +44,6 @@ export class ScopeCreationComponent implements OnInit {
   }
 
   create() {
-    // Force lowercase for scope key
-    this.scope.key = this.scope.key.toLowerCase();
     // set duration time for user consent
     if (this.expiresIn && this.unitTime) {
       this.scope.expiresIn = moment.duration(this.expiresIn, this.unitTime).asSeconds();
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.html b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.html
index c66ef77fc3..e822f32289 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.html
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.html
@@ -16,54 +16,69 @@
 
 -->
 <div class="scope-container">
+  <a [routerLink]="['..']" class="back-link"><small><< Back to scopes</small></a>
   <h1>{{scope.name}}</h1>
   <div class="scope-content">
-    <form (keydown.enter)="false" (ngSubmit)="update()" #scopeForm="ngForm" fxLayout="column" fxFlex="70">
-      <mat-form-field fxFlex>
-        <input matInput type="text" readonly placeholder="Key" name="key" [(ngModel)]="scope.key">
-      </mat-form-field>
+    <div fxLayout="column" fxFlex="70">
+      <form (keydown.enter)="false" (ngSubmit)="update()" #scopeForm="ngForm" fxLayout="column">
+        <mat-form-field fxFlex>
+          <input matInput type="text" placeholder="Key" name="key" [(ngModel)]="scope.key" disabled>
+          <mat-hint>A key for your scope.</mat-hint>
+        </mat-form-field>
 
-      <mat-form-field fxFlex>
-        <input matInput type="text" placeholder="Name" name="name" [(ngModel)]="scope.name" required>
-      </mat-form-field>
+        <mat-form-field fxFlex>
+          <input matInput type="text" placeholder="Name" name="name" [(ngModel)]="scope.name" required>
+          <mat-hint>A name for your scope.</mat-hint>
+        </mat-form-field>
 
-      <mat-form-field fxFlex>
-        <input matInput type="text" placeholder="Description" name="description" [(ngModel)]="scope.description" required>
-      </mat-form-field>
+        <mat-form-field fxFlex>
+          <input matInput type="text" placeholder="Description" name="description" [(ngModel)]="scope.description" required>
+          <mat-hint>A description for your scope.</mat-hint>
+        </mat-form-field>
 
-      <div fxLayout="column" class="user-consent-zone">
-        <h4>User consent</h4>
-        <small>Expiry/lapsing period for consent after which the user would need to confirm that scope.</small>
-        <p fxLayout="row" style="align-items: center; font-size: 90%;">
-          <mat-icon mat-list-icon>alarm</mat-icon>
-          <span>Current period :&nbsp;</span>
-          <span *ngIf="!scope.expiresIn" style="color: grey; font-style: italic;">No time set</span>
-          <span *ngIf="scope.expiresIn">{{ getScopeExpiry() }} <button mat-icon-button (click)="clearExpiry()" matTooltip="Clear expiry periode"><mat-icon>clear</mat-icon></button></span>
-        </p>
-        <div fxLayout="row">
-          <mat-form-field fxFlex style="margin-right: 20px;">
-            <input matInput type="number" placeholder="Duration time" name="expiresIn" min="0" [min]="0" [(ngModel)]="expiresIn">
-            <mat-hint>The duration for the user consent.</mat-hint>
-          </mat-form-field>
-          <mat-form-field fxFlex style="margin-right: 20px;">
-            <mat-select placeholder="Unit time" name="unitTime" [(ngModel)]="unitTime">
-              <mat-option value="seconds">SECONDS</mat-option>
-              <mat-option value="minutes">MINUTES</mat-option>
-              <mat-option value="hours">HOURS</mat-option>
-              <mat-option value="days">DAYS</mat-option>
-              <mat-option value="weeks">WEEKS</mat-option>
-              <mat-option value="months">MONTHS</mat-option>
-              <mat-option value="years">YEARS</mat-option>
-            </mat-select>
-            <mat-hint>Unit time for the user consent duration.</mat-hint>
-          </mat-form-field>
+        <div fxLayout="column" class="user-consent-zone">
+          <h4>User consent</h4>
+          <small>Expiry/lapsing period for consent after which the user would need to confirm that scope.</small>
+          <p fxLayout="row" style="align-items: center; font-size: 90%;">
+            <mat-icon mat-list-icon>alarm</mat-icon>
+            <span>Current period :&nbsp;</span>
+            <span *ngIf="!scope.expiresIn" style="color: grey; font-style: italic;">No time set</span>
+            <span *ngIf="scope.expiresIn">{{ getScopeExpiry() }} <button mat-icon-button (click)="clearExpiry()" matTooltip="Clear expiry periode"><mat-icon>clear</mat-icon></button></span>
+          </p>
+          <div fxLayout="row">
+            <mat-form-field fxFlex style="margin-right: 20px;">
+              <input matInput type="number" placeholder="Duration time" name="expiresIn" min="0" [min]="0" [(ngModel)]="expiresIn">
+              <mat-hint>The duration for the user consent.</mat-hint>
+            </mat-form-field>
+            <mat-form-field fxFlex style="margin-right: 20px;">
+              <mat-select placeholder="Unit time" name="unitTime" [(ngModel)]="unitTime">
+                <mat-option value="seconds">SECONDS</mat-option>
+                <mat-option value="minutes">MINUTES</mat-option>
+                <mat-option value="hours">HOURS</mat-option>
+                <mat-option value="days">DAYS</mat-option>
+                <mat-option value="weeks">WEEKS</mat-option>
+                <mat-option value="months">MONTHS</mat-option>
+                <mat-option value="years">YEARS</mat-option>
+              </mat-select>
+              <mat-hint>Unit time for the user consent duration.</mat-hint>
+            </mat-form-field>
+          </div>
         </div>
-      </div>
 
-      <div fxLayout="row" class="scope-form-actions">
-        <button mat-raised-button [disabled]="(!scopeForm.valid || scopeForm.pristine || formIsInvalid()) && !formChanged" type="submit">SAVE</button>
+        <div fxLayout="row" class="scope-form-actions">
+          <button mat-raised-button [disabled]="(!scopeForm.valid || scopeForm.pristine || formIsInvalid()) && !formChanged" type="submit">SAVE</button>
+        </div>
+      </form>
+
+      <div class="scope-delete-zone" fxLayout="column" *ngIf="!scope.system">
+        <p>This operation cannot be undone.</p>
+        Scope will be deleted, clients and users will not be able to use this scope as permission access.
+        <div class="scope-delete-button" fxLayout="row">
+          <span fxFlex></span>
+          <button mat-raised-button color="warn" (click)="delete($event)">DELETE</button>
+        </div>
       </div>
-    </form>
+    </div>
 
     <div class="scope-description" fxFlex>
       <h3>Manage Scope</h3>
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.scss b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.scss
index ff7d8fe5ad..7dc54239fa 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.scss
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.scss
@@ -3,8 +3,15 @@
   h1 {
     font-weight: 400;
   }
+  a.back-link {
+    color: #006CAB;
+    text-decoration: none;
+    &:hover {
+      color: #2196f3;
+      text-decoration: underline;
+    }
+  }
   .scope-content {
-    padding: 50px 0 20px;
     form {
       padding: 20px;
       background: #fafafa;
@@ -26,6 +33,17 @@
         margin-top: 20px;
       }
     }
+    .scope-delete-zone {
+      color: #f44336;
+      font-size: 75%;
+      margin-top: 20px;
+      padding: 20px;
+      border-radius: 2px;
+      border: 1px solid #e2e5e7;
+      .scope-delete-button {
+        margin-top: 20px;
+      }
+    }
     .scope-description {
       margin-left: 20px;
       h3 {
diff --git a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.ts b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.ts
index 1ace03aaa6..2ed46dd591 100644
--- a/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.ts
+++ b/gravitee-am-ui/src/app/domain/settings/scopes/scope/scope.component.ts
@@ -13,13 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import {Component, OnInit, ViewChild} from '@angular/core';
-import { ActivatedRoute } from "@angular/router";
+import { Component, OnInit, ViewChild } from '@angular/core';
+import { ActivatedRoute, Router } from "@angular/router";
 import { ScopeService } from "../../../../services/scope.service";
 import { SnackbarService } from "../../../../services/snackbar.service";
 import { BreadcrumbService } from "../../../../../libraries/ng2-breadcrumb/components/breadcrumbService";
 import * as moment from "moment";
-import {NgForm} from "@angular/forms";
+import { NgForm } from "@angular/forms";
+import { DialogService } from "../../../../services/dialog.service";
 
 @Component({
   selector: 'app-scope',
@@ -34,8 +35,12 @@ export class ScopeComponent implements OnInit {
   unitTime: any;
   @ViewChild('scopeForm') public scopeForm: NgForm;
 
-  constructor(private scopeService: ScopeService, private snackbarService: SnackbarService, private route: ActivatedRoute,
-              private breadcrumbService: BreadcrumbService) { }
+  constructor(private scopeService: ScopeService,
+              private snackbarService: SnackbarService,
+              private route: ActivatedRoute,
+              private router: Router,
+              private breadcrumbService: BreadcrumbService,
+              private dialogService: DialogService) { }
 
   ngOnInit() {
     this.domainId = this.route.snapshot.parent.parent.params['domainId'];
@@ -77,4 +82,18 @@ export class ScopeComponent implements OnInit {
     this.scope.expiresIn = null;
     this.formChanged = true;
   }
+
+  delete(event) {
+    event.preventDefault();
+    this.dialogService
+      .confirm('Delete Scope', 'Are you sure you want to delete this scope ?')
+      .subscribe(res => {
+        if (res) {
+          this.scopeService.delete(this.domainId, this.scope.id).subscribe(() => {
+            this.snackbarService.open("Scope deleted");
+            this.router.navigate(['/domains', this.domainId, 'settings', 'scopes']);
+          });
+        }
+      });
+  }
 }


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 d135af73ee..331f5d0a34 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
@@ -151,10 +151,34 @@ public class ScopeServiceTest {
     }
 
     @Test
-    public void shouldCreate_keyLowerCase() {
+    public void shouldCreate_keyUpperCase() {
         NewScope newScope = Mockito.mock(NewScope.class);
         when(newScope.getKey()).thenReturn("MY-SCOPE");
-        when(scopeRepository.findByDomainAndKey(DOMAIN, "my-scope")).thenReturn(Maybe.empty());
+        when(scopeRepository.findByDomainAndKey(DOMAIN, "MY-SCOPE")).thenReturn(Maybe.empty());
+        when(scopeRepository.create(any(Scope.class))).thenReturn(Single.just(new Scope()));
+        when(domainService.reload(any(), any())).thenReturn(Single.just(new Domain()));
+
+        TestObserver testObserver = scopeService.create(DOMAIN, newScope).test();
+        testObserver.awaitTerminalEvent();
+
+        testObserver.assertComplete();
+        testObserver.assertNoErrors();
+
+        verify(scopeRepository, times(1)).create(any(Scope.class));
+        verify(scopeRepository, times(1)).create(argThat(new ArgumentMatcher<Scope>() {
+            @Override
+            public boolean matches(Scope scope) {
+                return scope.getKey().equals("MY-SCOPE");
+            }
+        }));
+        verify(domainService, times(1)).reload(any(), any());
+    }
+
+    @Test
+    public void shouldCreate_whiteSpaces() {
+        NewScope newScope = Mockito.mock(NewScope.class);
+        when(newScope.getKey()).thenReturn("MY scope");
+        when(scopeRepository.findByDomainAndKey(DOMAIN, "MY_scope")).thenReturn(Maybe.empty());
         when(scopeRepository.create(any(Scope.class))).thenReturn(Single.just(new Scope()));
         when(domainService.reload(any(), any())).thenReturn(Single.just(new Domain()));
 
@@ -168,7 +192,7 @@ public class ScopeServiceTest {
         verify(scopeRepository, times(1)).create(argThat(new ArgumentMatcher<Scope>() {
             @Override
             public boolean matches(Scope scope) {
-                return scope.getKey().equals("my-scope");
+                return scope.getKey().equals("MY_scope");
             }
         }));
         verify(domainService, times(1)).reload(any(), any());