-
Notifications
You must be signed in to change notification settings - Fork 40
feat: authorization dataset tiles #2450
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| import { testConfig } from "../../fixtures/testData"; | ||
| import { mergeConfig } from "../../support/utils"; | ||
|
|
||
| describe("Dataset Detail Dynamic Tile Authorization", () => { | ||
| const authTestConfig = { | ||
| datasetDetailComponent: { | ||
| enableCustomizedComponent: true, | ||
| customization: [ | ||
| { | ||
| type: "regular", | ||
| label: "Admin Only Section", | ||
| order: 1, | ||
| row: 1, | ||
| col: 10, | ||
| authorization: ["admin"], | ||
| fields: [ | ||
| { element: "text", source: "datasetName", order: 0 }, | ||
| { element: "copy", source: "description", order: 1 }, | ||
| ], | ||
| }, | ||
| { | ||
| type: "regular", | ||
| label: "Public Section", | ||
| order: 2, | ||
| row: 1, | ||
| col: 10, | ||
| authorization: ["#all"], | ||
| fields: [ | ||
| { element: "text", source: "ownerEmail", order: 0 }, | ||
| { element: "tag", source: "keywords", order: 1 }, | ||
| ], | ||
| }, | ||
| { | ||
| type: "regular", | ||
| label: "Archive Manager Section", | ||
| order: 3, | ||
| row: 1, | ||
| col: 10, | ||
| authorization: ["archivemanager"], | ||
| fields: [ | ||
| { element: "text", source: "type", order: 0 }, | ||
| { element: "date", source: "creationTime", order: 1 }, | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| }; | ||
|
|
||
| const adminUsername = Cypress.env("username"); | ||
| const adminPassword = Cypress.env("password"); | ||
| const archiveManagerUsername = Cypress.env("secondaryUsername"); | ||
| const archiveManagerPassword = Cypress.env("secondaryPassword"); | ||
| const guestUsername = Cypress.env("guestUsername"); | ||
| const guestPassword = Cypress.env("guestPassword"); | ||
|
|
||
| beforeEach(() => { | ||
| cy.readFile("CI/e2e/frontend.config.e2e.json").then((baseConfig) => { | ||
| const mergedConfig = mergeConfig(baseConfig, authTestConfig); | ||
| cy.intercept("GET", "**/admin/config", mergedConfig).as( | ||
| "getFrontendConfig", | ||
| ); | ||
| }); | ||
|
|
||
| cy.login(adminUsername, adminPassword); | ||
| cy.createDataset({ type: "raw" }); | ||
| cy.visit("/datasets"); | ||
| cy.wait("@getFrontendConfig"); | ||
| }); | ||
|
|
||
| after(() => { | ||
| cy.removeDatasets(); | ||
| }); | ||
|
|
||
| it("should show all tiles to admin user", () => { | ||
| cy.finishedLoading(); | ||
|
|
||
| cy.get('[data-cy="text-search"]').clear().type("Cypress"); | ||
| cy.get('[data-cy="search-button"]').click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| cy.get("mat-row").contains("Cypress Dataset").click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| // Admin user should see all sections | ||
| cy.get('[data-cy="section-label"]').should("contain", "Admin Only Section"); | ||
| cy.get('[data-cy="section-label"]').should("contain", "Public Section"); | ||
| // But NOT archiv3e manager only section | ||
| cy.get('[data-cy="section-label"]:contains("Archive Manager Section")').should( | ||
| "not.exist", | ||
| ); | ||
|
|
||
| }); | ||
|
|
||
| it("should hide admin-only tile from archiveManager user", () => { | ||
| cy.login(archiveManagerUsername, archiveManagerPassword); | ||
| cy.visit("/datasets"); | ||
| cy.wait("@getFrontendConfig"); | ||
|
|
||
| cy.finishedLoading(); | ||
|
|
||
| cy.get('[data-cy="text-search"]').clear().type("Cypress"); | ||
| cy.get('[data-cy="search-button"]').click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| cy.get("mat-row").contains("Cypress Dataset").click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| // Archive Manager should see public and archiveManager sections | ||
| cy.get('[data-cy="section-label"]').should("contain", "Public Section"); | ||
| cy.get('[data-cy="section-label"]').should("contain", "Archive Manager Section"); | ||
| // But NOT admin-only section | ||
| cy.get('[data-cy="section-label"]:contains("Admin Only Section")').should( | ||
| "not.exist", | ||
| ); | ||
| }); | ||
|
|
||
| it("should only show public tile to guest user", () => { | ||
| cy.login(guestUsername, guestPassword); | ||
| cy.visit("/datasets"); | ||
| cy.wait("@getFrontendConfig"); | ||
|
|
||
| cy.finishedLoading(); | ||
|
|
||
| cy.get('[data-cy="text-search"]').clear().type("Cypress"); | ||
| cy.get('[data-cy="search-button"]').click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| cy.get("mat-row").contains("Cypress Dataset").click(); | ||
|
|
||
| cy.isLoading(); | ||
|
|
||
| // Guest user (user1) should only see public section | ||
| cy.get('[data-cy="section-label"]').should("contain", "Public Section"); | ||
| // Should NOT see admin-only or archiveManager sections | ||
| cy.get('[data-cy="section-label"]:contains("Admin Only Section")').should( | ||
| "not.exist", | ||
| ); | ||
| cy.get('[data-cy="section-label"]:contains("Archive Manager Section")').should( | ||
| "not.exist", | ||
| ); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,20 @@ | ||
| import { Component, OnInit, OnDestroy } from "@angular/core"; | ||
|
|
||
| import { MatDialog } from "@angular/material/dialog"; | ||
| import { Store } from "@ngrx/store"; | ||
| import { Subscription } from "rxjs"; | ||
| import { createSelector, Store } from "@ngrx/store"; | ||
| import { Subscription, Observable } from "rxjs"; | ||
| import { map } from "rxjs/operators"; | ||
|
|
||
| import { showMessageAction } from "state-management/actions/user.actions"; | ||
| import { | ||
| selectCurrentAttachments, | ||
| selectCurrentDataset, | ||
| selectCurrentDatasetWithoutFileInfo, | ||
| } from "state-management/selectors/datasets.selectors"; | ||
| import { selectIsLoading } from "state-management/selectors/user.selectors"; | ||
| import { | ||
| selectIsLoading, | ||
| selectProfile, | ||
| } from "state-management/selectors/user.selectors"; | ||
| import { selectCurrentInstrument } from "state-management/selectors/instruments.selectors"; | ||
|
|
||
| import { AppConfigService } from "app-config.service"; | ||
|
|
@@ -35,6 +39,12 @@ import { | |
| ActionItems, | ||
| } from "shared/modules/configurable-actions/configurable-action.interfaces"; | ||
|
|
||
| // profile.selectors.ts | ||
| export const selectProfileAccessGroups = createSelector( | ||
| selectProfile, | ||
| (profile) => profile?.accessGroups || [], | ||
| ); | ||
|
|
||
| /** | ||
| * Component to show customizable details for a dataset, using the | ||
| * form component | ||
|
|
@@ -50,7 +60,7 @@ import { | |
| export class DatasetDetailDynamicComponent implements OnInit, OnDestroy { | ||
| private subscriptions: Subscription[] = []; | ||
|
|
||
| datasetView: CustomizationItem[]; | ||
| datasetView$: Observable<CustomizationItem[]>; | ||
| form: FormGroup; | ||
| cols = 10; | ||
| gutterSize = 12; | ||
|
|
@@ -65,6 +75,8 @@ export class DatasetDetailDynamicComponent implements OnInit, OnDestroy { | |
| loading$ = this.store.select(selectIsLoading); | ||
| show = false; | ||
|
|
||
| userGroups$: Observable<string[]>; | ||
|
|
||
| instrument: Instrument | undefined; | ||
| dataset: OutputDatasetObsoleteDto | undefined; | ||
|
|
||
|
|
@@ -83,7 +95,9 @@ export class DatasetDetailDynamicComponent implements OnInit, OnDestroy { | |
| private router: Router, | ||
| private route: ActivatedRoute, | ||
| private snackBar: MatSnackBar, | ||
| ) {} | ||
| ) { | ||
| this.userGroups$ = this.store.select(selectProfileAccessGroups); | ||
| } | ||
|
|
||
| ngOnInit() { | ||
| this.form = this.fb.group({}); | ||
|
|
@@ -97,7 +111,13 @@ export class DatasetDetailDynamicComponent implements OnInit, OnDestroy { | |
| } | ||
| }); | ||
|
|
||
| this.datasetView = sortedDatasetView; | ||
| this.datasetView$ = this.userGroups$.pipe( | ||
| map((userGroups) => | ||
| sortedDatasetView.filter((section) => | ||
| this.canViewBlock(section.authorization, userGroups), | ||
| ), | ||
| ), | ||
| ); | ||
|
|
||
| this.subscriptions.push( | ||
| this.store.select(selectCurrentInstrument).subscribe((instrument) => { | ||
|
|
@@ -120,6 +140,29 @@ export class DatasetDetailDynamicComponent implements OnInit, OnDestroy { | |
| ); | ||
| } | ||
|
|
||
| /** | ||
| * Checks if the current user can view a block based on authorization | ||
| * @param blockAuthorization - Optional array of group names from block.authorization | ||
| * @param userGroups - Array of groups the current user belongs to | ||
| * @returns true if user can view the block | ||
| */ | ||
| canViewBlock( | ||
| blockAuthorization: string[] | undefined, | ||
| userGroups: string[], | ||
| ): boolean { | ||
| const auth = blockAuthorization || ["#all"]; | ||
|
|
||
| if (auth.includes("#all")) { | ||
| return true; | ||
| } | ||
|
|
||
| if (auth.length === 0) { | ||
| return false; | ||
| } | ||
|
Comment on lines
+158
to
+161
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this condition intentional? that I'm not sure which behavior is more intuitive if (auth.length === 0 || auth.includes("#all")) {
return true;
}or return false when authorization is an empty array
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this seems inconsistent, arising from the fact that an empty array is "trueish" in js ( as opposed to e.g python), casing line +153 to pass on the empty array. I would adjust the code either map empty to "#all" or if possible handle it in the config setting to set it to undefined/null if all elements are dropped |
||
|
|
||
| return auth.some((group) => userGroups.includes(group)); | ||
| } | ||
|
|
||
| onCopy(value: string) { | ||
| navigator.clipboard.writeText(value).then( | ||
| () => { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -50,6 +50,12 @@ export const light: Theme = { | |
| "--theme-header-4-lighter-contrast": "#000000", | ||
| "--theme-header-4-darker": "#9b429b", | ||
| "--theme-header-4-darker-contrast": "#ffffff", | ||
| "--theme-header-5-default": "#7f7f7f", | ||
| "--theme-header-5-default-contrast": "#ffffff", | ||
| "--theme-header-5-lighter": "#e5e5e5", | ||
| "--theme-header-5-lighter-contrast": "#000000", | ||
| "--theme-header-5-darker": "#4c4c4c", | ||
| "--theme-header-5-darker-contrast": "#ffffff", | ||
|
Comment on lines
+53
to
+58
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Backend needs to be updated accordingly |
||
| "--theme-hover-default": "#7f7f7f", | ||
| "--theme-hover-default-contrast": "#000000", | ||
| "--theme-hover-lighter": "#e5e5e5", | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be removed by adding
userGroups$ = this.store.select(selectProfileAccessGroups);on line 78