isAuthenticated in context for logout

[Shogoki]

Hi,

First of all thank you very much for making this. I just implemented that in our Beta Sveltekit app, and it works quite well :-)

For the logout part though, i had some struggle. (I know in the Readme it says it is not implemented yet, but looking at the code it seemed to me like it is)

Except for one thing. As i implemented it and clicking my logout Linkt i always got end-user already logged out, redirecting to... So i double checked the code and identified the following:

In the hooks File in getContextwe have:

  if (contextSession.user) {
            context.user = contextSession.user
            context.oidc = contextSession.oidc
        }

which is Okay, but in auth.js/Auth/handleLogout we have

const { isAuthenticated, oidc: cOidc} = context

which then leads to isAuthenticated always being false, as it is not in context.

To fix this, we should either add an isAuthenticated prop to the context inside the hooks (that´s what i did for now):

if (contextSession.user) {
      context.user = contextSession.user;
      context.oidc = contextSession.oidc;
      context.isAuthenticated = !!contextSession.user;
  }

or, what would probably the cleaner solution, change the code in auth.js to sth. like this.

const { user: isAuthenticated, oidc: cOidc} = context

What do you think about that?

[ChrisOgden]

Yes I did partially implement logout and forgot to update the README. I didn't feel it was as cleanly implemented. The usage of the Response context isn't great and feels very express-like.

I have already run into my own issues with where I handled things in hooks and have started move more of it into getContext instead of getSession. I will update the README with those changes once I finish them and test them.

As for the changes you are recommending I am not sure I am following correctly, could you link to lines either in the repo or create a PR with what you are suggesting? I do see that a few weeks ago I added context.isAuthenticated = true inside getContext but didn't update the README, that make be just what you mean.

As for putting the user into isAuthenticated, I prefer to keep isX style variables to be true/false instead of having to deal with truthy options. I also think since it is just a recommendation in that section it leaves things open for various styles of implementation.

[ChrisOgden]

I updated the readme based on changes in sveltekit and this discussion, please review and let me know if you feel this is still needed.