fix: add proper OIDC user role validation #247
+52
−24
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When creating OIDC users, the provider was calling UpdateUserRoles even with empty roles due to the default schema value, causing the server error "User Role Field is set in the OIDC configuration". OIDC users should get their roles exclusively from the OIDC provider's role mapping, not from explicit API calls. This fix: - Errors if explicit roles are provided for OIDC users - Skips role assignment entirely for OIDC users - Provides clear error messaging about OIDC role behavior 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
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.
Update the Read function to not populate roles from server response for OIDC users. This prevents Terraform from detecting config drift when OIDC users have roles assigned by the OIDC provider but an empty roles list in the Terraform config. Addresses review comment about config drift in PR #247. Co-authored-by: angrycub <464492+angrycub@users.noreply.github.com>
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 <464492+angrycub@users.noreply.github.com>
Fix formatting issues found by go fmt, specifically the closing brace placement in the ImportState function. Co-authored-by: angrycub <464492+angrycub@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When creating OIDC users, the provider was calling UpdateUserRoles even with empty roles due to the default schema value, causing the server error "User Role Field is set in the OIDC configuration".
OIDC users should get their roles exclusively from the OIDC provider's role mapping, not from explicit API calls. This fix:
🤖 Generated with Claude Code