Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor form builder #929

Closed
aeneasr opened this issue Dec 21, 2020 · 1 comment
Closed

Refactor form builder #929

aeneasr opened this issue Dec 21, 2020 · 1 comment
Assignees
Labels
corp/m5 Up for M5 at Ory Corp. rfc A request for comments to discuss and share ideas.

Comments

@aeneasr
Copy link
Member

aeneasr commented Dec 21, 2020

Is your feature request related to a problem? Please describe.

Currently, the form builder for registration and settings works as follows. Several methods/strategies compose it's payload:

  • Settings
    • profile: perform identity traits updates
    • password: perform password updates
    • oidc: perform oidc updates
    • totp: perform totp updates
    • ...
  • Registration
    • password: create password and submit identity traits
    • oidc: perform oidc flow, then submit identity traits if some are missing
    • totp: create totp and submit identity traits?

The problems here are - this was also noted in one of the issues some time ago:

  1. It is not possible to both update your password and your traits in one go. Two form submissions are required.
  2. In the registration flow, password and oidc create / parse the identity traits - while in settings we have the "profile" strategy. The profile strategy is used nowhere else!
  3. It is not possible to set up TOTP, and a password in one go in the registration step. This makes enforced 2FA basically impossible without a lot of logic.

Describe the solution you'd like

There are several questions to be answered:

  1. Should we move the traits parsing and creation out of password, totp methods?
  2. Should we have one form action which combines all methods, allowing us to both update the profile and the password in one flow?
    • But: for OpenID Connect or any federated flow in general, we need to open a link, or redirect to a URL in order to initialize the flow. This means that, if we want to both set profile data (or TOTP) and perform OIDC sign up, we would need to POST the data to the registration endpoint, and then redirect to the OIDC endpoint. This is already the case, but it is possible because OIDC endpoint acts on its own (does not parse other methods). If we are to change that as suggested, this would mean that the OIDC handler must also be the response writer. This however can lead to conflict with other methods (let's say OAuth 3) which might also want to redirect or write the the response.
    • If we instead use a link, the problem would be that any filled out form fields would be ignored because we are not submitting the form. This might be ok for certain fields but probably not for fields like TOTP. However, this might be a sensible solution. If the sign up is completed, but some data is still missing (e.g. TOTP), we could show the registration form and remove the social sign in buttons and others (e.g. password) and just show the relevant ones (e.g. profile).

If everything is merged, this could be a potential flow method payload:

{
  "method": "POST",
  "action": "https://.../selfservice/registration",
  "messages": [
    {
      "text": "something went wrong",
    }
  ],
  "elements":[ // Root elements. For example a submit button or anti-csrf tokens
    {
      "element": "input",
      "attributes": {
        "name": "csrf_token",
        "type": "hidden",
        "required": true,
        "value": "0enxVb8sUD/B+a0OXTa+i0s/FZA9hrGU6Lx54UelNjFWH/3wDmHwDLMIdtZ2hrdx3NSex/hroMJ89xyQBIZ0Fw=="
      }
    }
  ],
  "methods": {
    "traits": {
      // This was formerly part of "password"
      "elements": [
        {
          "element": "input",
          "attributes": {
            "type": "text",
            "name": "traits.email",
            "value": "...."
          },
          "messages": [
            {
              "text": "must be set"
            }
          ]
        }
      ]
    },
    "password": {
      "elements": [
        {
          "element": "input",
          "attributes": {
            "type": "password",
            "name": "password",
            "value": "...."
          }
        }
      ]
    },
    "oidc": {
      "elements": [
        {
          "element": "anchor",
          // The "Sign in with Google" link
          "attributes": {
            "data-provider": "google",
            // Not sure about this one
            "href": "https://kratos/selfservice/registration/methods/oidc/google"
          }
        }
      ]
    },
    "lookup": {
      // look up secrets or "recovery codes"
      "elements": [
        {
          "element": "text",
          "inner": "1234"
        },
        {
          "element": "text",
          "inner": "4321"
        },
        {
          "element": "text",
          "inner": "3214"
        },
        {
          "element": "text",
          "inner": "2143"
        },
      ],
    },
    "totp": {
      "elements": [
        {
          "element": "image",
          // the QR code
          "attributes": {
            "src": "png(base64,...)"
            // as base64 encoded
          }
        },
        {
          "element": "text",
          "inner": "the-secret-key"
          // The secret key - not sure about the inner thing.
        }
      ]
    }
  }
}

For comparison, this is the current format:

{
    "id": "f74b1716-ac4d-4a35-abb9-4410811e0318",
    "type": "browser",
    "expires_at": "2020-12-21T11:24:16.094697Z",
    "issued_at": "2020-12-21T10:24:16.094697Z",
    "request_url": "http://demo.tenants.staging.oryapis.dev/self-service/registration/browser",
    "messages": null,
    "methods": {
        "password": {
            "method": "password",
            "config": {
                "action": "https://demo.tenants.staging.oryapis.dev/api/kratos/public/self-service/registration/methods/password?flow=f74b1716-ac4d-4a35-abb9-4410811e0318",
                "method": "POST",
                "fields": [
                    {
                        "name": "csrf_token",
                        "type": "hidden",
                        "required": true,
                        "value": "0enxVb8sUD/B+a0OXTa+i0s/FZA9hrGU6Lx54UelNjFWH/3wDmHwDLMIdtZ2hrdx3NSex/hroMJ89xyQBIZ0Fw=="
                    },
                    {
                        "name": "password",
                        "type": "password",
                        "required": true
                    },
                    {
                        "name": "traits.email",
                        "type": "email"
                    },
                    {
                        "name": "traits.website",
                        "type": "url"
                    },
                    {
                        "name": "traits.name.first",
                        "type": "text"
                    },
                    {
                        "name": "traits.name.last",
                        "type": "text"
                    }
                ]
            }
        }
    }
}

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've
considered.

Additional context

Add any other context or screenshots about the feature request here.

@aeneasr aeneasr added rfc A request for comments to discuss and share ideas. corp/m5 Up for M5 at Ory Corp. labels Dec 21, 2020
@aeneasr aeneasr added this to the v0.6.0-alpha.1 milestone Dec 21, 2020
@aeneasr aeneasr self-assigned this Dec 21, 2020
@aeneasr aeneasr pinned this issue Dec 21, 2020
@aeneasr
Copy link
Member Author

aeneasr commented Apr 27, 2021

This is resolved!

@aeneasr aeneasr closed this as completed Apr 27, 2021
@aeneasr aeneasr unpinned this issue Apr 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
corp/m5 Up for M5 at Ory Corp. rfc A request for comments to discuss and share ideas.
Projects
None yet
Development

No branches or pull requests

1 participant