Skip to content

Commit c6b0377

Browse files
phlogistonjohnphlogistonjohn
committed
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 <[email protected]>
1 parent 00f2f71 commit c6b0377

File tree

7 files changed

+73
-275
lines changed

7 files changed

+73
-275
lines changed

‎src/pybind/mgr/smb/enums.py‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,12 @@ class AuthMode(_StrEnum):
4141

4242

4343
class JoinSourceType(_StrEnum):
44-
PASSWORD = 'password'
45-
HTTP_URI = 'http_uri'
4644
RESOURCE = 'resource'
4745

4846

4947
class UserGroupSourceType(_StrEnum):
50-
INLINE = 'inline'
51-
HTTP_URI = 'http_uri'
5248
RESOURCE = 'resource'
49+
EMPTY = 'empty'
5350

5451

5552
class ConfigNS(_StrEnum):

‎src/pybind/mgr/smb/handler.py‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,8 +1099,6 @@ def _save_pending_join_auths(
10991099
for idx, src in enumerate(checked(cluster.domain_settings).join_sources):
11001100
if src.source_type == JoinSourceType.RESOURCE:
11011101
javalues = checked(arefs[src.ref].auth)
1102-
elif src.source_type == JoinSourceType.PASSWORD:
1103-
javalues = checked(src.auth)
11041102
else:
11051103
raise ValueError(
11061104
f'unsupported join source type: {src.source_type}'
@@ -1124,9 +1122,8 @@ def _save_pending_users_and_groups(
11241122
if ugsv.source_type == UserGroupSourceType.RESOURCE:
11251123
ugvalues = augs[ugsv.ref].values
11261124
assert ugvalues
1127-
elif ugsv.source_type == UserGroupSourceType.INLINE:
1128-
ugvalues = ugsv.values
1129-
assert ugvalues
1125+
elif ugsv.source_type == UserGroupSourceType.EMPTY:
1126+
continue
11301127
else:
11311128
raise ValueError(
11321129
f'unsupported users/groups source type: {ugsv.source_type}'

‎src/pybind/mgr/smb/resources.py‎

Lines changed: 11 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -162,18 +162,17 @@ class JoinAuthValues(_RBase):
162162
class JoinSource(_RBase):
163163
"""Represents data that can be used to join a system to Active Directory."""
164164

165-
source_type: JoinSourceType
166-
auth: Optional[JoinAuthValues] = None
167-
uri: str = ''
165+
source_type: JoinSourceType = JoinSourceType.RESOURCE
168166
ref: str = ''
169167

170168
def validate(self) -> None:
171-
if self.ref:
169+
if not self.ref:
170+
raise ValueError('reference value must be specified')
171+
else:
172172
validation.check_id(self.ref)
173173

174174
@resourcelib.customize
175175
def _customize_resource(rc: resourcelib.Resource) -> resourcelib.Resource:
176-
rc.uri.quiet = True
177176
rc.ref.quiet = True
178177
return rc
179178

@@ -190,40 +189,21 @@ class UserGroupSettings(_RBase):
190189
class UserGroupSource(_RBase):
191190
"""Represents data used to set up user/group settings for an instance."""
192191

193-
source_type: UserGroupSourceType
194-
values: Optional[UserGroupSettings] = None
195-
uri: str = ''
192+
source_type: UserGroupSourceType = UserGroupSourceType.RESOURCE
196193
ref: str = ''
197194

198195
def validate(self) -> None:
199-
if self.source_type == UserGroupSourceType.INLINE:
200-
pfx = 'inline User/Group configuration'
201-
if self.values is None:
202-
raise ValueError(pfx + ' requires values')
203-
if self.uri:
204-
raise ValueError(pfx + ' does not take a uri')
205-
if self.ref:
206-
raise ValueError(pfx + ' does not take a ref value')
207-
if self.source_type == UserGroupSourceType.HTTP_URI:
208-
pfx = 'http User/Group configuration'
209-
if not self.uri:
210-
raise ValueError(pfx + ' requires a uri')
211-
if self.values:
212-
raise ValueError(pfx + ' does not take inline values')
213-
if self.ref:
214-
raise ValueError(pfx + ' does not take a ref value')
215196
if self.source_type == UserGroupSourceType.RESOURCE:
216-
pfx = 'resource reference User/Group configuration'
217197
if not self.ref:
218-
raise ValueError(pfx + ' requires a ref value')
219-
if self.uri:
220-
raise ValueError(pfx + ' does not take a uri')
221-
if self.values:
222-
raise ValueError(pfx + ' does not take inline values')
198+
raise ValueError('reference value must be specified')
199+
else:
200+
validation.check_id(self.ref)
201+
else:
202+
if self.ref:
203+
raise ValueError('ref may not be specified')
223204

224205
@resourcelib.customize
225206
def _customize_resource(rc: resourcelib.Resource) -> resourcelib.Resource:
226-
rc.uri.quiet = True
227207
rc.ref.quiet = True
228208
return rc
229209

‎src/pybind/mgr/smb/tests/test_enums.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,6 @@
1818
(smb.enums.State.UPDATED, "updated"),
1919
(smb.enums.AuthMode.USER, "user"),
2020
(smb.enums.AuthMode.ACTIVE_DIRECTORY, "active-directory"),
21-
(smb.enums.JoinSourceType.PASSWORD, "password"),
22-
(smb.enums.UserGroupSourceType.INLINE, "inline"),
2321
],
2422
)
2523
def test_stringified(value, strval):

‎src/pybind/mgr/smb/tests/test_handler.py‎

Lines changed: 36 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -31,11 +31,7 @@ def test_internal_apply_cluster(thandler):
3131
auth_mode=smb.enums.AuthMode.USER,
3232
user_group_settings=[
3333
smb.resources.UserGroupSource(
34-
source_type=smb.resources.UserGroupSourceType.INLINE,
35-
values=smb.resources.UserGroupSettings(
36-
users=[],
37-
groups=[],
38-
),
34+
source_type=smb.resources.UserGroupSourceType.EMPTY,
3935
),
4036
],
4137
)
@@ -50,11 +46,7 @@ def test_cluster_add(thandler):
5046
auth_mode=smb.enums.AuthMode.USER,
5147
user_group_settings=[
5248
smb.resources.UserGroupSource(
53-
source_type=smb.resources.UserGroupSourceType.INLINE,
54-
values=smb.resources.UserGroupSettings(
55-
users=[],
56-
groups=[],
57-
),
49+
source_type=smb.resources.UserGroupSourceType.EMPTY,
5850
),
5951
],
6052
)
@@ -72,11 +64,7 @@ def test_internal_apply_cluster_and_share(thandler):
7264
auth_mode=smb.enums.AuthMode.USER,
7365
user_group_settings=[
7466
smb.resources.UserGroupSource(
75-
source_type=smb.resources.UserGroupSourceType.INLINE,
76-
values=smb.resources.UserGroupSettings(
77-
users=[],
78-
groups=[],
79-
),
67+
source_type=smb.resources.UserGroupSourceType.EMPTY,
8068
),
8169
],
8270
)
@@ -109,8 +97,7 @@ def test_internal_apply_remove_cluster(thandler):
10997
'intent': 'present',
11098
'user_group_settings': [
11199
{
112-
'source_type': 'inline',
113-
'values': {'users': [], 'groups': []},
100+
'source_type': 'empty',
114101
}
115102
],
116103
}
@@ -141,8 +128,7 @@ def test_internal_apply_remove_shares(thandler):
141128
'intent': 'present',
142129
'user_group_settings': [
143130
{
144-
'source_type': 'inline',
145-
'values': {'users': [], 'groups': []},
131+
'source_type': 'empty',
146132
}
147133
],
148134
},
@@ -222,8 +208,7 @@ def test_internal_apply_add_joinauth(thandler):
222208
'intent': 'present',
223209
'user_group_settings': [
224210
{
225-
'source_type': 'inline',
226-
'values': {'users': [], 'groups': []},
211+
'source_type': 'empty',
227212
}
228213
],
229214
}
@@ -254,8 +239,7 @@ def test_internal_apply_add_usergroups(thandler):
254239
'intent': 'present',
255240
'user_group_settings': [
256241
{
257-
'source_type': 'inline',
258-
'values': {'users': [], 'groups': []},
242+
'source_type': 'empty',
259243
}
260244
],
261245
}
@@ -286,8 +270,7 @@ def test_generate_config_basic(thandler):
286270
'intent': 'present',
287271
'user_group_settings': [
288272
{
289-
'source_type': 'inline',
290-
'values': {'users': [], 'groups': []},
273+
'source_type': 'empty',
291274
}
292275
],
293276
},
@@ -338,15 +321,21 @@ def test_generate_config_ad(thandler):
338321
'realm': 'dom1.example.com',
339322
'join_sources': [
340323
{
341-
'source_type': 'password',
342-
'auth': {
343-
'username': 'testadmin',
344-
'password': 'Passw0rd',
345-
},
324+
'source_type': 'resource',
325+
'ref': 'foo1',
346326
}
347327
],
348328
},
349329
},
330+
'join_auths.foo1': {
331+
'resource_type': 'ceph.smb.join.auth',
332+
'auth_id': 'foo1',
333+
'intent': 'present',
334+
'auth': {
335+
'username': 'testadmin',
336+
'password': 'Passw0rd',
337+
},
338+
},
350339
'shares.foo.s1': {
351340
'resource_type': 'ceph.smb.share',
352341
'cluster_id': 'foo',
@@ -566,52 +555,6 @@ def test_apply_update_password(thandler):
566555
assert jdata == {'username': 'testadmin', 'password': 'Zm9vYmFyCg'}
567556

568557

569-
def test_apply_update_cluster_inline_pw(thandler):
570-
test_apply_full_cluster_create(thandler)
571-
to_apply = [
572-
smb.resources.Cluster(
573-
cluster_id='mycluster1',
574-
auth_mode=smb.enums.AuthMode.ACTIVE_DIRECTORY,
575-
domain_settings=smb.resources.DomainSettings(
576-
realm='MYDOMAIN.EXAMPLE.ORG',
577-
join_sources=[
578-
smb.resources.JoinSource(
579-
source_type=smb.enums.JoinSourceType.RESOURCE,
580-
ref='join1',
581-
),
582-
smb.resources.JoinSource(
583-
source_type=smb.enums.JoinSourceType.PASSWORD,
584-
auth=smb.resources.JoinAuthValues(
585-
username='Jimmy',
586-
password='j4mb0ree!',
587-
),
588-
),
589-
],
590-
),
591-
),
592-
]
593-
594-
results = thandler.apply(to_apply)
595-
assert results.success, results.to_simplified()
596-
assert len(list(results)) == 1
597-
598-
assert 'mycluster1' in thandler.public_store.namespaces()
599-
ekeys = list(thandler.public_store.contents('mycluster1'))
600-
assert len(ekeys) == 5
601-
assert 'cluster-info' in ekeys
602-
assert 'config.smb' in ekeys
603-
assert 'spec.smb' in ekeys
604-
assert 'join.0.json' in ekeys
605-
assert 'join.1.json' in ekeys
606-
607-
# we changed the password value. the store should reflect that
608-
jdata = thandler.public_store['mycluster1', 'join.0.json'].get()
609-
assert jdata == {'username': 'testadmin', 'password': 'Passw0rd'}
610-
# we changed the password value. the store should reflect that
611-
jdata2 = thandler.public_store['mycluster1', 'join.1.json'].get()
612-
assert jdata2 == {'username': 'Jimmy', 'password': 'j4mb0ree!'}
613-
614-
615558
def test_apply_add_second_cluster(thandler):
616559
test_apply_full_cluster_create(thandler)
617560
to_apply = [
@@ -622,15 +565,20 @@ def test_apply_add_second_cluster(thandler):
622565
realm='YOURDOMAIN.EXAMPLE.ORG',
623566
join_sources=[
624567
smb.resources.JoinSource(
625-
source_type=smb.enums.JoinSourceType.PASSWORD,
626-
auth=smb.resources.JoinAuthValues(
627-
username='Jimmy',
628-
password='j4mb0ree!',
629-
),
568+
source_type=smb.enums.JoinSourceType.RESOURCE,
569+
ref='coolcluster',
630570
),
631571
],
632572
),
633573
),
574+
smb.resources.JoinAuth(
575+
auth_id='coolcluster',
576+
auth=smb.resources.JoinAuthValues(
577+
username='Jimmy',
578+
password='j4mb0ree!',
579+
),
580+
linked_to_cluster='coolcluster',
581+
),
634582
smb.resources.Share(
635583
cluster_id='coolcluster',
636584
share_id='images',
@@ -643,7 +591,7 @@ def test_apply_add_second_cluster(thandler):
643591

644592
results = thandler.apply(to_apply)
645593
assert results.success, results.to_simplified()
646-
assert len(list(results)) == 2
594+
assert len(list(results)) == 3
647595

648596
assert 'mycluster1' in thandler.public_store.namespaces()
649597
ekeys = list(thandler.public_store.contents('mycluster1'))
@@ -865,13 +813,14 @@ def remove_smb_service(self, service_name):
865813
def test_all_resources(thandler):
866814
test_apply_add_second_cluster(thandler)
867815
rall = thandler.all_resources()
868-
assert len(rall) == 6
816+
assert len(rall) == 7
869817
assert rall[0].resource_type == 'ceph.smb.cluster'
870818
assert rall[1].resource_type == 'ceph.smb.share'
871819
assert rall[2].resource_type == 'ceph.smb.share'
872820
assert rall[3].resource_type == 'ceph.smb.cluster'
873821
assert rall[4].resource_type == 'ceph.smb.share'
874822
assert rall[5].resource_type == 'ceph.smb.join.auth'
823+
assert rall[6].resource_type == 'ceph.smb.join.auth'
875824

876825

877826
@pytest.mark.parametrize(
@@ -962,6 +911,10 @@ def test_all_resources(thandler):
962911
'resource_type': 'ceph.smb.join.auth',
963912
'auth_id': 'join1',
964913
},
914+
{
915+
'resource_type': 'ceph.smb.join.auth',
916+
'auth_id': 'coolcluster',
917+
},
965918
],
966919
),
967920
# cluster with id

0 commit comments

Comments
 (0)