Skip to content

Commit 9ecc2e1

Browse files
refactor: improve code style with consistent negative conditions
Update OIDC user role handling to use cleaner Go style: - Use negative conditions (loginType != codersdk.LoginTypeOIDC) for better readability - Simplify comments to be more concise and inline - Maintain all existing validation logic and functionality Co-authored-by: angrycub <[email protected]>
1 parent e7df6a1 commit 9ecc2e1

File tree

1 file changed

+23
-29
lines changed

1 file changed

+23
-29
lines changed

internal/provider/user_resource.go

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -214,15 +214,7 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
214214
data.Roles.ElementsAs(ctx, &roles, false)...,
215215
)
216216

217-
// OIDC users get their roles from the OIDC provider's role mapping
218-
if loginType == codersdk.LoginTypeOIDC {
219-
if len(roles) > 0 {
220-
resp.Diagnostics.AddError("Configuration Error", "Cannot set explicit roles for OIDC users. OIDC users get their roles from the OIDC provider's role mapping configuration.")
221-
return
222-
}
223-
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
224-
} else {
225-
// For non-OIDC users, set roles explicitly
217+
if loginType != codersdk.LoginTypeOIDC { // non-OIDC users get explicit roles
226218
tflog.Info(ctx, "updating user roles", map[string]any{
227219
"new_roles": roles,
228220
})
@@ -234,6 +226,13 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
234226
return
235227
}
236228
tflog.Info(ctx, "successfully updated user roles")
229+
} else {
230+
// OIDC users get roles from provider's role mapping
231+
if len(roles) > 0 {
232+
resp.Diagnostics.AddError("Configuration Error", "Cannot set explicit roles for OIDC users. OIDC users get their roles from the OIDC provider's role mapping configuration.")
233+
return
234+
}
235+
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
237236
}
238237

239238
if data.Suspended.ValueBool() {
@@ -278,21 +277,18 @@ func (r *UserResource) Read(ctx context.Context, req resource.ReadRequest, resp
278277
data.Email = types.StringValue(user.Email)
279278
data.Name = types.StringValue(user.Name)
280279
data.Username = types.StringValue(user.Username)
281-
282-
// For OIDC users, don't populate roles from server to avoid config drift
283-
// OIDC users get their roles from the OIDC provider's role mapping
284-
if user.LoginType == codersdk.LoginTypeOIDC {
285-
// Keep roles empty for OIDC users to match the expected Terraform config
286-
data.Roles = types.SetValueMust(types.StringType, []attr.Value{})
287-
} else {
288-
// For non-OIDC users, populate roles from server response
280+
281+
if user.LoginType != codersdk.LoginTypeOIDC { // populate roles from server for non-OIDC users
289282
roles := make([]attr.Value, 0, len(user.Roles))
290283
for _, role := range user.Roles {
291284
roles = append(roles, types.StringValue(role.Name))
292285
}
293286
data.Roles = types.SetValueMust(types.StringType, roles)
287+
} else {
288+
// OIDC users: keep roles empty to avoid config drift
289+
data.Roles = types.SetValueMust(types.StringType, []attr.Value{})
294290
}
295-
291+
296292
data.LoginType = types.StringValue(string(user.LoginType))
297293
data.Suspended = types.BoolValue(user.Status == codersdk.UserStatusSuspended)
298294

@@ -370,16 +366,8 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
370366
data.Roles.ElementsAs(ctx, &roles, false)...,
371367
)
372368

373-
// OIDC users get their roles from the OIDC provider's role mapping
374369
loginType := codersdk.LoginType(data.LoginType.ValueString())
375-
if loginType == codersdk.LoginTypeOIDC {
376-
if len(roles) > 0 {
377-
resp.Diagnostics.AddError("Configuration Error", "Cannot set explicit roles for OIDC users. OIDC users get their roles from the OIDC provider's role mapping configuration.")
378-
return
379-
}
380-
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
381-
} else {
382-
// For non-OIDC users, set roles explicitly
370+
if loginType != codersdk.LoginTypeOIDC { // non-OIDC users get explicit roles
383371
tflog.Info(ctx, "updating user roles", map[string]any{
384372
"new_roles": roles,
385373
})
@@ -391,6 +379,13 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
391379
return
392380
}
393381
tflog.Info(ctx, "successfully updated user roles")
382+
} else {
383+
// OIDC users get roles from provider's role mapping
384+
if len(roles) > 0 {
385+
resp.Diagnostics.AddError("Configuration Error", "Cannot set explicit roles for OIDC users. OIDC users get their roles from the OIDC provider's role mapping configuration.")
386+
return
387+
}
388+
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
394389
}
395390

396391
if data.LoginType.ValueString() == string(codersdk.LoginTypePassword) && !data.Password.IsNull() {
@@ -455,5 +450,4 @@ func (r *UserResource) ImportState(ctx context.Context, req resource.ImportState
455450
resp.Diagnostics.AddError("Client Error", "Invalid import ID format, expected a single UUID or a valid username")
456451
return
457452
}
458-
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), user.ID.String())...)
459-
}
453+
resp.Diagnostics.Append(resp.State.SetAttribute(ctx, path.Root("id"), user.ID.String())...)}

0 commit comments

Comments
 (0)