mgr/smb: remove embedded join source and users and groups types

Drop the embedded versions of the join source and usrers and groups
configs from the cluster resource type. This means that potentially
sensitive information is *always* stored in a separate resource
from the less-sensitive general cluster config. Future changes can
then make security related choices about those specific resources
without worrying about having embedded variants inside the cluster.

For convenience I added a new "emtpy" users and groups source type that
can be used for testing. It defines *no* users and groups (an empty set
basically). This wouldn't be useful in a production scenario today as it
would deploy an smbd that you couldn't log into. But for unit testing
it's extremely handy, and maybe it can be used for integration testing
too.

This is a large change because lots of unit tests relied on embedded
join auths or users-and-groups. I tried to handle all the breaking
changes in one go for bisect-ability.

Signed-off-by: John Mulligan <jmulligan@redhat.com>

diff --git a/src/pybind/mgr/smb/enums.py b/src/pybind/mgr/smb/enums.py
index 6e19c882dad469d59577b9403b3eceba899bf8ff..92b8705ebbab7f1b26deb2aa7f572a6d41f5f7ee 100644 (file)
--- a/src/pybind/mgr/smb/enums.py
+++ b/src/pybind/mgr/smb/enums.py
@@ -41,15 +41,12 @@ class AuthMode(_StrEnum):
 
 
 class JoinSourceType(_StrEnum):
-    PASSWORD = 'password'
-    HTTP_URI = 'http_uri'
     RESOURCE = 'resource'
 
 
 class UserGroupSourceType(_StrEnum):
-    INLINE = 'inline'
-    HTTP_URI = 'http_uri'
     RESOURCE = 'resource'
+    EMPTY = 'empty'
 
 
 class ConfigNS(_StrEnum):

diff --git a/src/pybind/mgr/smb/handler.py b/src/pybind/mgr/smb/handler.py
index ab97bc1248f6bd72ec2a0270341e9c0e27ec7887..9931fe0a688353a49cfc31ecd1b62f27dbe3ca10 100644 (file)
--- a/src/pybind/mgr/smb/handler.py
+++ b/src/pybind/mgr/smb/handler.py
@@ -1099,8 +1099,6 @@ def _save_pending_join_auths(
     for idx, src in enumerate(checked(cluster.domain_settings).join_sources):
         if src.source_type == JoinSourceType.RESOURCE:
             javalues = checked(arefs[src.ref].auth)
-        elif src.source_type == JoinSourceType.PASSWORD:
-            javalues = checked(src.auth)
         else:
             raise ValueError(
                 f'unsupported join source type: {src.source_type}'
@@ -1124,9 +1122,8 @@ def _save_pending_users_and_groups(
         if ugsv.source_type == UserGroupSourceType.RESOURCE:
             ugvalues = augs[ugsv.ref].values
             assert ugvalues
-        elif ugsv.source_type == UserGroupSourceType.INLINE:
-            ugvalues = ugsv.values
-            assert ugvalues
+        elif ugsv.source_type == UserGroupSourceType.EMPTY:
+            continue
         else:
             raise ValueError(
                 f'unsupported users/groups source type: {ugsv.source_type}'

diff --git a/src/pybind/mgr/smb/resources.py b/src/pybind/mgr/smb/resources.py
index bd5ca87812159745edf5821849b4b8d12c69a19a..1a40076538ee49c2c8e8aeda921a58b3b9632ed8 100644 (file)
--- a/src/pybind/mgr/smb/resources.py
+++ b/src/pybind/mgr/smb/resources.py
@@ -162,18 +162,17 @@ class JoinAuthValues(_RBase):
 class JoinSource(_RBase):
     """Represents data that can be used to join a system to Active Directory."""
 
-    source_type: JoinSourceType
-    auth: Optional[JoinAuthValues] = None
-    uri: str = ''
+    source_type: JoinSourceType = JoinSourceType.RESOURCE
     ref: str = ''
 
     def validate(self) -> None:
-        if self.ref:
+        if not self.ref:
+            raise ValueError('reference value must be specified')
+        else:
             validation.check_id(self.ref)
 
     @resourcelib.customize
     def _customize_resource(rc: resourcelib.Resource) -> resourcelib.Resource:
-        rc.uri.quiet = True
         rc.ref.quiet = True
         return rc
 
@@ -190,40 +189,21 @@ class UserGroupSettings(_RBase):
 class UserGroupSource(_RBase):
     """Represents data used to set up user/group settings for an instance."""
 
-    source_type: UserGroupSourceType
-    values: Optional[UserGroupSettings] = None
-    uri: str = ''
+    source_type: UserGroupSourceType = UserGroupSourceType.RESOURCE
     ref: str = ''
 
     def validate(self) -> None:
-        if self.source_type == UserGroupSourceType.INLINE:
-            pfx = 'inline User/Group configuration'
-            if self.values is None:
-                raise ValueError(pfx + ' requires values')
-            if self.uri:
-                raise ValueError(pfx + ' does not take a uri')
-            if self.ref:
-                raise ValueError(pfx + ' does not take a ref value')
-        if self.source_type == UserGroupSourceType.HTTP_URI:
-            pfx = 'http User/Group configuration'
-            if not self.uri:
-                raise ValueError(pfx + ' requires a uri')
-            if self.values:
-                raise ValueError(pfx + ' does not take inline values')
-            if self.ref:
-                raise ValueError(pfx + ' does not take a ref value')
         if self.source_type == UserGroupSourceType.RESOURCE:
-            pfx = 'resource reference User/Group configuration'
             if not self.ref:
-                raise ValueError(pfx + ' requires a ref value')
-            if self.uri:
-                raise ValueError(pfx + ' does not take a uri')
-            if self.values:
-                raise ValueError(pfx + ' does not take inline values')
+                raise ValueError('reference value must be specified')
+            else:
+                validation.check_id(self.ref)
+        else:
+            if self.ref:
+                raise ValueError('ref may not be specified')
 
     @resourcelib.customize
     def _customize_resource(rc: resourcelib.Resource) -> resourcelib.Resource:
-        rc.uri.quiet = True
         rc.ref.quiet = True
         return rc
 

diff --git a/src/pybind/mgr/smb/tests/test_enums.py b/src/pybind/mgr/smb/tests/test_enums.py
index f3f0f4eeb8b06a53d200ad789abc103017fdfdc5..ef0edf87acb9a27982661445f7f4626299c8aaaa 100644 (file)
--- a/src/pybind/mgr/smb/tests/test_enums.py
+++ b/src/pybind/mgr/smb/tests/test_enums.py
@@ -18,8 +18,6 @@ import smb.enums
         (smb.enums.State.UPDATED, "updated"),
         (smb.enums.AuthMode.USER, "user"),
         (smb.enums.AuthMode.ACTIVE_DIRECTORY, "active-directory"),
-        (smb.enums.JoinSourceType.PASSWORD, "password"),
-        (smb.enums.UserGroupSourceType.INLINE, "inline"),
     ],
 )
 def test_stringified(value, strval):

diff --git a/src/pybind/mgr/smb/tests/test_handler.py b/src/pybind/mgr/smb/tests/test_handler.py
index 1a22dbb6e15321e40481dd7e0112cf0c12289da4..ceaf044744d6490ec3496a70de14372205e710f5 100644 (file)
--- a/src/pybind/mgr/smb/tests/test_handler.py
+++ b/src/pybind/mgr/smb/tests/test_handler.py
@@ -31,11 +31,7 @@ def test_internal_apply_cluster(thandler):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -50,11 +46,7 @@ def test_cluster_add(thandler):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -72,11 +64,7 @@ def test_internal_apply_cluster_and_share(thandler):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -109,8 +97,7 @@ def test_internal_apply_remove_cluster(thandler):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -141,8 +128,7 @@ def test_internal_apply_remove_shares(thandler):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             },
@@ -222,8 +208,7 @@ def test_internal_apply_add_joinauth(thandler):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -254,8 +239,7 @@ def test_internal_apply_add_usergroups(thandler):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -286,8 +270,7 @@ def test_generate_config_basic(thandler):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             },
@@ -338,15 +321,21 @@ def test_generate_config_ad(thandler):
                     'realm': 'dom1.example.com',
                     'join_sources': [
                         {
-                            'source_type': 'password',
-                            'auth': {
-                                'username': 'testadmin',
-                                'password': 'Passw0rd',
-                            },
+                            'source_type': 'resource',
+                            'ref': 'foo1',
                         }
                     ],
                 },
             },
+            'join_auths.foo1': {
+                'resource_type': 'ceph.smb.join.auth',
+                'auth_id': 'foo1',
+                'intent': 'present',
+                'auth': {
+                    'username': 'testadmin',
+                    'password': 'Passw0rd',
+                },
+            },
             'shares.foo.s1': {
                 'resource_type': 'ceph.smb.share',
                 'cluster_id': 'foo',
@@ -566,52 +555,6 @@ def test_apply_update_password(thandler):
     assert jdata == {'username': 'testadmin', 'password': 'Zm9vYmFyCg'}
 
 
-def test_apply_update_cluster_inline_pw(thandler):
-    test_apply_full_cluster_create(thandler)
-    to_apply = [
-        smb.resources.Cluster(
-            cluster_id='mycluster1',
-            auth_mode=smb.enums.AuthMode.ACTIVE_DIRECTORY,
-            domain_settings=smb.resources.DomainSettings(
-                realm='MYDOMAIN.EXAMPLE.ORG',
-                join_sources=[
-                    smb.resources.JoinSource(
-                        source_type=smb.enums.JoinSourceType.RESOURCE,
-                        ref='join1',
-                    ),
-                    smb.resources.JoinSource(
-                        source_type=smb.enums.JoinSourceType.PASSWORD,
-                        auth=smb.resources.JoinAuthValues(
-                            username='Jimmy',
-                            password='j4mb0ree!',
-                        ),
-                    ),
-                ],
-            ),
-        ),
-    ]
-
-    results = thandler.apply(to_apply)
-    assert results.success, results.to_simplified()
-    assert len(list(results)) == 1
-
-    assert 'mycluster1' in thandler.public_store.namespaces()
-    ekeys = list(thandler.public_store.contents('mycluster1'))
-    assert len(ekeys) == 5
-    assert 'cluster-info' in ekeys
-    assert 'config.smb' in ekeys
-    assert 'spec.smb' in ekeys
-    assert 'join.0.json' in ekeys
-    assert 'join.1.json' in ekeys
-
-    # we changed the password value. the store should reflect that
-    jdata = thandler.public_store['mycluster1', 'join.0.json'].get()
-    assert jdata == {'username': 'testadmin', 'password': 'Passw0rd'}
-    # we changed the password value. the store should reflect that
-    jdata2 = thandler.public_store['mycluster1', 'join.1.json'].get()
-    assert jdata2 == {'username': 'Jimmy', 'password': 'j4mb0ree!'}
-
-
 def test_apply_add_second_cluster(thandler):
     test_apply_full_cluster_create(thandler)
     to_apply = [
@@ -622,15 +565,20 @@ def test_apply_add_second_cluster(thandler):
                 realm='YOURDOMAIN.EXAMPLE.ORG',
                 join_sources=[
                     smb.resources.JoinSource(
-                        source_type=smb.enums.JoinSourceType.PASSWORD,
-                        auth=smb.resources.JoinAuthValues(
-                            username='Jimmy',
-                            password='j4mb0ree!',
-                        ),
+                        source_type=smb.enums.JoinSourceType.RESOURCE,
+                        ref='coolcluster',
                     ),
                 ],
             ),
         ),
+        smb.resources.JoinAuth(
+            auth_id='coolcluster',
+            auth=smb.resources.JoinAuthValues(
+                username='Jimmy',
+                password='j4mb0ree!',
+            ),
+            linked_to_cluster='coolcluster',
+        ),
         smb.resources.Share(
             cluster_id='coolcluster',
             share_id='images',
@@ -643,7 +591,7 @@ def test_apply_add_second_cluster(thandler):
 
     results = thandler.apply(to_apply)
     assert results.success, results.to_simplified()
-    assert len(list(results)) == 2
+    assert len(list(results)) == 3
 
     assert 'mycluster1' in thandler.public_store.namespaces()
     ekeys = list(thandler.public_store.contents('mycluster1'))
@@ -865,13 +813,14 @@ def test_apply_remove_all_clusters(thandler):
 def test_all_resources(thandler):
     test_apply_add_second_cluster(thandler)
     rall = thandler.all_resources()
-    assert len(rall) == 6
+    assert len(rall) == 7
     assert rall[0].resource_type == 'ceph.smb.cluster'
     assert rall[1].resource_type == 'ceph.smb.share'
     assert rall[2].resource_type == 'ceph.smb.share'
     assert rall[3].resource_type == 'ceph.smb.cluster'
     assert rall[4].resource_type == 'ceph.smb.share'
     assert rall[5].resource_type == 'ceph.smb.join.auth'
+    assert rall[6].resource_type == 'ceph.smb.join.auth'
 
 
 @pytest.mark.parametrize(
@@ -962,6 +911,10 @@ def test_all_resources(thandler):
                     'resource_type': 'ceph.smb.join.auth',
                     'auth_id': 'join1',
                 },
+                {
+                    'resource_type': 'ceph.smb.join.auth',
+                    'auth_id': 'coolcluster',
+                },
             ],
         ),
         # cluster with id

diff --git a/src/pybind/mgr/smb/tests/test_resources.py b/src/pybind/mgr/smb/tests/test_resources.py
index 6fce09c2698555224063dc39525b5028649bac68..82446876a7c71d9dd4b5432dd2d45c1363ebefb7 100644 (file)
--- a/src/pybind/mgr/smb/tests/test_resources.py
+++ b/src/pybind/mgr/smb/tests/test_resources.py
@@ -117,10 +117,6 @@ domain_settings:
   join_sources:
     - source_type: resource
       ref: bob
-    - source_type: password
-      auth:
-        username: Administrator
-        password: fallb4kP4ssw0rd
 ---
 resource_type: ceph.smb.share
 cluster_id: chacha
@@ -168,13 +164,10 @@ def test_load_yaml_resource_yaml1():
     assert cluster.intent == enums.Intent.PRESENT
     assert cluster.auth_mode == enums.AuthMode.ACTIVE_DIRECTORY
     assert cluster.domain_settings.realm == 'CEPH.SINK.TEST'
-    assert len(cluster.domain_settings.join_sources) == 2
+    assert len(cluster.domain_settings.join_sources) == 1
     jsrc = cluster.domain_settings.join_sources
     assert jsrc[0].source_type == enums.JoinSourceType.RESOURCE
     assert jsrc[0].ref == 'bob'
-    assert jsrc[1].source_type == enums.JoinSourceType.PASSWORD
-    assert jsrc[1].auth.username == 'Administrator'
-    assert jsrc[1].auth.password == 'fallb4kP4ssw0rd'
 
     assert isinstance(loaded[1], smb.resources.Share)
     assert isinstance(loaded[2], smb.resources.Share)
@@ -427,7 +420,7 @@ domain_settings:
             "exc_type": ValueError,
             "error": "not supported",
         },
-        # u/g inline missing
+        # u/g empty with extra ref
         {
             "yaml": """
 resource_type: ceph.smb.cluster
@@ -435,89 +428,11 @@ cluster_id: randolph
 intent: present
 auth_mode: user
 user_group_settings:
-  - source_type: inline
-""",
-            "exc_type": ValueError,
-            "error": "requires values",
-        },
-        # u/g inline extra uri
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: inline
-    values:
-      users: []
-      groups: []
-    uri: http://foo.bar.example.com/baz.txt
-""",
-            "exc_type": ValueError,
-            "error": "does not take",
-        },
-        # u/g inline extra ref
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: inline
-    values:
-      users: []
-      groups: []
+  - source_type: empty
     ref: xyz
 """,
             "exc_type": ValueError,
-            "error": "does not take",
-        },
-        # u/g uri missing
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: http_uri
-""",
-            "exc_type": ValueError,
-            "error": "requires",
-        },
-        # u/g uri extra values
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: http_uri
-    values:
-      users: []
-      groups: []
-    uri: http://foo.bar.example.com/baz.txt
-""",
-            "exc_type": ValueError,
-            "error": "does not take",
-        },
-        # u/g uri extra ref
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: http_uri
-    uri: http://boop.example.net
-    ref: xyz
-""",
-            "exc_type": ValueError,
-            "error": "does not take",
+            "error": "ref may not be",
         },
         # u/g resource missing
         {
@@ -530,39 +445,7 @@ user_group_settings:
   - source_type: resource
 """,
             "exc_type": ValueError,
-            "error": "requires",
-        },
-        # u/g resource extra values
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: resource
-    ref: xyz
-    uri: http://example.net/foo
-""",
-            "exc_type": ValueError,
-            "error": "does not take",
-        },
-        # u/g resource extra resource
-        {
-            "yaml": """
-resource_type: ceph.smb.cluster
-cluster_id: randolph
-intent: present
-auth_mode: user
-user_group_settings:
-  - source_type: resource
-    ref: xyz
-    values:
-      users: []
-      groups: []
-""",
-            "exc_type": ValueError,
-            "error": "does not take",
+            "error": "reference value must be",
         },
     ],
 )

diff --git a/src/pybind/mgr/smb/tests/test_smb.py b/src/pybind/mgr/smb/tests/test_smb.py
index 09380cad869a7a26a8831f81879e6a3f3332e916..429a5cdec00233e63815d4ba41f828028f458034 100644 (file)
--- a/src/pybind/mgr/smb/tests/test_smb.py
+++ b/src/pybind/mgr/smb/tests/test_smb.py
@@ -39,11 +39,7 @@ def test_internal_apply_cluster(tmodule):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -58,11 +54,7 @@ def test_cluster_add_cluster_ls(tmodule):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -80,11 +72,7 @@ def test_internal_apply_cluster_and_share(tmodule):
         auth_mode=smb.enums.AuthMode.USER,
         user_group_settings=[
             smb.resources.UserGroupSource(
-                source_type=smb.resources.UserGroupSourceType.INLINE,
-                values=smb.resources.UserGroupSettings(
-                    users=[],
-                    groups=[],
-                ),
+                source_type=smb.resources.UserGroupSourceType.EMPTY,
             ),
         ],
     )
@@ -117,8 +105,7 @@ def test_internal_apply_remove_cluster(tmodule):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -149,8 +136,7 @@ def test_internal_apply_remove_shares(tmodule):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             },
@@ -230,8 +216,7 @@ def test_internal_apply_add_joinauth(tmodule):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -262,8 +247,7 @@ def test_internal_apply_add_usergroups(tmodule):
                 'intent': 'present',
                 'user_group_settings': [
                     {
-                        'source_type': 'inline',
-                        'values': {'users': [], 'groups': []},
+                        'source_type': 'empty',
                     }
                 ],
             }
@@ -296,15 +280,21 @@ def _example_cfg_1(tmodule):
                     'realm': 'dom1.example.com',
                     'join_sources': [
                         {
-                            'source_type': 'password',
-                            'auth': {
-                                'username': 'testadmin',
-                                'password': 'Passw0rd',
-                            },
+                            'source_type': 'resource',
+                            'ref': 'foo',
                         }
                     ],
                 },
             },
+            'join_auths.foo': {
+                'resource_type': 'ceph.smb.join.auth',
+                'auth_id': 'foo',
+                'intent': 'present',
+                'auth': {
+                    'username': 'testadmin',
+                    'password': 'Passw0rd',
+                },
+            },
             'shares.foo.s1': {
                 'resource_type': 'ceph.smb.share',
                 'cluster_id': 'foo',