feat(invite) add email autocomplete in invite #14610

raphaelbadawi · 2024-04-09T09:35:37Z

This is the follow up to this discussion in which the specs were discussed.

This adds an autocomplete for mere emails. Since emails are not Jitsi entity types, this also adds an alternative authentication mechanism for directories holding external entities (this authentication mechanism was also discussed in the issue above).

The documentation for this has been written too, here is the PR for this part => jitsi/handbook#503

Thanks!

…chToken config

…ail directory)

…ken (e. g. email invite type), invite icons updated

…s list

jitsi-jenkins · 2024-04-09T09:40:19Z

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

raphaelbadawi · 2024-04-12T07:23:17Z

Hello,

Here is the CLA signed by my CEO.

8x8_Corporate_CLA_rev.20181129.docx.pdf

Dev list.pdf

Best regards,
Raphaël.

github-actions · 2024-07-12T01:53:14Z

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

saghul · 2024-07-12T07:13:48Z

PIng @damencho

damencho

There are also a lot of lint errors.

damencho · 2024-07-12T07:53:55Z

react/features/invite/functions.ts

 ): Promise<any> {

    if (!inviteItems || inviteItems.length === 0) {
        return Promise.resolve();
    }

+    // Parse all the query strings of the search directory endpoint
+    const params = new URLSearchParams();
+    params.append('jwt', jwt);


This adds jwt to the URL params. This reverts behavior that was dropped in 9af0003.

damencho · 2024-07-12T07:55:05Z

react/features/invite/functions.ts

+    // Authentication params for external entities (e. g. email)
+    if (peopleSearchTokenLocation && peopleSearchTokenKey) {
+        const peopleSearchToken = localStorage.getItem(peopleSearchTokenLocation) ?? "";
+        params.append(peopleSearchTokenKey, peopleSearchToken);


Why do you need a second token parameter. Can't you use Authorization': `Bearer headers that are added.

Can you give more details on this one?
So the token is stored in localstorage? Who is storing it there?

That is a good question. The whole idea is to be able to authenticate against directories on instances where JWT is not enabled. In our case we added a little fetch request to our auth service at the bottom of index.html which retrieves the token if there is an active session.

What we added to index.html is very straightforward and looks like this:

document.addEventListener("DOMContentLoaded", () => { fetch("{{SERVER}}/api/login/token", { credentials: "include" }) .then((response) => response.json()) .then((data) => localStorage.setItem("service_token", data.token)); });

Do you see a problem with using 'Authorization': Bearer ${jwt} with the token you store in service_token?
The idea is to get rid of the URL params with authorization data, as normally the URLs are logged in different places in the backend and is hard to sanitize, so its a better practice to use the headers.

Don't see any problem doing this, as long as it works. Moreover it's cleaner. I'll let you know when the change will be done ;-)

Use local storage like this:

jitsi-meet/conference.js

Line 364 in b4a5e63

room.join(jitsiLocalStorage.getItem('xmpp_conference_password_override'), replaceParticipant);

All good and retested!

damencho · 2024-07-12T07:55:59Z

react/features/invite/functions.ts

+    params.append('query', query);
+    params.append('queryTypes', queryTypesString);
+    params.append('jwt', jwt);
+


We should not add jwt as URL param since: 9af0003

damencho · 2024-07-12T07:56:38Z

react/features/invite/functions.ts

+
+    if (peopleSearchTokenLocation && peopleSearchTokenKey) {
+        const peopleSearchToken = localStorage.getItem(peopleSearchTokenLocation) ?? "";
+        params.append(peopleSearchTokenKey, peopleSearchToken);


Same here why don't you use Authorization': `Bearer?

raphaelbadawi · 2024-07-12T08:07:17Z

Thank you for the quick review @damencho.

So first I will check all the linter errors today and make sure npm run lint passes.

As for the JWT in the headers, it was not there three months ago when I first did the PR. I just noticed the change today when remerging master to the branch, so I will update things consequently ;-)

I'll put a comment when everything will be settled.

damencho · 2024-07-12T08:12:29Z

Yep, the changes were added a month later after the PR, sorry it fell trough the cracks.

raphaelbadawi · 2024-07-13T09:28:50Z

Hmm there is yet a little regression to fix I will let you know when everything is OK.

raphaelbadawi · 2024-07-15T09:43:30Z

Everything has been retested again should be fine now. Thanks!

raphaelbadawi · 2024-07-29T08:14:19Z

Up

damencho · 2024-07-30T13:13:03Z

react/features/invite/functions.ts

    const headers = {
        ...jwt ? { 'Authorization': `Bearer ${jwt}` } : {},
        'Content-Type': 'application/json'
    };

    return fetch(
-        `${inviteServiceUrl}`,
+        `${inviteServiceUrl}?${params.toString()}`,


In the default case there will be an empty ? at the end.

damencho · 2024-07-30T13:16:10Z

Other than the questions I have and the small lint error LGTM.

damencho · 2024-08-01T16:06:50Z

@raphaelbadawi
Can you add // @ts-expect-error on the previous line before the js-utils import :) to fix the lint error

damencho · 2024-08-01T16:32:32Z

@raphaelbadawi Can you add // @ts-expect-error on the previous line before the js-utils import :) to fix the lint error

I did it. Let's see now 🤞

damencho · 2024-08-01T16:38:24Z

jenkins test this please.

raphaelbadawi · 2024-08-01T17:18:18Z

@raphaelbadawi Can you add // @ts-expect-error on the previous line before the js-utils import :) to fix the lint error

I did it. Let's see now 🤞

Thank you, I didn't react quickly enough.

jenkins test this please.

Do I have to do something for this part?

damencho · 2024-08-01T18:00:03Z

Do I have to do something for this part?

Nope, these is triggering the integration tests when the PR creator is not one of the devs.

damencho · 2024-08-01T18:01:20Z

Thank you for your contribution and patience :)

raphaelbadawi · 2024-08-01T18:10:11Z

Thank you for your contribution and patience :)

Thank you for your great insights ;-)

raphaelbadawi added 3 commits April 2, 2024 23:13

feat(invite) add email value in peopleSearchQueryTypes and peopleSear…

2f6b5aa

…chToken config

feat(invite) pass custom auth token in search directory (e. g. for em…

3d4da6d

…ail directory)

feat(invite) autocomplete and invitations working with custom auth to…

f70e443

…ken (e. g. email invite type), invite icons updated

raphaelbadawi mentioned this pull request Apr 9, 2024

Add documentation for invite configuration jitsi/handbook#503

Merged

raphaelbadawi force-pushed the email-autocomplete branch from 6c56b78 to 55e5b0a Compare April 9, 2024 09:37

feat(invite) remove newly documented config from undocumented setting…

5ed6d73

…s list

raphaelbadawi force-pushed the email-autocomplete branch from 55e5b0a to 5ed6d73 Compare April 9, 2024 09:39

raphaelbadawi mentioned this pull request Apr 9, 2024

Have "email" as a people search query type supported in the "Add People" dialog #14331

Closed

github-actions bot added the stale label Jul 12, 2024

Merge branch 'master' into email-autocomplete

ac0e4bc

damencho requested changes Jul 12, 2024

View reviewed changes

feat(invite) jwt are now passed in the invite requests headers

a4c3e81

github-actions bot removed the stale label Jul 13, 2024

feat(invite) linter-related formatting

d200f03

feat(invite) fix default user icon regression

aa1f180

Merge branch 'master' into email-autocomplete

9d2bceb

damencho reviewed Jul 30, 2024

View reviewed changes

raphaelbadawi added 3 commits July 30, 2024 16:47

feat(invite) last lint issues

c313139

feat(invite) pass alternate token in header, not in params

3f619a2

Merge branch 'master' into email-autocomplete

121a916

Fixes lint error

fc76aba

damencho approved these changes Aug 1, 2024

View reviewed changes

damencho merged commit 1e101af into jitsi:master Aug 1, 2024
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(invite) add email autocomplete in invite #14610

feat(invite) add email autocomplete in invite #14610

raphaelbadawi commented Apr 9, 2024

jitsi-jenkins commented Apr 9, 2024

raphaelbadawi commented Apr 12, 2024

github-actions bot commented Jul 12, 2024

saghul commented Jul 12, 2024

damencho left a comment

damencho Jul 12, 2024

damencho Jul 12, 2024

damencho Jul 30, 2024

raphaelbadawi Jul 30, 2024

damencho Jul 31, 2024

raphaelbadawi Jul 31, 2024

damencho Jul 31, 2024

damencho Jul 31, 2024

raphaelbadawi Aug 1, 2024

damencho Jul 12, 2024

damencho Jul 12, 2024

raphaelbadawi commented Jul 12, 2024

damencho commented Jul 12, 2024

raphaelbadawi commented Jul 13, 2024 •

edited

Loading

raphaelbadawi commented Jul 15, 2024

raphaelbadawi commented Jul 29, 2024

damencho Jul 30, 2024

damencho commented Jul 30, 2024

damencho commented Aug 1, 2024 •

edited

Loading

damencho commented Aug 1, 2024 •

edited

Loading

damencho commented Aug 1, 2024

raphaelbadawi commented Aug 1, 2024

damencho commented Aug 1, 2024

damencho commented Aug 1, 2024

raphaelbadawi commented Aug 1, 2024

feat(invite) add email autocomplete in invite #14610

feat(invite) add email autocomplete in invite #14610

Conversation

raphaelbadawi commented Apr 9, 2024

jitsi-jenkins commented Apr 9, 2024

raphaelbadawi commented Apr 12, 2024

github-actions bot commented Jul 12, 2024

saghul commented Jul 12, 2024

damencho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

raphaelbadawi commented Jul 12, 2024

damencho commented Jul 12, 2024

raphaelbadawi commented Jul 13, 2024 • edited Loading

raphaelbadawi commented Jul 15, 2024

raphaelbadawi commented Jul 29, 2024

Choose a reason for hiding this comment

damencho commented Jul 30, 2024

damencho commented Aug 1, 2024 • edited Loading

damencho commented Aug 1, 2024 • edited Loading

damencho commented Aug 1, 2024

raphaelbadawi commented Aug 1, 2024

damencho commented Aug 1, 2024

damencho commented Aug 1, 2024

raphaelbadawi commented Aug 1, 2024

raphaelbadawi commented Jul 13, 2024 •

edited

Loading

damencho commented Aug 1, 2024 •

edited

Loading

damencho commented Aug 1, 2024 •

edited

Loading