[NEW] Feature/custom oauth mail field and interpolation for mapped fields

[benkroeger]

CLOSES #11647
CLOSES #15693
CLOSES #13220

This PR adds the config option to pick email value from a different identity property (e.g. upn)

It came to live when trying to use the O365 Graph user api as identityPath. That endpoint provides the user's mail address under the mail property, which would cause the email field in the user's identity to be empty.
Also, since O365 Graph api adds a property @odata.context to the response json and mongodb doesn't allow document paths to contain . characters, this PR removes all properties that contain a . as the very last step in normalizeIdentity (happening last in order to allow intermediate steps to still work with those properties; e.g. lookup @odata.... in the getUsername method).

Also, when configuring Azure AD records to contain values like employeeId, this could become a powerful combination to populate user records in RocketChat (mail as email and employeeId as username)

UPDATE
73aebfa adds the possibility to use string interpolation in custom field mappings.
this allows to mix and match fragments of existing fields (identity response from provider) into new values (e.g. pick the leading part of email and use as username: {{/^(.+)@/::email}})

Note:

• if you want interpolation, the field mapping must contain text in double curly braces {{interpolation here}}
• interpolation can be anything from a simple field mapping {{email}} through a nested path {{foo.bar}} to a regexp applied to the value from a nested path {{/^(.+)@/::profile.email}}
• interpolations can be combined with static strings {{/^(.+)@/::email}}@rocket.chat. assuming email has the value foo@bar.com the result would be foo@rocket.chat
• regular expressions in interpolations are enclosed with slashes / and separated from the field selector with double colons ::
• regular expressions must have a single capture group. the value from the first capture group is used for the interpolation value

[benkroeger]

@rodrigok @engelgabriel can someone pls re-trigger the test-with-oplog-mongo-4-0? I didn't change anything related in the last commit (previous commit ran tests successfully). Suspecting a hick-up in circle-ci here.

thanks!

[tammer123]

Is there any updates with this?

[tammer123]

Updates?

[benkroeger]

@sampaiodiego since this one is similar to #15743, can you take a look?

[tammer123]

Updates?

[benkroeger]

nothing? in a month? is anything missing?

[tammer123]

Maybe you should @ them?

[benkroeger]

"them" whom? mentioned a few of the frequent contributors already

[benkroeger]

@tammer123 so you are a team member (now)?

[benkroeger]

@sampaiodiego since you authored the PR for the latest release, I assume you're pretty active in the development team. I've been trying to drag some attention to this PR for quite a while now. Can you maybe give a hint on what action I'm missing - or even help me get this thing processed? Any comment is highly appreciated

[benkroeger]

@rodrigok unit tests added - for simplicity in testing, I outsourced the newly added methods into a separate file. this also reduces the risk of code conflicts in the main file

[rodrigok]

@benkroeger great, thanks, I'll review it again soon

[rodrigok]

@benkroeger I tried this config:

With this value returning on /me:

And got this result:

The email is correct but the name and username are wrong

[benkroeger]

@rodrigok can not explain where die mismatch comes from. added unit test to process exactly your test data and template expression - receiving the expected results. I also added the normalizers to template_helpers and included 'normalizing' for the test data (suspected that some data manupulation might happen there).

is it possible that you ran tests a couple of times, maybe even with different setup or an existing user with the same id?
I can not reproduce what you see.

[benkroeger]

live test on a running instance also works fine for me (completely fresh meteor instance), using my actual azure ad oauth configuration:

[rodrigok]

@benkroeger you need to apply this change:

From 22db311f707550e480f1d208740ba6b5097facc7 Mon Sep 17 00:00:00 2001
From: Rodrigo Nascimento <rodrigoknascimento@gmail.com>
Date: Thu, 26 Mar 2020 15:25:26 -0300
Subject: [PATCH] Do not process OAuth fields 2 times

---
 app/custom-oauth/server/custom_oauth_server.js | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/app/custom-oauth/server/custom_oauth_server.js b/app/custom-oauth/server/custom_oauth_server.js
index 020da8782..c37e43b62 100644
--- a/app/custom-oauth/server/custom_oauth_server.js
+++ b/app/custom-oauth/server/custom_oauth_server.js
@@ -357,15 +357,15 @@ export class CustomOAuth {
 			}
 
 			if (this.usernameField) {
-				user.username = this.getUsername(user.services[this.name]);
+				user.username = user.services[this.name].username;
 			}
 
 			if (this.emailField) {
-				user.email = this.getEmail(user.services[this.name]);
+				user.email = user.services[this.name].email;
 			}
 
 			if (this.nameField) {
-				user.name = this.getCustomName(user.services[this.name]);
+				user.name = user.services[this.name].name;
 			}
 
 			if (this.mergeRoles) {
-- 
2.21.1 (Apple Git-122.3)

I've tried to push to your branch but I was blocked.

[rodrigok]

The current code is processing the fields 2 times

[benkroeger]

I've rebased my work on #develop regularly (last today with d223fb1). Is this something you just added or should this have been there before?
Also, this issue would happen even without my proposed changes. Shouldn't it be fixed in a separate PR?

[rodrigok]

@benkroeger I didn't add this to our code. It wasn't an issue before because wasn't possible to combine values via interpolation. For example, now the name property may be a mix of {{name}} {{anotherField}} and if this process again, since I'm using the current field inside the interpolation it will get name as the already changed value and will contact with anotherField again.

[benkroeger]

@rodrigok added your suggested modifications (and rebased again)

[rodrigok]

@benkroeger thanks man, it was a great contribution.

We are now in the feature freeze period during the Release Candidate of version 3.1. So we will merge this in a few days for the development of version 3.2.

[luixxiul]

@benkroeger Glückwunsch 🥳