[core] log out is not working

groegert commented

2021-05-20 17:06:42 +00:00

Member

session is not deleted on log out.

an additional session is beeing displayed when relogin and looking at settings
in backend there are 2 sessions after relogin (select * from session)

To reproduce

browser firefox
logged in as admin (default)
using the upper right button to log out
log in again
goto 'Admin->Einstellungen'
--> 2 sessions are shown
select * from user in backend database
--> 2 sessions are shown

session is not deleted on log out. * an additional session is beeing displayed when relogin and looking at settings * in backend there are 2 sessions after relogin (select * from session) To reproduce 1. browser firefox 2. logged in as admin (default) 3. using the upper right button to log out 4. log in again 5. goto 'Admin->Einstellungen' --> 2 sessions are shown 6. select * from user in backend database --> 2 sessions are shown

ferfissimo added the

🎒 backend

label 2021-05-20 20:56:53 +00:00

ferfissimo added this to the v2.0-beta milestone 2021-05-20 20:57:42 +00:00

groegert added the

📺 frontend

label 2021-05-21 07:40:32 +00:00

groegert commented

2021-05-21 08:25:44 +00:00

Author

Member

I think this should be fixed in frontend.
The call stores/index.ts->logout() 'api.delete('/auth/${token}')' should be done within a valid session. Otherwise everybody could delete sessions without beeing logged in.
So the sequence should be changed from:

this.$patch({
  session: undefined,
  user: undefined,
});
await api.delete(`/auth/${token}`);

to

await api.delete(`/auth/${token}`);
this.$patch({
  session: undefined,
  user: undefined,
});

I think this should be fixed in frontend. The call stores/index.ts->logout() 'api.delete('/auth/${token}')' should be done within a valid session. Otherwise everybody could delete sessions without beeing logged in. So the sequence should be changed from: ``` this.$patch({ session: undefined, user: undefined, }); await api.delete(`/auth/${token}`); ``` to ``` await api.delete(`/auth/${token}`); this.$patch({ session: undefined, user: undefined, }); ```

groegert commented

2021-05-21 08:29:34 +00:00

Author

Member

fixed with commit: f2b7f3a3b4

groegert closed this issue

2021-05-21 08:29:34 +00:00

groegert added the

🐞 bug

label 2021-05-21 08:30:39 +00:00

ferfissimo commented

2021-05-21 11:17:49 +00:00

Owner

I do not see how the changes should fix the problem.

The call stores/index.ts->logout() 'api.delete('/auth/${token}')' should be done within a valid session. Otherwise everybody could delete sessions without beeing logged in.

This is true, but should be checked on the backend not the frontend (check if valid session token is set and session belongs to user).

So the sequence should be changed from:

this.$patch({
  session: undefined,
  user: undefined,
});
await api.delete(`/auth/${token}`);

to

await api.delete(`/auth/${token}`);
this.$patch({
  session: undefined,
  user: undefined,
});

This was intentional, because if the session is expired the backend will emit a HTTP-401 on every API request, the frontend will handle this by logging out and go back to login page.
Now the logout will try to send a DELETE but gets a HTTP-401 which is intercepted and result in an other logout attempt, which leads to infinit logout requests.

So the idea was to first patch the session, as the token is "valid" when the function gets called" and then do the logout, because then every next logout attempt will get canceled (guard in line 77 )

But I think this might be not the ideal way of handling this, when we receive a 401 the token is outdated, so we rather should simply clear the stores and LocalStorage.

I do not see how the changes should fix the problem. > The call stores/index.ts->logout() 'api.delete('/auth/${token}')' should be done within a valid session. Otherwise everybody could delete sessions without beeing logged in. This is true, but should be checked on the backend not the frontend (check if valid session token is set and session belongs to user). > So the sequence should be changed from: > ``` > this.$patch({ > session: undefined, > user: undefined, > }); > await api.delete(`/auth/${token}`); > ``` > to > ``` > await api.delete(`/auth/${token}`); > this.$patch({ > session: undefined, > user: undefined, > }); > ``` This was intentional, because if the session is expired the backend will emit a `HTTP-401` on every API request, the frontend will handle this by logging out and go back to login page. Now the logout will try to send a `DELETE` but gets a `HTTP-401` which is intercepted and result in an other logout attempt, which leads to infinit logout requests. So the idea was to first patch the session, as the token is "valid" when the function gets called" and then do the logout, because then every next logout attempt will get canceled (guard in [line 77](https://flaschengeist.dev/Flaschengeist/flaschengeist-frontend/src/branch/develop/src/stores/index.ts#L77) ) But I think this might be not the ideal way of handling this, when we receive a 401 the token is outdated, so we rather should simply clear the stores and LocalStorage.

groegert commented

2021-05-24 09:33:34 +00:00

Author

Member

I see: store.logout does not only logout a valid session but does also handles an unauthorized session (HTTP-401), i.e. an expired session.
So solution would be to separate the 2 cases:

store.logout -- which logs out in backend
store.handleLoggedOut -- which cleans up to store to be in sync with backend

With this it is possible to have multiple (async) logouts. But these would just result in correctly cleaning up the store.

I see: `store.logout` does not only logout a valid session but does also handles an unauthorized session (`HTTP-401`), i.e. an expired session. So solution would be to separate the 2 cases: * `store.logout` -- which logs out in backend * `store.handleLoggedOut` -- which cleans up to store to be in sync with backend With this it is possible to have multiple (async) logouts. But these would just result in correctly cleaning up the store.

groegert reopened this issue

2021-05-24 09:33:35 +00:00

groegert commented

2021-05-24 17:37:00 +00:00

Author

Member

fixed with comit: 9940589d1a and 625ac55b0a

fixed with comit: [9940589d1a](https://flaschengeist.dev/Flaschengeist/flaschengeist-frontend/commit/9940589d1ac42f40f1761bd9b53f89ec4257f9ec) and [625ac55b0a](https://flaschengeist.dev/Flaschengeist/flaschengeist-frontend/commit/625ac55b0a2e88b18e008e2ced4974363c4eca4d)

groegert closed this issue

2021-05-24 17:47:13 +00:00

[core] log out is not working #7