wasp-lang / wasp

The fastest way to develop full-stack web apps with React & Node.js.
https://wasp-lang.dev
MIT License
13.78k stars 1.18k forks source link

Revisit our `auth` middleware behaviour Queries, Actions and APIs #1133

Open infomiho opened 1 year ago

infomiho commented 1 year ago

We have multiple places where we have some sort of auth related logic. Some of the logic is maybe a bit unclear and we might want to extend the existing logic.

1️⃣ Current state

Backend

Users can use the auth middleware with three features (Actions, Queries, APIs) which can be configured in the Wasp file by passing in the auth option.

// main.wasp
query getTasks {
  auth: true // <---
  fn: import { getTasks } from "@server/queries.js",
  entities: [Task],
}

// queries.js
export const getTasks = async (_args, context) => {
  if (!context.user) {
    throw new HttpError(401)
  }
  // ...
}

The logic in the auth middleware is like this:

Feature Option Behaviour
Actions, Queries, APIs auth: true (default) Allow request. If there a JWT token on request, put user -> req..
Actions, Queries, APIs auth: false Allow request. Don't put user on req.

If there is not auth token on the request, request is still allowed.

Frontend

Users can put authRequired: true on their pages to make them inaccessible to public.

page ProfilePage {
  authRequired: true, // <---
  component: import { ProfilePage } from "@client/pages/ProfilePage.tsx"
}
Feature Default Meaning
Pages authRequired: true Page can't be opened by unauthenticated users.
Pages authRequired: false (default) Page can be opened by unauthenticated users.

2️⃣ Issues with the current state

Backend

Behaviour auth: true is not the a regular user would expect. Also, we didn't document it for Actions and Queries, but only for APIs.

Based on experiences with other frameworks and with our own implementation for Pages, people assume that putting auth: true means -> no access for unauthenticated users. But in our case, it just means extract user from the auth token. To be fair, that's correct if we look at the strict definition, we are doing "authentication" and identifying the source of the request. But people don't really think about that when they put auth: true and they want to protect a route.

Frontend

Users can use authRequired to control access to certain pages, which is all good. But they can't control per page the redirect URL. That might a thing we could improve on. But overall, the logic works as expected ✅

3️⃣ What we could do

Backend

We could do several things:

  1. Remove the auth option and just keep the default behaviour (extract user if auth token present)
  2. Introduce a new concept of authRequired on the backend
    • If authRequired: true then the route is returning 401 for unauthenticated users
    • This would simplify the implementation of authenticated routes
    • Users wouldn't need to writeif (!context.user) { throw new HttpError(401) } in most cases
  3. In the future, explore User roles and offer proper role based access (not in the scope for this issue)

Frontend

Explore giving more control to the users in terms of redirects for unauthenticated visits to protected pages (not in scope for this issue).

shayneczyzewski commented 1 year ago

We should also keep in mind how it should behave with WebSockets, once that feature is added 👍🏻

Martinsos commented 4 weeks ago

Use got confused with what auth: true does from it name, though it requires user to be authenticated: https://discord.com/channels/686873244791210014/1297094807688056895/1297094807688056895 .