jacobobryant / biff

A Clojure web framework for solo developers.
https://biffweb.com
MIT License
829 stars 40 forks source link

Warn and log out user if deleted #182

Closed olavfosse closed 8 months ago

olavfosse commented 8 months ago

In development I sometimes rm -r storage. Unfortunately this leaves the users in a pseudo-loggedin state. It would be preferable if the user was logged out if not found in the db.

Also, Happy New Year!

jacobobryant commented 8 months ago

Happy new year 🙂

You can do this in "app space" by modifying the default wrap-signed-in middleware to do a DB query:

(defn wrap-signed-in [handler]
  (fn [{:keys [session biff/db] :as ctx}
       user (some->> (:uid session) (biff/lookup db))]
    (if user
      (handler (assoc ctx :user user))
      {:status 303
       :headers {"location" "/signin?error=not-signed-in"}
       :session (dissoc session :uid)})))

As for whether this should be the default implementation: not sure. Assuming your app has a sign-out button somewhere easily accessible, then you can also just click on that after doing rm -r storage right? I believe that'll work in the default example app anyway--the page will still load and the sign-out button will be visible, you just won't see any values in the default crud form because there's no user document.

I think I'd prefer to leave it at that rather than modifying the default middleware to do a DB query on every (authenticated) request. Those who want this behavior can make the decision to do that if they want. As it is, Biff's default middleware stack doesn't do any DB queries (I'm pretty sure?) so I wouldn't be able to add this functionality without introducing a new query somewhere.

olavfosse commented 8 months ago

I hear what you're saying. I still think the better default is to hit the db on every request though.

I've come to use this middleware instead:

(defn wrap-require-user [handler]
  "Ensures the user is logged in and that :user is associated
  with (xt/entity uid). If your handler or middleware requires :user,
  use it explicitly."
  (fn [{:keys [session biff/db] :as ctx}]
    (if-not (some? (:uid session))
      {:status 303
       :headers {"location" "/signin?error=not-signed-in"}}
      (if-let [user (or (:user ctx)
                        (xt/entity db (:uid session)))]
        (handler (assoc ctx :user user))
        {:status 303
         :headers {"location" "/signin?error=not-signed-in"}}))))
olavfosse commented 8 months ago

Btw feel free to close :^), or I can pr if u like the middleware

jacobobryant commented 8 months ago

I hear what you're saying. I still think the better default is to hit the db on every request though.

I still think it's definitely better not to do that by default 😉 .

Do you want to stick this middleware in a gist or write up a short blog post or something though? I'd be happy to add a link for it in the content library.

olavfosse commented 8 months ago

I think it's fine that it lives here for now :)