feat(UI): AccessManagement UI to access the role metadata for a dataset #8541

Ramendra761 · 2023-08-01T18:55:40Z

The aim of this feature is to add a new tab for Access Management to enable users to view the required roles (Roles/Policies defined in Access Management System of an organisation) for accessing a dataset in that organisation for a dataset. Also allow user to raise the request for a required role if user dont have access.

People can navigate to a dataset and view list of roles necessary to access that dataset. We can click on a dataset role and request access from that page.

Attaching the RFC for further details
rfc-DatahubUI.pdf
Uploading RFC.-.Addition.of.External.Access.Managemt.pdf…

reference PR - 8499

Checklist

The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
Links to related issues (if applicable)
Tests for the changes have been added/updated (if applicable)
Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

metadata-service/factories/src/main/resources/application.yml

Ramendra761 · 2023-08-04T06:52:08Z

@jjoyce0510
could you please take a look.
thanks ,
Ramendra

jjoyce0510 · 2023-08-17T19:59:18Z

...b-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagerDescription.tsx

+};
+
+export default function AccessManagerDescription({ description }: Props) {
+    const DescriptionContainer = styled.div`


please place these styled components at the top of the file (similar to other components)

done update

jjoyce0510 · 2023-08-17T20:00:12Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessRolesDetail.tsx

@@ -0,0 +1,27 @@
+export default function handleExternalRoles(externalRoles, loggedInUser) {


let's name this file utils.ts since it's just a utility function

we can also remove the "Default" in front of function

renamed to utils.ts

jjoyce0510 · 2023-08-17T20:01:40Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessRolesDetail.tsx

+        externalRoles?.dataset?.access.roles.length > 0
+    ) {
+        externalRoles?.dataset?.access?.roles?.forEach((userRoles) => {
+            const arr = {


what does "arr" mean here?

can we name this "role"

done update

jjoyce0510 · 2023-08-17T20:02:01Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessRolesDetail.tsx

+            const arr = {
+                name: userRoles?.role?.properties?.name || ' ',
+                description: userRoles?.role?.properties?.description || ' ',
+                accesstype: userRoles?.role?.properties?.type || ' ',


camelCase please!

accessType

done update

jjoyce0510 · 2023-08-17T20:02:19Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessRolesDetail.tsx

+                name: userRoles?.role?.properties?.name || ' ',
+                description: userRoles?.role?.properties?.description || ' ',
+                accesstype: userRoles?.role?.properties?.type || ' ',
+                access:


let's name this:

hasAccess

to imply that it is boolean

done update

jjoyce0510 · 2023-08-17T20:09:19Z

...b-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagerDescription.tsx

+        setIsExpanded(!expanded);
+    };
+
+    if (description.length > 150) {


can we combine these 2 blocks into 1?

const shouldTruncateDescription = description.length > 150; const [expanded, setIsExpanded] = useState(!shouldTruncateDescription); const finalDescription = expanded ? description : description.slice(0, 150); return ( <DescriptionContainer> {finalDescription} <Typography.Link onClick={() => { toggleExpanded(); }} > {shouldTruncate && (expanded ? ' Read Less' : '...Read More') || undefined} </Typography.Link> </DescriptionContainer> );

done update

jjoyce0510 · 2023-08-17T20:09:42Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+    ];
+
+    return (
+        <div>


can we remove this additional div?

done update

jjoyce0510 · 2023-08-17T20:10:07Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+            render: (text, record) => {
+                if (text) {
+                    return (
+                        <div


pleae use styled component

done update

jjoyce0510 · 2023-08-17T20:10:50Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+                    <button
+                        type="button"
+                        style={{
+                            backgroundColor: '#1890ff',


please use styled component

done update

jjoyce0510 · 2023-08-17T20:11:05Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+
+    const columns = [
+        {
+            title: 'RoleName',


done update

jjoyce0510 · 2023-08-17T20:11:26Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+import AccessManagerDescription from './AccessManagerDescription';
+
+export default function AccessManagement() {
+    const StyledTable = styled(Table)`


please place this at the top of the file, similar to toher components

done update

jjoyce0510 · 2023-08-17T20:11:41Z

datahub-web-react/src/app/entity/dataset/DatasetEntity.tsx

@@ -68,6 +70,8 @@ export class DatasetEntity implements Entity<Dataset> {

    isSearchEnabled = () => true;

+    isAccessManagementEnabled = useIsShowAccessManagementEnabled;


let's get rid of this

recieving below error
"React Hook "useIsShowAccessManagementEnabled" is called in function "visible" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter react-hooks/rules-of-hooks"

to resolve that we used isAccessManagementEnabled

done update

(please remove this line)

checking w my colleague @chriscollins3456

jjoyce0510 · 2023-08-17T20:12:09Z

datahub-web-react/src/app/entity/dataset/DatasetEntity.tsx

+                    name: 'Access Management',
+                    component: AccessManagement,
+                    display: {
+                        visible: (_, _1) => this.isAccessManagementEnabled(),


just use the hook here:

visible: (_, _1) => useIsShowAccessManagementEnabled(),

recieving below error
"React Hook "useIsShowAccessManagementEnabled" is called in function "visible" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter react-hooks/rules-of-hooks"

to resolve that we used isAccessManagementEnabled

done update

got it. here's what we want to do then... pass a new optional parameter into the function called "appConfig" in line 249 on EntityProfile.tsx

we can use a context object actually, then the method can just use that instead

hi @jjoyce0510 we used a context object still had hooks issue. as a solution we called "appConfig" in datasetEntity.tsx

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/featureflags/FeatureFlags.java

jjoyce0510 · 2023-08-29T22:45:48Z

datahub-web-react/src/app/shared/tags/Access/accessTerm.tsx

+    fontSize?: number;
+}
+
+export default function AccessTermstyle({ data, fontSize, highlightText }: Props) {


Rename the file name and let's revisit the name of this component

Maybe we name it AccessRoleLabel.tsx?

removed the changes as we cannot make changes in the default preview card

jjoyce0510 · 2023-08-31T17:28:35Z

datahub-web-react/src/app/shared/tags/TagTermGroup.tsx

+                if (maxShow && renderedTags > maxShow) return null;
+                return (
+                    <div>
+                        {console.log(role?.role)}


please remove...

removed the changes as we cannot make changes in the default preview card

jjoyce0510 · 2023-08-31T20:06:26Z

datahub-web-react/src/app/entity/Access/Access.tsx

+            entityType={EntityType.Role}
+            useEntityQuery={useGetExternalRoleQuery}
+            getOverrideProperties={this.getOverridePropertiesFromEntity}
+            tabs={[]}


Do you want Documentation tab?

Since you have the About section, which displays the same information

added RoleEntityProfile in renderpreview similar to tagProfile.tsx

jjoyce0510 · 2023-08-31T20:12:33Z

datahub-web-react/src/app/entity/dataset/DatasetEntity.tsx

@@ -252,6 +264,7 @@ export class DatasetEntity implements Entity<Dataset> {
                platformLogo={data.platform.properties?.logoUrl}
                platformInstanceId={data.dataPlatformInstance?.instanceId}
                owners={data.ownership?.owners}
+                access={this.isAccessManagementEnabled() ? data?.access : null}


Please do not change this preview DatasetEntity.tsx - what are you hoping to achieve with this?

done update

jjoyce0510 · 2023-08-31T20:13:06Z

datahub-web-react/src/app/entity/dataset/DatasetEntity.tsx

@@ -290,6 +303,7 @@ export class DatasetEntity implements Entity<Dataset> {
                glossaryTerms={data.glossaryTerms}
                subtype={data.subTypes?.typeNames?.[0]}
                container={data.container}
+                access={this.isAccessManagementEnabled() ? data?.access : null}


Please do not change this

done update

jjoyce0510 · 2023-08-31T20:13:18Z

datahub-web-react/src/app/entity/dataset/preview/Preview.tsx

@@ -101,6 +104,7 @@ export const Preview = ({
            tags={globalTags || undefined}
            owners={owners}
            domain={domain}
+            access={access}


Please do not change this

done update

jjoyce0510 · 2023-08-31T20:15:05Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+            key: 'hasAccess',
+            render: (text, record) => {
+                if (text) {
+                    return <Accessprovision>Provisioned</Accessprovision>;


Why are we using a Divider component to wrap this text? Let's use a or something similar

updated with StyledSection

jjoyce0510 · 2023-08-31T20:16:12Z

datahub-web-react/src/app/preview/DefaultPreviewCard.tsx

@@ -355,6 +359,8 @@ export default function DefaultPreviewCard({
                        {hasGlossaryTerms && <TagTermGroup uneditableGlossaryTerms={glossaryTerms} maxShow={3} />}
                        {((hasGlossaryTerms && hasTags) || ((dataProduct || domain) && hasTags)) && <TagSeparator />}
                        {hasTags && <TagTermGroup uneditableTags={tags} maxShow={3} />}
+                        {hasAccess && <TagSeparator />}
+                        {hasAccess && <TagTermGroup uneditableAccess={access} maxShow={3} />}


Remove this please. We do not want to accept changes to one of the most important components we have - the default preview card.

done update

jjoyce0510 · 2023-08-31T20:18:22Z

datahub-web-react/src/app/useAppConfig.ts

@@ -12,3 +12,8 @@ export function useIsShowAcrylInfoEnabled() {
    const appConfig = useAppConfig();
    return appConfig.config.featureFlags.showAcrylInfo;
 }
+
+export function useIsShowAccessManagementEnabled(): boolean {


Once we pass the appConfig into the visible function, we should no longer need this method

removed useIsShowAccessManagementEnabled()

anshbansal · 2023-09-07T07:06:49Z

datahub-graphql-core/src/main/resources/entity.graphql

@@ -1451,7 +1455,7 @@ type Role implements Entity {
    """
    Role properties to include Request Access Url
    """
-    properties: RoleProperties!
+    properties: RoleProperties


Why is this no longer an optional field? What happens if someone wishes to create a role without properties?

Hi @anshbansal we are facing issues in search and autocomplete in web-react search.graphql if we are making it as optional field. i.e why we update the same.

chriscollins3456

nice stuff here gang! this PR is very very close. I only have one real request and that's around an error I saw in the UI when no actors were associated with a role and so we weren't setting it on the role graphql object but we defined it as required on the graphql side (simple fix is simply make actors not required to return on graphql)

Otherwise I have a few small suggestions and wanted to call out how the profile page is pretty bare bones right now but maybe that's alright.

chriscollins3456 · 2023-09-07T14:52:07Z

datahub-web-react/src/app/entity/Access/RoleEntityProfile.tsx

+            <Paragraph style={{ fontSize: '12px', lineHeight: '15px', padding: '5px 0px' }}>
+                {data?.role?.properties?.description}
+            </Paragraph>


so you don't want to allow users to edit role entity descriptions through the UI? you could get the same functionality as other entities if you used EntityProfile and passed in a list of tabs and sidebar sections instead of defining your own entity profile page for this one entity

i'm just thinking this is pretty bare bones but maybe that is alright for now.

hi @chriscollins3456 for Roles the description is not editable. We have description for the accessRole coming from an external source we are updating in datahub if we provide option to edit the dscription we would not be able to update the same at source.

chriscollins3456 · 2023-09-07T15:33:58Z

datahub-graphql-core/src/main/resources/entity.graphql

@@ -1451,7 +1455,7 @@ type Role implements Entity {
    """
    Role properties to include Request Access Url
    """
-    properties: RoleProperties!
+    properties: RoleProperties

    """
    A standard Entity Type


i'm getting an error when trying locally if I don't set any role actors on a role saying that this is a required field that was not set.

I'd make this field not required for graphql since we aren't always setting actors on the result as you can see in RoleMapper.java

final EnvelopedAspect envelopedUsers = aspects.get(Constants.ROLE_ACTORS_ASPECT_NAME); if (envelopedUsers != null) { result.setActors(mapActor(new Actors(envelopedUsers.getValue().data()))); }

Or alternatively we could always set an empty list of actors and always return it saying we don't have any actors. either way we need to prevent this error from popping up:

updated the actor as optional.

chriscollins3456 · 2023-09-07T15:41:08Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/AccessManagement.tsx

+            render: (text, record) => {
+                if (text) {


text is a bit confusing here. can it be called hasAccess instead?

chriscollins3456 · 2023-09-07T15:42:08Z

datahub-web-react/src/app/entity/shared/tabs/Dataset/AccessManagement/utils.tsx

+                            (user) => user.user.urn === loggedInUser?.me?.corpUser.urn,
+                        )) ||
+                    false,
+                url: userRoles?.role?.properties?.requestUrl || window.location.href,


minor: so if there's no requestUrl on the role we just send them to this page? I feel like the button should either not exist to request access or it should be disabled with a tooltip saying this role has no request access URL associated with it

hi @chriscollins3456 updated we have removed request button if there is no request url

chriscollins3456

nice thanks for addressing those comments! this is looking good to me thanks for all of your work here 👍

YuriyGavrilov · 2024-01-09T14:13:40Z

@Ramendra761 Could you please describe shortly how to use this feature? just deployed 0.12 but didn't see any Access management tab in the UI.

stevenayers · 2024-04-08T13:16:43Z

@Ramendra761 nice work!

I enabled access management via the SHOW_ACCESS_MANAGEMENT env var in the gms container and I can see the tab in the UI, but I'm unsure how to add roles that can be granted. Are there any docs on this yet?

Ramendra761 mentioned this pull request Aug 1, 2023

feat(UI): AccessManagement UI to access the role metadata for a dataset #8499

Closed

5 tasks

github-actions bot added product PR or Issue related to the DataHub UI/UX devops PR or Issue related to DataHub backend & deployment labels Aug 1, 2023

vercel bot deployed to Preview August 1, 2023 19:52 View deployment

vercel bot deployed to Preview August 2, 2023 11:24 View deployment

anshbansal reviewed Aug 2, 2023

View reviewed changes

metadata-service/factories/src/main/resources/application.yml Outdated Show resolved Hide resolved

Ramendra761 force-pushed the accessmngtUI branch 2 times, most recently from 6d93f20 to be51228 Compare August 4, 2023 06:15

vercel bot deployed to Preview August 4, 2023 06:43 View deployment

Ramendra Srivastava and others added 3 commits August 6, 2023 19:44

Adding AccessManagement UI feature

e094431

adding UI feature with set FeatureFlag

49e23aa

modified application.yml and updated yarnlintfix

f4aeda1

Ramendra761 force-pushed the accessmngtUI branch from be51228 to f4aeda1 Compare August 6, 2023 14:14

vercel bot deployed to Preview August 6, 2023 14:30 View deployment

maggiehays assigned chriscollins3456 Aug 17, 2023

jjoyce0510 reviewed Aug 17, 2023

View reviewed changes

datahub-graphql-core/src/main/java/com/linkedin/datahub/graphql/featureflags/FeatureFlags.java Show resolved Hide resolved

jjoyce0510 reviewed Aug 29, 2023

View reviewed changes

jjoyce0510 reviewed Aug 31, 2023

View reviewed changes

vercel bot deployed to Preview September 1, 2023 09:43 View deployment

updating comments added by John in 8541

295743d

Ramendra761 force-pushed the accessmngtUI branch from 1b032a2 to 295743d Compare September 1, 2023 18:28

Ramendra761 added 2 commits September 2, 2023 00:04

update changes

eb6688b

updating tag params

452b155

vercel bot deployed to Preview September 1, 2023 19:05 View deployment

updated application.yml

93865cf

vercel bot deployed to Preview September 4, 2023 07:20 View deployment

Ramendra761 and others added 2 commits September 4, 2023 14:16

resolving conflict

473815b

Merge branch 'master' into accessmngtUI

2f36745

vercel bot deployed to Preview September 4, 2023 09:20 View deployment

anshbansal reviewed Sep 7, 2023

View reviewed changes

chriscollins3456 reviewed Sep 7, 2023

View reviewed changes

updated Chris comment 8541

7a4f546

vercel bot deployed to Preview September 8, 2023 12:40 View deployment

chriscollins3456 approved these changes Sep 8, 2023

View reviewed changes

anshbansal merged commit 75252a3 into datahub-project:master Sep 8, 2023
40 checks passed

This was referenced Mar 25, 2024

feat(accessRequests) #9728

Closed

demo(data-access-management): data model design #10061

Draft

		@@ -0,0 +1,27 @@
		export default function handleExternalRoles(externalRoles, loggedInUser) {

		@@ -68,6 +70,8 @@ export class DatasetEntity implements Entity<Dataset> {

		isSearchEnabled = () => true;

		isAccessManagementEnabled = useIsShowAccessManagementEnabled;

feat(UI): AccessManagement UI to access the role metadata for a dataset #8541

feat(UI): AccessManagement UI to access the role metadata for a dataset #8541

Conversation

Ramendra761 commented Aug 1, 2023

Checklist

Ramendra761 commented Aug 4, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ramendra761 Sep 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscollins3456 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Ramendra761 Sep 8, 2023 • edited Loading

Choose a reason for hiding this comment

chriscollins3456 Sep 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chriscollins3456 left a comment

Choose a reason for hiding this comment

YuriyGavrilov commented Jan 9, 2024

stevenayers commented Apr 8, 2024

Ramendra761 Sep 1, 2023 •

edited

Loading

Ramendra761 Sep 8, 2023 •

edited

Loading

chriscollins3456 Sep 7, 2023 •

edited

Loading