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

#4607 look up users on LDAP explicitly using a ldap-relay API #4627

Merged
merged 17 commits into from
Aug 24, 2022

Conversation

ioanmo226
Copy link
Collaborator

This PR looks up users on LDAP explicitly using a ldap-relay API

close #4607 // if this PR closes an issue


Tests (delete all except exactly one):

  • Tests added or updated

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@ioanmo226
Copy link
Collaborator Author

@tomholub With current implementation, Settings/attester page shows this error.
Do I have to ignore errors from ldap search results?

image

@tomholub
Copy link
Collaborator

buggy endpoint, that was supposed to be a 404

Please treat error 500 as error 404 on this particular endpoint - meaning no pubkeys found.

Meanwhile I've filed an issue to improve the endpoint https://github.com/FlowCrypt/flowcrypt-attester-ts/issues/264 (you won't see it but it will eventually get fixed)

@ioanmo226
Copy link
Collaborator Author

Got it

@ioanmo226 ioanmo226 requested a review from tomholub August 22, 2022 19:27
@ioanmo226 ioanmo226 marked this pull request as ready for review August 22, 2022 19:27
@ioanmo226 ioanmo226 requested a review from rrrooommmaaa August 23, 2022 12:49
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Thank you - see comments

@ioanmo226 ioanmo226 requested a review from tomholub August 23, 2022 16:23
@rrrooommmaaa rrrooommmaaa merged commit 7716ab8 into master Aug 24, 2022
@rrrooommmaaa rrrooommmaaa deleted the feature/issue-4607 branch August 24, 2022 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

look up users on LDAP explicitly using a ldap-relay API
3 participants