shell-ui : Add users group mapping in shell UI config #3196

JBWatenbergScality · 2021-03-11T17:01:49Z

Component:

shell-ui

Context:

In order to disable features in Metalk8s UI and Solutions UI according to user permissions we need an abstract way to define user groups independant from any OIDC provider capabilities.

Summary:

This PR add support for an userGroupsMapping property in shell ui configuration that allows static user groups configuration.

Acceptance criteria:

Shell-UI should expose aggregated users groups to Metalk8s and solutions UIs.

Refs : #3159

Depends on : #3154 and #3181

bert-e · 2021-03-11T17:01:51Z

Hello jbwatenbergscality,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Status report is not available.

bert-e · 2021-03-11T17:01:56Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

JBWatenbergScality · 2021-03-12T09:22:58Z

@bert-e after_pull_request=3181

gdemonet

LGTM 👌 you may want to address usage of this new configuration in the same PR (defining a mapping for Dex users).

gdemonet · 2021-03-16T11:02:01Z

shell-ui/src/auth/permissionUtils.spec.js

+  it('should return a merged array of groups when defined in OIDC claims and mapping ', () => {
+    //E
+    const oidcAndStaticGroups = ["group"];
+    const groups = getUserGroups({profile: {email: "test@test.com", groups: oidcAndStaticGroups}}, {'test@test.com': oidcAndStaticGroups});
+    //V
+    expect(groups).toEqual(oidcAndStaticGroups);
+  });


Maybe this test case could be improved a bit, by making sure the input OIDC and mapped groups are different (otherwise we're not really testing the "merge" behaviour):

have one case where OIDC groups are not set at all (to emulate Dex 😉)

have another case where there are some duplicates which get discarded by this method

gdemonet · 2021-03-16T11:05:06Z

shell-ui/src/index.js

@@ -21,6 +21,8 @@ export type MenuItems = {[path: string]: TranslationAndGroups }

 export type Options = { main: MenuItems, subLogin: MenuItems };

+export type UserGroupsMapping = {[email: string]: string[]};


Static mapping is fine for now I guess, maybe adding support for regular expressions could be useful in the future.

Anyway. This mapping would need to be defined from Salt (could be merged with user-provided values in UI CSC) by reading the Dex CSC, so we can always get the "platform-admin" (or whatever it's called) group to Dex static users.

In practice, I think this simply means updating the salt/metalk8s/addons/ui/config/shell-ui-config.yaml.j2 template to:

#!jinja|yaml {%- set dex_defaults = salt.slsutil.renderer('salt://metalk8s/addons/dex/config/dex.yaml.j2', saltenv=saltenv) %} {%- set dex = salt.metalk8s_service_configuration.get_service_conf('metalk8s-auth', 'metalk8s-dex-config', dex_defaults) %} # Defaults for shell UI configuration apiVersion: addons.metalk8s.scality.com/v1alpha1 kind: ShellUIConfig spec: oidc: providerUrl: "/oidc" redirectUrl: "https://{{ grains.metalk8s.control_plane_ip }}:8443/" clientId: "metalk8s-ui" responseType: "id_token" scopes: "openid profile email groups offline_access audience:server:client_id:oidc-auth-client" userGroupsMapping: {%- for user in dex.spec.config.staticPasswords | map(attribute='email') %} {{ user }}: [admin] {%- endfor %} options: main: "https://{{ grains.metalk8s.control_plane_ip }}:8443/": en: "Platform" fr: "Plateforme" "https://{{ grains.metalk8s.control_plane_ip }}:8443/alerts": en: "Alerts" fr: "Alertes" subLogin: "https://{{ grains.metalk8s.control_plane_ip }}:8443/docs": en: "Documentation" fr: "Documentation"

bert-e · 2021-03-22T13:51:16Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: after_pull_request

gdemonet · 2021-03-24T07:49:00Z

docs/operation/account_administration/user_authentication_and_identity_management.rst

+   .. parsed-literal::
+
+      root\@bootstrap $ kubectl exec -n kube-system -c salt-master \\
+                         --kubeconfig /etc/kubernetes/admin.conf \\
+                         salt-master-bootstrap -- salt-run state.sls \\
+                         metalk8s.addons.ui.deployed saltenv=metalk8s-|version|
+
+   .. parsed-literal::
+
+      root\@bootstrap $ kubectl exec -n kube-system -c salt-master \\
+                         --kubeconfig /etc/kubernetes/admin.conf \\
+                         salt-master-bootstrap -- salt-run state.sls \\
+                         metalk8s.addons.prometheus-operator.deployed \\
+                         saltenv=metalk8s-|version|
+


You can pass multiple SLS formulas to state.sls, so the following should be enough:

.. parsed-literal:: root\@bootstrap $ STATES=$(printf ",metalk8s.addons.%s.deployed" \\ dex prometheus-operator ui) root\@bootstrap $ kubectl exec -n kube-system -c salt-master \\ --kubeconfig /etc/kubernetes/admin.conf \\ salt-master-bootstrap -- salt-run state.sls \\ "${STATES:1}" saltenv=metalk8s-|version|

gdemonet · 2021-03-24T07:54:50Z

docs/operation/cluster_and_service_configuration.rst

-.. literalinclude:: ../../salt/metalk8s/addons/ui/config/metalk8s-shell-ui-config.yaml.j2
-   :language: yaml
-   :lines: 3-


Why not keep this include? It's easier to keep things in-sync this way IMO.

I think keeping Jinja syntax in there isn't really a problem (it's somewhat readable), but if it is, we could use the .. jinja:: directive to render some of the templated values (such as the userGroupsMapping). Not really straightforward though, since I guess we want to keep a placeholder for the control plane IP.

I had some doc build issues linked to yaml highlighting. Maybe using .. jinja:: directive could help solves this, I will try it.

I think I will remove highlighting for now WDYT ?

gdemonet · 2021-03-24T09:54:12Z

salt/metalk8s/addons/ui/config/metalk8s-shell-ui-config.yaml.j2

-    "admin@metalk8s.invalid":
-      - admin
+{%- for user in dex.spec.config.staticPasswords | map(attribute='email') %}
+    "{{ user }}": [admin]


So, IIUC, in practice all Dex static users have the admin group mapped. Do we want another PR to filter MetalK8s pages based on this group? Also, to keep things clean, I would name this group metalk8s:admin (it's configurable anyway).

bert-e · 2021-03-24T09:59:20Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following reviewers are expecting changes from the author, or must review again:

@gdemonet

The following options are set: after_pull_request

bert-e · 2021-03-24T15:40:30Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
one peer

Peer approvals must include at least 1 approval from the following list:

The following options are set: after_pull_request

JBWatenbergScality · 2021-03-24T16:02:43Z

/approve

bert-e · 2021-03-24T16:02:53Z

In the queue

The changeset has received all authorizations and has been added to the
relevant queue(s). The queue(s) will be merged in the target development
branch(es) as soon as builds have passed.

The changeset will be merged in:

✔️ development/2.9

The following branches will NOT be impacted:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

There is no action required on your side. You will be notified here once
the changeset has been merged. In the unlikely event that the changeset
fails permanently on the queue, a member of the admin team will
contact you to help resolve the matter.

IMPORTANT

Please do not attempt to modify this pull request.

Any commit you add on the source branch will trigger a new cycle after the
current queue is merged.
Any commit you add on one of the integration branches will be lost.

If you need this pull request to be removed from the queue, please contact a
member of the admin team now.

The following options are set: approve, after_pull_request

bert-e · 2021-03-25T13:21:44Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/2.9

The following branches have NOT changed:

development/1.0
development/1.1
development/1.2
development/1.3
development/2.0
development/2.1
development/2.2
development/2.3
development/2.4
development/2.5
development/2.6
development/2.7
development/2.8

Please check the status of the associated issue None.

Goodbye jbwatenbergscality.

JBWatenbergScality changed the base branch from development/2.9 to improvement/use-csc-framework-for-ui-configuration March 11, 2021 17:02

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch from e6ab115 to 480e0d9 Compare March 12, 2021 09:13

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 8b4553f to 1b413e8 Compare March 12, 2021 09:17

JBWatenbergScality marked this pull request as ready for review March 12, 2021 09:23

JBWatenbergScality requested a review from a team as a code owner March 12, 2021 09:23

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch from 480e0d9 to df63277 Compare March 12, 2021 09:36

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 1b413e8 to abb9dec Compare March 12, 2021 09:37

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch from df63277 to 04a7330 Compare March 15, 2021 11:35

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch 2 times, most recently from a57af98 to b2d7dc9 Compare March 15, 2021 15:46

gdemonet reviewed Mar 16, 2021

View reviewed changes

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch from 4c698b5 to 4faa805 Compare March 16, 2021 16:31

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from b2d7dc9 to 6b9a18b Compare March 16, 2021 16:48

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch from 4faa805 to a269acb Compare March 17, 2021 08:25

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 6b9a18b to 8f49d08 Compare March 17, 2021 09:00

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch 9 times, most recently from 40f51a0 to 8b33873 Compare March 18, 2021 12:57

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 8f49d08 to 4fdd9b4 Compare March 18, 2021 15:13

JBWatenbergScality force-pushed the improvement/use-csc-framework-for-ui-configuration branch 2 times, most recently from 9ec3291 to dc7625a Compare March 19, 2021 17:37

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch 2 times, most recently from f6f77c4 to c289959 Compare March 22, 2021 12:07

JBWatenbergScality mentioned this pull request Mar 22, 2021

salt: Use csc framework for UI configuration #3181

Merged

3 tasks

Base automatically changed from improvement/use-csc-framework-for-ui-configuration to development/2.9 March 22, 2021 13:51

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch 3 times, most recently from 505475e to 7abbd99 Compare March 23, 2021 08:26

shell-ui: Add ability to configure user group mapping

0f75454

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 7abbd99 to 6735163 Compare March 24, 2021 09:00

gdemonet suggested changes Mar 24, 2021

View reviewed changes

JBWatenbergScality added 3 commits March 24, 2021 12:26

salt: Sync DEX static users with shell ui static group mapping

a07beb4

doc: Add steps to apply user modifications in Dex

231ca9a

doc: Fix cluster and service configuration formating

0c9d5d1

JBWatenbergScality force-pushed the improvement/enable-users-group-mapping-in-shell-ui-config branch from 6735163 to 0c9d5d1 Compare March 24, 2021 11:31

gdemonet approved these changes Mar 24, 2021

View reviewed changes

bert-e merged commit aa097f2 into development/2.9 Mar 25, 2021

bert-e deleted the improvement/enable-users-group-mapping-in-shell-ui-config branch March 25, 2021 13:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

shell-ui : Add users group mapping in shell UI config #3196

shell-ui : Add users group mapping in shell UI config #3196

JBWatenbergScality commented Mar 11, 2021 •

edited

Loading

bert-e commented Mar 11, 2021

bert-e commented Mar 11, 2021

JBWatenbergScality commented Mar 12, 2021

gdemonet left a comment

gdemonet Mar 16, 2021

gdemonet Mar 16, 2021

gdemonet Mar 22, 2021

bert-e commented Mar 22, 2021

gdemonet Mar 24, 2021

gdemonet Mar 24, 2021

JBWatenbergScality Mar 24, 2021

JBWatenbergScality Mar 24, 2021

gdemonet Mar 24, 2021

bert-e commented Mar 24, 2021

bert-e commented Mar 24, 2021

JBWatenbergScality commented Mar 24, 2021

bert-e commented Mar 24, 2021

bert-e commented Mar 25, 2021

		@@ -21,6 +21,8 @@ export type MenuItems = {[path: string]: TranslationAndGroups }

		export type Options = { main: MenuItems, subLogin: MenuItems };

		export type UserGroupsMapping = {[email: string]: string[]};

shell-ui : Add users group mapping in shell UI config #3196

shell-ui : Add users group mapping in shell UI config #3196

Conversation

JBWatenbergScality commented Mar 11, 2021 • edited Loading

bert-e commented Mar 11, 2021

Hello jbwatenbergscality,

bert-e commented Mar 11, 2021

Waiting for approval

JBWatenbergScality commented Mar 12, 2021

gdemonet 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

bert-e commented Mar 22, 2021

Waiting for approval

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

bert-e commented Mar 24, 2021

Waiting for approval

bert-e commented Mar 24, 2021

Waiting for approval

JBWatenbergScality commented Mar 24, 2021

bert-e commented Mar 24, 2021

In the queue

bert-e commented Mar 25, 2021

JBWatenbergScality commented Mar 11, 2021 •

edited

Loading