From bed7063d979800e2cdc338b205f794641d0edeeb Mon Sep 17 00:00:00 2001 From: Huey Date: Fri, 9 Sep 2022 00:15:28 -0700 Subject: [PATCH 1/2] DMND-1080 refactor to not use hard coded strings per Devon S PR comment --- README.md | 2 +- .../config/application-dev.properties | 4 +-- ui/src/app/app-routing.module.ts | 21 +++++------ ui/src/app/auth/auth-roles.ts | 31 +++++++++++----- ui/src/app/auth/auth-service.ts | 6 ++-- ui/src/app/auth/auth.guard.spec.ts | 4 +-- ui/src/app/auth/auth.guard.ts | 2 +- .../collection/collections-list.component.ts | 13 +++---- .../detail/manage-collection.component.ts | 11 +++--- .../navigation/abstract-search.component.ts | 26 ++++++++++++-- .../commoncontrols-mobile.component.html | 6 ++-- .../navigation/commoncontrols.component.html | 6 ++-- .../navigation/commoncontrols.component.ts | 7 ---- ...skill-action-bar-horizontal.component.html | 12 +++---- ...e-skill-action-bar-vertical.component.html | 12 +++---- .../manage-rich-skill-action-bar.component.ts | 35 +++++++++++++++---- .../richskill/list/skills-list.component.ts | 15 ++++---- ui/test/resource/mock-stubs.ts | 10 ++---- 18 files changed, 135 insertions(+), 88 deletions(-) diff --git a/README.md b/README.md index 28aff03ac..43aab5e34 100644 --- a/README.md +++ b/README.md @@ -213,7 +213,7 @@ osmt.security.role.curator=ROLE_Osmt_Curator osmt.security.role.view=ROLE_Osmt_View osmt.security.scope.read=SCOPE_osmt.read ``` -* NOTE: if app.enableRoles=false, all endpoints will be exposed!! +* NOTE: if app.enableRoles=false, all endpoints will be accessible by any authenticated user. * You can use these values, or you can provide your own based on your own authorization tooling. For Okta, you will need to use the uppercase `ROLE_` prefix on your role. * `read` is a scope, not a role. This is for machine-to-machine access, rather than for authenticated OSMT users. diff --git a/api/src/main/resources/config/application-dev.properties b/api/src/main/resources/config/application-dev.properties index bf4b9f23a..abb86d934 100644 --- a/api/src/main/resources/config/application-dev.properties +++ b/api/src/main/resources/config/application-dev.properties @@ -5,8 +5,8 @@ app.baseUrl=http://localhost:8080 # Spring Boot will serve frontend files via port 8080 # If you are using ng serve to proxy static files built by Angular, set OSMT_FRONT_END_PORT to 4200 -#app.frontEndPort=${OSMT_FRONT_END_PORT:4200} -app.frontEndPort=${OSMT_FRONT_END_PORT:8080} +app.frontEndPort=${OSMT_FRONT_END_PORT:4200} +#app.frontEndPort=${OSMT_FRONT_END_PORT:8080} app.frontendUrl=http://localhost:${app.frontEndPort} app.security.cors.allowedOrigins=${app.baseUrl},${app.frontendUrl} diff --git a/ui/src/app/app-routing.module.ts b/ui/src/app/app-routing.module.ts index ce47f5e59..9a16f4b1a 100644 --- a/ui/src/app/app-routing.module.ts +++ b/ui/src/app/app-routing.module.ts @@ -20,7 +20,7 @@ import {ManageCollectionComponent} from "./collection/detail/manage-collection.c import {PublishCollectionComponent} from "./collection/detail/publish-collection.component"; import {CollectionSkillSearchComponent} from "./collection/collection-skill-search.component"; import {BatchImportComponent} from "./richskill/import/batch-import.component"; -import { ACTION_ROLES } from "./auth/auth-roles" +import { ActionByRoles, ButtonAction } from "./auth/auth-roles" const routes: Routes = [ @@ -33,7 +33,8 @@ const routes: Routes = [ component: RichSkillFormComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.SKILLS_CREATE + // roles: ACTION_ROLES.SKILLS_CREATE + roles: ActionByRoles.get(ButtonAction.SkillCreate) }, canDeactivate: [FormDirtyGuard] }, @@ -47,7 +48,7 @@ const routes: Routes = [ component: RichSkillFormComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.SKILL_UPDATE + roles: ActionByRoles.get(ButtonAction.SkillUpdate) }, canDeactivate: [FormDirtyGuard] }, @@ -57,7 +58,7 @@ const routes: Routes = [ canActivate: [AuthGuard], canDeactivate: [FormDirtyGuard], data: { - roles: ACTION_ROLES.SKILLS_CREATE + roles: ActionByRoles.get(ButtonAction.SkillCreate) }, }, // manage skill @@ -75,7 +76,7 @@ const routes: Routes = [ component: BatchImportComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.SKILLS_CREATE + roles: ActionByRoles.get(ButtonAction.SkillCreate) }, }, @@ -86,7 +87,7 @@ const routes: Routes = [ component: CollectionFormComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.COLLECTION_CREATE + roles: ActionByRoles.get(ButtonAction.CollectionCreate) }, canDeactivate: [FormDirtyGuard] }, @@ -100,7 +101,7 @@ const routes: Routes = [ component: CollectionFormComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.COLLECTION_UPDATE + roles: ActionByRoles.get(ButtonAction.CollectionUpdate) }, canDeactivate: [FormDirtyGuard] }, @@ -114,7 +115,7 @@ const routes: Routes = [ component: PublishCollectionComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.COLLECTION_PUBLISH + roles: ActionByRoles.get(ButtonAction.CollectionPublish) }, }, // find skills to add to a collection @@ -122,7 +123,7 @@ const routes: Routes = [ component: CollectionSkillSearchComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.COLLECTION_SKILLS_UPDATE + roles: ActionByRoles.get(ButtonAction.CollectionSkillsUpdate) }, }, // find a collection to add a selection of skills to @@ -130,7 +131,7 @@ const routes: Routes = [ component: AddSkillsCollectionComponent, canActivate: [AuthGuard], data: { - roles: ACTION_ROLES.COLLECTION_SKILLS_UPDATE + roles: ActionByRoles.get(ButtonAction.CollectionSkillsUpdate) }, }, // collections library diff --git a/ui/src/app/auth/auth-roles.ts b/ui/src/app/auth/auth-roles.ts index 12941b651..534fd2e51 100644 --- a/ui/src/app/auth/auth-roles.ts +++ b/ui/src/app/auth/auth-roles.ts @@ -1,15 +1,28 @@ // Default values of OSMT Roles +import {Button} from "protractor"; + const OSMT_ADMIN = "ROLE_Osmt_Admin" const OSMT_CURATOR = "ROLE_Osmt_Curator" -export const ENABLE_ROLES = false +export const ENABLE_ROLES = true -export const ACTION_ROLES : any = { - SKILL_UPDATE : [OSMT_ADMIN, OSMT_CURATOR], - SKILLS_CREATE : [OSMT_ADMIN, OSMT_CURATOR], - SKILL_PUBLISH : [OSMT_ADMIN], - COLLECTION_CREATE : [OSMT_ADMIN, OSMT_CURATOR], - COLLECTION_PUBLISH : [OSMT_ADMIN], - COLLECTION_UPDATE : [OSMT_ADMIN, OSMT_CURATOR], - COLLECTION_SKILLS_UPDATE : [OSMT_ADMIN] +export enum ButtonAction { + SkillUpdate, + SkillCreate, + SkillPublish, + CollectionUpdate, + CollectionCreate, + CollectionPublish, + CollectionSkillsUpdate } + +export const ActionByRoles = new Map([ + [ButtonAction.SkillUpdate, [OSMT_ADMIN, OSMT_CURATOR]], + [ButtonAction.SkillCreate, [OSMT_ADMIN, OSMT_CURATOR]], + [ButtonAction.SkillPublish, [OSMT_ADMIN]], + [ButtonAction.CollectionUpdate, [OSMT_ADMIN, OSMT_CURATOR]], + [ButtonAction.CollectionCreate, [OSMT_ADMIN, OSMT_CURATOR]], + [ButtonAction.CollectionPublish, [OSMT_ADMIN]], + [ButtonAction.CollectionSkillsUpdate, [OSMT_ADMIN]], + ]); + diff --git a/ui/src/app/auth/auth-service.ts b/ui/src/app/auth/auth-service.ts index 69e80da9c..f7f0e04c2 100644 --- a/ui/src/app/auth/auth-service.ts +++ b/ui/src/app/auth/auth-service.ts @@ -1,4 +1,4 @@ -import {ENABLE_ROLES, ACTION_ROLES} from "./auth-roles"; +import {ENABLE_ROLES, ButtonAction, ActionByRoles} from "./auth-roles"; import { Injectable } from "@angular/core" import { Router } from "@angular/router" import { DEFAULT_INTERRUPTSOURCES, Idle } from "@ng-idle/core" @@ -81,9 +81,9 @@ export class AuthService extends Whitelabelled implements IAuthService { return false } - isEnabledByRoles(key : string): boolean { + isEnabledByRoles(buttonAction : ButtonAction): boolean { if (ENABLE_ROLES) { - const allowedRoles = ACTION_ROLES[key]; + const allowedRoles = ActionByRoles.get(buttonAction) ?? []; const userRoles = this.getRole()?.split(","); return this.hasRole(allowedRoles, userRoles); } diff --git a/ui/src/app/auth/auth.guard.spec.ts b/ui/src/app/auth/auth.guard.spec.ts index 1197578f4..a03cc1a52 100644 --- a/ui/src/app/auth/auth.guard.spec.ts +++ b/ui/src/app/auth/auth.guard.spec.ts @@ -4,7 +4,7 @@ import { AuthService } from "./auth-service" import { ActivatedRouteSnapshot, Router, RouterStateSnapshot} from "@angular/router" import { HttpClientTestingModule } from "@angular/common/http/testing" import { AuthServiceStub, RouterStub } from "../../../test/resource/mock-stubs" -import {ACTION_ROLES, ENABLE_ROLES} from "./auth-roles" +import {ENABLE_ROLES, ActionByRoles, ButtonAction} from "./auth-roles" describe("AuthGuard", () => { @@ -38,7 +38,7 @@ describe("AuthGuard", () => { it("should return true", () => { // Arrange const route = Object.assign({}, ActivatedRouteSnapshot.prototype, { - data: {roles: ACTION_ROLES.SKILLS_CREATE} + data: {roles: ActionByRoles.get(ButtonAction.SkillCreate)} }) // Act and Assert diff --git a/ui/src/app/auth/auth.guard.ts b/ui/src/app/auth/auth.guard.ts index 1a06bc9ee..b7e38d272 100644 --- a/ui/src/app/auth/auth.guard.ts +++ b/ui/src/app/auth/auth.guard.ts @@ -1,6 +1,6 @@ import {Injectable} from "@angular/core" import {ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot} from "@angular/router" -import {ENABLE_ROLES, ACTION_ROLES} from "./auth-roles" +import {ENABLE_ROLES} from "./auth-roles" import {AuthService} from "./auth-service" import {ToastService} from "../toast/toast.service" diff --git a/ui/src/app/collection/collections-list.component.ts b/ui/src/app/collection/collections-list.component.ts index a85d64878..eaac2ecd3 100644 --- a/ui/src/app/collection/collections-list.component.ts +++ b/ui/src/app/collection/collections-list.component.ts @@ -12,6 +12,7 @@ import {TableActionDefinition} from "../table/skills-library-table/has-action-de import {TableActionBarComponent} from "../table/skills-library-table/table-action-bar.component" import {Whitelabelled} from "../../whitelabel"; import {AuthService} from "../auth/auth-service"; +import {ButtonAction} from "../auth/auth-roles"; @Component({ @@ -115,33 +116,33 @@ export class CollectionsListComponent extends Whitelabelled { publishVisible(collection?: ApiCollectionSummary): boolean { if (collection !== undefined) { - return collection.publishDate === undefined && this.authService.isEnabledByRoles("COLLECTION_PUBLISH") + return collection.publishDate === undefined && this.authService.isEnabledByRoles(ButtonAction.CollectionPublish) } else if ((this.selectedCollections?.length ?? 0) === 0) { return false } else { const unpublishCollection = this.selectedCollections?.find(s => s.publishDate === undefined) - return (unpublishCollection !== undefined) && (this.authService.isEnabledByRoles("COLLECTION_PUBLISH")) + return (unpublishCollection !== undefined) && this.authService.isEnabledByRoles(ButtonAction.CollectionPublish) } } archiveVisible(collection?: ApiCollectionSummary): boolean { if (collection !== undefined) { - return !checkArchived(collection) && this.authService.isEnabledByRoles("COLLECTION_UPDATE") + return !checkArchived(collection) && this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) } else if ((this.selectedCollections?.length ?? 0) === 0) { return false } else { const unarchCollection = this.selectedCollections?.find(s => !checkArchived(s)) - return unarchCollection !== undefined && this.authService.isEnabledByRoles("COLLECTION_UPDATE") + return unarchCollection !== undefined && this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) } } unarchiveVisible(collection?: ApiCollectionSummary): boolean { if (collection !== undefined) { - return checkArchived(collection) && this.authService.isEnabledByRoles("COLLECTION_UPDATE") + return checkArchived(collection) && this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) } else if ((this.selectedCollections?.length ?? 0) === 0) { return false } else { const archCollection = this.selectedCollections?.find(checkArchived) - return archCollection !== undefined && this.authService.isEnabledByRoles("COLLECTION_UPDATE") + return archCollection !== undefined && this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) } } diff --git a/ui/src/app/collection/detail/manage-collection.component.ts b/ui/src/app/collection/detail/manage-collection.component.ts index b93ad8e14..7a8b138e6 100644 --- a/ui/src/app/collection/detail/manage-collection.component.ts +++ b/ui/src/app/collection/detail/manage-collection.component.ts @@ -15,6 +15,7 @@ import {Observable, Subject} from "rxjs" import {TableActionBarComponent} from "../../table/skills-library-table/table-action-bar.component" import {Title} from "@angular/platform-browser"; import {AuthService} from "../../auth/auth-service"; +import {ButtonAction} from "../../auth/auth-roles"; @Component({ selector: "app-manage-collection", @@ -142,13 +143,13 @@ export class ManageCollectionComponent extends SkillsListComponent implements On icon: this.addIcon, primary: !this.collectionHasSkills, // Primary only if there are no skills callback: () => this.addSkillsAction(), - visible: () => this.authService.isEnabledByRoles("COLLECTION_SKILLS_UPDATE") + visible: () => this.authService.isEnabledByRoles(ButtonAction.CollectionSkillsUpdate) }), new TableActionDefinition({ label: "Edit Collection Name", icon: this.editIcon, callback: () => this.editAction(), - visible: () => this.authService.isEnabledByRoles("COLLECTION_UPDATE") + visible: () => this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) }) ] @@ -163,7 +164,7 @@ export class ManageCollectionComponent extends SkillsListComponent implements On label: "Publish Collection", icon: this.publishIcon, callback: () => this.publishAction(), - visible: () => this.authService.isEnabledByRoles("COLLECTION_PUBLISH") + visible: () => this.authService.isEnabledByRoles(ButtonAction.CollectionPublish) })) } @@ -173,7 +174,7 @@ export class ManageCollectionComponent extends SkillsListComponent implements On label: "Archive Collection ", icon: this.archiveIcon, callback: () => this.archiveAction(), - visible: () => this.authService.isEnabledByRoles("COLLECTION_UPDATE") + visible: () => this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) })) } else if (this.collection?.status === PublishStatus.Archived || this.collection?.status === PublishStatus.Deleted) { actions.push( @@ -181,7 +182,7 @@ export class ManageCollectionComponent extends SkillsListComponent implements On label: "Unarchive Collection ", icon: this.unarchiveIcon, callback: () => this.unarchiveAction(), - visible: () => this.authService.isEnabledByRoles("COLLECTION_UPDATE") + visible: () => this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate) })) } return actions diff --git a/ui/src/app/navigation/abstract-search.component.ts b/ui/src/app/navigation/abstract-search.component.ts index 83d92f7bd..b67a67c36 100644 --- a/ui/src/app/navigation/abstract-search.component.ts +++ b/ui/src/app/navigation/abstract-search.component.ts @@ -2,12 +2,21 @@ import {FormControl, FormGroup} from "@angular/forms"; import {SearchService} from "../search/search.service"; import {ActivatedRoute} from "@angular/router"; import {AuthService} from "../auth/auth-service"; +import {ButtonAction} from "../auth/auth-roles"; export class AbstractSearchComponent { searchForm = new FormGroup({ search: new FormControl("") }) + canSkillUpdate: boolean = false + canSkillCreate: boolean = false + canSkillPublish: boolean = false + canCollectionUpdate: boolean = false + canCollectionCreate: boolean = false + canCollectionPublish: boolean = false + canCollectionSkillsUpdate: boolean = false + constructor(protected searchService: SearchService, protected route: ActivatedRoute, protected authService: AuthService) { this.searchService.searchQuery$.subscribe(apiSearch => { if (apiSearch === undefined) { @@ -21,6 +30,17 @@ export class AbstractSearchComponent { this.searchForm.setValue({search: queryString}) } }) + this.setEnableFlags() + } + + setEnableFlags(): void { + this.canSkillUpdate = this.authService.isEnabledByRoles(ButtonAction.SkillUpdate); + this.canSkillCreate = this.authService.isEnabledByRoles(ButtonAction.SkillCreate); + this.canSkillPublish = this.authService.isEnabledByRoles(ButtonAction.SkillPublish); + this.canCollectionUpdate = this.authService.isEnabledByRoles(ButtonAction.CollectionUpdate); + this.canCollectionCreate = this.authService.isEnabledByRoles(ButtonAction.CollectionCreate); + this.canCollectionPublish = this.authService.isEnabledByRoles(ButtonAction.CollectionPublish); + this.canCollectionSkillsUpdate = this.authService.isEnabledByRoles(ButtonAction.CollectionSkillsUpdate); } clearSearch(): boolean { @@ -50,8 +70,8 @@ export class AbstractSearchComponent { return false } - isEnabled(path: string): boolean { - return this.authService.isEnabledByRoles(path); - } + // isEnabled(path: string): boolean { + // return this.authService.isEnabledByRoles(path); + // } } diff --git a/ui/src/app/navigation/commoncontrols-mobile.component.html b/ui/src/app/navigation/commoncontrols-mobile.component.html index 15ebd807b..995fafbd0 100644 --- a/ui/src/app/navigation/commoncontrols-mobile.component.html +++ b/ui/src/app/navigation/commoncontrols-mobile.component.html @@ -55,7 +55,7 @@