Skip to content

Commit

Permalink
fix: remove session cookie on logout (#1587)
Browse files Browse the repository at this point in the history
Before, the logout endpoint would invalidate the session cookie, but not remove it. This was a regression introduced in 0.7.0. This patch resolves that issue.

Closes #1584
  • Loading branch information
aeneasr committed Jul 23, 2021
1 parent cc34996 commit cdb30bb
Show file tree
Hide file tree
Showing 3 changed files with 7 additions and 2 deletions.
2 changes: 1 addition & 1 deletion selfservice/flow/logout/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ func (h *Handler) submitLogout(w http.ResponseWriter, r *http.Request, ps httpro
return
}

if err := h.d.SessionPersister().RevokeSessionByToken(r.Context(), sess.Token); err != nil {
if err := h.d.SessionManager().PurgeFromRequest(r.Context(), w, r); err != nil {
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
}
Expand Down
5 changes: 5 additions & 0 deletions selfservice/flow/logout/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package logout_test
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/cookiejar"
"net/url"
Expand Down Expand Up @@ -107,9 +108,11 @@ func TestLogout(t *testing.T) {
t.Run("type=browser", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
originalCookies := hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))
require.Contains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

body, res := makeBrowserLogout(t, hc, logoutUrl)
assert.EqualValues(t, public.URL+"/session/browser/get", res.Request.URL.String())
require.NotContains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

assert.EqualValues(t, http.StatusUnauthorized, res.StatusCode, "%s", body)
assert.EqualValues(t, "No active session was found in this request.", gjson.GetBytes(body, "error.reason").String())
Expand All @@ -121,11 +124,13 @@ func TestLogout(t *testing.T) {
t.Run("type=ajax", func(t *testing.T) {
hc, logoutUrl := getLogoutUrl(t)
originalCookies := hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))
require.Contains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

body, res := testhelpers.HTTPRequestJSON(t, hc, "GET", logoutUrl, nil)
assert.EqualValues(t, logoutUrl, res.Request.URL.String())

assert.EqualValues(t, http.StatusNoContent, res.StatusCode, "%s", body)
require.NotContains(t, fmt.Sprintf("%v", hc.Jar.Cookies(urlx.ParseOrPanic(public.URL))), "ory_kratos_session")

// Logout means CSRF has also been changed.
ensurePrincipalChange(t, originalCookies)
Expand Down
2 changes: 1 addition & 1 deletion spec/api.json
Original file line number Diff line number Diff line change
Expand Up @@ -2144,7 +2144,7 @@
"name": "Apache 2.0"
},
"title": "Ory Kratos API",
"version": "v0.7.1-alpha.1"
"version": ""
},
"openapi": "3.0.3",
"paths": {
Expand Down

0 comments on commit cdb30bb

Please sign in to comment.