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

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

Closed
tomholub opened this issue Aug 16, 2022 · 0 comments · Fixed by #4627
Closed

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

tomholub opened this issue Aug 16, 2022 · 0 comments · Fixed by #4627
Assignees

Comments

@tomholub
Copy link
Collaborator

tomholub commented Aug 16, 2022

should first do #4610

Until now, when the browser extension looked up public key on https://flowcrypt.com/attester/pub/[email protected], the server would:

  1. check if there is a public key in internal database, if yes return it
  2. check if there is a public key in LDAP at ldap://keys.[domain], if yes save & return
  3. check WKD at recipient domain, if yes save & return

We we are updating attester to be purely pulling public keys from its own database and ignore other sources. Browser extension already knows how to pull from WKD, and now we'll also teach it how to pull from LDAP. Since the browser cannot do arbitrary TCP connections, there is now a proxy for this: https://flowcrypt.com/attester/ldap-relay?server=keys.example.test&[email protected] . When a key was found, it will return status 200 and the armored key as text. Else it's 404. The format is the same as https://flowcrypt.com/attester/pub/[email protected] except that this ldap-relay endpoint can return more than one public key, concatenated with \n.

Steps, approximately:

  1. add a public method similar to doLookup maybe called doLookupLdap (the public method will also later be needed for stop using initialLegacySubmit #4609 )

The method accepts email and optional server. When server is not provided, it will derive it as follows: assume I'm looking up [email protected] then the server is keys.example.test. There is similar code in WKD and FES API code to extract the domain.

Since more than one armored block can be returned, you need to parse them apart with MsgBlockParser.detectBlocks if you get an error 200, then filter public keys from the parsed blocks, and then return the result as an array.

  1. in Attester.lookupEmail, after checking that lookup is enabled, instead of calling await this.doLookup(email) please call the following:
const results = await Promise.all([
  this.doLookup(email),  // get from flowcrypt.com public keyserver database
  this.doLookupLdap(email),  // get from recipient-specific LDAP server, if any, relayed through flowcrypt.com
  this.doLookupLdap(email, 'keyserver.pgp.com'), // get from keyserver.pgp.com, relayed through flowcrypt.com
])

Then please choose and return only one set of results, with the following priority, starting from highest:

  • if at least one public key was returned from customer-specific LDAP, return an array of these public keys and nothing else
  • else, if flowcrypt.com public keyserver returned any public key, return only array of these
  • else, return array of keys returned from keyserver.pgp.com (or empty array)

And return that from lookupEmail like before.

For tests, you can add attester mock endpoints, to write tests ensuring that the priority is followed and that ldap searches are also disabled when other attester searches are disabled.

Also test that it can parse and recognize more than one public key (eg respond with two public keys for one of the looked up addresses, and then check that both were imported in settings)

@tomholub tomholub added this to the 8.3.4 milestone Aug 16, 2022
@tomholub tomholub changed the title look up users on LDAP explicitly using a ldap_relay API look up users on LDAP explicitly using a ldap-relay API Aug 16, 2022
rrrooommmaaa pushed a commit that referenced this issue Aug 24, 2022
* feat: added lookup ldap result

* feat: added ui test

* fix: 500 ldap api

* fix: test

* fix: test

* fix: ui test

* fix: ui test

* fix: remove unused test

* fix: comment message

* fix: comment message

* fix: pr reviews

* fix: ldap search

* feat: added ui test

* fix: only

* fix: pr review and added ui test for retry pub key search logic

* fix: ui test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants