Discussion:
[PR PATCH] acess based share enum: handle permission set in configuration files
(too old to reply)
g***@samba.org
2016-02-24 13:02:29 UTC
Permalink
There is a new pull request by bud4 against master on the Samba Samba Github repository

https://github.com/bud4/samba master
https://github.com/samba-team/samba/pull/54

acess based share enum: handle permission set in configuration files
** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.

Signed-off-by: Alberto Maria Fiaschi <***@estar.toscana.it>

A patch file from https://github.com/samba-team/samba/pull/54.patch is attached
g***@samba.org
2016-02-28 12:30:07 UTC
Permalink
New comment by urisimchoni on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-189857051
Comment:
Hi Alberto,
Thanks for submitting this fix.
Some comments:

1. The checks for lp_valid_users and lp_invalid_users seem redundant - they are being done inside user_ok_token() without extra cost, it also seems that the proposed patch misses lp_only_user() this way.

2. The "username" parameter to user_ok_token() needs to be session_info->unix_info->unix_name (that's what you exchange for %U)

3. We need a bug in bugzilla and a BUG: line pointing to it in the commit message (see other commit messages in git log)

4. Please consult README.Coding - surround if blocks with curly braces. The preferred coding style also prefers multiple simple if statements over a complex condition, if possible. Something like:
```c
if (!user_ok_token(...)) {
return false;
}

return share_access_check(...);
```

Could you resubmit with those changes? I can later throw in some test
g***@samba.org
2016-02-29 10:38:35 UTC
Permalink
New comment by bud4 on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-190147183
Post by g***@samba.org
Hi Alberto,
Thanks for submitting this fix.
1.
The checks for lp_valid_users and lp_invalid_users seem redundant -
they are being done inside user_ok_token() without extra cost, it also
seems that the proposed patch misses lp_only_user() this way.
sorry I did not see that the user_ok_token function returns true if in
the configuration file is not set any permission.
Post by g***@samba.org
1.
The "username" parameter to user_ok_token() needs to be
session_info->unix_info->unix_name (that's what you exchange for %U)
ok
1.
We need a bug in bugzilla and a BUG: line pointing to it in the commit
message (see other commit messages in git log)
ok if the bug is not present in bugzilla then I'll create it
1.
Please consult README.Coding - surround if blocks with curly braces.
The preferred coding style also prefers multiple simple if statements over
if (!user_ok_token(...)) {
return false;
}
return share_access_check(...);
ok i will do it as soon as I can...
Bye
Alberto

Could you resubmit with those changes? I can later throw in some tests to
Post by g***@samba.org
verify correct behavior.
Thanks,
Uri
—
Reply to this email directly or view it on GitHub
<https://github.com/samba-team/samba/pull/54#issuecomment-189857051>.
g***@samba.org
2016-02-29 13:42:47 UTC
Permalink
There is an updated pull request by bud4 against master on the Samba Samba Github repository

https://github.com/bud4/samba master
https://github.com/samba-team/samba/pull/54

acess based share enum: handle permission set in configuration files
** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.

Signed-off-by: Alberto Maria Fiaschi <***@estar.toscana.it>

A patch file from https://github.com/samba-team/samba/pull/54.patch is attached
g***@samba.org
2016-02-29 14:24:06 UTC
Permalink
There is an updated pull request by bud4 against master on the Samba Samba Github repository

https://github.com/bud4/samba master
https://github.com/samba-team/samba/pull/54

acess based share enum: handle permission set in configuration files
** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.

Signed-off-by: Alberto Maria Fiaschi <***@estar.toscana.it>

A patch file from https://github.com/samba-team/samba/pull/54.patch is attached
g***@samba.org
2016-02-29 15:23:53 UTC
Permalink
There is an updated pull request by bud4 against master on the Samba Samba Github repository

https://github.com/bud4/samba master
https://github.com/samba-team/samba/pull/54

acess based share enum: handle permission set in configuration files
** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.

Signed-off-by: Alberto Maria Fiaschi <***@estar.toscana.it>

A patch file from https://github.com/samba-team/samba/pull/54.patch is attached
Uri Simchoni
2016-03-01 04:05:34 UTC
Permalink
Here's an updated patch with cleaned-up indentation, and a blackbox test.

Alberto - please confirm that it's OK.

Can I get another team reviewer?

Thanks,
Uri.
Post by g***@samba.org
There is an updated pull request by bud4 against master on the Samba Samba Github repository
https://github.com/bud4/samba master
https://github.com/samba-team/samba/pull/54
acess based share enum: handle permission set in configuration files
** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.
A patch file from https://github.com/samba-team/samba/pull/54.patch is attached
Andreas Schneider
2016-03-02 08:59:13 UTC
Permalink
Post by Uri Simchoni
Here's an updated patch with cleaned-up indentation, and a blackbox test.
Alberto - please confirm that it's OK.
Can I get another team reviewer?
RB+

Thanks for the test!
--
Andreas Schneider GPG-ID: CC014E3D
Samba Team ***@samba.org
www.samba.org
Uri Simchoni
2016-03-02 10:39:01 UTC
Permalink
Post by Andreas Schneider
Post by Uri Simchoni
Here's an updated patch with cleaned-up indentation, and a blackbox test.
Alberto - please confirm that it's OK.
Can I get another team reviewer?
RB+
Thanks for the test!
Pushed to autobuild after Alberto's OK.
Thanks,
Uri.
g***@samba.org
2016-03-01 21:46:01 UTC
Permalink
New comment by urisimchoni on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-190919761
Comment:
I replied in samba-technical list with patch that's essentially the same, save for coding style fixes, plus a blackbox test. Awaiting Alberto's approval a
g***@samba.org
2016-03-02 10:02:45 UTC
Permalink
New comment by bud4 on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-191166051
Comment:
for me ok
Alberto
Post by g***@samba.org
I replied in samba-technical list with patch that's essentially the same,
save for coding style fixes, plus a blackbox test. Awaiting Alberto's
approval and another reviewer.
—
Reply to this email directly or view it on GitHub
<https://github.com/samba-team/samba/pull/54#issuecomme
g***@samba.org
2016-03-03 05:39:03 UTC
Permalink
New comment by urisimchoni on Samba Github repository

https://github.com/samba-team/samba/pull/54#issuecomment-191591965
Comment:
Merged as 5036a0922b7890005bcc8b77368a6635c8ebeb4b (with some indentation / code style modificati
g***@samba.org
2016-03-03 05:39:03 UTC
Permalink
There's a closed pull request on the Samba Samba Github repository

acess based share enum: handle permission set in configuration files
https://github.com/samba-team/samba/pull/54
Description: ** access based share enum** not work with permission set in config file.
change function is_enumeration_allowed to check permissions set by
fields: valid users, invalid users, only user.

Signed-off-by: Alberto Maria Fiaschi <***@estar.toscana.
Loading...