Skip to content

Commit 0a7ce23

Browse files
ethanndicksonethanndickson
authored
fix: validate password logintype combos (#56)
1 parent 9aa27ba commit 0a7ce23

File tree

2 files changed

+56
-32
lines changed

2 files changed

+56
-32
lines changed

‎internal/provider/user_resource.go

+33-29
Original file line numberDiff line numberDiff line change
@@ -66,27 +66,23 @@ func (r *UserResource) Schema(ctx context.Context, req resource.SchemaRequest, r
6666
stringplanmodifier.UseStateForUnknown(),
6767
},
6868
},
69-
7069
"username": schema.StringAttribute{
7170
MarkdownDescription: "Username of the user.",
7271
Required: true,
7372
},
7473
"name": schema.StringAttribute{
75-
Computed: true,
7674
MarkdownDescription: "Display name of the user. Defaults to username.",
77-
Required: false,
75+
Computed: true,
7876
Optional: true,
79-
// Defaulted in Create
8077
},
8178
"email": schema.StringAttribute{
8279
MarkdownDescription: "Email address of the user.",
8380
Required: true,
8481
},
8582
"roles": schema.SetAttribute{
8683
MarkdownDescription: "Roles assigned to the user. Valid roles are 'owner', 'template-admin', 'user-admin', and 'auditor'.",
87-
Required: false,
88-
Optional: true,
8984
Computed: true,
85+
Optional: true,
9086
ElementType: types.StringType,
9187
Validators: []validator.Set{
9288
setvalidator.ValueStringsAre(
@@ -97,24 +93,24 @@ func (r *UserResource) Schema(ctx context.Context, req resource.SchemaRequest, r
9793
},
9894
"login_type": schema.StringAttribute{
9995
MarkdownDescription: "Type of login for the user. Valid types are 'none', 'password', 'github', and 'oidc'.",
100-
Required: false,
101-
Optional: true,
10296
Computed: true,
97+
Optional: true,
10398
Validators: []validator.String{
10499
stringvalidator.OneOf("none", "password", "github", "oidc"),
105100
},
106101
Default: stringdefault.StaticString("none"),
102+
PlanModifiers: []planmodifier.String{
103+
stringplanmodifier.RequiresReplaceIfConfigured(),
104+
},
107105
},
108106
"password": schema.StringAttribute{
109107
MarkdownDescription: "Password for the user. Required when login_type is 'password'. Passwords are saved into the state as plain text and should only be used for testing purposes.",
110-
Required: false,
111108
Optional: true,
112109
Sensitive: true,
113110
},
114111
"suspended": schema.BoolAttribute{
115-
Computed: true,
116112
MarkdownDescription: "Whether the user is suspended.",
117-
Required: false,
113+
Computed: true,
118114
Optional: true,
119115
Default: booldefault.StaticBool(false),
120116
},
@@ -164,14 +160,15 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
164160
}
165161

166162
tflog.Trace(ctx, "creating user")
167-
loginType := codersdk.LoginTypeNone
168-
if data.LoginType.ValueString() != "" {
169-
loginType = codersdk.LoginType(data.LoginType.ValueString())
170-
}
171-
if loginType == codersdk.LoginTypePassword && data.Password.ValueString() == "" {
163+
loginType := codersdk.LoginType(data.LoginType.ValueString())
164+
if loginType == codersdk.LoginTypePassword && data.Password.IsNull() {
172165
resp.Diagnostics.AddError("Data Error", "Password is required when login_type is 'password'")
173166
return
174167
}
168+
if loginType != codersdk.LoginTypePassword && !data.Password.IsNull() {
169+
resp.Diagnostics.AddError("Data Error", "Password is only allowed when login_type is 'password'")
170+
return
171+
}
175172
user, err := client.CreateUser(ctx, codersdk.CreateUserRequest{
176173
Email: data.Email.ValueString(),
177174
Username: data.Username.ValueString(),
@@ -189,13 +186,13 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
189186
data.ID = UUIDValue(user.ID)
190187

191188
tflog.Trace(ctx, "updating user profile")
192-
name := data.Username.ValueString()
189+
name := data.Username
193190
if data.Name.ValueString() != "" {
194-
name = data.Name.ValueString()
191+
name = data.Name
195192
}
196193
user, err = client.UpdateUserProfile(ctx, user.ID.String(), codersdk.UpdateUserProfileRequest{
197194
Username: data.Username.ValueString(),
198-
Name: name,
195+
Name: name.ValueString(),
199196
})
200197
if err != nil {
201198
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update newly created user profile, got error: %s", err))
@@ -290,18 +287,23 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
290287
return
291288
}
292289

290+
name := data.Username
291+
if data.Name.ValueString() != "" {
292+
name = data.Name
293+
}
293294
tflog.Trace(ctx, "updating user", map[string]any{
294295
"new_username": data.Username.ValueString(),
295-
"new_name": data.Name.ValueString(),
296+
"new_name": name.ValueString(),
296297
})
297298
_, err = client.UpdateUserProfile(ctx, user.ID.String(), codersdk.UpdateUserProfileRequest{
298299
Username: data.Username.ValueString(),
299-
Name: data.Name.ValueString(),
300+
Name: name.ValueString(),
300301
})
301302
if err != nil {
302303
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update user profile, got error: %s", err))
303304
return
304305
}
306+
data.Name = name
305307
tflog.Trace(ctx, "successfully updated user profile")
306308

307309
var roles []string
@@ -320,15 +322,17 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
320322
}
321323
tflog.Trace(ctx, "successfully updated user roles")
322324

323-
tflog.Trace(ctx, "updating password")
324-
err = client.UpdateUserPassword(ctx, user.ID.String(), codersdk.UpdateUserPasswordRequest{
325-
Password: data.Password.ValueString(),
326-
})
327-
if err != nil && !strings.Contains(err.Error(), "New password cannot match old password.") {
328-
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update password, got error: %s", err))
329-
return
325+
if data.LoginType.ValueString() == string(codersdk.LoginTypePassword) && !data.Password.IsNull() {
326+
tflog.Trace(ctx, "updating password")
327+
err = client.UpdateUserPassword(ctx, user.ID.String(), codersdk.UpdateUserPasswordRequest{
328+
Password: data.Password.ValueString(),
329+
})
330+
if err != nil && !strings.Contains(err.Error(), "New password cannot match old password.") {
331+
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update password, got error: %s", err))
332+
return
333+
}
334+
tflog.Trace(ctx, "successfully updated password")
330335
}
331-
tflog.Trace(ctx, "successfully updated password")
332336

333337
var statusErr error
334338
if data.Suspended.ValueBool() {

‎internal/provider/user_resource_test.go

+23-3
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,16 @@ func TestAccUserResource(t *testing.T) {
3232

3333
cfg2 := cfg1
3434
cfg2.Username = PtrTo("exampleNew")
35-
cfg2.Name = PtrTo("Example User New")
35+
36+
cfg3 := cfg2
37+
cfg3.Name = PtrTo("Example New")
38+
39+
cfg4 := cfg3
40+
cfg4.LoginType = PtrTo("github")
41+
cfg4.Password = nil
3642

3743
resource.Test(t, resource.TestCase{
44+
IsUnitTest: true,
3845
PreCheck: func() { testAccPreCheck(t) },
3946
ProtoV6ProviderFactories: testAccProtoV6ProviderFactories,
4047
Steps: []resource.TestStep{
@@ -66,10 +73,23 @@ func TestAccUserResource(t *testing.T) {
6673
Config: cfg2.String(t),
6774
Check: resource.ComposeAggregateTestCheckFunc(
6875
resource.TestCheckResourceAttr("coderd_user.test", "username", "exampleNew"),
69-
resource.TestCheckResourceAttr("coderd_user.test", "name", "Example User New"),
76+
resource.TestCheckResourceAttr("coderd_user.test", "name", "Example User"),
77+
),
78+
},
79+
{
80+
Config: cfg3.String(t),
81+
Check: resource.ComposeAggregateTestCheckFunc(
82+
resource.TestCheckResourceAttr("coderd_user.test", "username", "exampleNew"),
83+
resource.TestCheckResourceAttr("coderd_user.test", "name", "Example New"),
84+
),
85+
},
86+
// Replace triggered
87+
{
88+
Config: cfg4.String(t),
89+
Check: resource.ComposeAggregateTestCheckFunc(
90+
resource.TestCheckResourceAttr("coderd_user.test", "login_type", "github"),
7091
),
7192
},
72-
// Delete testing automatically occurs in TestCase
7393
},
7494
})
7595
}

0 commit comments

Comments
 (0)