Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

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.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
ericpaulsen merged 5 commits intomainfromfix/conceal-domains-in-auth-error
Mar 27, 2025

Conversation

ericpaulsen
Copy link
Member

@ericpaulsenericpaulsen commentedMar 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 withClaude Code

@github-actionsGitHub Actions
Copy link

github-actionsbot commentedMar 27, 2025•
edited
Loading

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

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.
@ericpaulsenericpaulsenforce-pushed thefix/conceal-domains-in-auth-error branch fromff34fcc to0fcce5fCompareMarch 27, 2025 12:19
@ericpaulsen
Copy link
MemberAuthor

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.

Copy link
Member

@johnstcnjohnstcn 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!

- 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
Copy link
Member

@mafredrimafredri 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 reacted with thumbs up emoji
Copy link
Member

@mafredrimafredri 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 reacted with thumbs up emoji
@ericpaulsenericpaulsenenabled auto-merge (squash)March 27, 2025 13:28
@ericpaulsenericpaulsen merged commit5bd2a3f intomainMar 27, 2025
30 checks passed
@ericpaulsenericpaulsen deleted the fix/conceal-domains-in-auth-error branchMarch 27, 2025 13:41
@github-actionsgithub-actionsbot locked and limited conversation to collaboratorsMar 27, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@mafredrimafredrimafredri approved these changes

@johnstcnjohnstcnjohnstcn approved these changes

@EmyrkEmyrkAwaiting requested review from Emyrk

Assignees

@ericpaulsenericpaulsen

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
[8]ページ先頭
©2009-2025 Movatter.jp