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

building/testing auth flow #142

Open
1 of 2 tasks
aayu5hgit opened this issue Aug 12, 2023 · 20 comments
Open
1 of 2 tasks

building/testing auth flow #142

aayu5hgit opened this issue Aug 12, 2023 · 20 comments
Assignees
Labels
🐛 bug Something isn't working 🗣️ discuss further discussion is required

Comments

@aayu5hgit
Copy link
Contributor

aayu5hgit commented Aug 12, 2023

Here we're building/testing/discussing the supabase auth flow.

This will be developed further in isolation here using a raw implementation of supabase-js because the nuxt module hides all of the logic and auth is a key part of our app that we need to understand.

Flows handled:

  • register
  • etc. will add when have time
@aayu5hgit aayu5hgit added 🐛 bug Something isn't working 🗣️ discuss further discussion is required labels Aug 12, 2023
@aayu5hgit aayu5hgit self-assigned this Aug 12, 2023
@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon

(bug): Improper login page reload

At the time, when auth is OFF, everything works normal.
But when auth is ON and login is clicked, there are times when it doesn't showcase the changes such as

  • The navbar still shows login and not logged in
  • Page doesn't gets updated

A forced page reload has to be made to see the changes.

@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon

(bug): Update Many returns null

When the auth and SMTP are toggled ON and Update Many is clicked, console shows up the following error

image

I couldn't understand the actual working of this functionality, so can you explain me just the rough flow of things taking place within this functionality?

@Drew-Macgibbon
Copy link
Contributor

@Drew-Macgibbon

(bug): Improper login page reload

At the time, when auth is OFF, everything works normal.
But when auth is ON and login is clicked, there are times when it doesn't showcase the changes such as

  • The navbar still shows login and not logged in
  • Page doesn't gets updated

A forced page reload has to be made to see the changes.

I think I fixed this in develop, but we lost power so I couldn't merge before I left.

I'll send you the updated env tomorrow so you can work on the dev branch.

@Drew-Macgibbon
Copy link
Contributor

@Drew-Macgibbon

(bug): Update Many returns null

When the auth and SMTP are toggled ON and Update Many is clicked, console shows up the following error

image

I couldn't understand the actual working of this functionality, so can you explain me just the rough flow of things taking place within this functionality?

This is expected behaviour, but a shoddy implementation by me. I was using data from private-data which is in the git ignore since it includes emails of users etc.

I just commented out the code and set users to an empty array since the code is only for use locally. And errors if the import isn't valid.

Although it is an admin route, so should return an error that you are not authorised. As I'm pretty sure I set isAdmin to false in the server/middleware/auth

@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon

(bug): Login creds & supabase auth

While logging in on the live website, some errors are being encountered:

  • Post (400); there is something going wrong with supabase auth
    image

  • After clicking submit button, the toast (which is used in profile updation page) is raising up. So we need to make another component which only handles the submission.

@Drew-Macgibbon
Copy link
Contributor

@Drew-Macgibbon

(bug): Login creds & supabase auth

While logging in on the live website, some errors are being encountered:

  • Post (400); there is something going wrong with supabase auth
    image

  • After clicking submit button, the toast (which is used in profile updation page) is raising up. So we need to make another component which only handles the submission.

Ah yes the feedback needs to be updated.

Check the supabase auth logs for more info on the error. You can also resend magic links from the auth tab in supabase.

If the createError is an object convert it to a template literal for logging the error.message

@Drew-Macgibbon
Copy link
Contributor

@aayu5hgit list all the bugs you're facing so I can help give you direction.

@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon

(bug): Auth API Error

When I tried to login locally, it wasn't happening, then I registered again and the verification email didn't arrive and the following error is being displayed in the console (if I try to enter credentials)

image

NOTE: Scenario was tested when

  • Auth was ON
  • SMTP was ON

@Drew-Macgibbon
Copy link
Contributor

@Drew-Macgibbon

(bug): Auth API Error

When I tried to login locally, it wasn't happening, then I registered again and the verification email didn't arrive and the following error is being displayed in the console (if I try to enter credentials)

image

NOTE: Scenario was tested when

  • Auth was ON
  • SMTP was ON

OK, just had a quick look at the supabase logs and am seeing permission denied for schema public

Which means we have a grant_permission error. The authenticator is the Postgres user without the permission. As far as I am aware the authenticator should only be accessing the auth schema. So I think the issue stems from using the auth.users.id as the primary key for public.users.id. This is likely confusing the authenticator

to test this theory I will add a new primary key and move the public.users.id to auth_id. Although I've just discovered that the auth.users.id is NOT immutable, which will require some further thought. I guess a trigger that updates auth_id if it's changed should work...

@Drew-Macgibbon
Copy link
Contributor

Drew-Macgibbon commented Aug 14, 2023

@aayu5hgit ok I have implemented the changes. I also added a trigger to watch the auth.users.id and update the row accordingly. This needed to be handled because if the user updates their email (obviously something we want to allow) it will change their auth.id.

You will need to update the prisma endpoints to check the auth_id instead of the id

hopefully this resolves the errors, I'll leave the testing in your capable hands.

@aayu5hgit
Copy link
Contributor Author

aayu5hgit commented Aug 14, 2023

@Drew-Macgibbon okay, so do I need to update the whole ‘prisma schema‘ too on the codebase?

@Drew-Macgibbon
Copy link
Contributor

thought of an edge case, if the user updates their email, the id and email will not match anything in the public.users table, so a new row would be created. fixed by using the OLD SQL selector:

CREATE OR REPLACE FUNCTION public.create_user_in_public() RETURNS TRIGGER AS $$
BEGIN
    -- Update the row with NEW values if a corresponding row exists with OLD.id
    UPDATE public.users SET auth_id = NEW.id, email = NEW.email WHERE auth_id = OLD.id;

    -- Upsert logic (handles updates where the OLD.id wasn't found)
    INSERT INTO public.users (auth_id, email)
    VALUES (NEW.id, NEW.email)
    ON CONFLICT (auth_id)
    DO UPDATE SET email = excluded.email, auth_id = excluded.auth_id;

    RETURN NEW;
END;
$$ LANGUAGE plpgsql;

@aayu5hgit
Copy link
Contributor Author

aayu5hgit commented Aug 15, 2023

@Drew-Macgibbon after going through the above snippet, I think that in the ON CONFLICT clause of the UPSERT logic, we don't need to explicitly set auth_id = excluded.auth_id because it's the same value as the one in the conflict target (param).

    INSERT INTO public.users (auth_id, email)
    VALUES (NEW.id, NEW.email)
    ON CONFLICT (auth_id)
    DO UPDATE SET email = excluded.email;

I might not be correct, but thought of discussing with you.😅

@Drew-Macgibbon
Copy link
Contributor

@aayu5hgit you're correct. Will update it.

@aayu5hgit
Copy link
Contributor Author

aayu5hgit commented Aug 15, 2023

Drew-Macgibbon I tried updating the prisma endpoints to check the auth_id instead of id, but it isn't resolving the error.
I updated the code as:

const user = await client.users.findFirst({
    where: {
      auth_id: id
    },
    include: {
      roles: true
    }
  })
// Do I need to change this 
const { id } = event.context.params

// To this
const authId = parseInt(req.params.auth_id)

// Or this
const { authId } = event.context.params

@Drew-Macgibbon
Copy link
Contributor

@aayu5hgit it depends on where the id param is coming from. If it's coming from the auth.user then it will be id if it's coming from public.user then it's auth_id. Both are strings.

I'll take a quick look at the code now and see if I can spot anything. Also, if you want help with errors make sure to post the error.

I also reccomend copy pasting the error into chatGPT with some context for immediate help.

@aayu5hgit
Copy link
Contributor Author

aayu5hgit commented Aug 16, 2023

@Drew-Macgibbon yes, I'll do the required and post things to maintain a record.

Also, I did change the public.user part's id with auth_id but I don't see any change in the error.

@Drew-Macgibbon
Copy link
Contributor

@aayu5hgit what is the error?

I've tried to perform a clean npm i but the network keeps dropping, so not much I can do from my end.

Maybe update the zod schemas if you haven't already.

UserBasicSchema should have these:

id: z.number(),
auth_id: z.string().optional(),

@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon not working, can you look into it?
I think there's something wrong with Supabase.

@aayu5hgit
Copy link
Contributor Author

@Drew-Macgibbon Finally working, wohoo!
Tested with a new email and it worked. Check out the recording for ref.

auth.mp4

I think there is a problem where the email that has already been verified is giving invalid credential error when passed the same in the env as testing email.

@Drew-Macgibbon Drew-Macgibbon changed the title Testing the Production building/testing auth flow Aug 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working 🗣️ discuss further discussion is required
Projects
None yet
Development

No branches or pull requests

2 participants