Hi Ron, Sounds good to me, thank you.
Regards, Rajini On Wed, Sep 2, 2020 at 8:00 PM Ron Dagostino <rndg...@gmail.com> wrote: > Thanks, Rajini. That's a good point about authorizing user creation and > ACL creation separately to enable that separation as an *option* -- I agree > with that, which I think argues for a separate ALTER_USER operation (or > ALTER_USER_CREDENTIAL operation; not sure what the best name is, but > probably the first one since it is shorter and these names tend to be > short). If we separate it out like this, then I don't think there is a > need to restrict delegation tokens from being able to > ALTER_USER_SCRAM_CREDENTIALS -- since it is possible to simply never > delegate authority to act as a user with those permissions. Do you agree > with that, or do you still believe we should explicitly restrict delegation > tokens from ALTER_USER_SCRAM_CREDENTIALS? I personally still agree with > Colin's point about avoiding second-class citizenry. > > That's also a good point about users perhaps wanting to change their own > password. I don't think that has come up. If we were to add this, then it > would be the case that a user would be authorized to change their own > password at all times. But in this case I think we would have to restrict > delegation tokens from changing the password of the underlying credential > since the user doesn't know the password to that account -- and due to that > lack of knowledge I think this is not a case of being a second-class > citizen and the restriction is justifiable. > > So, to summarize, I am tentatively proposing the following: > > 1) ALTER_USER_SCRAM_CREDENTIALS is allowed on any credential if a new > ALTER_USER operation on the CLUSTER resource is authorized -- even if > authenticated via delegation token > 2) Assuming (1) does not apply, ALTER_USER_SCRAM_CREDENTIALS is also > allowed if the only alterations requested are alterations to one or more > credentials associated with the authenticated user making the request, with > the added caveat that the authentication must not have been via delegation > token (i.e. you can't alter credentials, in the absence of (1) above, for > the user who delegated their authority to you). > > Thoughts? > > Ron > > > On Wed, Sep 2, 2020 at 2:16 PM Rajini Sivaram <rajinisiva...@gmail.com> > wrote: > > > Hi Ron, > > > > Not sure the person who creates/manages users is always the person who > > controls access to Kafka resources. Separate ACLs gives the flexibility > to > > keep them separate while you can still grant both to the user, while a > > combined ACL means that they can only be granted together. A related > > question is about password change. In the current model, User:Alice > > authenticated as Alice cannot change Alice's password without Alter > > permissions on the Cluster. The delegation token model where a user has > > more control over their own credential seems more appropriate in this > case. > > Not sure if we considered and rejected that approach. > > > > > > On Wed, Sep 2, 2020 at 5:57 PM Ron Dagostino <rndg...@gmail.com> wrote: > > > > > Hi Rajini. Thanks for the explanation. > > > > > > I think these are the APIs that are authorized via the ALTER CLUSTER > > > operation, none of which are restricted when authenticating via > > delegation > > > token: > > > > > > ALTER_PARTITION_REASSIGNMENTS > > > ALTER_REPLICA_LOG_DIRS > > > CREATE_ACL > > > DELETE_ACL > > > ELECT_LEADERS > > > > > > I think if we are going to ALTER_USER_SCRAM_CREDENTIALS then we are > > likely > > > going to want to CREATE_ACL as well -- it feels like there's no sense > in > > > creating a user but then not being able to authorize the user to do > > > anything. (Unless I am wrong here?). If this is correct, then > > > following that to its logical conclusion, it feels like we should > > authorize > > > ALTER_USER_SCRAM_CREDENTIALS via the same ALTER CLUSTER operation. And > > > then with respect to delegation tokens, I think we would either need to > > > allow delegation tokens to do both or we should prevent delegation > tokens > > > from altering credentials. And then that gets to Colin's point about > > > whether sessions authenticated via delegation token should be > > second-class > > > in some way, which I am inclined to think they should not. > > > > > > Ron > > > > > > > > > On Wed, Sep 2, 2020 at 11:23 AM Rajini Sivaram < > rajinisiva...@gmail.com> > > > wrote: > > > > > > > Hi Ron/Colin, > > > > > > > > Without any restrictions, if delegation tokens can be used to create > > new > > > > users or change the password of the user you are impersonating, you > > also > > > > get the power to create/renew a new token by authenticating as a > SCRAM > > > user > > > > you just created or updated. It seems like a new power that we are > > > granting > > > > in a new API using an existing ACL. User management is a new class of > > > > operations we are adding to the Kafka protocol. An alternative to > > > > restricting delegation tokens would be to add a new ACL operation > > instead > > > > of reusing `Alter` for user management : `AlterUsers/DescribeUsers` > > (like > > > > AlterConfigs/DescribeConfigs). > > > > > > > > Regards, > > > > > > > > Rajini > > > > > > > > > > > > On Wed, Sep 2, 2020 at 12:24 AM Colin McCabe <co...@cmccabe.xyz> > > wrote: > > > > > > > > > Hi Ron, > > > > > > > > > > Thanks. We can wait for Rajini's reply to finalize this, but for > > now I > > > > > guess that will unblock the PR at least. If we do decide we want > the > > > > > restriction we can do a follow-on PR. > > > > > > > > > > It's good to see this API moving forward! > > > > > > > > > > best, > > > > > Colin > > > > > > > > > > > > > > > On Tue, Sep 1, 2020, at 12:55, Ron Dagostino wrote: > > > > > > Hi Colin. I've removed that requirement from the KIP and updated > > the > > > > PR > > > > > > accordingly. > > > > > > > > > > > > Ron > > > > > > > > > > > > On Fri, Aug 28, 2020 at 2:27 PM Colin McCabe <cmcc...@apache.org > > > > > > wrote: > > > > > > > > > > > > > Hi Ron, > > > > > > > > > > > > > > Thanks for the update. I agree with all of these changes, > > except I > > > > > think > > > > > > > we should discuss this one further: > > > > > > > > > > > > > > On Wed, Aug 26, 2020, at 14:59, Ron Dagostino wrote: > > > > > > > > > > > > > > > > 2. We added a restriction to not allow users who > authenticated > > > > using > > > > > > > > delegation tokens to create or update user SCRAM credentials. > > We > > > > > don't > > > > > > > > allow such authenticated users to create new tokens, and it > > would > > > > be > > > > > odd > > > > > > > if > > > > > > > > they could create a new user or change the password of the > user > > > for > > > > > the > > > > > > > > token. > > > > > > > > > > > > > > > > > > > > > > I don't think these two restrictions are comparable. It seems > to > > > me > > > > > that > > > > > > > we forbid creating a new token based on an existing token in > > order > > > to > > > > > force > > > > > > > users of delegation tokens to re-authenticate periodically > > through > > > > the > > > > > > > regular auth system. If they could just create a new token > based > > > on > > > > > their > > > > > > > old token, there would be an obvious "wishing for more wishes" > > > > problem > > > > > and > > > > > > > they could just sidestep the regular authentication system > > entirely > > > > > once > > > > > > > they had a token. > > > > > > > > > > > > > > This issue doesn't exist here, since creating a new SCRAM user > > > > doesn't > > > > > do > > > > > > > anything to extend the life of the existing delegation token. > If > > > the > > > > > user > > > > > > > has the permission to change SCRAM users, I don't see any > reason > > > why > > > > we > > > > > > > should forbid them from doing just that. Users of delegation > > > tokens > > > > > > > shouldn't be second-class citizens. A user with ALTER on > CLUSTER > > > > should > > > > > > > have all the permissions associated with ALTER on CLUSTER, > > > regardless > > > > > of if > > > > > > > they logged in with Kerberos, delegation tokens, SCRAM, etc. > etc. > > > I > > > > > don't > > > > > > > think the proposed restriction you mention here is consistent > > with > > > > > that. > > > > > > > > > > > > > > best, > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > > > > >
