Ssouser group enhancement

[huangqun]

@ajefts Changes for #87 and #88

[huangqun]

@ajefts please take a look at this when you have time. Thanks!

[ajefts]

I'll deploy it to dev and take a look. fyi @mtwomey

[ajefts]

@huangqun Creating groups looks ok.

However, editing a users SSO properties isn't quite right yet...

• The user id should auto populate based on the user I'm editing. No need to edit the user id.
• We don't need provider and provider type. Combine these into a single dropdown called Provider. The lookup values should come from common_oltp:sso_login_provider.
• Add a manual entry field for "SSO User ID" and SSO User Name". These are both strings with max 100 chars. Both are required.

Please let me know if that makes sense.

Thanks,
Tony

[huangqun]

@ajefts my comments below:

1. I can see the fields getting populated when I tested on dev server: http://take.ms/0vgCk
2. The sso_loing_provider table looks like the following, so should we just pull the names into the single dropdown?
   
3. We can add these entry fields, but where should we save them? The following table?
   

[ajefts]

1. Did we miss some config or something? I don't see it working in dev or prod...

2. Yes

3. Yes, they get saved in the user_sso_login table.

[huangqun]

1. If you check the screenshot I sent, I was testing it on dev server:

I'll get 2 & 3 updated. But if you can double check #1 and let me know if that's an issue it would be helpful.

[huangqun]

@ajefts also do we already have api to pull the list of providers from database? Admin app is frontend only and can't do that.

[huangqun]

@ajefts if there's no such api yet, where should we add it? Perhaps the identity service since we already have the sso user api there?

[ajefts]

@huangqun I don't believe we have that endpoint, but identity would be the right place to add it. Thanks.

[huangqun]

@ajefts check this new PR that should address the changes requested above:
#106

However it seems that the v3/users api doesn't return all the profiles of that user but only the 1st one, and therefore after changing the sso provider, we can't see the updated provider (since api seems to create a new profile for each new sso provider) since only the 1st profile in the list is returned by the api. Not sure if that's something that needs to be fixed in the api and then in the admin-app (to show the list of profiles).

[huangqun]

@ajefts any things we need to do about the api issue I mentioned above?

[ajefts]

I'm not sure I fully understand. Is it creating new user records when you update the original user?

[huangqun]

@ajefts please check this: https://github.com/appirio-tech/tc1-api-core/blob/6a163effcbf50d989424f1450b60364f24391faf/tech.core/tech.core.service.identity/src/main/java/com/appirio/tech/core/service/identity/resource/UserResource.java#L176-L181

Whenever you set a new sso provider to an existing user, it will create a new SSO user (profile).

However the /v3/users api doesn't return all these sso users but only the 1st entry, so when you change the sso provider in admin-app, there's no way to verify the change was successful on the UI.

[ajefts]

for https://github.com/appirio-tech/tc1-api-core/blob/6a163effcbf50d989424f1450b60364f24391faf/tech.core/tech.core.service.identity/src/main/java/com/appirio/tech/core/service/identity/resource/UserResource.java#L176-L181, logically it looks like it's not adding a new record IF one already exists. Is that not true in reality. We only want 1 sso_user_login record per user

[huangqun]

Not really, in fact it creates a new record if the <userId, providerId> doesn't exist yet. I guess we need to fix this in the API first.

[ajefts]

@sudoster @ykohata I think I need your brains on this for a little help.

Are we supporting multiple sso identities per tc user?

[ykohata]

@ajefts - It should support multiple SSO identities per tc uid + provider-id. I think it can't handle multiple accounts within the same provider.

[huangqun]

@ykohata so the logic we have here right now is actually correct?

https://github.com/appirio-tech/tc1-api-core/blob/6a163effcbf50d989424f1450b60364f24391faf/tech.core/tech.core.service.identity/src/main/java/com/appirio/tech/core/service/identity/resource/UserResource.java#L176-L181

Each tc uid + provider-id pair will uniquely correspond to at most one SSO identity right?

[huangqun]

@ykohata so the logic we have here right now is actually correct?

https://github.com/appirio-tech/tc1-api-core/blob/6a163effcbf50d989424f1450b60364f24391faf/tech.core/tech.core.service.identity/src/main/java/com/appirio/tech/core/service/identity/resource/UserResource.java#L176-L181

Each tc uid + provider-id pair will uniquely correspond to at most one SSO identity right?

[ajefts]

@huangqun In that case then, do we need to be looking up the user sso info for the specific provider when making updates?

[huangqun]

@ajefts I think perhaps we need a new endpoint to get the list of sso identities for a user id, which can be used in the admin app when editing the sso users. The UI might need some tweaks as well since right now our UI doesn't support displaying multiple providers. For example:

1. When user changes the provider of an existing sso identity to a new one, should the app automatically create a new identity for that new provider?
2. Or should creation always be explicitly done using a "New Provider" button or something?
3. Should we display the list of sso identities on the popup and the admin has to click select one identity to be able to edit it?

[ajefts]

@huangqun yes, I agree with your thinking. We should provide support to create/edit multiple sso identities per user account. I think we should allow the admin user to explicitly "Add New Provider" and select an existing provider to edit.

I think we should update the modal for managing the SSO info to add these features.

[huangqun]

@ajefts ok, that makes sense, I'll get the API updated and then start the update on the admin-app.