On 05/26/2014 12:09 PM, Martin Kosek wrote:
On 05/26/2014 12:04 PM, Petr Viktorin wrote:
On 05/25/2014 09:29 PM, Martin Kosek wrote:
On 05/23/2014 04:50 PM, Simo Sorce wrote:
On Fri, 2014-05-23 at 10:59 +0200, Martin Kosek wrote:
On 05/22/2014 04:20 PM, Petr Viktorin wrote:
On 05/21/2014 12:14 PM, Simo Sorce wrote:
On Wed, 2014-05-21 at 08:03 +0200, Martin Kosek wrote:
On 05/16/2014 04:33 PM, Petr Viktorin wrote:
On 05/16/2014 01:54 PM, Martin Kosek wrote:
On 04/29/2014 11:00 PM, Petr Viktorin wrote:
Patch 0540 adds a bunch of managed read ACIs for user, as
discussed
previously
[0].

Patch 0541 is some minor refactoring for the next part.

Patch 0542 sets the read acces to addressbook attributes to
anonymous when
upgrading from pre-4.0.
I first this by checking if the update is run from
ipa-server-install or
not,
but then I realized the logic I want is simple: if the global
anon read ACI
exists, we want to preserve its spirit by setting addressbook
attribute
ACI to
anonymous.


[0]
http://www.redhat.com/archives/freeipa-devel/2014-April/msg00363.html
et
al.


540:

Looks good! The only attributes I am concerned about are special
IPA
attributes:

- ipauniqueid
- ipasshpubkey
- ipauserauthtype
- userclass

I personally do not think they should be included in POSIX
attributes
permissions, they are far from POSIX definition...

What about creating one more permission "System: Read User IPA
Attributes" as
these are specific to FreeIPA use and allowing that permission
for all
authenticated users?

Sounds reasonable. I assume we want this one to be also set to
anonymous when
upgrading from old versions.
Attaching updated patches.

Ok, looks good.

I am now just pondering whether "System: Read User POSIX
Attributes" is the
right name for the permission as there are not just POSIX
attributes, but also
attributes from organizationalPerson or inetOrgPerson objectclasses.

Maybe we should name it "System: Read User Core Attributes" or
"System: Read
User Basic Attributes"? Simo, any preference?

We could use: "System: Read User Standard Attributes"

I've used this one, then.


but the 'posix' version is also ok to me.

On Wed, 2014-05-21 at 08:03 +0200, Martin Kosek wrote:
Also, I just realized we forgot memberOf attribute - it needs to be
available
to authenticated users otherwise group membership will fall apart.

Good catch. Added.


We are very close to push this one - I have just one last concern about
userpkcs12 attribute. On upgrade, we previously hidden userpkcs12
from user,
now we added it to be read by default. This results in this warning
during upgrade:

Excluded attributes for System: Read User Addressbook Attributes:
userpkcs12

Simo (or others), is this OK or do we want to keep hiding userpkcs12
by default?

Is there any client that needs access to that information that we are
aware of ?

Simo.

I do not think so. Rob, do you know?

This was my mistake. We never allowed non-admins to see that attribute by
default, so we shouldn't start now.

ack, we probably had a good reason and it is much safer to keep this decision.

I'm glad the updater caught it, sorry that I didn't.

Actually, that means that you made the security checks in the updater right :-)

I diffed the change in the patch and it removed the last obstacle I saw with
this patch set. Thus, ACK for all 3.

Thanks for the thorough review!
Pushed to master: 63becae88c6c270b98f0432dc474b661b82f3119


--
PetrĀ³

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to