Skip to content
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

feat: Vendor verification #1208

Merged
merged 23 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
130f64e
First attempt at migrations and seeds for vendors
stalgiag Sep 4, 2024
21a1cbc
Correct vendors resolver
stalgiag Sep 4, 2024
3ad89f6
vendorByNameResolver
stalgiag Sep 4, 2024
4c90249
vendorById
stalgiag Sep 4, 2024
6fa7096
Add self to vendors for testing
stalgiag Sep 4, 2024
442130a
Association with "company" by User
stalgiag Sep 4, 2024
df70d43
Improve gql interface for vendor queries
stalgiag Sep 4, 2024
15f5ca8
Temporarily disallow admins for user and prevent review for incorrect…
stalgiag Sep 4, 2024
815f6f2
First pass on limiting reviews to appropriate vendors
stalgiag Sep 4, 2024
7675404
reduce default deep fetching of user vendor ats
stalgiag Sep 4, 2024
749ff98
schema test
stalgiag Sep 4, 2024
52f8835
Add and update spec tests for Vendor table
stalgiag Sep 4, 2024
263f631
Correct location for vendors schema test
stalgiag Sep 4, 2024
df8031a
Further testing adjustments, Vendor
stalgiag Sep 4, 2024
2eea45a
User company updates, testing schema
stalgiag Sep 4, 2024
7e5b4ed
Update mocks for Vendor
stalgiag Sep 4, 2024
0b2e110
Remove User.company from schema testing
stalgiag Sep 4, 2024
c650638
Associate vendor with fake vendor user
stalgiag Sep 4, 2024
48c4f2f
Set vendors back to previous state
stalgiag Sep 4, 2024
f1c1ab7
Add self back to admins.txt
stalgiag Sep 4, 2024
3273e14
Merge branch 'development' into vendor-verification
stalgiag Sep 25, 2024
051e41b
Trim vendor company name
stalgiag Sep 25, 2024
265e6d9
Remove unnecessary company from TestPlanReportStatusDialogMock, updat…
stalgiag Sep 25, 2024
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
45 changes: 33 additions & 12 deletions client/components/CandidateReview/TestPlans/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import ClippedProgressBar from '@components/common/ClippedProgressBar';
import { dates } from 'shared';
import './TestPlans.css';
import { calculations } from 'shared';
import { UserPropType } from '../../common/proptypes';

const FullHeightContainer = styled(Container)`
min-height: calc(100vh - 64px);
Expand Down Expand Up @@ -177,7 +178,7 @@ const None = styled.span`
}
`;

const TestPlans = ({ testPlanVersions }) => {
const TestPlans = ({ testPlanVersions, me }) => {
const [atExpandTableItems, setAtExpandTableItems] = useState({
1: true,
2: true,
Expand Down Expand Up @@ -503,20 +504,39 @@ const TestPlans = ({ testPlanVersions }) => {
.filter(t => t.isCandidateReview === true)
.filter(t => uniqueFilter(t, uniqueLinks, 'link'));

const canReview =
me.roles.includes('ADMIN') ||
(me.roles.includes('VENDOR') &&
me.company.ats.some(at => at.id === atId));

const getTitleEl = () => {
if (canReview) {
return (
<Link
to={`/candidate-test-plan/${testPlanVersion.id}/${atId}`}
>
{getTestPlanVersionTitle(testPlanVersion, {
includeVersionString: true
})}{' '}
({testsCount} Test
{testsCount === 0 || testsCount > 1 ? `s` : ''})
</Link>
);
}
return (
<>
{getTestPlanVersionTitle(testPlanVersion, {
includeVersionString: true
})}{' '}
({testsCount} Test
{testsCount === 0 || testsCount > 1 ? `s` : ''})
</>
);
};
return (
dataExists && (
<tr key={testPlanVersion.id}>
<th>
<Link
to={`/candidate-test-plan/${testPlanVersion.id}/${atId}`}
>
{getTestPlanVersionTitle(testPlanVersion, {
includeVersionString: true
})}{' '}
({testsCount} Test
{testsCount === 0 || testsCount > 1 ? `s` : ''})
</Link>
</th>
<th>{getTitleEl()}</th>
<CenteredTd>
<i>
{dates.convertDateToString(
Expand Down Expand Up @@ -778,6 +798,7 @@ TestPlans.propTypes = {
)
})
).isRequired,
me: UserPropType.isRequired,
triggerPageUpdate: PropTypes.func
};

Expand Down
6 changes: 5 additions & 1 deletion client/components/CandidateReview/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,16 @@ import { useQuery } from '@apollo/client';
import PageStatus from '../common/PageStatus';
import TestPlans from './TestPlans';
import { CANDIDATE_REVIEW_PAGE_QUERY } from './queries';
import { ME_QUERY } from '../App/queries';

const CandidateReview = () => {
const { loading, data, error } = useQuery(CANDIDATE_REVIEW_PAGE_QUERY, {
fetchPolicy: 'cache-and-network'
});

const { data: meData } = useQuery(ME_QUERY);
const { me } = meData;

if (error) {
return (
<PageStatus
Expand All @@ -33,7 +37,7 @@ const CandidateReview = () => {

const testPlanVersions = data.testPlanVersions;

return <TestPlans testPlanVersions={testPlanVersions} />;
return <TestPlans testPlanVersions={testPlanVersions} me={me} />;
};

export default CandidateReview;
17 changes: 14 additions & 3 deletions client/components/ConfirmAuth/index.jsx
Original file line number Diff line number Diff line change
@@ -1,15 +1,17 @@
import React from 'react';
import { Navigate } from 'react-router-dom';
import { Navigate, useParams } from 'react-router-dom';
import PropTypes from 'prop-types';
import { useQuery } from '@apollo/client';
import { ME_QUERY } from '../App/queries';
import { evaluateAuth } from '../../utils/evaluateAuth';

const ConfirmAuth = ({ children, requiredPermission }) => {
const ConfirmAuth = ({ children, requiredPermission, requireVendorForAt }) => {
const { data } = useQuery(ME_QUERY);
const { atId } = useParams();

const auth = evaluateAuth(data && data.me ? data.me : {});
const { roles, username, isAdmin, isSignedIn } = auth;
const company = data && data.me ? data.me.company : null;

if (!username) return <Navigate to={{ pathname: '/invalid-request' }} />;

Expand All @@ -21,6 +23,14 @@ const ConfirmAuth = ({ children, requiredPermission }) => {
return <Navigate to="/404" replace />;
}

// Check if the user's company is the vendor for the associated At
if (requireVendorForAt && !isAdmin) {
const isVendorForAt = company && company.ats.some(at => at.id === atId);
if (!isVendorForAt) {
return <Navigate to="/404" replace />;
}
}

Comment on lines +26 to +33
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏽 👍🏽

return children;
};

Expand All @@ -29,7 +39,8 @@ ConfirmAuth.propTypes = {
PropTypes.arrayOf(PropTypes.node),
PropTypes.node
]).isRequired,
requiredPermission: PropTypes.string
requiredPermission: PropTypes.string,
requireVendorForAt: PropTypes.bool
};

export default ConfirmAuth;
6 changes: 6 additions & 0 deletions client/components/common/fragments/Me.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ const ME_FIELDS = gql`
id
username
roles
company {
id
ats {
id
}
}
}
`;

Expand Down
15 changes: 14 additions & 1 deletion client/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,20 @@ window.signMeInAsTester = username => {
};

window.signMeInAsVendor = username => {
return signMeInCommon({ username, roles: [{ name: 'VENDOR' }] });
return signMeInCommon({
username,
roles: [{ name: 'VENDOR' }],
company: {
id: '1',
name: 'vispero',
ats: [
{
id: '1',
name: 'JAWS'
}
]
}
});
};

window.startTestTransaction = async () => {
Expand Down
2 changes: 1 addition & 1 deletion client/routes/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default () => (
exact
path="/candidate-test-plan/:testPlanVersionId/:atId"
element={
<ConfirmAuth requiredPermission="VENDOR">
<ConfirmAuth requiredPermission="VENDOR" requireVendorForAt={true}>
<CandidateTestPlanRun />
</ConfirmAuth>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,12 @@ export default (
me: {
id: '1',
username: 'foo-bar',
roles: ['ADMIN', 'TESTER']
roles: ['ADMIN', 'TESTER'],
company: {
id: '1',
name: 'Company',
ats: []
}
Copy link
Contributor

Choose a reason for hiding this comment

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

in the mock, would these need a company since the roles assigned here doesn't include 'VENDOR'?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great point! Addressed.

}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ export default testQueuePageQuery => [
__typename: 'User',
id: '1',
username: 'foo-bar',
roles: ['ADMIN', 'TESTER']
roles: ['ADMIN', 'TESTER'],
company: {
id: '1',
name: 'Company',
ats: []
}
},
users: [
{
Expand All @@ -18,23 +23,38 @@ export default testQueuePageQuery => [
username: 'foo-bar',
roles: ['ADMIN', 'TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
},
{
__typename: 'User',
id: '4',
username: 'bar-foo',
roles: ['TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
},
{
__typename: 'User',
id: '5',
username: 'boo-far',
roles: ['TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
}
],
ats: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ export default testQueuePageQuery => [
__typename: 'User',
id: '4',
username: 'bar-foo',
roles: ['TESTER']
roles: ['TESTER'],
company: {
id: '1',
name: 'Company',
ats: []
}
},
users: [
{
Expand All @@ -18,23 +23,38 @@ export default testQueuePageQuery => [
username: 'foo-bar',
roles: ['ADMIN', 'TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
},
{
__typename: 'User',
id: '4',
username: 'bar-foo',
roles: ['TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
},
{
__typename: 'User',
id: '5',
username: 'boo-far',
roles: ['TESTER'],
isBot: false,
ats: []
ats: [],
company: {
id: '1',
name: 'Company',
ats: []
}
}
],
ats: [],
Expand Down
44 changes: 43 additions & 1 deletion server/controllers/AuthController.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
const { User } = require('../models');
const { getOrCreateUser } = require('../models/services/UserService');
const {
getOrCreateUser,
addUserVendor,
getUserById
} = require('../models/services/UserService');
const { GithubService } = require('../services');
const getUsersFromFile = require('../util/getUsersFromFile');
const {
findVendorByName,
getOrCreateVendor
} = require('../models/services/VendorService');

const APP_SERVER = process.env.APP_SERVER;

Expand Down Expand Up @@ -70,6 +78,40 @@ const oauthRedirectFromGithubController = async (req, res) => {
transaction: req.transaction
});

if (roles.some(role => role.name === User.VENDOR)) {
const vendorEntry = vendors.find(
vendor => vendor.split('|')[0] === githubUsername
);
if (vendorEntry) {
const [, companyName] = vendorEntry.split('|');
const vendor = await findVendorByName({
name: companyName,
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to trim this just in case?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. I added a trim()

transaction: req.transaction
});
if (vendor) {
await addUserVendor(user.id, vendor.id, {
transaction: req.transaction
});
} else {
const vendor = await getOrCreateVendor({
where: { name: companyName },
transaction: req.transaction
});
await addUserVendor(user.id, vendor.id, {
transaction: req.transaction
});
}
// Fetch the user again with vendor and AT information
user = await getUserById({
id: user.id,
vendorAttributes: ['id', 'name'],
atAttributes: ['id', 'name'],
includeVendorAts: true,
transaction: req.transaction
});
}
}

req.session.user = user;

return loginSucceeded();
Expand Down
Loading
Loading