ciscoheat / sveltekit-flash-message

Send temporary data after redirect, usually from endpoints. Works with both SSR and client.
https://www.npmjs.com/package/sveltekit-flash-message
MIT License
246 stars 5 forks source link

Flash message not appearing when expected #12

Closed Josh-Nicholson closed 9 months ago

Josh-Nicholson commented 1 year ago

I'm struggling to get the following process to work with flash messages

  1. User is on the /settings/security route and changes their password
  2. On success the user is redirected back to the login screen with the following code throw redirect('/login', { type: 'success', message: 'Successfully changed password, please enter new password to login'}, event);
  3. When the user gets to the login screen a flash message appears

The issue is the flash message never appears. However it does appear once the user has logged in and gone from '/login' to '/'

Any idea why this might be?

Here is my layout code

<script lang="ts">
    import '../theme.postcss';
    import '@skeletonlabs/skeleton/styles/skeleton.css';
    import '../app.postcss';
    import {
        Toast,
        toastStore,
        type ToastSettings,
    } from '@skeletonlabs/skeleton';
    import { page } from '$app/stores';
    import type { LayoutData } from './$types';
    import { initFlash } from 'sveltekit-flash-message/client';
    export let data: LayoutData;

    $: onLoginPage = $page.url.pathname === '/login';

    const flash = initFlash(page);
    function displayFlashToast() {
        if ($flash) {
            if ($flash.type == 'success') {
                const t: ToastSettings = {
                    message: `<p>${$flash.message}</p>`,
                    autohide: true,
                    timeout: 5000,
                    background: 'variant-ghost-success'
                };
                toastStore.trigger(t);
            } else {
                const t: ToastSettings = {
                    message: `<p>${$flash.message}</p>`,
                    autohide: true,
                    timeout: 5000,
                    background: 'variant-ghost-error'
                };
                toastStore.trigger(t);
            }
        }
    }

    flash.subscribe(($flash) => {
        if ($flash) displayFlashToast();
    });
</script>

{#if onLoginPage}
    <slot />
{:else}
    <!-- My menu layout -->
{/if}
ciscoheat commented 1 year ago

Does it work if you don't use the event-style toast message, but just displaying it on the page?

ciscoheat commented 1 year ago

Try the RC 2 now, and see if it's worknig better.

Josh-Nicholson commented 1 year ago

RC2 has stopped duplication.

For some reason I am still not seeing a flash message when the user is logged out after changing their password. RC2 has fixed the issue where the flash message would appear when the user logs back in with the new password. However now the flash message never appears.

Josh-Nicholson commented 1 year ago

I think I've narrowed the issue down to the use of an inner layout.

My folder looks like this: src/routes/ │ settings/ │ ├ details/ │ ├ preferences/ │ ├ changepassword/ │ ├ +layout.server.ts │ └ +layout.svelte ├ admin/ ├ +layout.server.ts └ +layout.svelte

So when I trigger the redirect with flash message from within the changepassword route it hits the inner layout.server.ts which does not have export const load = loadFlashMessage(async (event) => {} code.

My most parent layout does.

Does my inner layout need this as well?

ciscoheat commented 1 year ago

I'm not sure, if you're passing the parent layout data to the inner layout, I don't think it's needed.

winston0410 commented 1 year ago

I have the same issue with this one and https://github.com/ciscoheat/sveltekit-flash-message/issues/10, I guess something is off. I will try and reproduce this.

@Josh-Nicholson have you been able to fix this?

winston0410 commented 1 year ago

@ciscoheat I believe the issue is this one: https://stackoverflow.com/questions/74915712/sveltekit-cookies-set-in-form-action-not-working

And I am able to fix it with a patch like this:

diff --git a/node_modules/sveltekit-flash-message/dist/server.js b/node_modules/sveltekit-flash-message/dist/server.js
index abda090..283cf5d 100644
--- a/node_modules/sveltekit-flash-message/dist/server.js
+++ b/node_modules/sveltekit-flash-message/dist/server.js
@@ -84,10 +84,10 @@ function realRedirect(status, location, message, event) {
         return redir(status, location.toString());
     if (!event)
         throw new Error('RequestEvent is required for redirecting with flash message');
-    event.cookies.set(cookieName, JSON.stringify(message), { httpOnly, path, maxAge });
+    event.cookies.set(cookieName, JSON.stringify(message), { httpOnly, path, maxAge, secure: false });
     return redir(status, location.toString());
 }
 export function setFlash(message, event) {
     const cookies = 'cookies' in event ? event.cookies : event;
-    cookies.set(cookieName, JSON.stringify(message), { httpOnly, path, maxAge });
+    cookies.set(cookieName, JSON.stringify(message), { httpOnly, path, maxAge, secure: false });
 }

I was using Brave to run the application. Seems like this is something need to be addressed?

ciscoheat commented 1 year ago

I'm not sure about the security implication of forcing insecure cookies. Does it only apply when using localhost?

winston0410 commented 1 year ago

I'm not sure about the security implication of forcing insecure cookies. Does it only apply when using localhost?

I am not sure as well, and seems to be browser specific. If we are designing this package to be only used with SvelteKit, I think we can use import { dev } from '$app/environment';, and secure: dev ? false : true. I guess every website in production will be in https and the cookie can be secure

ciscoheat commented 1 year ago

Yes, that was what I was thinking as well. If the problem occurs only in dev/localhost, that is?

winston0410 commented 1 year ago

Yup agree that will solve the issue

ciscoheat commented 1 year ago

The SvelteKit default settings for the secure option is here. Can you check if it works with setting secure to:

dev && location.protocol === 'http:' ? false : true
winston0410 commented 1 year ago

@ciscoheat I guess it has to be url.hostname === 'localhost' || url.protocol === 'http:', or even just url.protocol === 'http:'. It is not unusual to use 0.0.0.0 instead of localhost for development, when the backend is in Docker/K8s and you need to send cookies in that domain

ciscoheat commented 1 year ago

I'm curious, since it didn't work for you, do you see any problem with the SvelteKit default value for secure, which may have caused you the problem?

url.hostname === 'localhost' && url.protocol === 'http:' ? false : true
winston0410 commented 1 year ago

I'm curious, since it didn't work for you, do you see any problem with the SvelteKit default value for secure, which may have caused you the problem?

url.hostname === 'localhost' && url.protocol === 'http:' ? false : true

Oh I didn't see any log for that, and I guess there will be no log on server if the cookie cannot be set? Anyway my issue is I am running sveltekit in http://0.0.0.0:8000, thats why it does not work.

But indeed if that is the default, maybe the change should be made upstream in Sveltekit. I thought the link was coming from this package previously and you tried to fix it. Sorry for the confusion for my replies

ciscoheat commented 1 year ago

Yes, I think it's better to make a SvelteKit issue for this, to avoid any problems if they change things in the future. Maybe even suggest that dev is a better check instead of the hostname.

ciscoheat commented 11 months ago

@winston0410 Did you have time to make a issue for SvelteKit about this?

winston0410 commented 11 months ago

nope sorry I havn't done that yet

ciscoheat commented 9 months ago

Closing this, as it's a SvelteKit issue.