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

sveltekit helpers update #245

Merged
merged 47 commits into from Sep 26, 2022
Merged

sveltekit helpers update #245

merged 47 commits into from Sep 26, 2022

Conversation

david-plugge
Copy link
Collaborator

@david-plugge david-plugge commented Sep 11, 2022

Closes #230

What kind of change does this PR introduce?

Support new versions of sveltekit (around 1.0.0-next.440 upwards, random guess based on their changelog)

What is the current behavior?

We can´t use auth helpers with the current version of sveltekit.

Additional context

Since there is no need for a Svelte component we could build the sveltekit helpers with tsup like the other packages.

PS:
SvelteKit is in release candidate phase!

Edit: very sorry for this huge commit :/

@dlugoschvincent
Copy link
Contributor

dlugoschvincent commented Sep 11, 2022

Firstly, thank you for your work bringing the auth helpers up to date with the newest version of sveltekit.

I have one question. I don't see any example or possibility to trigger an action with authentication. All I see is authenticated data retrieval, but not authenticated data posting. Am I seeing this correctly, or how would I go about making an authenticated insert/update/delete from the server side?

@david-plugge
Copy link
Collaborator Author

david-plugge commented Sep 11, 2022

Using supabaseServerClient you get a supabase instance that sends the users accessToken on every request.

I´ll add an example though!

Edit:

Completely misread your comment, there is missing a helper for actions. The same logic would apply though.
And you can always do supabaseServerClient(locals.accessToken)

Something like this:

export const load: Actions = {
  createPost: actionWithSession(
    { status: 401, '/signin' },
    async ({ session, getSupabaseClient }) => {
      // do something
    }
  )
}

@david-plugge
Copy link
Collaborator Author

david-plugge commented Sep 11, 2022

To clarify a bit:

The helpers (load, serverLoad, server) check if there is a logged in user and if not redirects with the status and location provided. So the callback only runs when there is a session, which makes sense to me. There won´t be many pages that check if there is no session right?

There could be added even more to the event than just the session.

export const load: PageLoad = loadWithSession(
	{ status: 303, location: '/' },     	// the user is redirected if there is no session
	async ({ session }) => {				// we have a session and can make authenticated requests
		const { data: testTable } = await supabaseServerClient(session.accessToken)
			.from<TestTable>('test')
			.select('*');
		return {
			testTable,
			user: session.user  // the user is correctly typed, no need to check if it´s nullish
		};
	}
);

Since the helper has access to the session this is possible aswell:

export const load: PageLoad = loadWithSession(
	{ status: 303, location: '/' }, 
	async ({ getSupabaseClient }) => {
		const { data: testTable } = await getSupabaseClient()
			.from<TestTable>('test')
			.select('*');
		return {
			testTable,
        };
	}
);

Thoughts?

@dlugoschvincent
Copy link
Contributor

Ok, thank you, I figured as much, but how do I get the access token in an action as there is a loadWithSession function but no “action with session” function. Maybe I am confused.

Imagine a scenario like this, where I want to make sure only authenticated users can insert into the database.
Would something like this be possible?

export const actions: Actions = {
  add: async ({ request }) => {
    const json = await request.json();
    supabaseServerClient(json.accessToken)
      .from<definitions['Food']>('Food')
      .insert({ bar_code: json.barCode });
  }
}

@david-plugge
Copy link
Collaborator Author

You can use locals in actions

export const actions: Actions = {
  add: async ({ request, locals }) => {
    if (!locals.accessToken) {
        throw redirect(303, '/signin');
    }
    const json = await request.json();
    supabaseServerClient(locals.accessToken)
      .from<definitions['Food']>('Food')
      .insert({ bar_code: json.barCode });
  }
}

@dlugoschvincent
Copy link
Contributor

Okay, thank you so much, I will give it a try! Sorry for taking your time. Having an example for actions would be really helpful. I could think one up once this PR gets merged.

@david-plugge
Copy link
Collaborator Author

Updated docs

Feel free to leave some comments

@thorwebdev thorwebdev added enhancement New feature or request sveltekit SvelteKit specific functionality labels Sep 15, 2022
@kvetoslavnovak
Copy link

kvetoslavnovak commented Sep 15, 2022

I guess this dangerous SvelteKit data leaking in SSR issue should be checked as well.
There is even a proposed solution for a custom store reinventing the depreciated session store.
Edit: Regarding the draft of updated documentation it seems to me there is still depreciated import { session } from '$app/stores'; mentioned or is not?

@david-plugge
Copy link
Collaborator Author

Good catch, i found a few more issues in the readme.
Since we are only using the page store there is no chance data can leak.

@david-plugge
Copy link
Collaborator Author

I noticed one problem though that is also included in the current version.
The supabaseServerClient() method returns the supabase instance and sets the accessToken. If there is no token it wont be set. So now the global instance on the server uses that accessToken to make api calls and if a developer does not check if the access token is valid before using the helper data will leak.

  1. user with valid accessToken makes a request -> global instance accessToken is set
  2. user without token makes a request -> global instance reuses the accessToken from first request

I can think of two possible solutions:

  1. throw an error if there is no accessToken
  2. unset the accessToken
  3. create a new supabase client on every request and store it in locals (PocketBase does it like this)

Option 3 sounds nice, but since we don´t have access to locals in load functions this isn´t the best option.

I prefer option 1 as there is no reason to use the helper method if you want to serve public data as you can simply use the global supabaseClient instance. That way you get feedback if you are doing something wrong.

@kvetoslavnovak
Copy link

@david-plugge please, can you check in your updated docs (Server-side data fetching with RLS and Protecting API routes) whether your imports are right?
Especially importing import { supabaseServerClient, withApiAuth} from '@supabase/auth-helpers-sveltekit'; but using getSupabaseClient and withAuth later on.

Thank you very much.

@yzn-h
Copy link

yzn-h commented Sep 24, 2022

nice work guys, i really need this to be merged 😅

@maxkozlowski
Copy link

maxkozlowski commented Sep 24, 2022

Great stuff guys, really appreciated all the work that has gone here!
Seconding @yzn-h - if you could merge it, that would help me a lot! 🙏

Edit: I've forked and published the new version via npm, but getting an error when trying to install.
npm ERR! Unsupported URL Type "workspace:": workspace:*

@david-plugge
Copy link
Collaborator Author

I've forked and published the new version via npm, but getting an error when trying to install.
npm ERR! Unsupported URL Type "workspace:": workspace:*

Have you published the package with npm or pnpm? I think npm doesn´t replace workspace:* with the actual version.

And you didn´t publish the auth-helpers-shared package right?

pnpm i @maxkozlowski/auth-helpers-sveltekit
> ERR_PNPM_NO_MATCHING_VERSION_INSIDE_WORKSPACE  In : No matching version found for @supabase/auth-helpers-shared@* inside the workspace

@maxkozlowski
Copy link

Have you published the package with npm or pnpm?
I published it with npm.

And you didn´t publish the auth-helpers-shared package right?
That's right, I didn't publish auth-helpers-shared.

Happy to try to publish both libraries with pnpm instead if that should work?

@jamalmoir
Copy link

Hopefully, it will be merged soon. It was just the weekend, and this is a company, so I assume they don't work on the weekends.

@david-plugge
Copy link
Collaborator Author

Happy to try to publish both libraries with pnpm instead if that should work?

It should, if not there is a bug. Make sure to change the package name of the shared helpers in the dependencies of the sveltekit helpers too

@silentworks silentworks merged commit 9c82084 into supabase:main Sep 26, 2022
@silentworks
Copy link
Contributor

Thanks for this @david-plugge, I will get a release out during this week after writing up the migration docs.

@maxkozlowski
Copy link

maxkozlowski commented Sep 27, 2022

Thank you for merging the change and releasing so quickly! I've downloaded the library but I'm getting an error with one of imports in src/hooks.server.ts.

import { auth } from '@supabase/auth-helpers-sveltekit/server';
Cannot find module '@supabase/auth-helpers-sveltekit/server' or its corresponding type declarations.

Does anyone experience the same? Not sure how to fix this 🤔

@david-plugge
Copy link
Collaborator Author

david-plugge commented Sep 27, 2022

Yeah that´s a known issue. If you have multiple entry points it´s hard to get ts intellisense working if they are not located in the packages root folder. I´ve setup publishConfig in the package.json to publish the dist folder only. It seems like @silentworks decided to remove that. To get it working like this (publishing the whole package, not just dist) we need to add this to the package.json:

"typesVersions": {
  "*": {
    "server": ["./dist/server/index.d.ts"],
    "./": ["./dist/index.d.ts"]
  }
}

I created a PR #267

@maxkozlowski
Copy link

Thank you David! Does that mean that we'll need to wait for the new release now?

@mitjakukovec
Copy link

@silentworks, was publishConfig in the package.json removed intentionally or if it will be re-introduced? Thanks!

@jamalmoir
Copy link

I'm getting ERR_MODULE_NOT_FOUND due to imports not having the extensions specified eg. import {...} from './utils/cookie' rather than import {...} from './utils/cookie.js'

How are people getting this to work?

@david-plugge
Copy link
Collaborator Author

Does that mean that we'll need to wait for the new release now?

In the meantime you can use this alias in your svelte.config.js

alias: {
	'@supabase/auth-helpers-sveltekit/server': './node_modules/@supabase/auth-helpers-sveltekit/dist/server'
}

But i think there are a few more issues that need to be addressed. It seems like $app/* is not accessible in a npm package while it works locally in the monorepo.

@david-plugge
Copy link
Collaborator Author

I'm getting ERR_MODULE_NOT_FOUND due to imports not having the extensions specified eg. import {...} from './utils/cookie' rather than import {...} from './utils/cookie.js'

How are people getting this to work?

Before the update there was a script that adds .js to every imported file to solve that issue. I removed it since it shouldn´t be an issue if the files are located in the packages root. I´ll add that script back in.

@jamalmoir
Copy link

I'm getting ERR_MODULE_NOT_FOUND due to imports not having the extensions specified eg. import {...} from './utils/cookie' rather than import {...} from './utils/cookie.js'
How are people getting this to work?

Before the update there was a script that adds .js to every imported file to solve that issue. I removed it since it shouldn´t be an issue if the files are located in the packages root. I´ll add that script back in.

So rather than it living in node_modules, it should live in your project root?

@david-plugge
Copy link
Collaborator Author

So rather than it living in node_modules, it should live in your project root?

No i mean in the folder node_modules/@supabase/auth-helpers-sveltekit. Currently they live in node_modules/@supabase/auth-helpers-sveltekit/dist. But i´m not 100% confident about that.

@jamalmoir
Copy link

So rather than it living in node_modules, it should live in your project root?

No i mean in the folder node_modules/@supabase/auth-helpers-sveltekit. Currently they live in node_modules/@supabase/auth-helpers-sveltekit/dist. But i´m not 100% confident about that.

Ah, OK I see. Got you, thanks

@maxkozlowski
Copy link

Has anyone got the new release to work in the end? I am unable to I'm afraid.

It would be fantastic to either get clear instructions as to how to get it to work or ETA when we might get a release that works.

@jamalmoir
Copy link

@maxkozlowski #267

@maxkozlowski
Copy link

maxkozlowski commented Sep 28, 2022

Looks like the new release works well 👏
The only thing I'm struggling with now is log in. I've followed the instructions and used saveSession but getting an error:
No action with name 'default' found.

Has anyone come across this?

@ak4zh
Copy link

ak4zh commented Sep 28, 2022

@maxkozlowski You need to create named action.

async signin({ request, cookies, url }) {}

to

signin: async ({ request, cookies, url }) => {}

and in your form use action=“?/signin”

@david-plugge
Copy link
Collaborator Author

Take a look at this example https://github.com/supabase/auth-helpers/blob/main/examples/sveltekit-email-password/src/routes/(app)/%2Bpage.server.ts :)

@ak4zh
Copy link

ak4zh commented Sep 28, 2022

@david-plugge

  1. Here It mentions auth callback api/auth/callback do we need to create that endpoint?

or it’s auto handled if we use the export const handle = auth();

  1. Here it’s redirecting to /logging-in is that a special endpoint that needs to be created and do some special cookie saving stuff? Or we can redirect to any page including root and it is all handled by hooks.server.js ?

@david-plugge
Copy link
Collaborator Author

david-plugge commented Sep 28, 2022

@ak4zh

  1. auth() creates the endpoint for you
  2. Thats just a page the user gets redirected to after successfull login. But since it´s email+password login it´s actually not needed there. You can use it when signing in using oAuth or magic-link.

@maxkozlowski
Copy link

Thanks both!

One more question: I'm getting some issues when trying to do password reset. I use supabaseClient.auth.api.resetPasswordForEmail with user's email and redirect them upon clicking the link to create the new password.

Even though the user appears to be logged in, when I try to do

const { user, error } = await supabaseClient.auth.update({
			password: password,
		})

I get an error Error: Not logged in.. Has anyone successfully completed password reset and could help?

Thank you 🙏

@mitjakukovec
Copy link

mitjakukovec commented Sep 30, 2022

I followed all the steps on how to setup @supabase/auth-helpers-sveltekit.

Everything seems so trivial, but when I try to sign into my page using an authentication provider like google it will not work.

I am sure I am missing something here but I just can't figure it out.

On calling signing method like this

// +page.server.ts

const { user, session, error } = await supabase.auth.signIn({ provider: 'google' });

console.log('user:', user);
console.log('session:', session);
console.log('error:', error);

It will just return null for every property

Log output:

user: null
session: null
error: null

It is the same even if I use an authentication provider name, that is unsupported by Supabase.
How is it possible that it even doesn't return an error of some kind?

P. S.
On the Supabase page google is configured and enabled as an authentication provider.

Any ideas are more than welcome, Thanks!

@david-plugge
Copy link
Collaborator Author

@mitjakukovec you can´t get the session like that! signIn({ provider: 'google' }) returns { url: string }. You need to redirect the user to that url. After the user confirms the propmt by google he will be redirected back to your page with a token in the url hash. The supabase client will read that token and sync the session to the server.

So in case you are trying to achieve oauth login on the server, you simply can´t at the moment (maybe possible in the future).

Just call supabase.auth.signIn({ provider: 'google' }); on the client and you should be good to go. example

@mitjakukovec
Copy link

mitjakukovec commented Sep 30, 2022

Thank you @david-plugge, this is exactly what I was doing wrong! I was trying to log in on the server and then somehow sync the session back to the client and not vice versa. As you can see I am not a web developer by trade, but C++ guy with mainly embedded systems experience.

Does it make sense to do the signin call on the server and then redirect the client to the received URL?

Something like this

// +page.server.ts

export const actions: Actions = {
    async signin({ request }) {
        const data = await request.formData();
        const provider = data.get('provider') as Provider;

        const { url } = await supabase.auth.signIn({ provider });
        throw redirect(303, url as string);
    },

    async signout({ cookies }) {
        deleteSession(cookies);
        throw redirect(303, '/');
    }
};

@david-plugge
Copy link
Collaborator Author

Does it make sense to do the signin call on the server and then redirect the client to the received URL?

Not really, since you need client js anyway to retrieve the token.
For oAuth signin i´d suggest using the client only to keep it simple :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request sveltekit SvelteKit specific functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SvelteKit removed Session store