Skip to content

Commit

Permalink
chore: cleanup unused code
Browse files Browse the repository at this point in the history
  • Loading branch information
Benehiko committed Jun 27, 2023
1 parent a2af146 commit 68d0539
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 29 deletions.
7 changes: 0 additions & 7 deletions selfservice/flow/login/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ func WithFormErrorMessage(messages []text.Message) FlowOption {
}
}

func WithAuthenticatorAssuranceLevel(auth identity.AuthenticatorAssuranceLevel) FlowOption {
return func(f *Flow) {
f.RequestedAAL = auth
}
}

func (h *Handler) NewLoginFlow(w http.ResponseWriter, r *http.Request, ft flow.Type, opts ...FlowOption) (*Flow, *session.Session, error) {
conf := h.d.Config()
f, err := NewFlow(conf, conf.SelfServiceFlowLoginRequestLifespan(r.Context()), h.d.GenerateCSRFToken(r), r, ft)
Expand Down Expand Up @@ -458,7 +452,6 @@ func (h *Handler) createBrowserLoginFlow(w http.ResponseWriter, r *http.Request,
}

rt, err := h.d.Hydra().AcceptLoginRequest(r.Context(), string(hydraLoginChallenge), sess.IdentityID.String(), sess.AMR)

if err != nil {
h.d.SelfServiceErrorManager().Forward(r.Context(), w, r, err)
return
Expand Down
15 changes: 4 additions & 11 deletions selfservice/flow/settings/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,7 @@ import (
"github.com/ory/kratos/x"
)

var (
ErrHookAbortFlow = errors.New("aborted settings hook execution")
)
var ErrHookAbortFlow = errors.New("aborted settings hook execution")

type (
errorHandlerDependencies interface {
Expand Down Expand Up @@ -85,7 +83,8 @@ func (e *FlowNeedsReAuth) EnhanceJSONError() interface{} {
func NewFlowNeedsReAuth() *FlowNeedsReAuth {
return &FlowNeedsReAuth{
DefaultError: herodot.ErrForbidden.WithID(text.ErrIDNeedsPrivilegedSession).
WithReasonf("The login session is too old and thus not allowed to update these fields. Please re-authenticate.")}
WithReasonf("The login session is too old and thus not allowed to update these fields. Please re-authenticate."),
}
}

func NewErrorHandler(d errorHandlerDependencies) *ErrorHandler {
Expand Down Expand Up @@ -166,13 +165,7 @@ func (s *ErrorHandler) WriteFlowError(
if shouldRespondWithJSON {
s.d.Writer().WriteError(w, r, aalErr)
} else {
if aalErr.RedirectTo == "" {
http.Redirect(w, r, urlx.CopyWithQuery(
urlx.AppendPaths(s.d.Config().SelfPublicURL(r.Context()), login.RouteInitBrowserFlow),
url.Values{"aal": {string(identity.AuthenticatorAssuranceLevel2)}}).String(), http.StatusSeeOther)
} else {
http.Redirect(w, r, aalErr.RedirectTo, http.StatusSeeOther)
}
http.Redirect(w, r, aalErr.RedirectTo, http.StatusSeeOther)
}
return
}
Expand Down
39 changes: 34 additions & 5 deletions selfservice/flow/settings/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func TestHandleError(t *testing.T) {
t.Cleanup(ts.Close)

_ = testhelpers.NewSettingsUIFlowEchoServer(t, reg)
errorTS := testhelpers.NewErrorTestServer(t, reg)
_ = testhelpers.NewErrorTestServer(t, reg)
loginTS := testhelpers.NewLoginUIFlowEchoServer(t, reg)

h := reg.SettingsFlowErrorHandler()
Expand All @@ -72,6 +72,10 @@ func TestHandleError(t *testing.T) {
h.WriteFlowError(w, r, flowMethod, settingsFlow, &id, flowError)
})

router.GET("/fake-redirect", func(w http.ResponseWriter, r *http.Request, p httprouter.Params) {
reg.LoginHandler().NewLoginFlow(w, r, flow.TypeBrowser)
})

reset := func() {
settingsFlow = nil
flowError = nil
Expand Down Expand Up @@ -318,17 +322,42 @@ func TestHandleError(t *testing.T) {

settingsFlow = newFlow(t, time.Minute, flow.TypeBrowser)
settingsFlow.IdentityID = id.ID
flowError = errors.WithStack(session.NewErrAALNotSatisfied(""))
flowError = errors.WithStack(session.NewErrAALNotSatisfied(conf.SelfServiceFlowLoginUI(ctx).String()))
flowMethod = settings.StrategyProfile

res, err := ts.Client().Get(ts.URL + "/error")
client := &http.Client{}
*client = *ts.Client()

// disable redirects
client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
return http.ErrUseLastResponse
}

res, err := client.Get(ts.URL + "/error")
require.NoError(t, err)

loc, err := res.Location()
require.NoError(t, err)
assert.Contains(t, res.Request.URL.String(), errorTS.URL)

// we should end up at the login URL since the NewALLNotSatisfied takes in a redirect to URL.
assert.Contains(t, loc.String(), loginTS.URL)

// test the JSON resoponse as well
request, err := http.NewRequest("GET", ts.URL+"/error", nil)
require.NoError(t, err)

request.Header.Add("Accept", "application/json")

res, err = client.Do(request)
require.NoError(t, err)

// the body should contain the reason for the error
body := x.MustReadAll(res.Body)
require.NoError(t, res.Body.Close())

require.NotEmpty(t, gjson.GetBytes(body, "error.reason").String(), "%s", body)
// We end up at the error endpoint with an aal2 error message because ts.client has no session.
assert.Equal(t, "You can not requested a higher AAL (AAL2/AAL3) without an active session.", gjson.GetBytes(body, "reason").String(), "%s", body)
assert.Equal(t, session.NewErrAALNotSatisfied("").Reason(), gjson.GetBytes(body, "error.reason").String(), "%s", body)
})

t.Run("case=session old error", func(t *testing.T) {
Expand Down
11 changes: 5 additions & 6 deletions selfservice/flow/settings/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package settings

import (
"net/http"
"net/url"
"time"

"github.com/julienschmidt/httprouter"
Expand Down Expand Up @@ -415,12 +416,10 @@ func (h *Handler) fetchFlow(w http.ResponseWriter, r *http.Request) error {
return errors.WithStack(herodot.ErrForbidden.WithID(text.ErrIDInitiatedBySomeoneElse).WithReasonf("The request was made for another identity and has been blocked for security reasons."))
}

// we cannot redirect back to the settings flow
// because it would just return a json response which is not what we want
requestURL := h.d.Config().SelfServiceFlowSettingsUI(r.Context())
query := requestURL.Query()
query.Set("flow", rid.String())
requestURL.RawQuery = query.Encode()
// we cannot redirect back to the request URL (/self-service/settings/flows?id=...) since it would just redirect
// to a page displaying raw JSON to the client (browser), which is not what we want.
// Let's rather carry over the flow ID as a query parameter and redirect to the settings UI URL.
requestURL := urlx.CopyWithQuery(h.d.Config().SelfServiceFlowSettingsUI(r.Context()), url.Values{"flow": {rid.String()}})
if err := h.d.SessionManager().DoesSessionSatisfy(r, sess, h.d.Config().SelfServiceSettingsRequiredAAL(r.Context()), session.WithRequestURL(requestURL.String())); err != nil {
return err
}
Expand Down

0 comments on commit 68d0539

Please sign in to comment.