Missing error.id field when trying to initialize settings flow

[Quasarman]

Describe the bug

When wanting to init a settings flow which has the method password and there is currently no active user session I expect an error object. Based on the error object I want to display a custom UI. From the documentation I read that there are several values for the error.id which would come in handy for displaying the UI differently.

When I try it on my end I do get the correct error object when the init flow is called without the user having a valid session. But the error object does not contain the expected error id which makes it hard for me to differentiate between i.e., forbidden_return_to and no_active_session, thus making it unreliable to display the correct UI.

What is working however is when trying to complete a settings flow if a user is logged in (has a valid session cookie) and the privileged_session_max_age has expired I do get the correct error object with the error.id set to session_refresh_required as described in the documentation for "Complete Settings Flow" here: https://www.ory.sh/docs/reference/api/#operation/submitSelfServiceSettingsFlow

This makes me wonder, shouldn't the error id be present for any error object on any flow? Note I have not tested every flow I just noticed it while implementing the settings flow.

Reproducing the bug

Steps to reproduce the behavior:

Try to init the settings flow when no valid active session is present. Then look at the error object to see that the id property is not present.

Expected behavior

Error id should be present in any returned error object as described in the documentation, regardless of init or complete flows.

Environment

Docker v0.8.0.pre

Additional context

I could try to parse the according error messages but that just seems very hacky to do when working on auth related stuff...

[aeneasr]

Thank you, that does look like an oversight. If you'd like, the best way to fix this fast would be to create a PR :)

You can find an example of an error like the one from submitselfserviceflow here:

kratos/selfservice/flow/error.go

Lines 65 to 88 in 02c9e26

	type BrowserLocationChangeRequiredError struct {
	*herodot.DefaultError `json:"error"`
	// Since when the flow has expired
	RedirectBrowserTo string `json:"redirect_browser_to"`
	}
	func (e *BrowserLocationChangeRequiredError) EnhanceJSONError() interface{} {
	return e
	}
	func NewBrowserLocationChangeRequiredError(redirectTo string) *BrowserLocationChangeRequiredError {
	return &BrowserLocationChangeRequiredError{
	RedirectBrowserTo: redirectTo,
	DefaultError: &herodot.DefaultError{
	IDField: text.ErrIDSelfServiceBrowserLocationChangeRequiredError,
	CodeField: http.StatusUnprocessableEntity,
	StatusField: http.StatusText(http.StatusUnprocessableEntity),
	ReasonField: fmt.Sprintf("In order to complete this flow please redirect the browser to: %s", redirectTo),
	DebugField: "",
	ErrorField: "browser location change required",
	},
	}
	}

This function is very important, without it, the extra info will be stripped (if you have any extra info to be included such as e.g. redirect_browser_to).

kratos/selfservice/flow/error.go

Line 72 in 02c9e26

func (e *BrowserLocationChangeRequiredError) EnhanceJSONError() interface{} {

The error itself is set here:

kratos/selfservice/flow/settings/handler.go

Line 87 in 9dfde7d

h.d.Writer().WriteError(w, r, errors.WithStack(herodot.ErrForbidden.WithReason("Please include a valid session cookie or session token when calling this endpoint.")))

I would probably use this ID:

kratos/text/message_error.go

Line 15 in 02c9e26

ErrNoActiveSession = "session_inactive"

Hope this helps

[Quasarman]

Hey @aeneasr,

Sounds good I'm not to familiar with Go but will try to make this change. Thanks for the input!

[Quasarman]

Hello @aeneasr,

After looking at the examples you gave me above. Wouldn't the only change we have to to is simply extend this line:

kratos/selfservice/flow/settings/handler.go

Line 87 in 9dfde7d

h.d.Writer().WriteError(w, r, errors.WithStack(herodot.ErrForbidden.WithReason("Please include a valid session cookie or session token when calling this endpoint.")))

with this:

ErrForbidden.WithID(text.ErrNoActiveSession)

To get something similar like here:

kratos/selfservice/flow/settings/handler.go

Line 367 in 9dfde7d

return errors.WithStack(herodot.ErrForbidden.WithID(text.ErrIDInitiatedBySomeoneElse).WithReasonf("The request was made for another identity and has been blocked for security reasons."))

Final line 87 in settings/handlers.go with the correct ID field would look like this: (I don't understand the difference between WithReason and WithReasonf tho)

h.d.Writer().WriteError(w, r, errors.WithStack(herodot.ErrForbidden.WithID(text.ErrNoActiveSession).WithReasonf("Please include a valid session cookie or session token when calling this endpoint."))) 

That should be it, shouldn't it?

[aeneasr]

Yes, exactly! :)