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
nsklikas committed Sep 20, 2024
1 parent 6099d68 commit 6cf22f8
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 35 deletions.
1 change: 1 addition & 0 deletions README.md
View file
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
2 changes: 1 addition & 1 deletion cmd/serve.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func serve() {
panic("Invalid authorization model provided")
}

router := web.NewRouter(kClient, kAdminClient, hClient, authorizer, distFS, specs.MFAEnabled, specs.BaseURL, tracer, monitor, logger)
router := web.NewRouter(kClient, kAdminClient, hClient, authorizer, distFS, specs.MFAEnabled, specs.BaseURL, specs.KratosSessionCookieName, tracer, monitor, logger)

logger.Infof("Starting server on port %v", specs.Port)

Expand Down
2 changes: 2 additions & 0 deletions internal/config/specs.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ type EnvSpec struct {
KratosAdminURL string `envconfig:"kratos_admin_url"`
HydraAdminURL string `envconfig:"hydra_admin_url"`

KratosSessionCookieName string `envconfig:"kratos_session_cookie_name" default:"ory_kratos_session"`

ApiScheme string `envconfig:"openfga_api_scheme" default:""`
ApiHost string `envconfig:"openfga_api_host"`
ApiToken string `envconfig:"openfga_api_token"`
Expand Down
34 changes: 29 additions & 5 deletions pkg/kratos/handlers.go
View file
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 @@ -19,10 +20,11 @@ import (
const RegenerateBackupCodesError = "regenerate_backup_codes"

type API struct {
mfaEnabled bool
service ServiceInterface
baseURL string
contextPath string
mfaEnabled bool
service ServiceInterface
baseURL string
kratosSessionCookieName string
contextPath string

logger logging.LoggerInterface
}
Expand Down Expand Up @@ -457,6 +459,11 @@ func (a *API) handleCreateRecoveryFlow(w http.ResponseWriter, r *http.Request) {
}

setCookies(w, cookies)
// 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,12 +567,29 @@ func (a *API) handleCreateSettingsFlow(w http.ResponseWriter, r *http.Request) {
w.Write(resp)
}

func NewAPI(service ServiceInterface, mfaEnabled bool, baseURL string, logger logging.LoggerInterface) *API {
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: a.kratosSessionCookieName,
Value: "",
Path: "/",
Expires: time.Unix(0, 0),
HttpOnly: true,
Secure: true,
}
http.SetCookie(w, c)
}

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

a.mfaEnabled = mfaEnabled
a.service = service
a.baseURL = baseURL
a.kratosSessionCookieName = kratosSessionCookieName

fullBaseURL, err := url.Parse(baseURL)
if err != nil {
Expand Down
98 changes: 70 additions & 28 deletions pkg/kratos/handlers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import (

const (
BASE_URL = "https://example.com"
KRATOS_SESSION_COOKIE_NAME = "ory_kratos_session"
HANDLE_CREATE_FLOW_URL = BASE_URL + "/api/kratos/self-service/login/browser"
HANDLE_UPDATE_LOGIN_FLOW_URL = BASE_URL + "/api/kratos/self-service/login"
HANDLE_GET_LOGIN_FLOW_URL = BASE_URL + "/api/kratos/self-service/login/flows"
Expand Down Expand Up @@ -59,7 +60,7 @@ func TestHandleCreateFlowWithoutSession(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -108,7 +109,7 @@ func TestHandleCreateFlowWithoutSessionFailOnCreateBrowserLoginFlow(t *testing.T

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -146,7 +147,7 @@ func TestHandleCreateFlowWithoutSessionFailOnFilterProviders(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -183,7 +184,7 @@ func TestHandleCreateFlowWithoutSessionWhenNoProvidersAllowed(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -230,7 +231,7 @@ func TestHandleCreateFlowWithSession(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -273,7 +274,7 @@ func TestHandleCreateFlowWithSessionFailOnAcceptLoginRequest(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -304,7 +305,7 @@ func TestHandleGetLoginFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -348,7 +349,7 @@ func TestHandleGetLoginFlowFail(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -388,7 +389,7 @@ func TestHandleUpdateFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -436,7 +437,7 @@ func TestHandleUpdateFlowWhenProviderNotAllowed(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -472,7 +473,7 @@ func TestHandleUpdateFlowFailOnParseLoginFlowMethodBody(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -530,7 +531,7 @@ func TestHandleUpdateLoginFlowRedirectToRegenerateBackupCodes(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -571,7 +572,7 @@ func TestHandleUpdateFlowFailOnUpdateOIDCLoginFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -608,7 +609,7 @@ func TestHandleUpdateFlowFailOnCheckAllowedProvider(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -637,7 +638,48 @@ func TestHandleCreateRecoveryFlow(t *testing.T) {

w := httptest.NewRecorder()
mux := chi.NewMux()
NewAPI(mockService, false, BASE_URL, mockLogger).RegisterEndpoints(mux)
NewAPI(mockService, false, BASE_URL, KRATOS_SESSION_COOKIE_NAME, 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)
}
}

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, KRATOS_SESSION_COOKIE_NAME, mockLogger).RegisterEndpoints(mux)

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -669,7 +711,7 @@ func TestHandleCreateRecoveryFlowFailOnCreateBrowserRecoveryFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -701,7 +743,7 @@ func TestHandleGetRecoveryFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -745,7 +787,7 @@ func TestHandleGetRecoveryFlowFail(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -783,7 +825,7 @@ func TestHandleUpdateRecoveryFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -828,7 +870,7 @@ func TestHandleUpdateRecoveryFlowFailOnParseRecoveryFlowMethodBody(t *testing.T)

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -857,7 +899,7 @@ func TestHandleCreateSettingsFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -894,7 +936,7 @@ func TestHandleCreateSettingsFlowWithRedirect(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -926,7 +968,7 @@ func TestHandleCreateSettingsFlowFailOnCreateBrowserSettingsFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -958,7 +1000,7 @@ func TestHandleGetSettingsFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -1006,7 +1048,7 @@ func TestHandleGetSettingsFlowWithRedirect(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -1042,7 +1084,7 @@ func TestHandleGetSettingsFlowFail(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -1078,7 +1120,7 @@ func TestHandleUpdateSettingsFlow(t *testing.T) {

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

mux.ServeHTTP(w, req)

Expand Down Expand Up @@ -1120,7 +1162,7 @@ func TestHandleUpdateSettingsFlowFailOnParseSettingsFlowMethodBody(t *testing.T)

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

mux.ServeHTTP(w, req)

Expand Down
Loading

0 comments on commit 6cf22f8

Please sign in to comment.