Skip to content

Commit

Permalink
fix: delete session on recovery
Browse files Browse the repository at this point in the history
  • Loading branch information
nsklikas committed Sep 20, 2024
1 parent 6099d68 commit 427b26e
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 1 deletion.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ At the moment the application is sourcing the following from the environment:
`log.txt`. **Make sure application user has permissions to write**.
- `PORT` - HTTP server port, defaults to `8080`
- `BASE_URL` - the base url that the application will be running on
- `KRATOS_SESSION_COOKIE_NAME` - the kratos session cookie name, defaults to `ory_kratos_session`
- `KRATOS_PUBLIC_URL` - address of Kratos Public APIs
- `KRATOS_ADMIN_URL` - address of Kratos Admin APIs
- `HYDRA_ADMIN_URL` - address of Hydra admin APIs
Expand Down
37 changes: 36 additions & 1 deletion pkg/kratos/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"net/url"
"path"
"strconv"
"time"

"github.com/go-chi/chi/v5"
client "github.com/ory/kratos-client-go"
Expand All @@ -17,6 +18,7 @@ import (
)

const RegenerateBackupCodesError = "regenerate_backup_codes"
const KRATOS_SESSION_COOKIE_NAME = "ory_kratos_session"

type API struct {
mfaEnabled bool
Expand Down Expand Up @@ -456,7 +458,12 @@ func (a *API) handleCreateRecoveryFlow(w http.ResponseWriter, r *http.Request) {
return
}

setCookies(w, cookies)
setCookiesWithExclude(w, cookies, []string{KRATOS_SESSION_COOKIE_NAME})
// We delete any active Kratos sessions. If there were any active Kratos sessions,
// recovery wouldn't be needed.
// See https://github.com/canonical/kratos-operator/issues/259 for more info.
a.deleteKratosSession(w)

w.WriteHeader(http.StatusOK)
w.Write(resp)
}
Expand Down Expand Up @@ -560,6 +567,22 @@ func (a *API) handleCreateSettingsFlow(w http.ResponseWriter, r *http.Request) {
w.Write(resp)
}

func (a *API) deleteKratosSession(w http.ResponseWriter) {
// To delete the session we delete the kratos session cookie.
// This is hacky as it does not call the Kratos API and is likely to break on
// a new Kratos version, but there is no easy way to delete the session
// from the Kratos API
c := &http.Cookie{
Name: KRATOS_SESSION_COOKIE_NAME,
Value: "",
Path: "/",
Expires: time.Unix(0, 0),
HttpOnly: true,
Secure: true,
}
http.SetCookie(w, c)
}

func NewAPI(service ServiceInterface, mfaEnabled bool, baseURL string, logger logging.LoggerInterface) *API {
a := new(API)

Expand All @@ -580,6 +603,18 @@ func NewAPI(service ServiceInterface, mfaEnabled bool, baseURL string, logger lo
return a
}

func setCookiesWithExclude(w http.ResponseWriter, cookies []*http.Cookie, exclude []string) {
l1:
for _, c := range cookies {
for _, n := range exclude {
if c.Name == n {
continue l1
}
}
http.SetCookie(w, c)
}
}

func setCookies(w http.ResponseWriter, cookies []*http.Cookie) {
for _, c := range cookies {
http.SetCookie(w, c)
Expand Down
55 changes: 55 additions & 0 deletions pkg/kratos/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"net/http/httptest"
"net/url"
"testing"
"time"

"github.com/go-chi/chi/v5"
gomock "go.uber.org/mock/gomock"
Expand Down Expand Up @@ -651,6 +652,60 @@ func TestHandleCreateRecoveryFlow(t *testing.T) {
}
}

func TestHandleCreateRecoveryFlowWithSession(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()

mockLogger := NewMockLoggerInterface(ctrl)
mockService := NewMockServiceInterface(ctrl)

redirect := "https://example.com/ui/reset_email"

req := httptest.NewRequest(http.MethodGet, HANDLE_CREATE_RECOVERY_FLOW_URL, nil)
values := req.URL.Query()
req.URL.RawQuery = values.Encode()

sessionCookie := &http.Cookie{
Name: KRATOS_SESSION_COOKIE_NAME,
Value: "some_value",
Path: "/",
HttpOnly: true,
Secure: true,
}
req.AddCookie(sessionCookie)

flow := kClient.NewRecoveryFlowWithDefaults()
mockService.EXPECT().CreateBrowserRecoveryFlow(gomock.Any(), redirect, req.Cookies()).Return(flow, req.Cookies(), nil)

w := httptest.NewRecorder()
mux := chi.NewMux()
NewAPI(mockService, false, BASE_URL, mockLogger).RegisterEndpoints(mux)

mux.ServeHTTP(w, req)

res := w.Result()

if _, err := json.Marshal(flow); err != nil {
t.Fatalf("Expected error to be nil got %v", err)
}
if res.StatusCode != http.StatusOK {
t.Fatal("Expected HTTP status code 200, got: ", res.Status)
}
deleted := false
for _, c := range res.Cookies() {
if c.Name == KRATOS_SESSION_COOKIE_NAME {
if c.Expires.Equal(time.Unix(0, 0)) {
deleted = true
} else {
t.Fatal("Kratos session cookie was set")
}
}
}
if !deleted {
t.Fatal("Kratos session cookie was not deleted")
}
}

func TestHandleCreateRecoveryFlowFailOnCreateBrowserRecoveryFlow(t *testing.T) {
ctrl := gomock.NewController(t)
defer ctrl.Finish()
Expand Down

0 comments on commit 427b26e

Please sign in to comment.