Skip to content

Commit

Permalink
Fix/1744 permission guards (#2258)
Browse files Browse the repository at this point in the history
* fix role selector

* permission guard component to conditionally render based on redux state

* use permission guard for data file page

* use permission guard for data file nav item

* add component tests for data file permission guard

* require account approval to access data files

* update ci?

* fix ci img

* ci img

* rm old heroku keys workaround

Co-authored-by: Andrew <[email protected]>
  • Loading branch information
jtimpe and andrew-jameson authored Nov 22, 2022
1 parent d1ea6b0 commit fe177c6
Show file tree
Hide file tree
Showing 9 changed files with 381 additions and 23 deletions.
15 changes: 6 additions & 9 deletions tdrs-frontend/src/components/Header/Header.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
} from '../../selectors/auth'

import NavItem from '../NavItem/NavItem'
import PermissionGuard from '../PermissionGuard'

/**
* This component is rendered on every page and contains the navigation bar.
Expand All @@ -28,13 +29,6 @@ function Header() {
const userAccessRequestPending = useSelector(accountIsInReview)
const userAccessRequestApproved = useSelector(accountStatusIsApproved)

const hasPermission = (permissionName) =>
user?.roles?.[0]?.permissions?.some(
(perm) => perm.codename === permissionName
)

const canViewDataFiles = hasPermission('view_datafile')

const menuRef = useRef()

const keyListenersMap = useMemo(() => {
Expand Down Expand Up @@ -118,13 +112,16 @@ function Header() {
{authenticated && (
<>
<NavItem pathname={pathname} tabTitle="Home" href="/home" />
{canViewDataFiles && (
<PermissionGuard
requiresApproval
requiredPermissions={['view_datafile', 'add_datafile']}
>
<NavItem
pathname={pathname}
tabTitle="Data Files"
href="/data-files"
/>
)}
</PermissionGuard>
{(userAccessRequestPending || userAccessRequestApproved) && (
<NavItem
pathname={pathname}
Expand Down
74 changes: 74 additions & 0 deletions tdrs-frontend/src/components/Header/Header.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -168,4 +168,78 @@ describe('Header', () => {
expect(queryByText('Profile')).not.toBeInTheDocument()
expect(queryByText('Admin')).not.toBeInTheDocument()
})

it('should NOT show data-files nav item when the user does not have view_datafile and add_datafile permissions', () => {
const state = {
...initialState,
auth: {
user: {
email: '[email protected]',
roles: [{ id: 1, name: 'Developer', permissions: [] }],
access_request: true,
account_approval_status: 'Approved',
},
authenticated: true,
},
}

const store = mockStore(state)

const { queryByText } = render(
<Provider store={store}>
<Header />
</Provider>
)

expect(queryByText('Data Files')).not.toBeInTheDocument()
expect(queryByText('Profile')).toBeInTheDocument()
expect(queryByText('Admin')).toBeInTheDocument()
})

it('should NOT show data-files nav item when the user is not in an approved status', () => {
const state = {
...initialState,
auth: {
user: {
email: '[email protected]',
roles: [
{
id: 1,
name: 'Developer',
permissions: ['add_datafile', 'view_datafile'],
},
],
access_request: true,
account_approval_status: 'Pending',
},
authenticated: true,
},
}

const store = mockStore(state)

const { queryByText } = render(
<Provider store={store}>
<Header />
</Provider>
)

expect(queryByText('Data Files')).not.toBeInTheDocument()
expect(queryByText('Profile')).toBeInTheDocument()
expect(queryByText('Admin')).toBeInTheDocument()
})

it('should show data-files nav item when the user has view_datafile and add_datafile permissions and is approved', () => {
const store = mockStore(initialState)

const { queryByText } = render(
<Provider store={store}>
<Header />
</Provider>
)

expect(queryByText('Data Files')).toBeInTheDocument()
expect(queryByText('Profile')).toBeInTheDocument()
expect(queryByText('Admin')).toBeInTheDocument()
})
})
47 changes: 47 additions & 0 deletions tdrs-frontend/src/components/PermissionGuard/PermissionGuard.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { useSelector } from 'react-redux'
import {
accountStatusIsApproved,
selectUserPermissions,
} from '../../selectors/auth'

const isAllowed = (
{ permissions, isApproved },
requiredPermissions,
requiresApproval
) => {
if (requiresApproval && !isApproved) {
return false
}

if (!requiredPermissions) {
return true
}

for (var i = 0; i < requiredPermissions.length; i++) {
if (!permissions.includes(requiredPermissions[i])) {
return false
}
}

return true
}

const PermissionGuard = ({
children,
requiresApproval = false,
requiredPermissions = [],
notAllowedComponent = null,
}) => {
const permissions = useSelector(selectUserPermissions)
const isApproved = useSelector(accountStatusIsApproved)

return isAllowed(
{ permissions, isApproved },
requiredPermissions,
requiresApproval
)
? children
: notAllowedComponent
}

export default PermissionGuard
194 changes: 194 additions & 0 deletions tdrs-frontend/src/components/PermissionGuard/PermissionGuard.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
/* eslint-disable no-param-reassign */
import React from 'react'
import thunk from 'redux-thunk'
import { Provider } from 'react-redux'
import configureStore from 'redux-mock-store'
import { render, screen } from '@testing-library/react'

import PermissionGuard from '.'

describe('PermissionGuard.js', () => {
const mockStore = configureStore([thunk])

const setup = (initialState, requiredPermissions, requiresApproval = false) =>
render(
<Provider store={mockStore(initialState)}>
<PermissionGuard
requiresApproval={requiresApproval}
requiredPermissions={requiredPermissions}
notAllowedComponent={<p>not allowed</p>}
>
<p>hello, world</p>
</PermissionGuard>
</Provider>
)

describe('not allowed', () => {
it('shows not allowed if missing one permission', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [{ permissions: [{ codename: 'some_stuff' }] }],
},
},
},
['allowed']
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})

it('shows not allowed if missing multiple permissions', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [{ permissions: [{ codename: 'allowed' }] }],
},
},
},
['allowed', 'super_allowed', 'super_duper_allowed']
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})

it('shows not allowed if multiple roles missing permissions', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [
{ permissions: [{ codename: 'allowed' }] },
{ permissions: [{ codename: 'super_allowed' }] },
{ permissions: [{ codename: 'nothing' }] },
],
},
},
},
['allowed', 'super_allowed', 'super_duper_allowed']
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})

it('shows not allowed if user has no roles', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: null,
},
},
},
['anything']
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})

it('shows not allowed if user has no permissions', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [{ permissions: [] }],
},
},
},
['anything']
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})

it('shows not allowed if requiresApproval and not approved', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [{ permissions: ['anything'] }],
account_approval_status: 'Pending',
},
},
},
[],
true
)

expect(screen.queryByText('hello, world')).not.toBeInTheDocument()
expect(screen.queryByText('not allowed')).toBeInTheDocument()
})
})

describe('allowed', () => {
it('shows allowed if no required permissions given', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: null,
},
},
},
null
)

expect(screen.queryByText('hello, world')).toBeInTheDocument()
expect(screen.queryByText('not allowed')).not.toBeInTheDocument()
})

it('shows allowed if user matches all permissions', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [
{ permissions: [{ codename: 'allowed' }] },
{ permissions: [{ codename: 'super_allowed' }] },
{ permissions: [{ codename: 'super_duper_allowed' }] },
],
},
},
},
['allowed', 'super_allowed', 'super_duper_allowed']
)

expect(screen.queryByText('hello, world')).toBeInTheDocument()
expect(screen.queryByText('not allowed')).not.toBeInTheDocument()
})

it('shows allowed if requiresApproval and approved', () => {
setup(
{
auth: {
authenticated: true,
user: {
roles: [{ permissions: ['anything'] }],
account_approval_status: 'Approved',
},
},
},
[],
true
)

expect(screen.queryByText('hello, world')).toBeInTheDocument()
expect(screen.queryByText('not allowed')).not.toBeInTheDocument()
})
})
})
3 changes: 3 additions & 0 deletions tdrs-frontend/src/components/PermissionGuard/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import PermissionGuard from './PermissionGuard'

export default PermissionGuard
Loading

0 comments on commit fe177c6

Please sign in to comment.