odpi / egeria

Egeria core
https://egeria-project.org
Apache License 2.0
806 stars 260 forks source link

Egeria UI 'demo' users (H2) should be consistent with sample security plugin #1490

Closed planetf1 closed 2 years ago

planetf1 commented 5 years ago

If Egeria is configured with security as per the 'metadata-server-config' lab notebook, only specified users are allowed to access the server.

The sample security pluginfor coco has hardcoded names including 'faithbroker', 'garygeeke' etc which follow the list of coco pharmaceuticals users already references in documentation, ldif samples etc.

This can be seen at: open-metadata-resources/open-metadata-samples/open-metadata-security-samples/src/main/java/org/odpi/openmetadata/metadatasecurity/samples/CocoPharmaServerSecurityConnector.java

However the Egeria UI when initially setup - if not pointed at an ldap server - has hardcoded users such as 'faith' (ie not faith broker). If UI applications pass the logged in user to the underlaying server, but the server has security enabled, then access will fail.

It would make sense to have the H2 database be pre-populated with our standard coco users for consistency (in addition to configuring ldap in our k8s sample environments)

mandy-chessell commented 5 years ago

I have added David to the discussion as this needs to be implemented in one of the connectors proposed for multi-tenancy in the UI platform. Once we have that connector in place, we could pull out the "in-memory" user directory into a separate class that both connectors use.

It would be nice if this class tried to load from LDAP and only used the in-memory list if LDAP is not found in the environment.

For reference, the hardcoded list of userIds and groups is here:

https://github.com/odpi/egeria/blob/master/open-metadata-resources/open-metadata-samples/open-metadata-security-samples/src/main/java/org/odpi/openmetadata/metadatasecurity/samples/CocoPharmaServerSecurityConnector.java

The UI would also need access to their passwords too.

planetf1 commented 5 years ago

The UIs will not work with the current notebook setup consistently (as they do enable security) until this is addressed.

We could add notes in notebook to only use optionally until then, or make a quick H2 update, or fix properly. But it will delay proper use of the notebooks in a 'simple' way cc: @moyabrannan

For k8s we might be able to hook the UI into the ldap already installed. Config is tougher in the docker-compose environemnt, but may be possible. However until we have the connector pointing to the same source it would rely on matching configuration.

planetf1 commented 5 years ago

Surely the UI only needs access to the NPA for the server - if it has that it is 'trusted' to correctly provide the right userid to use for the context of any request.

The UI would authenticate using LDAP - but it would never know the real password. Just what the user entered - the LDAP server would be the only thing knowing the password. In fact if using SSO there could be another level of trust that means we don't even do that lookup from within the UI.

mandy-chessell commented 5 years ago

The call to LDAP needs to go through the UI security connector so that organizations can change the technology for the user directory. We cannot force them to use LDAP.

planetf1 commented 5 years ago

A further note....

The ldap .ldif files currently have user ids of the form Bob_Nitter Faith_Broker

whilst the sample security plugin will use bobnitter faithbroker

The H2 database is populated via DemoInMemoryInitializingBean.java

@bogdan-sava @davidradl As a quick-fix to gain consistency and get the notebook environment running (with security) I will adopt the latter format and explicitly add these userids (rather than do a generic ldap import or connector). I will leave the existing users in place to avoid confusion/breaking demos Should we update ldap accordingly @mandy-chessell

planetf1 commented 5 years ago

This is a minimal change. Icons are all default (not working anyway) and passwords are all 'admin' However the crucial impact is we can now demonstrate security with our lab notebooks whilst awaiting for more refactoring of the UI, and an ldap connector for user repo.

tested with 'faithbroker' (pass=admin)

bogdan-sava commented 5 years ago

Maybe we can add also the user roles in H2. There is also a table called roles, and oneToMany from user to roles, but no hierarchy.

I found this https://opengovernance.odpi.org/coco-pharmaceuticals/personas/ where the job title are. Shall I made user roles from them?

mandy-chessell commented 5 years ago

The job titles are all different - we may want courser grained roles - like clinical trials team, finance team, IT, executive etc.

cmgrote commented 5 years ago

Indeed, that's what I setup for the samples in IGC -- see https://github.com/odpi/egeria/blob/master/open-metadata-resources/open-metadata-deployment/sample-data/coco-pharmaceuticals/vars/users.yml

Top of the YAML you'll see the list of groups (and their IGC-specific permissions, which you should of course ignore); and further down you'll see the list of users and how they've been mapped to the groups. Would be good if we could keep this consistent across the different deployments (simple for labs, IGC, and LDAP)?

planetf1 commented 5 years ago

Good to go with roles but if we add let's make h2, ldap, & our sample plugins consistent. I suggest merging the user change in any case (as without that the UI just won't work at all in the notebook environment) and continue with the role/extended discussion.

The current ldap definitions for roles is found at ./open-metadata-resources/open-metadata-deployment/sample-data/coco-pharmaceuticals/ldap/03-groups.ldif

@mandy-chessell also we still need to make the ldap users consistent - ok with changing Faith_Broker to faithbroker in ldap? I'll wait until we decide what we do with groups?

mandy-chessell commented 5 years ago

I think this is a good illustration of why the user details synching through the Community Profile OMAS will be useful for organizations - we are in a mess with only 3 components :)

I agree with @planetf1 that we should keep LDAP in sync and also have connector implementations that can use our LDAP for each of these environments.

When we are feeling brave, we could also look at deploying multiple LDAP instances in our K8 environment and add to the demo to show how can Egeria keep them in sync.

planetf1 commented 5 years ago

@cmgrote thanks for jumping in - I forgot about the IGC/IS side ... Ultimately there's no question we should use ldap in anything more than a basic tutorial - actually tbh even in a simple docker environment we should probably add in ldap. How feasible it is to support that with IGC/IS I don't know. I'm guessing technically it's fine, but could be tricky since IGC will often already be deployed in an environment?

Another possibility would be to build all these files within the build process from a template automatically. a step back would be to use a tool as a one off gen, or finally we could just manually keep them consistent.

Knowing what is definitive is important - we can see now we had 4 definitions for many users faith (original UI definition) Faith_Broker (ldap) faithbroker (security plugin) fbroker (IGC) ......

If they are constant I'd probably just make the update I suggested to ldap, and bring IGC in line (if > 8chars are allowed).

But the same then applies to groups...

As to 'definitive' ldap would seem to be the best ?

planetf1 commented 5 years ago

@mandy-chessell I'm wondering how likely a typicaly install would allow updates to userids - depends on the trust in egeria I guess. Not necessarily a technical restriction of course. Adding more ldaps is easier, the trick is deciding what we do with them ;-)

I'm not familar enough with community profile, but will it allow for mapping between users on different systems? For example userids in IGC vs what we use in egeria? I wonder if we could need that?

cmgrote commented 5 years ago

For now, for simplicity of alignment, I'd suggest just putting into this issue the definitive list of:

(ie. list them rather than pointing to a file, as each file sounds like it might not be the complete definitive list.)

I'm happy to take the action to ensure IGC is in line with such a list.

mandy-chessell commented 5 years ago

@planetf1 - these are good points. Egeria supports multiple userids associated with the same profile (ie person or engine). It gets very emotional if two actors (either people or engines) try to claim the same userId. So as long as userIds are unique, we are good to go from a synching point of view. We may want more explicit ways of modelling which users are valid in each security domain.

What we don't have yet is the ability to automatically switch the userId as a call flows through different security domains. Not difficult to do technically, but need governance around this.

This relates to the comment from @cmgrote about trust. This is critical and we need to also focus on the governance program around this. However it is not much different from governance of glossary terms and classifications sufficiently to trust them to be able to control access to data in the VDC schenario. The human side of this is key.

Where I see this as particularly valuable is in a multi-cloud environment. Organizations often have a centralized user admin and LDAP/single signon for on premises systems - but when they go to SaaS and other cloud offerings on different platforms, it becomes fragmented again.

The design around community profile OMAS is that userIds/profiles/roles are mastered outside of Egeria and fed through an Egeria governance server into Community Profile OMAS which distributes across the cohort. This info is disseminated further by other instances of Community Profile OMAS deployed on other members of the cohort. Other instances of the Egeria governance server for CP OMAS can have connectors deployed to push this info to local user directories.

mandy-chessell commented 5 years ago

One of the pieces of feedback I had from our security expert on userIds and GDPR is that using UUIDs for userIds rather than meaningful names helps to obscure who is doing what - but need tools that users log on to to manage the mapping from somthing memerable to the UUID.

The mapping to the real person would have to be closely guarded as very easy to correlate on UUID :)

mandy-chessell commented 5 years ago

Looks like this is a good topic for a Thursday call

mandy-chessell commented 5 years ago

This would probably be the afternoon call rather than the morning call as more of an architecture/requirement discussion. I did not get any interest in the CP OMAS and how it is used when I reviewed it in one of our F2F workshops a while back.

From my perspective it would be helpful to get a broader view.

planetf1 commented 5 years ago

I will update the ldap version so that we go with the security plugin version ie 'faithbroker' @cmgrote - for IGC, I could update the files, but a) wonder if there are any other restrictions b) may cordinate better if you make change c) ultimately we should allow the external ids to differ in any case

cmgrote commented 5 years ago

Are we sure this user ID is the only thing that's different? I suggested explicitly listing the names and group assignments so that we're certain... Just checked and answered my own question: I don't see that the lists are aligned at all -- we could fix this one username, but that would only align that one user, and everything else would still be different, which doesn't make much sense to me...

cmgrote commented 5 years ago

So, taking the hard-coded list as the "master", we have the following groups and users (which themselves don't seem to follow a consistent convention, but I'll assume that's on purpose?):

Users

Groups

(Edited per feedback below: removed mixed-case names -- everything now lowercase only -- and removed underscores; also renamed npa_account to npa to remove redundant account 😉)

cmgrote commented 5 years ago

If we can all give a "thumbs-up" to the list above, then we can create consistency from that list across the various samples (for now) -- which I think would be useful for running any subset of them with whatever configuration (LDAP or not) and still have consistency in what we demo (?)

planetf1 commented 5 years ago

My initial focus was on users (as code wouldn't work without that fix)... expanding to groups.. I would suggest we make the group names avoid punctuation also ie npaaccount metadataarchitect (and no camelcase either). This seems to be how most system groups I've seen are listed (though am thinking more unix than ldap specifically..). What do you think @mandy-chessell

A quick google search suggests handling of special chars (spaces to underscores, punctuation etc) is a common cause of bugs in many sw components, and it would be useful to avoid getting into that space for a simple out of the box demo?

bogdan-sava commented 4 years ago

I'll create the following roles for in memory H2 mode of the UI:

Not sure what views will be allowed for roles, but it will be configurable.

github-actions[bot] commented 4 years ago

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

github-actions[bot] commented 4 years ago

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

github-actions[bot] commented 3 years ago

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

davidradl commented 3 years ago

commenting to keep this open

github-actions[bot] commented 3 years ago

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

github-actions[bot] commented 3 years ago

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

github-actions[bot] commented 3 years ago

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

planetf1 commented 3 years ago

Is this sufficiently complete now? At least until we move to ldap etc?

github-actions[bot] commented 3 years ago

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