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

Client-only guard() hook #1600

Closed
tszyan opened this issue Apr 5, 2024 · 12 comments
Closed

Client-only guard() hook #1600

tszyan opened this issue Apr 5, 2024 · 12 comments
Labels
enhancement ✨ New feature or request

Comments

@tszyan
Copy link

tszyan commented Apr 5, 2024

Description

It appears that the guard() hook is not functioning correctly when used with the following configuration:

  meta: {
    guard: {
      env: {
        server: false,
        client: true,
      }
    }
  }

No additional logic is implemented within the guard, just:

throw render(404);

Upon the initial page load or reloading with F5, the page loads without the guard hook being triggered.

It's the same behavior observed with Solid and React batijs scaffolds.

@tszyan tszyan added the bug 💥 label Apr 5, 2024
@brillout
Copy link
Member

brillout commented Apr 6, 2024

What is it you're trying to achieve?

It isn't clear to me what should happen with that configuration. Because the main purpose of the guard() hook is to protect unauthorized access, e.g. to protect an admin page from regular users. Given such use case calling guard() only the server-side is potentially a safety hazard, but maybe that's something we can warn the user about.

Closing but let's continue the conversation and let's see if the guard() can be improved.

@brillout brillout closed this as not planned Won't fix, can't repro, duplicate, stale Apr 6, 2024
@tszyan
Copy link
Author

tszyan commented Apr 6, 2024

In my case, guard() is necessary to display the authentication page if the user is not authenticated (no token or it has expired).

Utilized:

  • REST API (server-side not in JavaScript)
  • Authentication using access and refresh tokens.

No server-side JavaScript logic for data retrieval, checks, etc. is planned.

In guard(), I plan the following:

  1. Reading the token from the browser's local storage.
  2. Checking the token's expiration date (JWT token stores this information).
  3. Rendering "/auth" if any of the first two steps are not successfully passed.

If the token is valid, another layout and page components are used, within which requests to the REST API are made, and server-side checks are performed to decide which response to return - whether to send requested data, prompt for re-authentication, or something else.

@brillout
Copy link
Member

Correct me if I'm wrong, but I don't think what you want makes sense. Because the very first page the user visits triggers an SSR rendering and you'll need to decide whether to redirect the user already at that point. Thus you need to have your guard() hook be called on the server-side.

@brillout brillout changed the title guard() hook not working on client side Client-only guard() hook Apr 10, 2024
@brillout brillout added enhancement ✨ New feature or request and removed bug 💥 labels Apr 10, 2024
@tszyan
Copy link
Author

tszyan commented May 19, 2024

I apologize for not paying attention to the topic I initiated myself.

To further clarify, the issue is related to a SPA scenario, so ssr is set to false. In this context, I believe it makes perfect sense to handle the redirection on the client side.

@brillout
Copy link
Member

Indeed, that makes sense. Re-opening.

@brillout brillout reopened this May 20, 2024
@Blankeos
Copy link

Blankeos commented Jun 9, 2024

@tszyan Correct me if I'm wrong, but isn't handling redirection/guarding on a SPA not Vike's job, but rather the frontend framework (client code) you're using?

For me on SolidJS, I'm personally using a combination of:

  • a custom auth.context.ts to expose functions for login, register, logout, user, and loading.
  • a custom protected-route.ts
  • Vike's navigate function.

Here's some sample code:

@tszyan
Copy link
Author

tszyan commented Jun 11, 2024

Certainly, you're right that such functionality can be implemented independently on the client side using a frontend framework. However, considering that the guard() functionality exists for these purposes, albeit currently only on the server side, it would be logical to extend its implementation to the client side within Vike. Given Vike's support for both SSR and SPA, it would enhance consistency to have the ability to implement this functionality uniformly across both models.

@ignition42
Copy link

I agree, this could be very useful on the client as well, since routing can be done on the client it makes sense.

I had the same concern and yesterday I opened a discussion. I'm still reading through the documentation, doing some experiments and trying to understand how the framework works though.

@brillout
Copy link
Member

I can't see a concrete use case for this. What's your architecture that explains your wish to call guard() always and only on the client-side?

brillout added a commit that referenced this issue Jun 20, 2024
@brillout
Copy link
Member

https://vike.dev/guard now contains information about where and when guard() is called, and how to control this.

It may (or may not) address the issues of OP, let me know whether it does 👀

@tszyan
Copy link
Author

tszyan commented Jun 21, 2024

@brillout Thank you.
At the time of opening the issue, the biggest problem was that the documentation did not provide a clear understanding of the limitations of the guard() hook. I had to experimentally figure out what was what, including opening this issue. Currently, there are no such documentation problems, and possible solutions are even indicated. I believe this is a satisfactory solution for most developers.

However, I still see issues related to SPA both in the documentation and in how Vike works in this mode (according to the documentation, as I haven't personally checked yet). In particular, it is stated that both data() and guard() are executed on the server side during the initial page load (https://vike.dev/hooks#order), which is fundamentally incorrect for classic SPA applications where only static assets are hosted on the server.

Despite the popularity of SSR, the demand for SPA applications is still high. In order for SPA developers not to have to experiment with what works and how, and for the behavior of the Vike framework to be predictable in such scenarios, it is better to separate the description of configuration and how Vike works in different modes, especially SPA and SSR, in the documentation.

@brillout
Copy link
Member

@tszyan The docs were incorrect and I updated them: b675e81. I believe the use cases are covered by Vike's design, as well as now documented (atlhough there is room for improvement as you rightfully suggest). Let me know if you still believe something's missing from either the docs or Vike's design — supporting client-centric apps is very much a priority.

If someone's company is up for it we're looking for sponsors, as it makes a massiv difference to the project.

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

No branches or pull requests

4 participants