kratos icon indicating copy to clipboard operation
kratos copied to clipboard

feat: passkeys!

Open hperl opened this issue 1 year ago • 5 comments

BREAKING CHANGES: This feature enables two-step registration per default. Two-step registration is a significantly improved sign up flow and recommended when using more than one sign up methods. To disable two-step registration, set `selfservice.flows.registration.enable_legacy_flow` to `true`. This value defaults to `false`.

Related issue(s)

Checklist

  • [ ] I have read the contributing guidelines.
  • [ ] I have referenced an issue containing the design document if my change introduces a new feature.
  • [ ] I am following the contributing code guidelines.
  • [ ] I have read the security policy.
  • [ ] I confirm that this pull request does not address a security vulnerability. If this pull request addresses a security vulnerability, I confirm that I got the approval (please contact [email protected]) from the maintainers to push the changes.
  • [ ] I have added tests that prove my fix is effective or that my feature works.
  • [ ] I have added or changed the documentation.

Further Comments

hperl avatar Feb 08 '24 11:02 hperl

Codecov Report

Attention: Patch coverage is 74.36123% with 291 lines in your changes are missing coverage. Please review.

Project coverage is 78.23%. Comparing base (cfa3074) to head (316ecde). Report is 25 commits behind head on master.

Files Patch % Lines
...fservice/strategy/profile/two_step_registration.go 30.00% 66 Missing and 4 partials :warning:
selfservice/strategy/passkey/passkey_settings.go 68.83% 36 Missing and 31 partials :warning:
selfservice/strategy/passkey/passkey_login.go 73.79% 33 Missing and 27 partials :warning:
...lfservice/strategy/passkey/passkey_registration.go 73.45% 24 Missing and 19 partials :warning:
...rvice/strategy/passkey/passkey_schema_extension.go 62.22% 13 Missing and 4 partials :warning:
selfservice/strategy/webauthn/registration.go 73.91% 10 Missing and 2 partials :warning:
selfservice/strategy/code/strategy.go 10.00% 7 Missing and 2 partials :warning:
selfservice/hook/two_step_registration.go 77.77% 2 Missing and 2 partials :warning:
x/webauthnx/aaguid/aaguid.go 75.00% 2 Missing and 2 partials :warning:
selfservice/strategy/webauthn/login.go 85.00% 0 Missing and 3 partials :warning:
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3748      +/-   ##
==========================================
+ Coverage   78.05%   78.23%   +0.17%     
==========================================
  Files         355      355              
  Lines       24836    24912      +76     
==========================================
+ Hits        19386    19489     +103     
+ Misses       3962     3927      -35     
- Partials     1488     1496       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 08 '24 16:02 codecov[bot]

Unfortunately, the PR still suffers from various bugs when both passkey and webauthn strategy are enabled:

config

log:
  level: trace
  leak_sensitive_values: true
secrets:
  cookie:
    - PLEASE-CHANGE-ME-I-AM-VERY-INSECURE
selfservice:
  default_browser_return_url: http://localhost:4455/
  allowed_return_urls:
    - http://localhost:4455
    - http://localhost:19006/Callback
    - exp://example.com/Callback
    - https://www.ory.sh/
    - https://example.org/
    - https://www.example.org/
  methods:
    link:
      config:
        lifespan: 1h
    code:
      config:
        lifespan: 1h
    totp:
      enabled: true
      config:
        issuer: issuer.ory.sh
    lookup_secret:
      enabled: true
    webauthn:
      enabled: true
      config:
        passwordless: true
        rp:
          id: localhost
          origins:
            - http://localhost:4455
          display_name: Ory
    passkey:
      enabled: true
      config:
        rp:
          id: localhost
          origins:
            - http://localhost:4455
          display_name: Ory
  flows:
    settings:
      ui_url: http://localhost:4455/settings
      privileged_session_max_age: 5m
      required_aal: aal1
    logout:
      after:
        default_browser_return_url: http://localhost:4455/login
    registration:
      ui_url: http://localhost:4455/registration
      after:
        password:
          hooks:
            - hook: session
        passkey:
          hooks:
            - hook: web_hook
              config:
                body: base64://ZnVuY3Rpb24oY3R4KSBjdHg=
                url: https://webhook-target-gsmwn5ab4a-uc.a.run.app/documents/0.4nwpiet8hx5
                method: PUT
            - hook: session
    login:
      ui_url: http://localhost:4455/login
    error:
      ui_url: http://localhost:4455/error
    verification:
      ui_url: http://localhost:4455/verify
    recovery:
      ui_url: http://localhost:4455/recovery
serve:
  public:
    base_url: http://localhost:4455/
    cors:
      enabled: true
      allowed_origins:
        - http://localhost:3000
        - http://localhost:19006
      allowed_headers:
        - Authorization
        - Content-Type
        - X-Session-Token
  admin:
    base_url: http://kratos:4434/
hashers:
  algorithm: argon2
  argon2:
    memory: 1KB
    iterations: 1
    parallelism: 1
courier:
  smtp:
    connection_uri: smtps://test:test@localhost:1025/?skip_ssl_verify=true
session:
  whoami:
    required_aal: aal1
identity:
  schemas:
    - id: default
      url: file://test/e2e/profiles/passkey/identity.traits.schema.json

traits.json

{
  "$id": "https://schemas.ory.sh/presets/kratos/quickstart/email-password/identity.schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Person",
  "type": "object",
  "properties": {
    "traits": {
      "type": "object",
      "properties": {
        "email": {
          "type": "string",
          "format": "email",
          "title": "Your E-Mail",
          "minLength": 3,
          "ory.sh/kratos": {
            "credentials": {
              "password": {
                "identifier": true
              },
              "webauthn": {
                "identifier": true
              },
              "passkey": {
                "display_name": true
              }
            }
          }
        },
        "website": {
          "title": "Your website",
          "type": "string",
          "format": "uri",
          "minLength": 10
        }
      },
      "required": ["email", "website"],
      "additionalProperties": false
    }
  }
}

Unable to sign in using WebAuthn passwordless strategy

https://github.com/ory/kratos/assets/3372410/d70a24b9-7212-4e68-81e6-129143329e41

Neither hardware keys nor passkeys work when I click "Use security key".

Settings page shows duplicate entries

"Add security key" is shown twice, and the hardware key is missing from the list:

Screenshot 2024-02-12 at 12 46 25

Hardware key is hidden from view when it is the only method

Only when I add two hardware keys, the keys are actually shown:

Screenshot 2024-02-12 at 12 49 48

Instead, I would expect to see the key with a disabled remove button.

Left to be done

Can you please add e2e test cases for these and other cases? In my view these bugs are there because the following test suites are missing:

  • Migration scenarios where both PassKey and WebAuthn + Passwordless are enabled
  • Using the keys interchangeably (use WebAuthn keys to sign in using PassKey strategy) during login
  • Changing settings while both strategies are enabled (add, delete)

Documentation and migration guide

Side note: This is what it looks like when you sign in using WebAuthn + Passwordless with a hardware key, disable WebAuthn and just have Passkey enabled, and then try to sign in using the hardware key:

Screenshot 2024-02-12 at 12 53 19

I think we should document this limitation somewhere.

aeneasr avatar Feb 12 '24 11:02 aeneasr

For two-step registration, there seems to be an issue:

Screenshot 2024-02-12 at 12 59 18 Screenshot 2024-02-12 at 12 59 29

For two-step registration I would also appreciate to see a couple more use cases where several different sign up methods are used to ensure that all of them work properly (including scenarios where the form doesn't validate).

aeneasr avatar Feb 12 '24 12:02 aeneasr

To do:

  1. Submit button is on the top of the UI nodes list (should be last item) for API and browser flows
  2. Social sign in needs to be an option on the first step, not the second one:
    • Add test case for missing traits
  3. Do not use 422 but instead 400 for API flows and Browser + JSON flows
  4. Add tests for mobile app
  5. Back button for sign up with code

aeneasr avatar Feb 22 '24 10:02 aeneasr

I unfortunately still receive an error when both passkeys + webauthn with passwordless are enabled and I try to sign up using webauthn:

Screenshot 2024-02-22 at 13 44 40

aeneasr avatar Feb 22 '24 12:02 aeneasr

Hi there,

First time Ory-er (Kratos-er?) here, thanks for building great software! I think some of the docs need to be updated (example), because they show the createRegistrationFlow response containing a password field, and I could not figure out for the life of me why I wasn't getting a password field in my response with the default identity schema.

After enabling selfservice.flows.registration.enable_legacy_one_step, the password field showed up in the response.

bcspragu avatar Jul 21 '24 13:07 bcspragu