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

New "weakest MFA" property breaks user comparisons #51209

Open
zmb3 opened this issue Jan 17, 2025 · 0 comments
Open

New "weakest MFA" property breaks user comparisons #51209

zmb3 opened this issue Jan 17, 2025 · 0 comments
Assignees
Labels

Comments

@zmb3
Copy link
Collaborator

zmb3 commented Jan 17, 2025

I have an old Teleport local user that was likely created before we tracked whether the user has a password set and that I haven't logged in with in a long time.

I noticed that every time I log in to this old user account, I see an error in the logs which happens when we try to set the password state to "has a password".

Failed to set password state error:[
ERROR REPORT:
Original Error: *trace.CompareFailedError user zac did not match expected existing value

The compare and swap operation fails due to a mismatch in the new MfaWeakestDevice field.

  &types.UserV2{
        ... // 3 identical fields
        Metadata: {Name: "zac", Namespace: "default", ...},
        Spec:     {Roles: {"editor", "access", "auditor"}, Traits: {"aws_role_arns": nil, "db_names": nil, "db_users": nil, "kubernetes_groups": nil, ...}, CreatedBy: {Time: s"2022-05-29 17:47:02.403016 +0000 UTC", User: {Name: "b507c22c-685e-45bd-a796-7a5bcd63e3e6.zac"}}},
        Status: types.UserStatusV2{
                PasswordState:    s"PASSWORD_STATE_UNSPECIFIED",
-               MfaWeakestDevice: s"MFA_DEVICE_KIND_UNSET",
+               MfaWeakestDevice: s"MFA_DEVICE_KIND_UNSPECIFIED",
                ... // 3 ignored fields
        },
        ... // 3 ignored fields
  }

Note that the user object in backend storage has no status field at all (and therefore no MfaWeakestDevice setting):

{
  "kind": "user",
  "version": "v2",
  "metadata": {
    "name": "zac"
  },
  "spec": {
    "roles": [
      "editor",
      "access",
      "auditor"
    ],
    "traits": {
      "aws_role_arns": null,
      "db_names": null,
      "db_users": null,
      "kubernetes_groups": null,
      "kubernetes_users": null,
      "logins": null,
      "windows_logins": null
    },
    "status": {
      "is_locked": false,
      "locked_time": "0001-01-01T00:00:00Z",
      "lock_expires": "0001-01-01T00:00:00Z",
      "recovery_attempt_lock_expires": "0001-01-01T00:00:00Z"
    },
    "expires": "0001-01-01T00:00:00Z",
    "created_by": {
      "time": "2022-05-29T17:47:02.403016Z",
      "user": {
        "name": "b507c22c-685e-45bd-a796-7a5bcd63e3e6.zac"
      }
    }
  }
}

The comparison appears to fail because of new code added to GetUser that mutates the user object
instead of returning exactly what existed in storage:

if auth != nil {
// when reading with secrets, we can populate the data automatically.
user.SetWeakestDevice(getWeakestMFADeviceKind(auth.MFA))
}

As a side note, SetWeakestDevice is not a great name, as not all MFA methods are devices. SetWeakestMFA would have been a better choice.

@zmb3 zmb3 added the bug label Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants