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

Warn and log out user if deleted #182

Closed
olavfosse opened this issue Dec 28, 2023 · 5 comments
Closed

Warn and log out user if deleted #182

olavfosse opened this issue Dec 28, 2023 · 5 comments

Comments

@olavfosse
Copy link
Contributor

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@jacobobryant
Copy link
Owner

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants