Skip to content

fix: conceal sensitive domain information in auth error messages #17132

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

Jump to bottom
Merged
merged 5 commits into from
Mar 27, 2025
Merged

fix: conceal sensitive domain information in auth error messages #17132

merged 5 commits into from
Mar 27, 2025

Conversation

ericpaulsen
Copy link
Member

@ericpaulsen ericpaulsen commented Mar 27, 2025 •
edited by matifali
Loading

Summary

  • Removes exposure of allowed domain list in OIDC authentication error messages
  • Replaces detailed error messages with a generic message that doesn't expose internal domains
  • Adds "Please contact your administrator" to guide users seeking assistance
  • Addresses security concern where third-party contractors could see internal domain information

Test plan

  • Test accessing Coder with an email that doesn't match allowed domains
  • Verify error message no longer displays the list of authorized domains
  • Verify message now includes guidance to contact administrator

Fixes issue related to domain information exposure during authentication. Closes #17130

🤖 Generated with Claude Code

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 27, 2025 •
edited
Loading

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@ericpaulsen
fix: conceal sensitive domain information in auth error messages
0fcce5f 
Remove exposure of allowed domain list in OIDC authentication error messages to enhance security. Third-party contractors no longer see internal domain lists when accessing Coder with unauthorized email addresses.
@ericpaulsen ericpaulsen force-pushed the fix/conceal-domains-in-auth-error branch from ff34fcc to 0fcce5f Compare March 27, 2025 12:19
@ericpaulsen ericpaulsen requested review from Emyrk and johnstcn March 27, 2025 12:20
@ericpaulsen
Copy link
Member Author

FYI - this is PR is for a strategic customer, but ClaudeCode did the work here. I just supplied it the linked issue. Let me know if further iterations are needed.

@ericpaulsen ericpaulsen requested a review from mafredri March 27, 2025 12:38
johnstcn
johnstcn approved these changes Mar 27, 2025
View reviewed changes
Copy link
Member

@johnstcn johnstcn left a comment

Choose a reason for hiding this comment

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

Good catch!

@ericpaulsen
test: add test cases for domain concealment in auth error messages
bbdd048 
- Verifies the error message no longer shows domain list - Adds tests for both invalid domain and malformed email cases - Includes test for successful login with allowed domain - Fixes response body closing in test
mafredri
mafredri requested changes Mar 27, 2025
View reviewed changes
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

The code change seems fine, but tests could use a bit of work. Personally I'd like to see much less comments that state the same thing that the code does. I.e. comments that don't explain why are usually not high-value.

@ericpaulsen ericpaulsen requested a review from mafredri March 27, 2025 13:24
mafredri
mafredri approved these changes Mar 27, 2025
View reviewed changes
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for making the changes 👍🏻

@ericpaulsen ericpaulsen enabled auto-merge (squash) March 27, 2025 13:28
@ericpaulsen ericpaulsen merged commit 5bd2a3f into main Mar 27, 2025
30 checks passed
@ericpaulsen ericpaulsen deleted the fix/conceal-domains-in-auth-error branch March 27, 2025 13:41
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mafredri mafredri mafredri approved these changes

@johnstcn johnstcn johnstcn approved these changes

@Emyrk Emyrk Awaiting requested review from Emyrk

Assignees

@ericpaulsen ericpaulsen

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

conceal email domains from default login screen message
3 participants
@ericpaulsen @mafredri @johnstcn