Specify and store the provider ID of a collaborator

[aedelmann]

Motivation
Currently we support multiple oauth providers that can be managed as collaborators. However, at the moment, we cannot differentiate between a GitHub or a BoschID user. If a 3rd party oauth provider is added, possibly holding the same userID as GitHub, we would run into a clash.

Solution

• Store the provider ID together with the user in the user management DB.

Confirmations:

• (Automated) DB Migration of existing users, that add the provider ID to each user entry (BoschIDs start with S-*** and have a fixed length whereby GitHub users can be arbitrary. The migration script should be able to detect and handle it properly
• During User Registration process, the user and its provider ID are stored in the user management DB
• Assignment of technical collaborators to a namespace, would automatically create a technical user account in the Vorto Repository, if not exists
• Under namespace management, all collaborators are displayed with their corresponding provider IDs as well as a namespace admin must specify the provider ID when adding a collaborator to his/her namespace.

[ghost]

@aedelmann as discussed, happy to take this if not taken already. I could definitely use a bit more specs/elaboration on stories in that case though.

[ghost]

@aedelmann elaborating with a few questions:

• I don't understand the difference between a "technical" user and just a user - the code base has plenty of variable names / methods called ...technicalUser... but I seem to have trouble conceptually on this one. Is it a matter of permissions?
• As far as I can see, the controller side of things is handled by the AccountController only, which already seems to have some functionality to guess the user's provider upon creation (but I could be wrong - the two new oauth modules are brand new from your contribution yesterday). Bottomline, what is the value of having the admin user pick up a provider explicitly?
• By subject, do you mean contextually to security, i.e. as opposed to principal?

Let me know if discussing here suits, or whether we should take this offline.

[aedelmann]

Hi @mena-bosch
You are right. Currently I do not see any difference between a technical or an end user. Both are managed in the same way with their roles. However, what we are missing is the information about where the user originates from, eg. Github, BoschID or BoschIoTSuiteAuth. If the latter, it is a actually a technical user. Hence, I changed the issue context a bit, see confirmations.

I can share with you the concept in the Wiki Page what we are doing for the Device Provisioning process. That it might get clearer :)

[erlemantos]

Store the provider ID together with the user in the user management DB.

We already have this.

During User Registration process, the user and its provider ID are stored in the user management DB

We already have this.

Under namespace management, all collaborators are displayed with their corresponding provider IDs as well as a namespace admin must specify the provider ID when adding a collaborator to his/her namespace.

We don't have this.

(Automated) DB Migration of existing users,

We don't have this.

[erlemantos]

(Automated) DB Migration of existing users,

Could be done via the Upgrade Task mechanism that we have. Check the interface IUpgradeTask and some of the implementation in TargetPlatformUpgradeTask

[erlemantos]

I don't understand the difference between a "technical" user and just a user - the code base has plenty of variable names / methods called ...technicalUser... but I seem to have trouble conceptually on this one. Is it a matter of permissions?

From a technical perspective, the difference between a technical user and an ordinary user is when adding a collaborator to a namespace.

If we are adding a normal collaborator to a namespace, we check if the collaborator already exist (has already registered). We flag it as an error if he doesn't yet exist.

If we are adding a technical user, we don't check if he already exist. We create him if he doesn't yet exist, then add him to the namespace.

[ghost]

Thanks @erlemantos for the clarifications and @aedelmann for the session (@erlemantos we were on a call with Kevin and Alex while you were writing these :)
I will gather my notes later and add a summary of what I think is required for this specific task (which will partially repeat items already stated by Erle here).

[ghost]

Wrapping this up in a "workflow" from the admin perspective.

1. An administrator needs to add a user to a namespace.
   For now, we do not care about the origin of the add request and consider it as manual work for the administrator.
   For technical users, we might want to provide some candy at some point, to semi-automatize the process.

2. The administrator selects the namespace, then types the user ID in the text box. An auto-complete mechanism is in place, that also searches for existing users all across .
   a. If the user exists, the unmodifiable details regarding OAuth provider (unmodifiable dropdown) and whether it is a technical user (edit-disabled checkbox) appear below the user name. Upon confirming, the user is added to the namespace with no further operation.
   b. If the user does not exist, the administrator is prompted to select from a drop-down of supported authentication providers (for now, github or bosch id) and notified that the user can only be added as technical user. Upon confirming, the user is persisted along with their provider and information on whether technical or not .

Basically there are:

• Two new UI elements (user provider representation - dropdown, and technical user - checkbox).
• A new listener associated to an existing UI element (auto-complete and user search in text box)
• Maybe also an "alert" before adding a new user as technical user (this can also be handled by checking and disabling the relevant checkbox, and adding a succint message somewhere)

Remaining problem

• At this time, we are not supporting suit OAuth as a provider, and we cannot assume Bosch ID users (and let alone GitHub users) are technical users
• As such, the only "guessing" we can do with regards to a user being technical is when the administrator wants to add them when they don't already exist
• This seems to imply that, for now, we persist an additional flag for users specifying whether they are technical or not
• Similarly, the migration script can only do one thing in that regard, which is to assume users that have not been added as technical are in fact, not technical. Of course the migration script can also guess the provider based on the id, and once suite oauth is supported, it may be able to guess whether the user is technical based on whether their provider is suite oauth or not - but that's not for right now.

@aedelmann @erlemantos will start working towards that direction - do not hesitate to get back to me if I am still missing the point somewhere here. Also do we have any plans for suite oauth integration?

[ghost]

Taking back my last "remaining problem": the suite auth provider seems to be expressed as one of 3 enum types, so while I cannot see the actual authentication mechanism implemented anywhere, at least the checks should work already.

[aedelmann]

Taking back my last "remaining problem": the suite auth provider seems to be expressed as one of 3 enum types, so while I cannot see the actual authentication mechanism implemented anywhere, at least the checks should work already.

This is legacy code and this enum should be removed because we should not statically define oauth providers in our system. The https://github.com/eclipse/vorto/blob/development/repository/repository-core/src/main/java/org/eclipse/vorto/repository/oauth/IOAuthProviderRegistry.java provides a way to dynamically look up configured oauth providers in the system.

If the user does not exist, the administrator is prompted to select from a drop-down of supported authentication providers (for now, github or bosch id) and notified that the user can only be added as technical user. Upon confirming, the user is persisted along with their provider and information on whether technical or not .

We supporting Github, BoschID, BoschIoTSuiteAuth currently. Please refer to the registry (comment above) how to resolve them.

Just to re-confirm your statement, that any provider which is supported in Vorto, allows to store technical users, that means currently, GitHub, BoschID and BoschIoTSuiteAuth, correct ?

[erlemantos]

Taking back my last "remaining problem": the suite auth provider seems to be expressed as one of 3 enum types, so while I cannot see the actual authentication mechanism implemented anywhere, at least the checks should work already.

This is already fixed in a code I'm trying to pull request. Will ping you when this is done.

This seems to imply that, for now, we persist an additional flag for users specifying whether they are technical or not

This is also being included in a code i'm trying to pull request now. Basically, we add a isTechnicalUser flag to the User domain object, and also a isTechnicalUser flag in the Collaborator object (the object you use in the REST request).

I will ping you when the pull request pulls through.

[ghost]

Linking tickets for better visibility: in #2180, I have pushed a simple SQL script that does the following:

1. Adds a boolean IS_TECHNICAL_USER field to the USER table if not present
2. Adds a string AUTHENTICATION_PROVIDER_ID field to the USER table if not present
   (note: does not add a AUTHENTICATION_PROVIDER field as it seems redundant after talking to @aedelmann. @erlemantos we should probably discuss the new table design as we did not quite understand why there were two such fields together)
3. Adds a string SUBJECT field to the USER table if not present
   (note: not sure how to use this in migration - probably it can only be populated when the user actually logs in?)
4. Sets the AUTHENTICATION_PROVIDER_ID to 'BOSCH-ID' with constraint: (USERNAME LIKE 'S-%-%-%-%-%-%-%' AND CHAR_LENGTH(USERNAME) = 47)
5. Sets the AUTHENTICATION_PROVIDER_ID to 'GITHUB' with constraint: (USERNAME NOT LIKE 'S-%-%-%-%-%-%-%' OR CHAR_LENGTH(USERNAME) <> 47)
6. Sets all existing users to not technical

I cannot yet test this on the dev environment at this time, due to complications with AWS.
Again, I suspect we want to discuss further when @erlemantos is back.
That script is not pull-requested yet either.

Finally, as discussed I will now get back to the "add tenant to namespace" UI - i.e. this scope, while trying to keep it barebones (as the legacy Angular JS framework is too complicated for me to do much in terms of UX).

[ghost]

Note: removing CHAR_LENGTH(USERNAME) = 47 from constraints as it appears bosch id is not fixed length in dev.

[ghost]

Back to working on this after a lot of sidetracking...
Thinking back on the following:

Under namespace management, all collaborators are displayed with their corresponding provider IDs as well as a namespace admin must specify the provider ID when adding a collaborator to his/her namespace.

--> Ok for the first part, but shouldn't the second part be restricted to adding new users?
What sense does it make to ask for the authentication provider ID of existing users, when our DB will already contain that data (which is why we can display it in the first place for existing collaborators)?

My interpretation is:

• Admin adds user to namespace
• User exists? Added and Bob's your uncle
• User does not exist? Admin prompted to add as technical user with specific authentication provider ID chosen from supported set

This workflow would slightly differ from:

If we are adding a normal collaborator to a namespace, we check if the collaborator already exist (has already registered). We flag it as an error if he doesn't yet exist.

If we are adding a technical user, we don't check if he already exist. We create him if he doesn't yet exist, then add him to the namespace.

--> because we would never reach an error state again when a user does not exist. We would simply do the check first, and if the user does not exist, only create it as technical user after prompting the admin.

My final and unresolved dilemma - assuming the ideas above make sense to you guys - is: can we and should we enforce creation of technical users only for a subset of the authentication providers available? I vaguely recall broaching this during a phone call with @aedelmann but sadly, got side-tracked too many times to remember what was the outcome, assuming there was one.

TL;DR

• I'm going to get back to coding under the assumptions above until further notice - give me a shout if I'm mistaken. So:
• Additional modal for non-existing users, prompting for creation of tech user with authentication provider of admin's choice (no limitations enforced for now) - this will factually replace the "User does not exist" error
• With apologies for the verbosity - I still feel I'm walking through a functional mine field here

[ghost]

PR here. As mentioned in the PR comments @aedelmann please don't merge eagerly. If you have time, try it out (either you or Erle) - otherwise please give me a shout and I'll try and accommodate some time before my official return to work (Jan 7th) in order to discuss.

[ghost]

Tentatively closing this as scoped for authentication providers only. The subject part is handled by #2208 and presently being pullrequested (I expect it will go through once we deal with the compliance issues that have been popping up en masse these past few days).

[ghost]

Edit I cannot close this myself - @aedelmann could you close please? Thanks