Discussion:
[PATCH] Password Settings Object (PSO) support for Samba
William Brown via samba-technical
2018-05-14 02:59:14 UTC
Permalink
Attached is a rough set of patches. I still need to tidy a few things
up, get some clean autobuilds, and investigate if there's any
http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/tim
-pso
I think the main thing I'd say is that the netcmd.pso is the wrong
place. Either it belongs in domain.py, or it needs to become:

domain/
__init__.py
pso.py

Because of the hierarchy of the commands here.
Let me know if anyone has any feedback on the changes at this point.
Other wise, it's a really long patch set, but a skim read looks pretty
reasonable.
Thanks,
Tim
Tim Beale via samba-technical
2018-05-15 23:28:54 UTC
Permalink
Hi William,

Thanks for the feedback. That's a good point about the netcmd code layout.

Given that the new PSO samba-tool code is pretty self-contained, and
domain.py is already 4000+ lines long, I think it'd make sense to keep
it as a separate file.

However, one problem is the python import won't like having both a
domain.py file and a domain/ sub-directory. So are you suggesting that I
move netcmd/domain.py --> netcmd/domain/__init__.py? I just thought I'd
double-check, as that might have ramifications for other people with
back-porting patches, or for work in progress changes, etc.

I guess another option might be to have a more generically named
sub-directory, e.g.

netcmd/
    domain.py
    subcmd/
        __init__.py
        pso.py

The python import would then look something like: from
samba.netcmd.subcmd.pso import cmd_domain_passwordsettings_pso

Let me know if you (or anyone else) have any thoughts on this.

Thanks,
Tim
Post by William Brown via samba-technical
Attached is a rough set of patches. I still need to tidy a few things
up, get some clean autobuilds, and investigate if there's any
http://git.catalyst.net.nz/gw?p=samba.git;a=shortlog;h=refs/heads/tim
-pso
I think the main thing I'd say is that the netcmd.pso is the wrong
domain/
__init__.py
pso.py
Because of the hierarchy of the commands here.
Let me know if anyone has any feedback on the changes at this point.
Other wise, it's a really long patch set, but a skim read looks pretty
reasonable.
Thanks,
Tim
Andrew Bartlett via samba-technical
2018-05-16 07:50:47 UTC
Permalink
Post by Tim Beale via samba-technical
Hi William,
Thanks for the feedback. That's a good point about the netcmd code layout.
Given that the new PSO samba-tool code is pretty self-contained, and
domain.py is already 4000+ lines long, I think it'd make sense to keep
it as a separate file.
However, one problem is the python import won't like having both a
domain.py file and a domain/ sub-directory. So are you suggesting that I
move netcmd/domain.py --> netcmd/domain/__init__.py? I just thought I'd
double-check, as that might have ramifications for other people with
back-porting patches, or for work in progress changes, etc.
I think this (the rename of domain.py -> domain/__init__.py) is the
most resaonble, but once we land the backup changes so as not to make
that any harder than is already is.

Thanks,

Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
William Brown via samba-technical
2018-05-16 07:58:23 UTC
Permalink
Post by Andrew Bartlett via samba-technical
Post by Tim Beale via samba-technical
Hi William,
Thanks for the feedback. That's a good point about the netcmd code layout.
Given that the new PSO samba-tool code is pretty self-contained, and
domain.py is already 4000+ lines long, I think it'd make sense to keep
it as a separate file.
However, one problem is the python import won't like having both a
domain.py file and a domain/ sub-directory. So are you suggesting that I
move netcmd/domain.py --> netcmd/domain/__init__.py? I just thought I'd
double-check, as that might have ramifications for other people with
back-porting patches, or for work in progress changes, etc.
I think this (the rename of domain.py -> domain/__init__.py) is the
most resaonble, but once we land the backup changes so as not to make
that any harder than is already is.
This is what I meant :)

I have some changes to domain.py too I think in my recent patchsets ...
Post by Andrew Bartlett via samba-technical
Thanks,
Andrew Bartlett
Andrew Bartlett via samba-technical
2018-05-16 08:52:25 UTC
Permalink
Hi,
I'm currently working on adding support to Samba for PSOs (Password
Settings Objects). This AD feature is also known as Fine-Grained
Password Policies (FGPP).
Thanks Tim. I'm following your current branch with interest.

Thanks in particular for looking into the performance issues and
addressing them so that if there is no PSO we avoid the group recursion
cost.

I'll look over the patches you have here tomorrow and see what more we
can get into master:
https://gitlab.com/catalyst-samba/samba/commits/tim-pso

Thanks,

Andrew Bartlett
--
Andrew Bartlett http://samba.org/~abartlet/
Authentication Developer, Samba Team http://samba.org
Samba Developer, Catalyst IT http://catalyst.net.nz/services/samba
Loading...