nextjournal / garden-id

Authentication for application.garden
ISC License
1 stars 1 forks source link

Logout doesn't work #3

Open licht1stein opened 3 months ago

licht1stein commented 3 months ago

Hi,

Everything works in development, but when deployed logout doesn't seem to do anything, and neither does deleting all the cookies. Here's the app initialization:

(defmethod ig/init-key :sidework/server [_ {:keys [port] :as opts}]
  (println ::init :port port)
  (let [app (make-app opts)]
    (serv/run-server (-> app
                           (garden-id/wrap-auth)
                           (session/wrap-session {:store (cookie-store)})) (select-keys opts [:port]))))

And here's the deployed app: https://sidework.apps.garden

Please advise what am I doing wrong?

licht1stein commented 3 months ago

It seems to be related to CORS:

image
leahneukirchen commented 3 months ago

That link should be visited as a regular link, not using AJAX.

This is because you use hx-boost on the whole page.

leahneukirchen commented 3 months ago

But something else seems to be wrong, as I was logged in as @zampino when I opened that page the first time oO

licht1stein commented 3 months ago

Removing hx-boost fixed the CORS error, but didn't log me out.

licht1stein commented 3 months ago

But something else seems to be wrong, as I was logged in as @zampino when I opened that page the first time oO

That is seriously strange :)

licht1stein commented 3 months ago

A lot of things are happening when I click logout, but logging out is not one of them:

https://github.com/nextjournal/garden-id/assets/35418634/f565b170-a10e-41cd-ab09-adc9e5792fbe

jackrusher commented 3 months ago

In addition to the "wrong user" horror, I was unable to login with Apple creds at all

jackrusher commented 3 months ago

(Thanks for reporting, @licht1stein! This stuff is very early 😆)

licht1stein commented 3 months ago

@jackrusher I'm very glad to help!

leahneukirchen commented 3 months ago

Your app redirects to the login when there's not user in the session...

licht1stein commented 3 months ago

Sure, cause you need to be logged in to use it.

licht1stein commented 3 months ago

This is the middleware that does it:

(defn login-checker
  [handler]
  (fn [{:keys [app/db] :as request}]
    (if-let [user (garden-id/get-user request)]
      (handler (assoc request :app/user (users/ensure-user db {:user/email (:email user)
                                                               :user/name (:name user)})))
      (redirect garden-id/login-uri))))
leahneukirchen commented 3 months ago

But there is no login dialog if you are already logged in at application.garden, so it transparently logs you in again...

licht1stein commented 3 months ago

True, but I expected to see this:

image

licht1stein commented 3 months ago

Well, not the impersonate part. The enter your credentials part. So the user is logged out of my site, but not logged out of the OIDC, and that's why they get back in.

But if user pressed logout, they probably should be prompted to confirm logging back in?

leahneukirchen commented 3 months ago

You need to visit https://login.auth.application.garden/logout, then you need to reauthenticate to log in

licht1stein commented 3 months ago

You need to visit https://login.auth.application.garden/logout, then you need to reauthenticate to log in

So this should be the logout link instead of garden-id/logout-uri?

leahneukirchen commented 3 months ago

This will log out the user of every garden app, so imo no.

licht1stein commented 3 months ago

I mean, I can go around this limitation by adding a /login page to my app, that will have a big friendly login button. But I have a feeling that the entire flow is a bit non-intuitive at the moment.

In addition to non-intuitive part, every garden user goes around transmitting their personal data to every garden.app, even if they never wanted to use it. This doesn't feel right.

licht1stein commented 3 months ago

My expectation of a website is that the moment I press "Logout" I'm using it as an anonymous user. This isn't the case with garden-id, right?

For this app it's no problem — I wanted to make a public home page anyway, and I can redirect to the public homepage later. But I wouldn't be comfortable with putting this into a client's app.

leahneukirchen commented 3 months ago

If you press logout, there's no session anymore to identify you, so it's an anonymous user.

The thing is that you are logged in automatically if you redirect to the login endpoint.

We should probably make an interstitial there to at least ask if the user logs in to an app they have not used before. (C.f. how github does it.)

jackrusher commented 2 months ago

@leahneukirchen let's add that interstitial

zampino commented 2 months ago

Would it make things easier/different if we'd store the name of the app (from the container's env var) on the session after the successful OIDC callback, and make the garden-id/logged-in? check more than just the "global" user?

https://github.com/nextjournal/garden-id/blob/06b67b5cf77d4d0573975f6cbe9bb6d69cce60eb/src/nextjournal/garden_id.clj#L263