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

Auth smoke tests #4673

Merged
merged 48 commits into from
Mar 15, 2022
Merged

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Mar 6, 2022

Add Auth smoke tests

This PR does the following:

  1. Modifies the test-project generator (and updates fixture) to add a new profile page that:
    a) Is wrappedin <Private>
    b) Uses useAuth and prints some of the values
    c) Adds signup and login tests
    d) Adds playwright tests to check auth props are rendered correctly
    e) Adds tests to check for redirects
    f) Adds RBAC checks to see
    i) If hasRole("ADMIN") returns true
    ii) deleteContact with @requireAuth(roles: ["ADMIN"]) directive works as expected
    g) Adds unit test for ProfilePage, which lets us double check mockCurrentUser is working in unit tests

Todo:

Closes #4486

@dac09 dac09 added the release:chore This PR is a chore (means nothing for users) label Mar 6, 2022
dac09 added 3 commits March 9, 2022 00:07
…ecks-smoke-test

* 'main' of github.com:redwoodjs/redwood:
  Update dependency graphql-helix to v1.12.0 (redwoodjs#4692)
  Update dependency @supabase/supabase-js to v1.31.1 (redwoodjs#4690)
  Update dependency css-loader to v6.7.1 (redwoodjs#4691)
  update yarn.lock
  v0.49.0
  Update dependency @azure/msal-browser to v2.22.1 (redwoodjs#4686)
  Update dependency magic-sdk to v8.1.0 (redwoodjs#4684)
  run smoke-test on Node 14 and 16
  Update typescript-eslint monorepo to v5.14.0 (redwoodjs#4687)
  Update dependency fastify to v3.27.3 (redwoodjs#4683)
  Update dependency @supabase/supabase-js to v1.31.0 (redwoodjs#4677)
  Update dependency @auth0/auth0-spa-js to v1.20.1 (redwoodjs#4682)
  Codemod for webhook verifier option renaming (redwoodjs#4675)
@dac09
Copy link
Contributor Author

dac09 commented Mar 8, 2022

It's alive! It's alive!

https://s.tape.sh/w6waC16z

@dac09
Copy link
Contributor Author

dac09 commented Mar 10, 2022

@thedavidprice do you want to start reviewing? Awaiting merging the other PR (just holding till Rob does a once over) - then will regenerate the fixture.


describe('ProfilePage', () => {
it('renders successfully', async () => {
mockCurrentUser({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This lets us verify that mockCurrentUser is working

</tr>
</thead>
<tbody>
{Object.keys(currentUser).map((key) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just prints the keys from currentUser in useAuth.

// "port" fixture uses a unique value of the worker process index.
await use(9000 + workerInfo.workerIndex)
await use(9000)
Copy link
Contributor Author

@dac09 dac09 Mar 10, 2022

Choose a reason for hiding this comment

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

Changes here are because we actually don't want to spin up multiple dev servers (because our dev process regenerates some files)

This does not affect running tests in parallel though, because browser contexts are isolated

fs.writeFileSync(
path.join(process.env.PROJECT_PATH, 'scripts/makeAdmin.ts'),
`
import { db } from 'api/src/lib/db'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We use rw exec to convert the user to an admin

'<td>EMAIL</td><td>[email protected]</td>'
)

// const isAdminRow = await page.waitForSelector('*css=tr >> text=Is Admin')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will enable once the other PR is merged

Copy link
Contributor

Choose a reason for hiding this comment

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

SuperHelpfulReminder™

  • do the thing you were waiting to do here

return root
.findJSXElements('Route')
.filter(
j.filters.JSXElement.hasAttributes({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Finds the <Route path="/profile"/> and wraps in a <Private> component

@thedavidprice
Copy link
Contributor

@dac09 Amazing work! I just checked out the Tape.

Understood about dependency on #4678

Will review today ✅

@dac09
Copy link
Contributor Author

dac09 commented Mar 14, 2022

@thedavidprice ready to merge! ✅✅✅✅✅✅✅

dac09 added 13 commits March 15, 2022 11:14
…d into feat/auth-checks-smoke-test

* 'feat/auth-checks-smoke-test' of github.com:dac09/redwood:
  fix(deps): update dependency esbuild to v0.14.27 (redwoodjs#4755)
  chore(deps): update dependency cypress to v9.5.2 (redwoodjs#4754)
  chore(deps): update dependency @types/lodash to v4.14.180 (redwoodjs#4751)
  fix(deps): update typescript-eslint monorepo to v5.15.0 (redwoodjs#4752)
  fix(deps): update dependency esbuild to v0.14.26 (redwoodjs#4747)
  Setup futureExpiresDate in constructor (redwoodjs#4742)
  chore(deps): update dependency @clerk/clerk-sdk-node to v2.9.10 (redwoodjs#4746)
  fix(deps): update dependency eslint-plugin-react to v7.29.4 (redwoodjs#4745)
Use contacts count to have more reliable tests
…d into feat/auth-checks-smoke-test

* 'feat/auth-checks-smoke-test' of github.com:dac09/redwood:
  chore(deps): update dependency @playwright/test to v1.20.0 (redwoodjs#4757)
…ecks-smoke-test

* 'main' of github.com:redwoodjs/redwood:
  ignore @types/lru-cache (redwoodjs#4759)
  chore(deps): update dependency zx to v5.3.0 (redwoodjs#4758)
@dac09
Copy link
Contributor Author

dac09 commented Mar 15, 2022

I've updated it again. I had some git issues apparently (or just a problem between the keyboard and the chair) - didn't have the latest RBAC tests.

I've also updated the auth checks, because I realised that we were getting false positives with the error selectors... painfulllll 😭

@thedavidprice
Copy link
Contributor

@dac09 Contacts work looks 🤯

Just incase you're reviewing another PR later, I think we should enforce that we don't modify src files in fixtures or in the test from now on! It makes it harder, but it'll save us soooo much effort in the future!

Just so I'm clear, you mean:

  1. we do not allow manual, direct changes to any of the __fixtures__/ projects, correct?
  2. instead, if we need to update, we should add the modification to the build:test-project tasks, correct?

Local Test-project dbAuth still not working

I still can't get local dbAuth to work (see previous comment above). I hope it's just because I'm missing something obvious.

If I'm reading the auth tests correctly, you're able to create a user and login (note: the login and home redirect should happen at the same time new user successfully created). But I can do that successfully when I try locally.

Steps to reproduce:

  1. check out this PR branch
  2. yarn build:test-project --ts --link <directory>
  3. cd and yarn rw dev
  4. try to create a new user and log in

Working
I can confirm a user is being created in the DB. API Get is good. Networking shows function calls.

Not working
I cannot log in. Redirect and login flow is not working.

I'm at a loss... something obvious I'm missing?

Safari-15Mar22_12-54-22.mp4

Testing Main Branch

I now believe dbAuth is broken in main. Will confirm, open an issue if necessary, then link to this.

@thedavidprice
Copy link
Contributor

thedavidprice commented Mar 15, 2022

Confirmed this is a bug in main. See #4767

@dac09 Again, I don't know why the auth smoke test is passing if I understand correctly that it's stepping through creating a user and logging in. If the test was failing and/or we confirm the behavior is 1-to-1, it will be helpful to merge this PR in order to resolve #4767 And/or the fix can be applied here.

I could be doing something wrong, but right now I can't figure out what that might be.

Copy link
Contributor

@thedavidprice thedavidprice left a comment

Choose a reason for hiding this comment

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

Need to resolve dbAuth login bug #4767

TBD if we resolve here and then merge or merge and use this to resolve

@thedavidprice
Copy link
Contributor

Issue was specific to Safari. We were missing #4722

I'm will update and rebuild fixture. Then we're gtg 🚀

@thedavidprice thedavidprice merged commit 3982d32 into redwoodjs:main Mar 15, 2022
@jtoar jtoar added this to the next-release milestone Mar 15, 2022
@thedavidprice thedavidprice modified the milestones: next-release, v0.50.0 Mar 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:chore This PR is a chore (means nothing for users)
Projects
No open projects
Status: Archived
Development

Successfully merging this pull request may close these issues.

[CI] Add dbAuth functional test
3 participants