Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve troubleshooting for LDAP authentication errors #42948

Merged
merged 2 commits into from
Oct 16, 2024
Merged

Conversation

zmb3
Copy link
Collaborator

@zmb3 zmb3 commented Jun 13, 2024

This introduces two small changes:

  1. Use an aggregate error to make sure the original error is included along with our attempt at providing a better error message. This change should help distinguish between bad authn/invalid cert and valid authentication but insufficient user permissions.
  2. Make the CRL Distribution Point in Windows certs optional. This metadata is required in the certs we issue for users to RDP, but it doesn't need to be present in the certs we use to authenticate our service account. The problem with including it when it is not needed is it causes windows to perform a revocation check and log a failure, which can lead to wasted time when troubleshooting.

This introduces two small changes:

1. Use an aggregate error to make sure the original error is included
   along with our attempt at providing a better error message. This
   change should help distinguish between bad authn/invalid cert
   and valid authentication but insufficient user permissions.
2. Make the CRL Distribution Point in Windows certs optional. This
   metadata is required in the certs we issue for users to RDP,
   but it doesn't need to be present in the certs we use to
   authenticate our service account. The problem with including it
   when it is not needed is it causes windows to perform a revocation
   check and log a failure, which can lead to wasted time when
   troubleshooting.
@zmb3 zmb3 marked this pull request as ready for review June 23, 2024 22:50
@github-actions github-actions bot added desktop-access size/sm tctl tctl - Teleport admin tool labels Jun 23, 2024
Copy link

The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with changelog: followed by the changelog entries for the PR.

@zmb3 zmb3 added the no-changelog Indicates that a PR does not require a changelog entry label Jun 24, 2024
@zmb3 zmb3 requested a review from rosstimothy July 7, 2024 19:24
@zmb3 zmb3 enabled auto-merge October 16, 2024 18:10
@zmb3
Copy link
Collaborator Author

zmb3 commented Oct 16, 2024

Merging for v17 test plan and will evaluate backports after that.

@zmb3 zmb3 added this pull request to the merge queue Oct 16, 2024
Merged via the queue into master with commit 3f88208 Oct 16, 2024
40 checks passed
@zmb3 zmb3 deleted the zmb3/desktop-certs branch October 16, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
desktop-access no-changelog Indicates that a PR does not require a changelog entry size/sm tctl tctl - Teleport admin tool
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants