From 6263acf0e6ffa53be8a558db7c51bcdbb07922f2 Mon Sep 17 00:00:00 2001 From: Alexander Flenniken Date: Tue, 5 Mar 2024 13:04:37 -0500 Subject: [PATCH] Allow End-to-End Testing as an Admin or Tester (#908) * Initial smoke test before testing in CI * Make adjustments for CI * Testing if it is just slow * Comment out new test * Reenable test * Try headless chrome * Try running just one test * Add logging * Add more logging * Found potential environmental issue * Try fixing database connection * Try turning off sandbox mode * Try using a package to kill the server * Run full test suite instead of just one test * Some final polish * PR polish * Enable sign-in in e2e tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Work on fixing tests * Carry over changes from main * Add todo to list of todos * PR feedback * Merge config * Use services.deprecated to fix test * Fix sandbox config --- .vscode/launch.json | 14 ++ client/components/App/App.jsx | 4 +- client/components/App/useSigninUrl.js | 12 -- client/index.js | 30 ++++ client/tests/smokeTest.test.js | 166 ++++++------------ client/tests/util/getPage.js | 142 +++++++++++++++ config/dev.env | 2 +- config/test.env | 2 +- deploy/files/config-development.env | 2 +- deploy/files/config-sandbox.env | 80 ++++----- docs/local-development.md | 25 ++- server/controllers/AuthController.js | 24 +-- server/controllers/FakeUserController.js | 36 ++++ server/models/loaders/AtLoader.js | 35 ++-- server/models/loaders/BrowserLoader.js | 24 +-- server/routes/auth.js | 2 + server/services/GithubService.js | 5 +- .../tests/integration/authentication.test.js | 109 +----------- server/tests/util/mock-github-server.js | 4 +- 19 files changed, 382 insertions(+), 336 deletions(-) delete mode 100644 client/components/App/useSigninUrl.js create mode 100644 client/tests/util/getPage.js create mode 100644 server/controllers/FakeUserController.js diff --git a/.vscode/launch.json b/.vscode/launch.json index f87c86d42..37f8ff00e 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -25,6 +25,20 @@ "windows": { "program": "${workspaceFolder}/server/node_modules/jest/bin/jest" } + }, + { + "type": "node", + "request": "launch", + "name": "Jest Client Debug Current Test File", + //"env": { "NODE_ENV": "test" }, + "envFile": "${workspaceFolder}/config/test.env", + "program": "${workspaceFolder}/client/node_modules/.bin/jest", + "args": ["${fileBasenameNoExtension}", "--config", "client/jest.setup.js"], + "console": "integratedTerminal", + "disableOptimisticBPs": true, + "windows": { + "program": "${workspaceFolder}/client/node_modules/jest/bin/jest" + } } ] } diff --git a/client/components/App/App.jsx b/client/components/App/App.jsx index f4a21550e..d1282ccf5 100644 --- a/client/components/App/App.jsx +++ b/client/components/App/App.jsx @@ -10,13 +10,11 @@ import { evaluateAuth } from '../../utils/evaluateAuth'; import ScrollFixer from '../../utils/ScrollFixer'; import routes from '../../routes'; import { ME_QUERY } from './queries'; -import useSigninUrl from './useSigninUrl'; import SkipLink from '../SkipLink'; import './App.css'; const App = () => { const location = useLocation(); - const signinUrl = useSigninUrl(); const { client, loading, data } = useQuery(ME_QUERY); const [isNavbarExpanded, setIsNavbarExpanded] = useState(false); @@ -159,7 +157,7 @@ const App = () => { as={Link} to="#" onClick={() => - (window.location.href = signinUrl) + (window.location.href = `/api/auth/oauth`) } > Sign in with GitHub diff --git a/client/components/App/useSigninUrl.js b/client/components/App/useSigninUrl.js deleted file mode 100644 index 17fef40d7..000000000 --- a/client/components/App/useSigninUrl.js +++ /dev/null @@ -1,12 +0,0 @@ -const useSigninUrl = () => { - // Allows for quickly logging in with different roles - changing - // roles would otherwise require leaving and joining GitHub teams - const matchedFakeRole = window.location.href.match(/fakeRole=(\w*)/); - let dataFromFrontend = ''; - if (matchedFakeRole) { - dataFromFrontend += `fakeRole-${matchedFakeRole[1]}`; - } - return `/api/auth/oauth?dataFromFrontend=` + dataFromFrontend; -}; - -export default useSigninUrl; diff --git a/client/index.js b/client/index.js index 363b77b44..6cc1a86ea 100644 --- a/client/index.js +++ b/client/index.js @@ -18,3 +18,33 @@ root.render( ); + +const signMeInCommon = async user => { + if (!user.username) throw new Error('Please provide a username'); + + const response = await fetch('/api/auth/fake-user', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify(user) + }); + + const responseText = await response.text(); + if (!response.ok) throw responseText; + + location.reload(); +}; + +window.signMeInAsAdmin = username => { + return signMeInCommon({ + username, + roles: [{ name: 'ADMIN' }, { name: 'TESTER' }, { name: 'VENDOR' }] + }); +}; + +window.signMeInAsTester = username => { + return signMeInCommon({ username, roles: [{ name: 'TESTER' }] }); +}; + +window.signMeInAsVendor = username => { + return signMeInCommon({ username, roles: [{ name: 'VENDOR' }] }); +}; diff --git a/client/tests/smokeTest.test.js b/client/tests/smokeTest.test.js index 25377d111..e30143a68 100644 --- a/client/tests/smokeTest.test.js +++ b/client/tests/smokeTest.test.js @@ -1,142 +1,88 @@ -const puppeteer = require('puppeteer'); -const path = require('path'); -const spawn = require('cross-spawn'); -const treeKill = require('tree-kill'); +import getPage, { setup, teardown } from './util/getPage'; /* End-to-End Testing TODO: - The server start and close functions should be handled in one place by Jest's setup and teardown scripts. -- There needs to be a utility function to enable common tasks like getting a -fresh end-to-end page. -- End-to-end testing should support signing in with different roles, A.K.A. as -an admin, tester or vendor. - It should be possible to reset the database to a predictable state after each test, ideally in such a way that concurrency remains possible. See the POC here: https://github.com/w3c/aria-at-app/pull/895 +- Add section to docs for end-to-end testing */ -const PORT = 8033; -const CLIENT_PORT = 3033; -const AUTOMATION_SCHEDULER_PORT = 8833; - -const startServer = async serverOrClient => { - return new Promise(resolve => { - const server = spawn('yarn', ['workspace', serverOrClient, 'dev'], { - cwd: path.resolve(__dirname, '../../'), - env: { - PATH: process.env.PATH, - PORT, - CLIENT_PORT: 3033, - AUTOMATION_SCHEDULER_PORT: 8833, - API_SERVER: `http://localhost:${PORT}`, - APP_SERVER: `http://localhost:${CLIENT_PORT}`, - AUTOMATION_SCHEDULER_URL: `http://localhost:${AUTOMATION_SCHEDULER_PORT}`, - PGDATABASE: 'aria_at_report_test', - PGPORT: 5432, - ENVIRONMENT: 'test' - } - }); - - const killServer = async () => { - await new Promise((resolve, reject) => { - treeKill(server.pid, error => { - if (error) return reject(error); - resolve(); - }); - }); - }; +describe('smoke test', () => { + beforeAll(async () => { + await setup(); + }, 30000); - server.stdout.on('data', data => { - const output = data.toString(); - console.info(output); // eslint-disable-line no-console + afterAll(async () => { + await teardown(); + }, 30000); - if ( - (serverOrClient === 'server' && - output.includes(`Listening on ${PORT}`)) || - (serverOrClient === 'client' && - output.includes('compiled successfully')) - ) { - resolve({ close: killServer }); - } - }); + it('can sign in as admin, tester, vendor, or logged out', async () => { + await Promise.all([ + getPage({ role: 'admin', url: '/test-queue' }, async page => { + // Only admins can remove rows from the test queue + await page.waitForSelector('td.actions ::-p-text(Remove)'); + }), - server.stderr.on('data', data => { - const output = data.toString(); - console.info(output); // eslint-disable-line no-console - }); - }); -}; + getPage({ role: 'tester', url: '/test-queue' }, async page => { + // Testers can assign themselves + await page.waitForSelector('table ::-p-text(Assign Yourself)'); + const adminOnlyRemoveButton = await page.$( + 'td.actions ::-p-text(Remove)' + ); + expect(adminOnlyRemoveButton).toBe(null); + }), -describe('smoke test', () => { - let browser; - let backendServer; - let clientServer; + getPage( + { role: 'vendor', url: '/test-queue' }, + async (page, { baseUrl }) => { + // Vendors get the same test queue as signed-out users + await page.waitForSelector( + 'td.actions ::-p-text(View tests)' + ); + // Unlike signed-out users, they will get tables on this page + await page.goto(`${baseUrl}/candidate-review`); + await page.waitForSelector('table'); + } + ), - beforeAll(async () => { - // eslint-disable-next-line no-console - console.info( - 'Starting dev servers. This is required for end-to-end testing' - ); - [clientServer, backendServer] = await Promise.all([ - startServer('client'), - startServer('server') + getPage({ role: false, url: '/test-queue' }, async page => { + // Signed-out users can only view tests, not run them + await page.waitForSelector('td.actions ::-p-text(View tests)'); + }) ]); - - browser = await puppeteer.launch({ - headless: 'new', - args: ['--no-sandbox'] // Required for GitHub environment - }); - const [extraBlankPage] = await browser.pages(); - extraBlankPage.close(); - }, 60000); - - afterAll(async () => { - await Promise.all([backendServer.close(), clientServer.close()]); - - // Browser might not be defined, if it failed to start - if (browser) await browser.close(); - }, 60000); + }, 10000); it('loads various pages without crashing', async () => { - let homeH1; - let reportsH1; - let dataManagementH1; - await Promise.all([ - (async () => { - const page = await browser.newPage(); - await page.goto(`http://localhost:${CLIENT_PORT}/`); + getPage({ role: false, url: '/' }, async page => { await page.waitForSelector('h1'); const h1Handle = await page.waitForSelector('h1'); - homeH1 = await h1Handle.evaluate(h1 => h1.innerText); - })(), - (async () => { - const page = await browser.newPage(); - await page.goto(`http://localhost:${CLIENT_PORT}/reports`); + const h1Text = await h1Handle.evaluate(h1 => h1.innerText); + expect(h1Text).toBe( + 'Enabling Interoperability for Assistive Technology Users' + ); + }), + getPage({ role: false, url: '/reports' }, async page => { // Wait for an h2 because an h1 will show while the page is // still loading await page.waitForSelector('h2'); const h1Handle = await page.waitForSelector('h1'); - reportsH1 = await h1Handle.evaluate(h1 => h1.innerText); - })(), - (async () => { - const page = await browser.newPage(); - await page.goto( - `http://localhost:${CLIENT_PORT}/data-management` + const h1Text = await h1Handle.evaluate(h1 => h1.innerText); + expect(h1Text).toBe( + 'Assistive Technology Interoperability Reports' ); + }), + getPage({ role: false, url: '/data-management' }, async page => { // Wait for an h2 because an h1 will show while the page is // still loading await page.waitForSelector('h2'); const h1Handle = await page.waitForSelector('h1'); - dataManagementH1 = await h1Handle.evaluate(h1 => h1.innerText); - })() + const h1Text = await h1Handle.evaluate(h1 => h1.innerText); + expect(h1Text).toBe('Data Management'); + }) ]); - - expect(homeH1).toBe( - 'Enabling Interoperability for Assistive Technology Users' - ); - expect(reportsH1).toBe('Assistive Technology Interoperability Reports'); - expect(dataManagementH1).toBe('Data Management'); - }, 60000); + }, 10000); }); diff --git a/client/tests/util/getPage.js b/client/tests/util/getPage.js new file mode 100644 index 000000000..13cc8d5af --- /dev/null +++ b/client/tests/util/getPage.js @@ -0,0 +1,142 @@ +const puppeteer = require('puppeteer'); +const path = require('path'); +const spawn = require('cross-spawn'); +const treeKill = require('tree-kill'); + +let clientServer; +let backendServer; +let browser; + +const PORT = 8033; +const CLIENT_PORT = 3033; +const AUTOMATION_SCHEDULER_PORT = 8833; +const baseUrl = `http://localhost:${CLIENT_PORT}`; + +const startServer = async serverOrClient => { + return new Promise(resolve => { + const server = spawn('yarn', ['workspace', serverOrClient, 'dev'], { + cwd: path.resolve(__dirname, '../../'), + env: { + PATH: process.env.PATH, + PORT, + CLIENT_PORT, + AUTOMATION_SCHEDULER_PORT, + API_SERVER: `http://localhost:${PORT}`, + APP_SERVER: baseUrl, + AUTOMATION_SCHEDULER_URL: `http://localhost:${AUTOMATION_SCHEDULER_PORT}`, + PGDATABASE: 'aria_at_report_test', + PGPORT: 5432, + ENVIRONMENT: 'test' + } + }); + + const killServer = async () => { + await new Promise((resolve, reject) => { + treeKill(server.pid, error => { + if (error) return reject(error); + resolve(); + }); + }); + }; + + server.stdout.on('data', data => { + const output = data.toString(); + console.info(output); // eslint-disable-line no-console + + if ( + (serverOrClient === 'server' && + output.includes(`Listening on ${PORT}`)) || + (serverOrClient === 'client' && + output.includes('compiled successfully')) + ) { + resolve({ close: killServer }); + } + }); + + server.stderr.on('data', data => { + const output = data.toString(); + console.info(output); // eslint-disable-line no-console + }); + }); +}; + +const setup = async () => { + // eslint-disable-next-line no-console + console.info( + 'Starting dev servers. This is required for end-to-end testing' + ); + [clientServer, backendServer] = await Promise.all([ + startServer('client'), + startServer('server') + ]); + + browser = await puppeteer.launch({ + headless: 'new', + args: ['--no-sandbox'] // Required for GitHub environment + }); +}; + +const teardown = async () => { + await Promise.all([backendServer.close(), clientServer.close()]); + + // Browser might not be defined, if it failed to start + if (browser) await browser.close(); +}; + +let incognitoContexts = {}; + +/** + * Load a page for end-to-end testing. + * @param {*} options + * @param {'admin'|'tester'|'vendor'|false} options.role - The login state + * of the user. + * @param {string} options.url - The URL to start on. + * @param {function} callback - the first argument of the callback function is + * the page object as documented in Puppeteer. The second argument is an object + * with the current baseUrl including the localhost port being used - which is + * needed for navigation. + */ +const getPage = async (options, callback) => { + const { role, url } = options; + if (role == null || !['admin', 'tester', 'vendor', false].includes(role)) { + throw new Error('Please provide a valid role'); + } + + if (!incognitoContexts[role]) { + incognitoContexts[role] = await browser.createIncognitoBrowserContext(); + } + const incognitoContext = incognitoContexts[role]; + + const page = await incognitoContext.newPage(); + + if (!url) { + throw new Error('Please provide a URL, even if it it is simply "/"'); + } + + await page.goto(`http://localhost:${CLIENT_PORT}${url}`); + + if (role) { + await page.waitForSelector('::-p-text(Sign in with GitHub)'); + + const username = `joe-the-${role}`; + + if (role === 'admin') { + await page.evaluate(`signMeInAsAdmin("${username}")`); + } + if (role === 'tester') { + await page.evaluate(`signMeInAsTester("${username}")`); + } + if (role === 'vendor') { + await page.evaluate(`signMeInAsVendor("${username}")`); + } + + await page.waitForSelector('::-p-text(Signed in)'); + } + + await callback(page, { baseUrl }); + + await page.close(); +}; + +export default getPage; +export { setup, teardown }; diff --git a/config/dev.env b/config/dev.env index 8da4ead11..8bf64c9f2 100644 --- a/config/dev.env +++ b/config/dev.env @@ -7,7 +7,7 @@ PGPASSWORD=atr PGHOST=localhost PGPORT=5432 ENVIRONMENT=dev -ALLOW_FAKE_ROLE=true +ALLOW_FAKE_USER=true IMPORT_CONFIG=../config/dev.env GITHUB_OAUTH_SERVER=https://github.com GITHUB_GRAPHQL_SERVER=https://api.github.com diff --git a/config/test.env b/config/test.env index 237a2971d..6d32deed8 100644 --- a/config/test.env +++ b/config/test.env @@ -7,7 +7,7 @@ PGPASSWORD=atr PGHOST=localhost PGPORT=5432 ENVIRONMENT=test -ALLOW_FAKE_ROLE=true +ALLOW_FAKE_USER=true IMPORT_CONFIG=../config/test.env # Older and newer v1 test format tests, respectively diff --git a/deploy/files/config-development.env b/deploy/files/config-development.env index aa0edb832..c2a71c63f 100644 --- a/deploy/files/config-development.env +++ b/deploy/files/config-development.env @@ -7,7 +7,7 @@ PGPASSWORD=atr PGHOST=localhost PGPORT=5432 ENVIRONMENT=vagrant -ALLOW_FAKE_ROLE=true +ALLOW_FAKE_USER=true IMPORT_CONFIG=/home/aria-bot/config.env GITHUB_OAUTH_SERVER=https://github.com GITHUB_GRAPHQL_SERVER=https://api.github.com diff --git a/deploy/files/config-sandbox.env b/deploy/files/config-sandbox.env index 6eccb76f0..53de7550d 100644 --- a/deploy/files/config-sandbox.env +++ b/deploy/files/config-sandbox.env @@ -1,41 +1,41 @@ $ANSIBLE_VAULT;1.1;AES256 -31616139333234333762333037363164633235306537633338363561653261663165626361623832 -3734353062653839333964383962346437323261383930390a316130313736333265623763633232 -31643665313033623665346137353334623261346638666134653964396364323637343737623862 -6531376665323132640a646665646361313633383031646562653235323165633262393962643034 -66383233396262376566643337353730636532313136613661643035613730303262306231393934 -36663033636336383735363735303338396533373035343361633833663933393832316665343139 -32653362333739306163636539383434646662363734336132646337613538353037643130623933 -65303933646233303430356132636430636335363637393333346231623938323934613363646130 -39363265646531623036376531396130616163656138386539393964336638633939386536336464 -62333830366366653164353832626465653537313265366638643134383536326663306363616332 -37396361396563613065613737336535653030653463636263623664643032343562663430636130 -33663266396130626434633038646633336561323133336333323536346663366265626537376563 -32626530386635356238363032643231393961346135636664366261363262343565313039363464 -65336366653134313233643765353839363737393639323965666164636163396262623963346162 -39373563363636633838336463626634626130353463653164333331323362363937663535613238 -30386138666161393532646132353437326533326164336161393137313961306165303131616630 -65636239613963383532633462343561343533376462653963356563373134643235393435396464 -63336430373732316633323338336164373363313637646331376263353232346465376162356164 -30623935663232633463373734633039623437636564663630643531633938613733366233396466 -30343162313936323438363131343635663133616364333364653335393761653436363831373562 -31373736333037643536646633663537663661346434376530333931343836363934376366386239 -62613739616637643962613534313062383735616434646161663339646534613333333230306637 -63303364303963663666626137336435303530323963653462343236303232393736373934616230 -32363037376231653865636337643266333261383631353436393937616239653962303365306461 -63323564326334386235626632633961633166343938323331346134623834353630623961366332 -33633861646238343465646237313064636362646564613531653536633634373337633364313364 -62396235313530333066316162393161653763623937336665353231333663346564343832333161 -35373830383061303866326266333264396562326531656364643861626264623139323465346661 -36663966306632643163363965316239623765326165633564366337396139643438656665663265 -64333832303439646137323731353033613539616136303166393335366462343636616563353438 -61656263303336666531643063383363656534333438353131323537373933326162656337623633 -64663039393561343430633739303239383138393831353561393562316438393666386636656363 -35663933666661303039656436346462333638336163356265313562346161363131626663303438 -39653538653761316335356136393137313438633564663135646131366535636561633232636538 -35383732366166303635336434653164663038313538363630646633303938656266373365623762 -65613961376337326336623963333333633261313936636239333032343133353935653839353037 -63383934353837626631333635616336336337396233376532666261633530393730656464623366 -32666530356462333866653231653233643962373734356233643630626462663361643332303363 -33353937356234623830663666616535326464633433336631383231663735346338366332386435 -63303861396334363161 +30663031343864653133313365303935306566663531306138343464353631343730653964376239 +3363306662346534646433333130343264346631623035650a356564633964366262623631633138 +38666339633665636466363962366633623634333839623833636537646366373566626163633265 +6337626339343937330a636438653362366166643538343834646162383563343163663666636237 +31666330303834393032626264643064343864353566653763356338343135363633323364386139 +38323163623265316333316538333339656533373935376366616135386338303930356335313530 +31396434666337306164353637386662633037613435383136646330333137653362376437653937 +39663433653837363762666637643364356131636130636163313232613336343565653133353263 +37663331643633383739366164663830646339336562366338633463306335666237653335336530 +66393261363066346261383865316237636361323738356364336665323063373864656237326430 +38343532373463633738623032343730646533316263656232373937653439306263643938643336 +37326235663163653339366266633965363239616339393833346665393731363138326530663164 +32633035333636656333623536353937656530626664626164343037633965366330623538326230 +39616635363039333063333637386531636434613333363733343135663538353639323839396566 +38646130633630353138326532393235326438373336383434616565353231613834633563633066 +30343965366430613263633038326437643461336539663266316666646132626137356163393034 +61346537353937313862353637366666366137356538396535353236333639363362643139616662 +63323932306264306162313765666164663265313264313963303334383330316331336261393735 +64366463646563306132396564636630393736646136613465343934326166303134336138366264 +33383130323334616566393765333239333138343235363935653237366237306266333032303365 +65646561396661323938396461623030663566646638343038633230633263393938396165616265 +30643839653133306236613061376264633263663463303531363761663262633865653066383864 +39323765393535613865326531383262666564396436656131333562303863353138613339373936 +66393237613562623039653930616635333261613833323932313265616138323866356633346661 +63353538333731626435363334336330303266343135613432363738306365346239636264393635 +30336165303634353463363831646361623538303535366465323536626239626231643837346137 +39313235316164353365396335306238623936366561633533343230616533613664316233663062 +61363538373965346364373837323864643138333832656466643737626363313836616637326566 +38613331343238333231316436353366366331346663333536653138303561613962636132363438 +39396164363838393935316635643435306336323531626336363162666165386538663561666339 +31396261363163356664653961353630383432323735383633626231356637326265333938323433 +66633263626330356564376435376364653263303136646536303134383232653163643664373830 +34383436363862353962396234333137643866663336326563323234313463363032326165363066 +31666230656564346663313732666635643864316461303837386464336162306538313165313566 +38633939356631383334313162633462633163336130656663613164393864326635613662366564 +31356533373137333466303439383637633239663165303438326265323363643361326261643461 +33663762366266326161313364643764303563383936323938313738623566613634343933623736 +38386138396538326365363834623436363861316632353031663865636336343365323439363366 +38333037323632663265306137663232633731313539333930616133333735626638623931643233 +63626136333030616562 diff --git a/docs/local-development.md b/docs/local-development.md index 12ac7e252..918d42b8b 100644 --- a/docs/local-development.md +++ b/docs/local-development.md @@ -29,12 +29,25 @@ ARIA-AT App determines if you are authorized to sign in as an admin, tester, or Another way to log in as either a tester or admin, useful for quick testing and not requiring editing testers.txt or membership within any GitHub organizations or teams, is described below. -1. Sign out and return to the home page. -2. Add `?fakeRole=admin` to the URL bar and press enter. Alternatively use `?fakeRole=tester` to log in as a tester only or `?fakeRole=` to preview logging in without a role. -3. Follow the sign in steps as normal. -4. After signing in, your selected role will be used for the duration of your session. - -This functionality is available in development environments where the ALLOW_FAKE_ROLE environment variable is "true". +1. With the app running, open the browser DevTools. +2. Go to the DevTools console. +3. Paste in the following code: + - To become an admin: + ``` + signMeInAsAdmin("joe-the-admin") + ``` + - To become a tester: + ``` + signMeInAsTester("joe-the-tester") + ``` + - To become a vendor: + ``` + signMeInAsVendor("joe-the-vendor") + ``` + +The part in quotes is the username, feel free to change the username to whatever you prefer. + +This functionality is available in development environments where the ALLOW_FAKE_USER environment variable is "true". ## Debugging diff --git a/server/controllers/AuthController.js b/server/controllers/AuthController.js index c710d4835..3d8c6ef99 100644 --- a/server/controllers/AuthController.js +++ b/server/controllers/AuthController.js @@ -5,12 +5,10 @@ const { const { GithubService } = require('../services'); const getUsersFromFile = require('../util/getUsersFromFile'); -const ALLOW_FAKE_ROLE = process.env.ALLOW_FAKE_ROLE === 'true'; const APP_SERVER = process.env.APP_SERVER; const oauthRedirectToGithubController = async (req, res) => { - const { dataFromFrontend: state } = req.query; - const oauthUrl = GithubService.getOauthUrl({ state }); + const oauthUrl = GithubService.getOauthUrl(); res.redirect(303, oauthUrl); res.end(); }; @@ -34,7 +32,7 @@ const oauthRedirectFromGithubController = async (req, res) => { ); }; - const { code, state: dataFromFrontend } = req.query; + const { code } = req.query; const githubAccessToken = await GithubService.getGithubAccessToken(code); if (!githubAccessToken) return loginFailedDueToGitHub(); @@ -75,24 +73,6 @@ const oauthRedirectFromGithubController = async (req, res) => { [] ); - // Allows for quickly logging in with different roles - changing - // roles would otherwise require leaving and joining GitHub teams. - // Should not be saved to database. - const matchedFakeRole = - dataFromFrontend && dataFromFrontend.match(/fakeRole-(\w*)/); - if (ALLOW_FAKE_ROLE && matchedFakeRole) { - user = user.get({ plain: true }); - user.roles = - matchedFakeRole[1] === '' - ? [] - : [{ name: matchedFakeRole[1].toUpperCase() }]; - if (user.roles[0] && user.roles[0].name === User.ADMIN) { - user.roles.push({ name: User.TESTER }); // Admins are always testers - user.roles.push({ name: User.VENDOR }); - } - } - if (user.roles.length === 0) return loginFailedDueToRole(); - req.session.user = user; return loginSucceeded(); diff --git a/server/controllers/FakeUserController.js b/server/controllers/FakeUserController.js new file mode 100644 index 000000000..4110139cd --- /dev/null +++ b/server/controllers/FakeUserController.js @@ -0,0 +1,36 @@ +const { + getOrCreateUser +} = require('../models/services.deprecated/UserService'); + +const ALLOW_FAKE_USER = process.env.ALLOW_FAKE_USER === 'true'; + +const setFakeUserController = async (req, res) => { + if (!ALLOW_FAKE_USER) { + return res + .status(400) + .send('Feature not supported in this environment'); + } + + const userToCreate = req.body; + if ( + !userToCreate || + !userToCreate.username || + !userToCreate?.roles.length || + userToCreate.roles.some( + role => !['TESTER', 'ADMIN', 'VENDOR'].includes(role.name) + ) + ) { + return res.status(400).send('Invalid user'); + } + + let [user] = await getOrCreateUser( + { username: userToCreate.username }, + { roles: userToCreate.roles } + ); + + req.session.user = user; + + res.status(200).send(''); +}; + +module.exports = setFakeUserController; diff --git a/server/models/loaders/AtLoader.js b/server/models/loaders/AtLoader.js index 91efbb7b3..9ef12eeec 100644 --- a/server/models/loaders/AtLoader.js +++ b/server/models/loaders/AtLoader.js @@ -20,23 +20,26 @@ const AtLoader = () => { return activePromise; } - activePromise = getAts(); - ats = await activePromise; + activePromise = getAts().then(ats => { + // Sort date of atVersions subarray in desc order by releasedAt date + ats.forEach(item => + item.atVersions.sort((a, b) => b.releasedAt - a.releasedAt) + ); + + ats = ats.map(at => ({ + ...at.dataValues, + candidateBrowsers: at.browsers.filter( + browser => browser.AtBrowsers.isCandidate + ), + recommendedBrowsers: at.browsers.filter( + browser => browser.AtBrowsers.isRecommended + ) + })); + + return ats; + }); - // Sort date of atVersions subarray in desc order by releasedAt date - ats.forEach(item => - item.atVersions.sort((a, b) => b.releasedAt - a.releasedAt) - ); - - ats = ats.map(at => ({ - ...at.dataValues, - candidateBrowsers: at.browsers.filter( - browser => browser.AtBrowsers.isCandidate - ), - recommendedBrowsers: at.browsers.filter( - browser => browser.AtBrowsers.isRecommended - ) - })); + ats = await activePromise; return ats; }, diff --git a/server/models/loaders/BrowserLoader.js b/server/models/loaders/BrowserLoader.js index 8326535f7..aa1f0a089 100644 --- a/server/models/loaders/BrowserLoader.js +++ b/server/models/loaders/BrowserLoader.js @@ -20,19 +20,21 @@ const BrowserLoader = () => { return activePromise; } - activePromise = getBrowsers(); + activePromise = getBrowsers().then(browsers => { + browsers = browsers.map(browser => ({ + ...browser.dataValues, + candidateAts: browser.ats.filter( + at => at.AtBrowsers.isCandidate + ), + recommendedAts: browser.ats.filter( + at => at.AtBrowsers.isRecommended + ) + })); - browsers = await activePromise; + return browsers; + }); - browsers = browsers.map(browser => ({ - ...browser.dataValues, - candidateAts: browser.ats.filter( - at => at.AtBrowsers.isCandidate - ), - recommendedAts: browser.ats.filter( - at => at.AtBrowsers.isRecommended - ) - })); + browsers = await activePromise; return browsers; }, diff --git a/server/routes/auth.js b/server/routes/auth.js index 1ee74f20a..6822c6940 100644 --- a/server/routes/auth.js +++ b/server/routes/auth.js @@ -4,11 +4,13 @@ const { oauthRedirectFromGithubController, signoutController } = require('../controllers/AuthController'); +const setFakeUserController = require('../controllers/FakeUserController'); const router = Router(); router.get('/oauth', oauthRedirectToGithubController); router.get('/authorize', oauthRedirectFromGithubController); router.post('/signout', signoutController); +router.post('/fake-user', setFakeUserController); module.exports = router; diff --git a/server/services/GithubService.js b/server/services/GithubService.js index ded14d137..b69f2bbca 100644 --- a/server/services/GithubService.js +++ b/server/services/GithubService.js @@ -68,11 +68,10 @@ const getAllIssues = async () => { module.exports = { graphQLEndpoint, - getOauthUrl: ({ state = '' }) => { + getOauthUrl: () => { return ( `${GITHUB_OAUTH_SERVER}/login/oauth/authorize?scope=` + - `${permissionScopesURI}&client_id=${GITHUB_CLIENT_ID}` + - `&state=${state}` + `${permissionScopesURI}&client_id=${GITHUB_CLIENT_ID}` ); }, async getGithubAccessToken(code) { diff --git a/server/tests/integration/authentication.test.js b/server/tests/integration/authentication.test.js index 14e70b3fc..d508aa53b 100644 --- a/server/tests/integration/authentication.test.js +++ b/server/tests/integration/authentication.test.js @@ -17,7 +17,7 @@ const followRedirects = async firstUrl => { RegExp(`^${process.env.GITHUB_OAUTH_SERVER}`) ); expect(res1.headers.location).toMatch( - /\/login\/oauth\/authorize\?scope=[\w:%]+&client_id=\w+&state=[%\w-]*$/ + /\/login\/oauth\/authorize\?scope=[\w:%]+&client_id=\w+$/ ); const res2 = await fetch(res1.headers.location, { redirect: 'manual' }); @@ -27,7 +27,7 @@ const followRedirects = async firstUrl => { RegExp(`^${process.env.API_SERVER}`) ); expect(res2.headers.get('location')).toMatch( - /\/api\/auth\/authorize\?code=\w+&state=[%\w-]*$/ + /\/api\/auth\/authorize\?code=\w+$/ ); const removeDomain = process.env.API_SERVER.length; @@ -206,109 +206,4 @@ describe('authentication', () => { expect(second.me).toBe(null); }); }); - - it('allows fake roles to be applied for the duration of a session', async () => { - await dbCleaner(async () => { - // A1 - const _dataFromFrontend = 'fakeRole-tester'; - mockGithubServer.nextLogin({ - githubUsername: knownAdmin - }); - - // A2 - await followRedirects( - `/api/auth/oauth?dataFromFrontend=${_dataFromFrontend}` - ); - - const { - body: { data: firstLogin } - } = await sessionAgent.post('/api/graphql').send({ - query: ` - query { - me { - roles - } - } - ` - }); - - await sessionAgent.post('/api/auth/signout'); - - await followRedirects(`/api/auth/oauth`); - - const { - body: { data: secondLogin } - } = await sessionAgent.post('/api/graphql').send({ - query: ` - query { - me { - roles - } - } - ` - }); - - // A3 - expect(firstLogin.me.roles).toEqual(['TESTER']); - expect(secondLogin.me.roles.sort()).toEqual( - ['ADMIN', 'TESTER', 'VENDOR'].sort() - ); - }); - }); - - it('allows faking no teams', async () => { - await dbCleaner(async () => { - // A1 - const _dataFromFrontend = 'fakeRole-'; - mockGithubServer.nextLogin({ - githubUsername: knownAdmin - }); - - // A2 - const res = await followRedirects( - `/api/auth/oauth?dataFromFrontend=${_dataFromFrontend}` - ); - - // A3 - expect(res.status).toBe(303); - expect(res.headers.location).toBe( - `${process.env.APP_SERVER}/signup-instructions` - ); - }); - }); - - it('allows faking vendor', async () => { - await dbCleaner(async () => { - // A1 - const _dataFromFrontend = 'fakeRole-vendor'; - mockGithubServer.nextLogin({ - githubUsername: knownAdmin - }); - - // A2 - const res = await followRedirects( - `/api/auth/oauth?dataFromFrontend=${_dataFromFrontend}` - ); - - // A3 - expect(res.status).toBe(303); - expect(res.headers.location).toBe( - `${process.env.APP_SERVER}/test-queue` - ); - - const { - body: { data } - } = await sessionAgent.post('/api/graphql').send({ - query: ` - query { - me { - roles - } - } - ` - }); - - expect(data.me.roles).toEqual(['VENDOR']); - }); - }); }); diff --git a/server/tests/util/mock-github-server.js b/server/tests/util/mock-github-server.js index d8c4a7d50..047c034f3 100644 --- a/server/tests/util/mock-github-server.js +++ b/server/tests/util/mock-github-server.js @@ -45,10 +45,8 @@ const setUpMockGithubServer = async () => { expressApp.get('/login/oauth/authorize', (req, res) => { const code = randomStringGenerator(); - const { state } = req.query; res.status(303).redirect( - `${process.env.API_SERVER}/api/auth/authorize` + - `?code=${code}&state=${state}` + `${process.env.API_SERVER}/api/auth/authorize?code=${code}` ); });