Skip to content

PM-3904 Restrict engagements tab to only admins #1730

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.

Sign up for GitHub

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

Merged
kkartunov merged 1 commit into from
Feb 17, 2026
Merged

PM-3904 Restrict engagements tab to only admins #1730

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/components/Tab/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ const Tab = ({
selectTab,
projectId,
canViewAssets,
canViewEngagements,
canViewEngagements, // Admin or TM
isAdmin, // Only admin
onBack
}) => {
const projectTabs = [
Expand All @@ -21,7 +22,7 @@ const Tab = ({
: [
{ id: 1, label: 'All Work' },
{ id: 2, label: 'Projects' },
...(canViewEngagements ? [{ id: 3, label: 'Engagements' }] : []),
...(isAdmin ? [{ id: 3, label: 'Engagements' }] : []),
Comment thread
kkartunov marked this conversation as resolved.
{ id: 4, label: 'Users' },
{ id: 5, label: 'Self-Service' },
{ id: 6, label: 'TaaS' },
Expand Down Expand Up @@ -88,6 +89,7 @@ Tab.defaultProps = {
projectId: null,
canViewAssets: true,
canViewEngagements: false,
isAdmin: false,
onBack: () => {}
}

Expand All @@ -97,6 +99,7 @@ Tab.propTypes = {
projectId: PT.oneOfType([PT.string, PT.number]),
canViewAssets: PT.bool,
canViewEngagements: PT.bool,
isAdmin: PT.bool,
onBack: PT.func
}

Expand Down
24 changes: 18 additions & 6 deletions src/containers/Tab/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,13 @@ class TabContainer extends Component {
return !!resolvedToken && checkAdminOrTalentManager(resolvedToken)
}

getIsAdmin (props = this.props) {
const { token: currentToken } = this.props
const { token } = props
const resolvedToken = token || currentToken
return !!resolvedToken && checkAdmin(resolvedToken)
}

componentDidMount () {
const {
projectId,
Expand All @@ -63,8 +70,9 @@ class TabContainer extends Component {

const canViewAssets = this.getCanViewAssets()
const canViewEngagements = this.getCanViewEngagements()
const isAdmin = this.getIsAdmin()
this.setState({
currentTab: this.getTabFromPath(history.location.pathname, projectId, canViewAssets, canViewEngagements)
currentTab: this.getTabFromPath(history.location.pathname, projectId, canViewAssets, canViewEngagements, isAdmin)
})
}

Expand All @@ -77,8 +85,9 @@ class TabContainer extends Component {

const canViewAssets = this.getCanViewAssets(nextProps)
const canViewEngagements = this.getCanViewEngagements(nextProps)
const isAdmin = this.getIsAdmin(nextProps)
this.setState({
currentTab: this.getTabFromPath(nextProps.history.location.pathname, projectId, canViewAssets, canViewEngagements)
currentTab: this.getTabFromPath(nextProps.history.location.pathname, projectId, canViewAssets, canViewEngagements, isAdmin)
})
if (
isLoading ||
Expand Down Expand Up @@ -130,7 +139,7 @@ class TabContainer extends Component {
return 0
}

getTabFromPath (pathname, projectId, canViewAssets = true, canViewEngagements = false) {
getTabFromPath (pathname, projectId, canViewAssets = true, canViewEngagements = false, isAdmin = false) {
if (projectId) {
return this.getProjectTabFromPath(pathname, projectId, canViewAssets, canViewEngagements)
}
Expand All @@ -141,7 +150,7 @@ class TabContainer extends Component {
return 2
}
if (pathname === '/engagements') {
return canViewEngagements ? 3 : 0
return isAdmin ? 3 : 0
Copy link
Copy Markdown

@github-actions github-actions Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
The logic change here from canViewEngagements to isAdmin could potentially alter the intended access control. Ensure that this change aligns with the business requirements, as it restricts access to the '/engagements' path strictly to admins, whereas previously it was accessible to both admins and talent managers.

}
if (pathname === '/users') {
return 4
Expand Down Expand Up @@ -178,7 +187,8 @@ class TabContainer extends Component {
onTabChange (tab) {
const { history, resetSidebarActiveParams, projectId } = this.props
const canViewAssets = this.getCanViewAssets()
const canViewEngagements = this.getCanViewEngagements()
const canViewEngagements = this.getCanViewEngagements() // admin OR TM
const isAdmin = this.getIsAdmin() // admin
if (projectId) {
if ((tab === 2 && !canViewEngagements) || (tab === 3 && !canViewAssets)) {
return
Expand All @@ -200,7 +210,7 @@ class TabContainer extends Component {
history.push('/projects')
this.props.unloadProjects()
this.setState({ currentTab: 2 })
} else if (tab === 3 && canViewEngagements) {
} else if (tab === 3 && isAdmin) {
Copy link
Copy Markdown

@github-actions github-actions Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[❗❗ correctness]
Similar to the change on line 153, this modification restricts access to the '/engagements' tab to only admins. Verify that this change is intentional and aligns with the updated access control requirements.

history.push('/engagements')
this.setState({ currentTab: 3 })
} else if (tab === 4) {
Expand All @@ -225,6 +235,7 @@ class TabContainer extends Component {
const { currentTab } = this.state
const canViewAssets = this.getCanViewAssets()
const canViewEngagements = this.getCanViewEngagements()
const isAdmin = this.getIsAdmin()

return (
<Tab
Expand All @@ -233,6 +244,7 @@ class TabContainer extends Component {
projectId={this.props.projectId}
canViewAssets={canViewAssets}
canViewEngagements={canViewEngagements}
isAdmin={isAdmin}
onBack={this.onBackToHome}
/>
)
Expand Down
6 changes: 3 additions & 3 deletions src/routes.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ class Routes extends React.Component {
<FooterContainer />
)()}
/>
{canAccessEngagements && (
{isAdmin && (
Copy link
Copy Markdown

@github-actions github-actions Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The change from canAccessEngagements to isAdmin restricts access to the /engagements route to only admins. Ensure that this change aligns with the intended access control policy, as it removes access for Talent Managers.

<Route exact path='/engagements'
render={() => renderApp(
<EngagementsList allEngagements />,
Expand All @@ -166,12 +166,12 @@ class Routes extends React.Component {
)()}
/>
)}
{!canAccessEngagements && (
{!isAdmin && (
Copy link
Copy Markdown

@github-actions github-actions Bot Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[⚠️ correctness]
The change from !canAccessEngagements to !isAdmin means that non-admin users will see the warning message. Verify that this behavior is intended, as it alters the access logic for non-admin users.

<Route exact path='/engagements'
render={() => renderApp(
<Challenges
menu='NULL'
warnMessage={'You need Admin or Talent Manager role to view all engagements'}
warnMessage={'You need Admin role to view all engagements'}
/>,
<TopBarContainer />,
<Tab />,
Expand Down
Loading