jfrog/terraform-provider-artifactory

User with `internal_password_disabled = false` cannot be updated

HannesHerrmannAA opened this issue · 17 comments

Describe the bug

The artifact_user rescource with internal_password_disabled=false once created, cannot be updated for any of its fields.

Given a user was once created with the following resource:

resource "artifactory_user" "team_user" {
  name                       = "test"
  email                      = "test@test.local"
  admin                      = false
  profile_updatable          = true
  disable_ui_access          = false
  internal_password_disabled = false
  groups                     = []
}

When updating for example the group

resource "artifactory_user" "team_user" {
  name                       = "test"
  email                      = "test@test.local"
  admin                      = false
  profile_updatable          = true
  disable_ui_access          = false
  internal_password_disabled = false
  groups                     = ["readers"]
}

the apply fails with

│ Error: Unable to Update Resource
│ 
│   with artifactory_user.team_user,
│   on internal_users.tf line 33, in resource "artifactory_user" "team_user":
│   33: resource "artifactory_user" "team_user" {
│ 
│ An unexpected error occurred while updating the resource update request. Please report this issue to the provider developers.
│ 
│ Error: BAD_REQUEST - Cannot create a user without a password
|

Expected behavior
The user resource should be updated as expected. In this case the group should be changed without an error.

Used versions:
OpenTofu: 1.6.2
Artifactory Provider 10.4.3
Artifactory version: Cloud hosted (7.83.1)

Requirements for and issue
Perhaps the parameter should be omited internal_password_disabled when updating the user.

@HannesHerrmannAA Thanks for the report. I've added this to our sprint plan.

@alexhung
Thanks for the quick action. However, the fix introcues new issues for us:

We used the artifact_user resource to generate user accounts similar to a invite function.
We generated an account, and the corresponding user then set their own password with the password reset flow.

With this fix now we would override these passwords. In that sense this fix breaks earlier behaviour.
Couldn't you alternatively not update the password once set?

@HannesHerrmannAA In your use case, the artifactory_unmanaged_user resource may be better fit. This resource, implies by it's name, is for user whose password is not managed by Terraform. This auto generates the password during resource creation and the original purpose is to not save the auto-generated password in the state. The last update changes that so I can revert the change so artifactory_unmanaged_user should work for your use case.

@HannesHerrmannAA Actually I made a mistaken in my last PR (it's been a while since I worked on these resources). Both artifactory_user and artifactory_unmanaged_user were both designed to allow creation of users without specifying password. So for resource creation, the provider auto-generate a password. However, the new JFrog API requires the password field for update unless internal_password_disabled is set to true. See JFrog API doc. The previous Artifactory API allows omission of password field which enables the original resource design.

This means that if provider does not store the auto-generated password value in the state:

  1. The resource cannot be updated. JFrog API will fails because there is no password for the update process.
  2. Set internal_password_disabled to true to allow password to be not set but this is not possible unless you are using external authentication mechanism.

If provider stores the auto-generated password value, then:

  1. Resource can be updated but the auto-generated password will overwrite the user's current password on JFrog instance.

Given the constraint from JFrog API, I'm not sure there is a solution or workaround right now. I'll open an internal ticket with the Access team over the requirement of password field in the API, and see if we can replicate the old API behavior.

In the meantime, I'll revert the code and remove storing of password in the state from last PR.

Are you sure you need to set the password always in the API?

When
internal_password_disabled=false
and this field was true before, the ‘password” field is mandatory.

Isn't it only obligatory when we set internal_password_disabled=false? What happens if both internal_password_disabled=false and the password is ommitted if there are no updates for these fields?

I also issued an bug report some time ago here, but it did not have had any effect help https://jfrog.atlassian.net/browse/RTFACT-30415

@HannesHerrmannAA Yes I'm sure. I filed an internal ticket to the Access team and turns out it was already fixed in 7.83.1 which is released to Cloud version, but not yet in Self-Hosted.

So my team is running into this as well. Slightly different presentation of the issue, but it sounds like the same root cause.

Data

  • Artifactory Enterprise+ v7.77.18
  • Terraform 1.8.1 (also OpenTofu 1.6.2)
  • Artifactory Provider v10.6.1

Status

  • We provision "service accounts" with this provider (among other things).
  • Human users come in from Okta. We don't care about them in this case.
  • Service accounts (robot users), with the exception of of 4 of them, are all service accounts with modern Access Tokens.

In our organization, service accounts use access tokens exclusively, and no passwords are ever shared with users who leverage the service accounts. Also of note is that since Terraform manages these users, they are configured as artifactory_managed_user resources.

The comments I've left in my code are:

# …snip…

internal_password_disabled = true

# Random password; MUST be set as of Artifactory Provider v4.0.0. But we don't
# use it. As of Artifactory Provider v6.4.0, `artifactory_managed_user`
# requires a password while `artifactory_unmanaged_user` does not.
password = "${random_id.timestamp.keepers.ts}-${random_id.timestamp.hex}"

lifecycle {
  ignore_changes = [
    internal_password_disabled,
    password,
    # …snip…
  ]
}

Problem (and Actual behavior)

Today, our Terraform specifies the Artifactory Provider version range as ~> 10.0 (i.e., any v10.x release). We picked up v10.6.1 today, and received an error when trying to provision a new user.

Error: BAD_REQUEST - Cannot set a password to a user for user with disabled internal password ('internal_password_disabled'=true)

OK. Error message is new, but I recalled my comments in the code (above) about how we previously needed to set a password that we just wouldn't use.

So, we removed the password attribute and ran Terraform again. This time was an error about the password being required.

So we re-added the password = "" attribute with an empty string. This time was an error that the password needed to be at least 12 characters.

…which led me to realize we were hitting a bug somewhere. :)

Expected behavior

  1. We remove the password (since it's disabled anyway), and it works. Or…
  2. We set the password to an empty string (since it's disabled anyway), and it works.

Workaround

My team was able to workaround the issue by rolling the Artifactory Provider back to v10.1.2 and pinning that specific version.

But working-around a problem is not the same as solving the problem, so I wanted to report this unexpected behavior.

Also, we learned last week that although Amazon Linux 2 is supported until June 30, 2025, self-hosting Artifactory on Amazon Linux 2 is now deprecated (for all new Artifactory versions moving forward).

We cannot upgrade beyond Artifactory v7.77.x without also migrating from the CentOS-based Amazon Linux 2 → the Fedora-based Amazon Linux 2023.

This will take some time, as this will be the first of our systems to upgrade to Amazon Linux 2023 (we need to go through and do CIS security hardening of the AMI first), which means upgrading to the next-available Artifactory release is something we're hoping to avoid as a solution to this more immediate issue.

@skyzyx I think for your use case, you should switch to artifactory_unmanaged_user resource, once my PR is merged and reverts back to previous behavior.

This differs from artifactory_managed_user being password attribute is optional (it will auto generate one for you for creation if internal_password_disabled is false but you don't care of it anyway). It also does not store the password in TF state which means provider does not include this in update API requests and thus not trigger error in Artifactory.

One possible interim solution (for @HannesHerrmannAA use case) is to bump the Artifactory version check to 7.83.1 (leverage work from this PR) so that the provider uses old security API for versions prior to 7.83.1. I'll test it out today and see if that's a good approach.

@HannesHerrmannAA My testing shows my idea seems to work so I've included in #937.

Sorry, but I have to admit I'm really confused now.

My original bug report already used 7.83.1.
I just tested with the latest provider verison 10.6.2 and with an cloud instance using 7.83.3 and the terraform provider still cannot update the user.
Am I missing something? Do the same versions behave differently for some reason I'm not aware of?

@HannesHerrmannAA In 10.3.2, we've switched to using the new Access API to manage users. A bug in this new Access API (unable to omit 'password' field to not update it) causes the issue you originally reported.

These new API was introduced in Artifactory 7.49.3, and thus in provider 10.4.3 we added backward compatibility for customers who are using Artifactory 7.49.2 and earlier. This adds support for using both old Artifactory Security API and new Access API, based on Artifactory version.

So with this capability already in the provider, I was able to change the Artifactory version that determine with set of APIs to use to 7.83.1.

The original Access API bug is marked as fixed in 7.83.0 and should be released to Cloud version. I'll double check if the fix is released or not.

@HannesHerrmannAA The internal ticket is marked with 7.84.1 as the product version. I'll try to retest with this version and get back to you. If that's correct, I'll need to bump that version check in the resources to this version and release a patch.

I just tried it with the latest provider version 10.7.0 and artifactory version 7.84.3.
When we change the users group in an artifactory_user resource, the password is still overridden. Shouldn't this have been reverted by your change in #937 ?

@HannesHerrmannAA No, the fix is in #940 which is still in review.

@skyzyx I think for your use case, you should switch to artifactory_unmanaged_user resource, once my PR is merged and reverts back to previous behavior.

OK. That sounds fine. How do I migrate the state all of the accounts to the new resource type without deleting/recreating them in Artifactory? Is it just a terraform import by… username?

@skyzyx I think so. Something like these:

  1. Add new TF resources to configuration by duplicating the existing artifactory_managed_user resources with artifactory_unmanaged_user resource type
  2. Import users as these new resources into TF state
  3. Delete artifactory_managed_user resources from configuration
  4. Remove them from TF state