Can not update an identity using PUT /identities/{id}

[landerss1]

Describe the bug
When using the identity schema below, I can not update an identity using the PUT /identities/{id} endpoint:

{
  "$id": "https://example.com/registration.schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Person",
  "type": "object",
  "properties": {
    "email": {
      "type": "string",
      "format": "email",
      "ory.sh/kratos": {
        "credentials": {
          "password": {
            "identifier": true
          }
        },
        "verification": {
          "via": "email"
        }
      }
    },
    "firstName": {
      "type": "string"
    },
    "lastName": {
      "type": "string"
    },
    "phoneNumber": {
      "type": "string"
    },
    "mobileNumber": {
      "type": "string"
    },
    "timeZone": {
      "type": "string"
    },
    "language": {
      "type": "string"
    }
  }
}

Reproducing the bug

To reproduce, first create an identity:

curl -v -X POST http://localhost:4434/identities -d "{\"traits_schema_id\":\"default\",\"traits\":{\"email\":\"[email protected]\",\"lastName\":\"Doe\",\"firstName\":\"John\"}}"
< HTTP/1.1 201 Created
< Content-Type: application/json; charset=utf-8
< Location: http://127.0.0.1:4434/identities/be0ab6e6-25b0-453a-af33-f6a2efbb2ea4
< Date: Wed, 27 May 2020 09:08:27 GMT
< Content-Length: 395
<
{"id":"be0ab6e6-25b0-453a-af33-f6a2efbb2ea4","traits_schema_id":"default","traits_schema_url":"http://127.0.0.1:4433/schemas/default","traits":{"email":"[email protected]","lastName":"Doe","firstName":"John"},"addresses":[{"id":"5b2806ea-658d-4c07-a161-b2f4efe4bce1","value":"[email protected]","verified":false,"via":"email","verified_at":null,"expires_at":"2020-05-28T09:08:27.084960997Z"}]}

Then, take the response body as request body and try to update the identity:

curl -v -X PUT http://localhost:4434/identities/be0ab6e6-25b0-453a-af33-f6a2efbb2ea4 -d "{\"id\":\"be0ab6e6-25b0-453a-af33-f6a2efbb2ea4\",\"traits_schema_id\":\"default\",\"traits_schema_url\":\"http://127.0.0.1:4433/schemas/default\",\"traits\":{\"email\":\"[email protected]\",\"lastName\":\"Doe\",\"firstName\":\"John\"},\"addresses\":[{\"id\":\"5b2806ea-658d-4c07-a161-b2f4efe4bce1\",\"value\":\"[email protected]\",\"verified\":false,\"via\":\"email\",\"verified_at\":null,\"expires_at\":\"2020-05-28T09:08:27.084960997Z\"}]}"
< HTTP/1.1 403 Forbidden
< Content-Type: application/json
< Date: Wed, 27 May 2020 09:17:36 GMT
< Content-Length: 363
<
{"error":{"code":403,"status":"Forbidden","reason":"A field was modified that updates one or more credentials-related settings. This action was blocked because an unprivileged method was used to execute the update. This is either a configuration issue or a bug and should be reported to the system administrator.","message":"The requested action was forbidden"}}

Server logs

time="2020-05-27T09:27:54Z" level=info msg="started handling request" method=PUT name="admin#http://127.0.0.1:4434" remote="127.0.0.1:56402" request=/identities/be0ab6e6-25b0-453a-af33-f6a2efbb2ea4
time="2020-05-27T09:27:54Z" level=error msg="An error occurred while handling a request" code=403 debug= details="map[]" error="The requested action was forbidden" reason="A field was modified that updates one or more credentials-related settings. This action was blocked because an unprivileged method was used to execute the update. This is either a configuration issue or a bug and should be reported to the system administrator." request-id= status=403 writer=JSON
time="2020-05-27T09:27:54Z" level=info msg="completed handling request" method=PUT name="admin#http://127.0.0.1:4434" remote="127.0.0.1:56402" request=/identities/be0ab6e6-25b0-453a-af33-f6a2efbb2ea4 status=403 text_status=Forbidden took=31.68649ms

Server configuration

courier:
      smtp:
        connection_uri: smtps://<user-password>@<host>:465?skip_ssl_verify=true
        from_address: <from>
    dsn: <dsn>
    hashers:
      argon2:
        iterations: 2
        key_length: 32
        memory: 1048576
        parallelism: 4
        salt_length: 16
    identity:
      traits:
        default_schema_url: file:///etc/config/default_identity_schema.json
        schemas:
        - id: other
          url: http://test.kratos.ory.sh/other-identity.schema.json
    log:
      format: text
      level: debug
    secrets:
      session:
      - <key-1>
      -<key-2>
    security:
      session:
        cookie:
          same_site: Lax
    selfservice:
      login:
        after:
          default_return_to: https://self-service/login/return_to
          oidc:
            hooks:
            - hook: revoke_active_sessions
          password:
            default_return_to: https://self-service/login/password/return_to
            hooks:
            - hook: revoke_active_sessions
        request_lifespan: 99m
      logout:
        redirect_to: http://127.0.0.1:4435/auth/login
      registration:
        after:
          default_return_to: https://self-service/registration/return_to
          password:
            hooks:
            - hook: session
            - hook: verify
        request_lifespan: 98m
      settings:
        after:
          default_return_to: https://self-service/settings/return_to
          password:
            default_return_to: https://self-service/settings/password/return_to
          profile:
            hooks:
            - hook: verify
        privileged_session_max_age: 5m
        request_lifespan: 99m
      strategies:
        oidc:
          config:
            providers:
            - client_id: a
              client_secret: b
              id: github
              mapper_url: http://test.kratos.ory.sh/default-identity.schema.json
              provider: github
          enabled: false
        password:
          enabled: true
      verify:
        link_lifespan: 24h
        request_lifespan: 1h
        return_to: https://self-service/verification/return_to
    serve:
      admin:
        host: 0.0.0.0
      public:
        host: 0.0.0.0
    urls:
      default_return_to: http://localhost:4000/login
      error_ui: <error-ui>
      login_ui: <login-ui>
      mfa_ui: http://127.0.0.1:4435/mfa
      registration_ui: http://127.0.0.1:4435/auth/registration
      self:
        admin: http://127.0.0.1:4434
        public: http://127.0.0.1:4433
      settings_ui: http://127.0.0.1:4435/settings
      verify_ui: http://127.0.0.1:4435/verify
      whitelisted_return_to_urls:
      - http://localhost:4000/login

Expected behavior

I expect it would be possible to update the identity. It should also be possible to update only the traits of the identity, without having to send the verifiable addresses.

Environment

• Ory Kratos: v0.3.0-alpha.1
• Database: Postgres 11
• Kubernetes 1.17.1 running in AWS

[aeneasr]

Yeah it's currently not supported to update identities in a way that changes their credentials, because we don't have the infrastructure to e.g. set a password using the Admin API. This is definitely a feature/bug.

You will probably succeed with your request if you don't change the email address.

[landerss1]

No, even with the email address left out, it fails:

curl -v -X PUT http://localhost:4434/identities/be0ab6e6-25b0-453a-af33-f6a2efbb2ea4 -d "{\"id\":\"be0ab6e6-25b0-453a-af33-f6a2efbb2ea4\",\"traits_schema_id\":\"default\",\"traits_schema_url\":\"http://127.0.0.1:4433/schemas/default\",\"traits\":{\"lastName\":\"Doe\",\"firstName\":\"John\"},\"addresses\":[{\"id\":\"5b2806ea-658d-4c07-a161-b2f4efe4bce1\",\"value\":\"[email protected]\",\"verified\":false,\"via\":\"email\",\"verified_at\":null,\"expires_at\":\"2020-05-28T09:08:27.084960997Z\"}]}"
< HTTP/1.1 403 Forbidden
< Content-Type: application/json
< Date: Wed, 27 May 2020 10:50:05 GMT
< Content-Length: 363
<
{"error":{"code":403,"status":"Forbidden","reason":"A field was modified that updates one or more credentials-related settings. This action was blocked because an unprivileged method was used to execute the update. This is either a configuration issue or a bug and should be reported to the system administrator.","message":"The requested action was forbidden"}}

[aeneasr]

You need to include the email, and keep it unchanged!

[landerss1]

Yes, that's what I did in the first place. I created the identity as [email protected], then updated the identity using the response from the POST operation, so I tried to update john.doe2@acme com without any changes whatsoever.

[aeneasr]

Ok, we definitely need to give the Admin API some love. Sorry about that! But we're addressing some other hard-pressing issues (account recovery) first and we don't have enough bandwidth to solve this simoultaneously. Unfortunately, this needs a lot of internal knowledge that isn't documented so it probably won't be easy to contribute, but you can try of course :)

[landerss1]

The Admin API definitely need some love. And your right, it's not that easy to understand the design and be able to contribute, but my analysis shows that the offending line is this one:

kratos/identity/credentials.go

Line 121 in 6da1bd6

if string(expect.Config) != string(actual.Config) {

I put a debug statement there and can see that we have:

expect={}, actual=

So, to me the design seems a bit off. Why is this comparison even happening. The request payload doesn't even accept credentials (?), so why are we comparing them in the first place? Furthermore, the Update method in manager.go also compares verifiable addresses. I don't understand why this is being compared either. Updating an identity should only look at the traits, and potentially the credentials, if there was a way to specify the credentials as part of the payload. The current implementation compares both credentials and verifiable addresses against what's in the database, and that doesn't make much sense?

So, what's needed is probably a bit of refactoring of the Admin API. First, the part of the identity that we are interested in updating are the traits, so the request payload should basically only require the id of the identity, the schema id and the traits. Everything else should be handled under the hood. Verifiable addresses seems like an internal construct, and will probably be of little interest to the client. Secondly, we need a way to specify the credentials as part of the update operation (we would need this for the POST operation as well).

For now, I can fix this by disabling a few line of code in the relevant places, but I'm afraid I don't know the code well enough, or understand the basic design well enough to be able to contribute a fix for this. But hopefully, I've saved you some time to analyze the problem.

[aeneasr]

Good analysis! So the problem with the Admin API is the following: An Admin should not be able to set the password for a user manually. Instead, a flow where the user, for example, receives an email with a link that activates their account is preferred. Google solved this pretty well also, where you get a temporary password that you have to change once logged in.

As you can see, it would be foolish to allow an Admin to just "set" the password. We do however need that functionality also, because it might need to be possible to "import" users. Here the question however is how we're going to deal with the different password hashing algorithms.

Unfortunately, the traits compose several credentials. This includes verified addresses, recovery addresses, and - amongst others - the login with password id (eg your email)�. Allowing an admin to make modifications for those would be pretty bad unless we have a good strategy to solve the above points.

What you're looking at is code that prevents someone from changing an identity's credential. We'll only be allowing that through the Admin API if the above questions have been answered.

What you're seeing however appears to be a bug! Because the comparison is made between a null JSON string and an empty object. To fix the bug, we would have to figure out why there's credentials in the first place - I suspect it's the password ID schema extension, which sees an email and adds it to the credentials. I think what we need to do to fix that is fetch the identity beforehand from the store (with credentials), then update the traits for that identity, run schema validation, and put it without credentials back to the store.

[jertel]

I ran into this today and came to similar conclusions. I noticed there's an UpdateTraits() method in manager.go that first copies the original identity into the updated identity and then sets the Traits over the top of the updated identity object. I thought perhaps there's another API call I should be using to make use of this method but I couldn't find anything actually invoking this UpdateTraits() method. Even if this is the way to go would it support email address changes? Since the email address is both a trait, and in the addresses[] array I wasn't sure if updating only the email address trait would suffice.