ory / kratos

The most scalable and customizable identity server on the market. Replace your Homegrown, Auth0, Okta, Firebase with better UX and DX. Has all the tablestakes: Passkeys, Social Sign In, Multi-Factor Auth, SMS, SAML, TOTP, and more. Written in Go, cloud native, headless, API-first. Available as a service on Ory Network and for self-hosters.
https://www.ory.sh/?utm_source=github&utm_medium=banner&utm_campaign=kratos
Apache License 2.0
11.2k stars 959 forks source link

Registration flow is not correctly updated with error context on failed password field validation or webhook errors #3560

Open rbnbr opened 1 year ago

rbnbr commented 1 year ago

Preflight checklist

Ory Network Project

No response

Describe the bug

The registration flow doesn't seem to be properly updated with the error context on failed password validation. While it is showing error messages like "the registration flow has expired .. ago", or "an account with this identifier already exists", the returned UI Messages of the Flow are empty if the password validation has failed. However, looking into the self-hosted kratos container logs, I can find the log message containing: [..] the password does not fulfill the password policy because: password length must be at least 8 characters but only got 3 [..].

EDIT: The same seems to happen for webhook errors. They are ignored in the error output and I cannot find them in the flow context.

Reproducing the bug

Run kratos in docker compose and perform the registration self-service flow with an invalid or already pwned password.

Captcha Webhook Handler on Mismatch:

w.WriteHeader(http.StatusBadRequest)
err = json.NewEncoder(w).Encode(WebHookResponse{Messages: []messages_{{
    InstancePtr: "#/traits/flow/transient_payload/captcha",
    Messages: []messages{{
        ID:   12121004,
        Text: "bad captcha or token",
        Type: "error",
    }}},
}})
return

with structs defined as:

type messages struct {
    ID      int             `json:"id"` // Unique numeric ID of the error that helps the frontend to interpret this message.
    Text    string          `json:"text"`
    Type    string          `json:"type"`
    Context json.RawMessage `json:"context,omitempty"`
}

type messages_ struct {
    InstancePtr string     `json:"instance_ptr"` // Points to the field that failed validation.
    Messages    []messages `json:"messages"`
}

type WebHookResponse struct {
    Messages []messages_ `json:"messages"`
}

Relevant log output

kratos  | time=2023-10-07T18:50:46Z level=info msg=Encountered self-service flow error. func=github.com/ory/kratos/selfservice/flow/registration.(*ErrorHandler).WriteFlowError file=/project/selfservice/flow/registration/error.go:92 audience=audit error=map[message:I[#/pass
word] S[] the password does not fulfill the password policy because: password length must be at least 8 characters but only got 3 stack_trace:
kratos  | github.com/ory/kratos/schema.NewPasswordPolicyViolationError
kratos  |       /project/schema/errors.go:87
[...]

identity_id=00000000-0000-0000-0000-000000000000 service_name=Ory Kratos service_version=v1.0.0
kratos  | time=2023-10-07T20:33:06Z level=info msg=Encountered self-service flow error. func=github.com/ory/kratos/selfservice/flow/registration.(*ErrorHandler).WriteFlowError file=/project/selfservice/flow/registration/error.go:92 audience=audit error=map[message:1 valida
tion errors occurred:
kratos  | (0) I[#/traits/flow/transient_payload/captcha] S[] a webhook target returned an error stack_trace:
kratos  | github.com/ory/kratos/schema.NewValidationListError
kratos  |       /project/schema/errors.go:274
kratos  | github.com/ory/kratos/selfservice/hook.parseWebhookResponse
kratos  |       /project/selfservice/hook/web_hook.go:485

Relevant configuration

version: v1.0.0

methods:
  password:
    enabled: true
    config:
      min_password_length: 12
      identifier_similarity_check_enabled: false    

(flows)
    registration:
      lifespan: 10m
      ui_url: http://127.0.0.1:8082/register
      after:
        password:
          hooks:
            - hook: web_hook
              config:
                url: http://host.docker.internal:8082/api/webhooks/create_account_verify_captcha_after_registration_webhook
                method: POST
                body: base64://ZnVuY3Rpb24oY3R4KSB7DQogIGNhcHRjaGE6IGN0eC5mbG93LnRyYW5zaWVudF9wYXlsb2FkLmNhcHRjaGEsDQogIGNhcHRjaGFUb2tlbjogY3R4LmZsb3cudHJhbnNpZW50X3BheWxvYWQuY2FwdGNoYV90b2tlbg0KfQ==
                response:
                  ignore: false
                  parse: true
                auth:
                  type: api_key
                  config:
                    name: Authorization
                    value: #apikey
                    in: header
            - hook: web_hook
              config:
                url: http://host.docker.internal:8082/api/webhooks/create_account_after_registration_webhook
                method: POST
                body: base64://ZnVuY3Rpb24oY3R4KSB7IHVzZXJfaWQ6IGN0eC5pZGVudGl0eS5pZCB9
                response:
                  ignore: false
                  parse: false
                auth:
                  type: api_key
                  config:
                    name: Authorization
                    value: #apikey
                    in: header
        default_browser_return_url: http://127.0.0.1:8082/profile

Version

v1.0.0

On which operating system are you observing this issue?

Windows

In which environment are you deploying?

Docker Compose

Additional Context

No response

rbnbr commented 1 year ago

I just noticed that when using the "Accept: application/json" header, the error messages are being returned in the response ui body. However, they are still not a part of the flows UI Nodes when retrieving the flow using the SDK and only exist in the json response.

If they were returned with the flow itself, we could render the error messages server-side while using only standard html form post action without requiring any custom Javascript logic (as I do for the messages that work as described above).

rbnbr commented 1 year ago

Okay, I did some debugging and as it turns out, the UI messages are actually returned and exist in the server response. However, when unmarshaling the response into the ResponseFlowObject, the UI nodes are all just nil. So the bug lies in the auto-generated SDK of the the kratos client where the response is unmarshaled.

aeneasr commented 1 year ago

Thank you for the report! There is indeed an issue in the Go SDK which is causing this and it is related to the openapi Go generator template having a serious bug in the discriminator type when "additionalProperties" is allowed. It's going to be a bit of work to fix that :(

rbnbr commented 1 year ago

I just checked and it looks like I can still read the original body of the response which is luckily returned together with the Flow Object. So I'll do the parsing of the UI attributes in the backend myself for now. Nevertheless, it would be obviously more convenient if it would just work out of the box ;)