Replace Auth #37

tmyracle · 2024-01-12T18:20:26Z

I wanted to provide a draft PR on this to make sure everyone is aligned with the path I'm going down here. Comments, feedback, recommendations all greatly appreciated. This is a big change!

but first, here's a demo showing registration, sign out, and signing back in:

CleanShot.2024-01-12.at.12.02.23.mp4

A lot going on here, here's a quick breakdown:

Add NextAuth/AuthJS
Implement email/password register/auth (figured its easiest for self host, can add more providers later)
Add routes and services for finding existing user by email, creating user
Add Register and Sign In pages, took best stab at styling using design library (if you have mocks from back in the day I'm happy to match previous styles)
The newly added Auth models are all prefixed with "Auth" to prevent collisions with existing User and Account models
Add authId to User model
Update _app.tsx logic to check for session and redirect to sign in if user doesn't have session

TO-DO:

Update JWT to match fields that were populated by Auth0
~~Update middleware and JWT checks~~
~~Create User and associate with AuthUser~~
~~Figure out how to kick off onboarding for new users~~
Continue replacing Auth0 functionality with NextAuth
??? Probably a lot more that we'll find as we continue fixing things.

Hopefully this is some decent progress on #16
/claim #16

Shpigford · 2024-01-12T18:25:52Z

Amazing. Thanks for the first pass on this, @tmyracle!

tmyracle · 2024-01-12T18:27:51Z

apps/server/src/app/app.ts

@@ -148,6 +149,7 @@ app.use('/tools', devOnly, toolsRouter)
 app.use('/v1', webhooksRouter)

 app.use('/v1', publicRouter)
+app.use('/v1/auth-users', authUserRouter)


Need to figure out a better authorization pattern for this route / refactor checking for existing user by email and creating new users so those routes aren't open to the wild.

McPizza0 · 2024-01-12T20:49:17Z

Nice work @tmyracle !

credentials provider is discouraged, see https://authjs.dev/getting-started/providers/credentials-tutorial

@Shpigford do you specifically want user/pass logins or would you consider some oAuth options as well?
considering the purpose of this app, I think a more secure method would be suitable
theres an open PR for native passkey support, I have it running and can help with implementation if you want to go that way

Shpigford · 2024-01-12T22:53:46Z

@Shpigford do you specifically want user/pass logins or would you consider some oAuth options as well?

User/email + pass should be the entry point and then in the future we can add other providers if there's demand. No reason to google/apple/facebook/etc at this stage.

tmyracle · 2024-01-13T00:40:07Z

Made a little more progress I think? I can at least access some of the app. Had to rework things to place nice with Express since NextAuth expects just a Next.js environment, hence the cookie parsing in the middleware check. Now when registering it is creating the User object in the database automatically and associating it with the AuthUser object that is owned by NextAuth.

tmyracle · 2024-01-13T05:55:14Z

Onboarding is back, also fixed a few react warnings/bugs.
Video here: https://github.com/maybe-finance/maybe/assets/1218724/b5b0db6b-ced7-48c4-95c5-13e11a553f16

Looks like Auth0 took care of sending email verification so I'll need to go in and create an email and related services to handle sending the email verification and handling updating when user clicks the link in the email. For now I just added a flag to skip adding the email verification step to onboarding.

Next going to remove all use of the Auth0 libraries, server endpoints, and anything else that might be calling Auth0.

Shpigford · 2024-01-13T15:50:29Z

Looks like Auth0 took care of sending email verification so I'll need to go in and create an email and related services to handle sending the email verification and handling updating when user clicks the link in the email. For now I just added a flag to skip adding the email verification step to onboarding.

@tmyracle I agree with this move. Let's keep the flag to skip email verification and then we'll address email sending and email verification in a future issue.

gamedevsam

I think you need to look for uses of useAuth0Profile and replace or delete those. Great work though, this is very exciting!

apps/client/pages/_app.tsx

apps/client/pages/api/auth/[...nextauth].ts

apps/client/pages/login.tsx

apps/server/src/app/routes/auth-user.router.ts

gamedevsam · 2024-01-13T16:49:33Z

apps/server/src/app/routes/users.router.ts

@@ -276,6 +286,7 @@ router.get(
    })
 )

+// TODO: Remove this endpoint or refactor to work with new Auth


Are these comments relevant? I don't understand their purpose.

It's purely for me to track things down as I continue working on this. I wanted to share a draft to get feedback on overall direction. They'll be removed as the changes are made.

gamedevsam · 2024-01-13T16:50:24Z

libs/client/features/src/onboarding/steps/CountryWaitlist.tsx

    const { useDelete } = useUserApi()

    const deleteUser = useDelete({
        onSuccess() {
            toast.success(`Account deleted`)
-            setTimeout(() => logout({ logoutParams: { returnTo: window.location.origin } }), 500)
+            setTimeout(() => signOut(), 500)


Why is there a delay to log out here?

good question, will dig deeper

I'm guessing its to give time for the Toast to be displayed so the user has verification that the account was deleted prior to logging them out.

EduardsE · 2024-01-13T18:02:25Z

apps/client/components/APM.tsx

 import { useEffect } from 'react'
 import * as Sentry from '@sentry/react'
+import { useSession } from 'next-auth/react'

 export default function APM() {


What does APM stand for?

I believe its Application Performance Monitoring

coman3 · 2024-01-13T20:33:15Z

@Shpigford do you specifically want user/pass logins or would you consider some oAuth options as well?

User/email + pass should be the entry point and then in the future we can add other providers if there's demand. No reason to google/apple/facebook/etc at this stage.

Part of me would suggest to at least enable google and Microsoft (personal + 365) login support as a lot of home lab users use these services to secure their self-hosted stuff, myself included.

Enabling support for OpenID connect workflows would be enough to make this flexible for everyone and I think this is a must have for a self-hosted app.

Shpigford · 2024-01-13T20:44:06Z

@coman3 i 100% planning on supporting it, but it's just outside the scope/purpose of this issue/PR.

PierreAndreis · 2024-01-13T21:07:48Z

AuthJS is very flexible, once in place it can support any provider with a few lines in. Let the man do the ground work and later come and introduce google sign in if you want

kkoppenhaver · 2024-01-14T06:53:44Z

This looks awesome! Thanks for all the work on this, y'all.

When I try to run this locally, I get the following error, which makes me think my API service isn't running somehow? Anyone seen this/have an idea how to resolve?

useUserApi.ts:55 
GET http://localhost:3333/v1/users/onboarding/main net::ERR_CONNECTION_REFUSED

tmyracle · 2024-01-14T07:01:09Z

This looks awesome! Thanks for all the work on this, y'all.

When I try to run this locally, I get the following error, which makes me think my API service isn't running somehow? Anyone seen this/have an idea how to resolve?
useUserApi.ts:55 

GET http://localhost:3333/v1/users/onboarding/main net::ERR_CONNECTION_REFUSED

Hmm yeah looks like server might not be running. See anything in the console logs? Can also hit localhost:3333/health and check if you get an ok message back to make sure it's running.

kkoppenhaver · 2024-01-14T07:14:23Z

Looks like it's just refusing to connect all together (see screenshot).

When I boot it up, I get

error: uncaughtException: Must provide a clientSecret
ArgumentError: Must provide a clientSecret

So I'm thinking maybe that's it? But that would make it seem like it's still looking at Auth0 for that secret...

kkoppenhaver · 2024-01-14T07:44:46Z

I was able to get this working. I just needed NX_AUTH0_MGMT_CLIENT_SECRET and NX_AUTH0_CLIENT_SECRET to have a value. And then NX_SESSION_SECRET and NEXTAUTH_SECRET needed to be filled out.

🎉

tmyracle · 2024-01-14T17:03:56Z

Ok did a bunch of cleanup, should be pretty close to move to ready for review. I decided to remove the admin router for now, I'll leave it up to @Shpigford if he wants to remove the admin functionality or re-write it to work without Auth0 which would involve re-working the roles framework and refactoring how roles are assigned since those are currently owned by the Auth0 pages and sign up flow.

Things out of scope for now, but should be addressed in next PR since this is already massive:

Update all of the tests, they won't run now since there's a lot of Auth0 mocking
Use Postmark to add email validation since this was taken care of by Auth0 previously (if we want this added back)
Revisit admin functionality depending on whether it's a necessary feature for OSS version

Depending on decisions here we can spin up some new issues for folks to tackle some of these items.

tmyracle · 2024-01-14T17:06:57Z

apps/server/src/app/__tests__/utils/axios.ts

Testing will be addressed in future PR, focusing on unblocking dev and getting app functional.

tmyracle · 2024-01-14T17:07:44Z

apps/server/src/app/routes/e2e.router.ts

Testing will be addressed in future PR, focusing on unblocking dev and getting app functional.

tmyracle · 2024-01-14T17:11:10Z

libs/client/features/src/onboarding/steps/setup/AddFirstAccount.tsx

@@ -59,6 +59,8 @@ export function AddFirstAccount({ title, onNext }: StepProps) {
                                            loader={BrowserUtil.enhancerizerLoader}
                                            src={`financial-institutions/white/${src}.svg`}


These assets are missing so they won't load during onboarding. @Shpigford do you have these or should we plan to replace them?

tmyracle · 2024-01-14T17:30:19Z

Gonna go ahead and make this ready for review @Shpigford. The focus is getting an initial auth replacement implemented and pull out Auth0 and things that depend on it so that we can unblock development for other folks that want to contribute. This effort isn't complete, but I feel like its a good stopping point that achieves the goal stated above and can be built upon with future changes.

Future PRs will address:

Updating tests and add additional tests for NextAuth/Auth.js for coverage
Adding in email verification using Postmark (Auth0 handled this previously)
Add forgot password flow with Postmark as email provider (might look at abstracting this so people can use whatever email provider they want)
Remove remaining auth0 dependencies in package.json
Implementing roles (if that's something that is necessary for the open source version of this)

Issues for those changes will be added once this is reviewed and merged 🚀

Shpigford · 2024-01-14T18:06:01Z

@tmyracle really great work! after registration, I'm getting an "Unable to load onboarding" error and this in the console:

Error in token validation JWEDecryptionFailed: decryption operation failed
    at gcmDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:67:15)
    at decrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:92:20)
    at flattenedDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/flattened/decrypt.js:143:52)
    at compactDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/compact/decrypt.js:18:23)
    at jwtDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwt/decrypt.js:8:23)
    at decode (/Users/joshpigford/Development/maybe/node_modules/next-auth/jwt/index.js:66:7) {
  code: 'ERR_JWE_DECRYPTION_FAILED'
}

tmyracle · 2024-01-14T18:16:31Z

@tmyracle really great work! after registration, I'm getting an "Unable to load onboarding" error and this in the console:

Error in token validation JWEDecryptionFailed: decryption operation failed
    at gcmDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:67:15)
    at decrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:92:20)
    at flattenedDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/flattened/decrypt.js:143:52)
    at compactDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/compact/decrypt.js:18:23)
    at jwtDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwt/decrypt.js:8:23)
    at decode (/Users/joshpigford/Development/maybe/node_modules/next-auth/jwt/index.js:66:7) {
  code: 'ERR_JWE_DECRYPTION_FAILED'
}

Hmm, I'm assuming you have NEXTAUTH_SECRET populated in your .env?

Shpigford · 2024-01-14T18:19:56Z

Hmm, I'm assuming you have NEXTAUTH_SECRET populated in your .env?

@tmyracle Doh. Totally missed needing to do that! I'm set. Works great. 🎉

coltonehrman · 2024-01-14T18:24:14Z

@tmyracle really great work! after registration, I'm getting an "Unable to load onboarding" error and this in the console:

Error in token validation JWEDecryptionFailed: decryption operation failed
    at gcmDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:67:15)
    at decrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:92:20)
    at flattenedDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/flattened/decrypt.js:143:52)
    at compactDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/compact/decrypt.js:18:23)
    at jwtDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwt/decrypt.js:8:23)
    at decode (/Users/joshpigford/Development/maybe/node_modules/next-auth/jwt/index.js:66:7) {
  code: 'ERR_JWE_DECRYPTION_FAILED'
}

Hmm, I'm assuming you have NEXTAUTH_SECRET populated in your .env?

Maybe we could include a basic secret as part of the example instead of leaving it empty?

tmyracle · 2024-01-14T18:28:44Z

@tmyracle really great work! after registration, I'm getting an "Unable to load onboarding" error and this in the console:

Error in token validation JWEDecryptionFailed: decryption operation failed
    at gcmDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:67:15)
    at decrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/runtime/decrypt.js:92:20)
    at flattenedDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/flattened/decrypt.js:143:52)
    at compactDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwe/compact/decrypt.js:18:23)
    at jwtDecrypt (/Users/joshpigford/Development/maybe/node_modules/jose/dist/node/cjs/jwt/decrypt.js:8:23)
    at decode (/Users/joshpigford/Development/maybe/node_modules/next-auth/jwt/index.js:66:7) {
  code: 'ERR_JWE_DECRYPTION_FAILED'
}

Hmm, I'm assuming you have NEXTAUTH_SECRET populated in your .env?

Maybe we could include a basic secret as part of the example instead of leaving it empty?

Updated the README to add instructions for populating prior to install and run. I don't like leaving placeholder secrets because it's easy for people to not change them and introduce vulnerabilities.

coltonehrman · 2024-01-14T18:30:35Z

Was able to run locally fine! :D LGTM 🎉

Shpigford · 2024-01-14T18:36:20Z

Alrighty, so 3 folks confirmed running it with the new auth. I'm gonna call it. YOLO, etc etc.

AMAZING job on this @tmyracle!

tmyracle added 5 commits January 11, 2024 21:16

work on auth replacement

3c7b461

basic auth sign in

2c3da54

minimal register and login

b0e4746

clean up

343d7db

fix conflicts

e740fcb

tmyracle commented Jan 12, 2024

View reviewed changes

tmyracle added 2 commits January 12, 2024 18:34

add middleware check, refactor axios provider

633765d

Merge remote-tracking branch 'upstream/main' into replace-auth

18a8d7a

tmyracle added 3 commits January 12, 2024 21:43

add basic error messages for login and register

6323495

use fancier logo and remove ask the advisor notif settings

093949f

add back onboarding

e405fb8

tmyracle added 3 commits January 13, 2024 10:21

refactor

22ade8b

Merge remote-tracking branch 'upstream/main' into replace-auth

e4df737

Merge remote-tracking branch 'upstream/main' into replace-auth

5f3ebb1

gamedevsam suggested changes Jan 13, 2024

View reviewed changes

tmyracle added 2 commits January 13, 2024 11:03

continue cleaning up and removing Auth0

f94eb5b

move zapier feedback hook url to env

6663963

EduardsE reviewed Jan 13, 2024

View reviewed changes

Shpigford mentioned this pull request Jan 13, 2024

Rebuild authentication (remove Auth0) #16

Closed

replace getToken with decode

9ebc9fa

fix json parsing issue in image-loader

7267c30

remove most of the Auth0 useage

6100ab1

tmyracle commented Jan 14, 2024

View reviewed changes

tmyracle added 2 commits January 14, 2024 11:19

more clean up

56c7885

remove @auth0 deps

30fa84b

tmyracle marked this pull request as ready for review January 14, 2024 17:24

tmyracle changed the title ~~DRAFT: Replace Auth~~ Replace Auth Jan 14, 2024

Shpigford added this to the v1.0.0 milestone Jan 14, 2024

update readme

c56d528

tmyracle added 2 commits January 14, 2024 12:25

update readme to generate secret

e5edb1e

typo

cedbe1b

Minor readme update for building the app

85b56ca

Shpigford merged commit 4705620 into maybe-finance:main Jan 14, 2024

algora-pbc bot added the 🙋 Bounty claim label Jan 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Replace Auth #37

Replace Auth #37

tmyracle commented Jan 12, 2024 •

edited

Loading

Shpigford commented Jan 12, 2024

tmyracle Jan 12, 2024

McPizza0 commented Jan 12, 2024

Shpigford commented Jan 12, 2024

tmyracle commented Jan 13, 2024 •

edited

Loading

tmyracle commented Jan 13, 2024 •

edited

Loading

Shpigford commented Jan 13, 2024

gamedevsam left a comment

gamedevsam Jan 13, 2024

tmyracle Jan 13, 2024

gamedevsam Jan 13, 2024

tmyracle Jan 13, 2024

tmyracle Jan 14, 2024

EduardsE Jan 13, 2024

tmyracle Jan 13, 2024

coman3 commented Jan 13, 2024

Shpigford commented Jan 13, 2024

PierreAndreis commented Jan 13, 2024 •

edited

Loading

kkoppenhaver commented Jan 14, 2024

tmyracle commented Jan 14, 2024 •

edited

Loading

kkoppenhaver commented Jan 14, 2024

kkoppenhaver commented Jan 14, 2024

tmyracle commented Jan 14, 2024

tmyracle Jan 14, 2024

tmyracle Jan 14, 2024

tmyracle Jan 14, 2024

tmyracle commented Jan 14, 2024

Shpigford commented Jan 14, 2024

tmyracle commented Jan 14, 2024

Shpigford commented Jan 14, 2024

coltonehrman commented Jan 14, 2024

tmyracle commented Jan 14, 2024

coltonehrman commented Jan 14, 2024

Shpigford commented Jan 14, 2024

		@@ -59,6 +59,8 @@ export function AddFirstAccount({ title, onNext }: StepProps) {
		loader={BrowserUtil.enhancerizerLoader}
		src={`financial-institutions/white/${src}.svg`}

Replace Auth #37

Replace Auth #37

Conversation

tmyracle commented Jan 12, 2024 • edited Loading

Shpigford commented Jan 12, 2024

Choose a reason for hiding this comment

McPizza0 commented Jan 12, 2024

Shpigford commented Jan 12, 2024

tmyracle commented Jan 13, 2024 • edited Loading

tmyracle commented Jan 13, 2024 • edited Loading

Shpigford commented Jan 13, 2024

gamedevsam left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coman3 commented Jan 13, 2024

Shpigford commented Jan 13, 2024

PierreAndreis commented Jan 13, 2024 • edited Loading

kkoppenhaver commented Jan 14, 2024

tmyracle commented Jan 14, 2024 • edited Loading

kkoppenhaver commented Jan 14, 2024

kkoppenhaver commented Jan 14, 2024

tmyracle commented Jan 14, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tmyracle commented Jan 14, 2024

Shpigford commented Jan 14, 2024

tmyracle commented Jan 14, 2024

Shpigford commented Jan 14, 2024

coltonehrman commented Jan 14, 2024

tmyracle commented Jan 14, 2024

coltonehrman commented Jan 14, 2024

Shpigford commented Jan 14, 2024

tmyracle commented Jan 12, 2024 •

edited

Loading

tmyracle commented Jan 13, 2024 •

edited

Loading

tmyracle commented Jan 13, 2024 •

edited

Loading

PierreAndreis commented Jan 13, 2024 •

edited

Loading

tmyracle commented Jan 14, 2024 •

edited

Loading