Skip to content

fix: add proper OIDC user role validation #247

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

fix: add proper OIDC user role validation #247

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d8d4d6f
fix: add proper OIDC user role validation
angrycub Aug 14, 2025
e7df6a1
fix: prevent config drift for OIDC users in Read function
blink-so[bot] Aug 15, 2025
9ecc2e1
refactor: improve code style with consistent negative conditions
blink-so[bot] Aug 15, 2025
2e40dab
style: run go fmt to fix formatting
blink-so[bot] Aug 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 43 additions & 20 deletions internal/provider/user_resource.go
View file
Open in desktop
Copy link
Member

@ethanndickson ethanndickson Aug 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need to update Read as well here, right? I don't have a Coder deployment w/ an IDP handy*, but I assume if you gave the user managed by Terraform roles via OIDC, Terraform would complain about config drift on every subsequent apply.

*For the same reason, we probably won't be able to have a test for this :( All our provider tests use a containerized coder, and adding a fake IDP for those tests sounds painful.

Original file line number Diff line number Diff line change
Expand Up @@ -213,17 +213,28 @@ func (r *UserResource) Create(ctx context.Context, req resource.CreateRequest, r
resp.Diagnostics.Append(
data.Roles.ElementsAs(ctx, &roles, false)...,
)
tflog.Info(ctx, "updating user roles", map[string]any{
"new_roles": roles,
})
user, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
Roles: roles,
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update newly created user roles, got error: %s", err))
return

// OIDC users get their roles from the OIDC provider's role mapping
if loginType == codersdk.LoginTypeOIDC {
if len(roles) > 0 {
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.")
return
}
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
} else {
// For non-OIDC users, set roles explicitly
tflog.Info(ctx, "updating user roles", map[string]any{
"new_roles": roles,
})
user, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
Roles: roles,
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update newly created user roles, got error: %s", err))
return
}
tflog.Info(ctx, "successfully updated user roles")
}
tflog.Info(ctx, "successfully updated user roles")

if data.Suspended.ValueBool() {
_, err = client.UpdateUserStatus(ctx, data.ID.ValueString(), codersdk.UserStatus("suspended"))
Expand Down Expand Up @@ -348,17 +359,29 @@ func (r *UserResource) Update(ctx context.Context, req resource.UpdateRequest, r
resp.Diagnostics.Append(
data.Roles.ElementsAs(ctx, &roles, false)...,
)
tflog.Info(ctx, "updating user roles", map[string]any{
"new_roles": roles,
})
_, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
Roles: roles,
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update user roles, got error: %s", err))
return

// OIDC users get their roles from the OIDC provider's role mapping
loginType := codersdk.LoginType(data.LoginType.ValueString())
if loginType == codersdk.LoginTypeOIDC {
if len(roles) > 0 {
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.")
return
}
tflog.Info(ctx, "skipping role assignment for OIDC user (roles come from OIDC provider)")
} else {
// For non-OIDC users, set roles explicitly
tflog.Info(ctx, "updating user roles", map[string]any{
"new_roles": roles,
})
_, err = client.UpdateUserRoles(ctx, user.ID.String(), codersdk.UpdateRoles{
Roles: roles,
})
if err != nil {
resp.Diagnostics.AddError("Client Error", fmt.Sprintf("Unable to update user roles, got error: %s", err))
return
}
tflog.Info(ctx, "successfully updated user roles")
}
tflog.Info(ctx, "successfully updated user roles")

if data.LoginType.ValueString() == string(codersdk.LoginTypePassword) && !data.Password.IsNull() {
tflog.Info(ctx, "updating password")
Expand Down
Loading