RFC: PermissionBackend API

145 views
Skip to first unread message

Shawn Pearce

unread,
Feb 19, 2017, 6:26:00 PM2/19/17
to repo-discuss
I've started a large refactoring of how Gerrit evaluates permissions, and dubbed the new API PermissionBackend.

I think there is some elegance to this approach, for example PutTopic:

  @Override
  public Response<String> apply(ChangeResource req, Input input)
      throws UpdateException, RestApiException, PermissionBackendException {
    req.permissions().check(ChangePermission.EDIT_TOPIC_NAME);

The check method verifies the permission and throws AuthException (403 Forbidden) if the calling user is not permitted. A getDescription method is equally straightforward:

  public UiAction.Description getDescription(ChangeResource rsrc) {
    return new UiAction.Description()
        .setLabel("Edit Topic")
        .setVisible(rsrc.permissions().testOrFalse(ChangePermission.EDIT_TOPIC_NAME));
  }


One important aspect is the difference between check and test. The check method should be used early in a state changing operation, where a denial can be returned to the user. The test method should be used in other contexts like computing UI button states, where lack of the permission is not fatal to the read operation completing normally.


Later I'd like to make PermissionBackend pluggable(ish), so that an implementation can decide how to handle check and test.

This could be useful for integrated systems, where permission management is handled outside of Gerrit. Both CollabNet and GerritHub come to mind as systems that could benefit from being able to swap the PermissionBackend and skip writing permissions with custom GroupBackend associations to refs/meta/config[1].

Being able to swap PermissionBackend may also be useful for auditing. A failed check is a user trying to use a permission they do not have. An auditing PermissionBackend could wrap the "real" PermissionBackend and observe check operations, providing a more clear picture of successful and failed permissions, without sorting through the HTTP logs trying to map REST API URLs to permission names. Its especially challenging for the PostReview handler, where labels aren't even visible in the URL and a logger has to dig through the input object.


Its going to be a lot of work, but I'd like to have ProjectControl, RefControl and ChangeControl become internal implementation details of DefaultPermissionBackend, and hide these types entirely from the rest of the server (and plugins).

My permissionbackend topic contains a lot of changes because I've been going through removing a number of references to see if the PermissionBackend API is suitable for its intended purpose. Thus far its proving to be reasonably straightforward with the PermissionBackend, which suggests this may be a viable API to work with.



[1] You can still use a custom GroupBackend to facilitate the implementation. Both check and test have access to a CurrentUser, which obviously has group memberships accessible. What you avoid is writing AccessSection entries to refs/meta/config.

Jacek Centkowski

unread,
Feb 21, 2017, 11:05:26 AM2/21/17
to Repo and Gerrit Discussion
That sounds interesting (will look into details of proposed changes in the following days) however there is one aspect of existing approach (writing AccessSection entries in refs/meta/config) that seems to be missing in this proposal which is visibility of group's (and particular user as group member) permissions and also track of its changes (in refs/meta/config history). When in doubts one can go and immediately check what is currently granted to given group at certain project level - this is not perfect but allows for pretty quick investigation when permissions needs to be examined...

Regards
Jacek

Luca Milanesio

unread,
Feb 21, 2017, 12:30:33 PM2/21/17
to git...@googlegroups.com, Shawn Pearce, Repo and Gerrit Discussion, Jacek Centkowski
Looping the GitBlit Team ... because this change is *amazing* from a GitBlit plugin integration perspective.

One of the things that people like about GitBlit, is the ease of setup and use.
The GitBlit access control screen is very simple [1] and with this "pluggability" of Gerrit permissions it could be delegated to Gitblit altogether.

The result would be:
- Gitblit UX for repo browsing and ACL administration
- Gerrit as code-review backend 

The Gerrit + Gitblit combination could really bring the Gerrit Code Review workflow to a wider audience.

Luca.

--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Shawn Pearce

unread,
Feb 21, 2017, 12:40:22 PM2/21/17
to Jacek Centkowski, Repo and Gerrit Discussion
On Tue, Feb 21, 2017 at 8:05 AM, Jacek Centkowski <geminica...@gmail.com> wrote:
That sounds interesting (will look into details of proposed changes in the following days) however there is one aspect of existing approach (writing AccessSection entries in refs/meta/config) that seems to be missing in this proposal which is visibility of group's (and particular user as group member) permissions

Yes, I haven't gotten to group permissions. There's a few reasons for that. The installation where I want to use PermissionBackend doesn't want to use the internal group system, the customer only wants to use an external group system, so I don't immediately need PermissionBackend to understand that concept. I also kind of wanted to eject the internal group system out of the core server into a plugin, especially as part of the NoteDb migration. Actually, I've always wanted to eject it, once we had GroupBackend pluggable I started drafting that code, but we never got it completed.

and also track of its changes (in refs/meta/config history).

Eh. PermissionBackend leaves this open to the implementation. DefaultPermissionBackend uses the existing refs/meta/config access section format, so it does still offer this history view. Other PermissionBackend implementations could record data in a different format, or a different data store, at which point the concept of tracking history is up to the implementation of that specific PermissionBackend.

When in doubts one can go and immediately check what is currently granted to given group at certain project level - this is not perfect but allows for pretty quick investigation when permissions needs to be examined...

Agreed, this can be very useful, but some installations don't need this in refs/meta/config. They might have the data elsewhere and are converting it to refs/meta/config, or they may not consider history to be valuable. PermissionBackend gives more flexibility to decide these trade-offs.

 

Regards
Jacek


On Monday, 20 February 2017 00:26:00 UTC+1, Shawn Pearce wrote:
I've started a large refactoring of how Gerrit evaluates permissions, and dubbed the new API PermissionBackend.

I think there is some elegance to this approach, for example PutTopic:

  @Override
  public Response<String> apply(ChangeResource req, Input input)
      throws UpdateException, RestApiException, PermissionBackendException {
    req.permissions().check(ChangePermission.EDIT_TOPIC_NAME);

The check method verifies the permission and throws AuthException (403 Forbidden) if the calling user is not permitted. A getDescription method is equally straightforward:

  public UiAction.Description getDescription(ChangeResource rsrc) {
    return new UiAction.Description()
        .setLabel("Edit Topic")
        .setVisible(rsrc.permissions().testOrFalse(ChangePermission.EDIT_TOPIC_NAME));
  }


One important aspect is the difference between check and test. The check method should be used early in a state changing operation, where a denial can be returned to the user. The test method should be used in other contexts like computing UI button states, where lack of the permission is not fatal to the read operation completing normally.


Later I'd like to make PermissionBackend pluggable(ish), so that an implementation can decide how to handle check and test.

This could be useful for integrated systems, where permission management is handled outside of Gerrit. Both CollabNet and GerritHub come to mind as systems that could benefit from being able to swap the PermissionBackend and skip writing permissions with custom GroupBackend associations to refs/meta/config[1].

Being able to swap PermissionBackend may also be useful for auditing. A failed check is a user trying to use a permission they do not have. An auditing PermissionBackend could wrap the "real" PermissionBackend and observe check operations, providing a more clear picture of successful and failed permissions, without sorting through the HTTP logs trying to map REST API URLs to permission names. Its especially challenging for the PostReview handler, where labels aren't even visible in the URL and a logger has to dig through the input object.


Its going to be a lot of work, but I'd like to have ProjectControl, RefControl and ChangeControl become internal implementation details of DefaultPermissionBackend, and hide these types entirely from the rest of the server (and plugins).

My permissionbackend topic contains a lot of changes because I've been going through removing a number of references to see if the PermissionBackend API is suitable for its intended purpose. Thus far its proving to be reasonably straightforward with the PermissionBackend, which suggests this may be a viable API to work with.



[1] You can still use a custom GroupBackend to facilitate the implementation. Both check and test have access to a CurrentUser, which obviously has group memberships accessible. What you avoid is writing AccessSection entries to refs/meta/config.

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

Shawn Pearce

unread,
Feb 21, 2017, 12:41:54 PM2/21/17
to Luca Milanesio, git...@googlegroups.com, Repo and Gerrit Discussion, Jacek Centkowski
On Tue, Feb 21, 2017 at 9:30 AM, Luca Milanesio <luca.mi...@gmail.com> wrote:
Looping the GitBlit Team ... because this change is *amazing* from a GitBlit plugin integration perspective.

Thanks Luca.
 
One of the things that people like about GitBlit, is the ease of setup and use.
The GitBlit access control screen is very simple [1] and with this "pluggability" of Gerrit permissions it could be delegated to Gitblit altogether.

The result would be:
- Gitblit UX for repo browsing and ACL administration
- Gerrit as code-review backend 

The Gerrit + Gitblit combination could really bring the Gerrit Code Review workflow to a wider audience.

Yes, exactly!

The existing Gerrit permission system is very powerful and requires an "advanced user" to make use of it. One of the applications of PermissionBackend that I was thinking of is coupling Gerrit with a system that has less knobs, but still helps the team accomplish its goals. Gitblit is a perfect example.

 

Luca.


More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

Jacek Centkowski

unread,
Feb 21, 2017, 3:34:14 PM2/21/17
to Repo and Gerrit Discussion, luca.mi...@gmail.com, git...@googlegroups.com, geminica...@gmail.com


On Tuesday, 21 February 2017 18:41:54 UTC+1, Shawn Pearce wrote:
On Tue, Feb 21, 2017 at 9:30 AM, Luca Milanesio <luca.mi...@gmail.com> wrote:
Looping the GitBlit Team ... because this change is *amazing* from a GitBlit plugin integration perspective.

Thanks Luca.
 
One of the things that people like about GitBlit, is the ease of setup and use.
The GitBlit access control screen is very simple [1] and with this "pluggability" of Gerrit permissions it could be delegated to Gitblit altogether.

The result would be:
- Gitblit UX for repo browsing and ACL administration
- Gerrit as code-review backend 

The Gerrit + Gitblit combination could really bring the Gerrit Code Review workflow to a wider audience.

Yes, exactly!

The existing Gerrit permission system is very powerful and requires an "advanced user" to make use of it. One of the applications of PermissionBackend that I was thinking of is coupling Gerrit with a system that has less knobs, but still helps the team accomplish its goals. Gitblit is a perfect example.

Many will benefit from this feature especially in this aspect - no doubts about it :)
 

Luca.

To unsubscribe, email repo-discuss...@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
To unsubscribe, email repo-discuss...@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

David Pursehouse

unread,
Feb 22, 2017, 12:43:09 AM2/22/17
to Shawn Pearce, repo-discuss
On Mon, Feb 20, 2017 at 8:25 AM 'Shawn Pearce' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:
I've started a large refactoring of how Gerrit evaluates permissions, and dubbed the new API PermissionBackend.


Do you intend to get this feature into the next release (2.14) or can we postpone merging it until after stable-2.14 is cut, like we're going to do with the private changes series as agreed in the review of Patrick's proposal for that feature?


 
--

luca.mi...@gmail.com

unread,
Feb 22, 2017, 2:00:38 AM2/22/17
to David Pursehouse, Shawn Pearce, repo-discuss
This is a key feature IMHO, would be nice to have it in 2.14 !

Luca

Sent from my iPhone

Shawn Pearce

unread,
Feb 22, 2017, 10:04:33 AM2/22/17
to Luca Milanesio, David Pursehouse, repo-discuss
Although its "nice to have" in 2.14, I'm not rushing to make this large refactoring into a specific release. I'm skeptical I could get it all done in time for 2.14 to actually ship in a reasonable time frame after branching.

Its best for this to be post-2.14.


More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.

Kenny Ho

unread,
Feb 22, 2017, 3:23:35 PM2/22/17
to Repo and Gerrit Discussion, luca.mi...@gmail.com, git...@googlegroups.com, geminica...@gmail.com
Just some clarifying questions...

If I understand this correctly, there's two parts to this: 1) exposing an API to external applications to take advantage of Gerrit's access control mechanism, 2) make the access control mechanism swap-able such that Gerrit can use external mechanism, is that correct?

For 2), can it accommodate external mechanism that uses a different access control paradigm (like ABAC for example?) 


Luca.

To unsubscribe, email repo-discuss...@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
To unsubscribe, email repo-discuss...@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.

Shawn Pearce

unread,
Feb 22, 2017, 9:51:21 PM2/22/17
to Kenny Ho, Repo and Gerrit Discussion, Luca Milanesio, git...@googlegroups.com, Jacek Centkowski
On Wed, Feb 22, 2017 at 12:23 PM, Kenny Ho <y2k...@gmail.com> wrote:
Just some clarifying questions...

If I understand this correctly, there's two parts to this: 1) exposing an API to external applications to take advantage of Gerrit's access control mechanism,

No. With this work, we are not the existing access control systems to external applications. That could become easier, as the API surface is smaller (just PermissionBackend).
 
2) make the access control mechanism swap-able such that Gerrit can use external mechanism, is that correct?

This is correct, and the purpose of this work.

For 2), can it accommodate external mechanism that uses a different access control paradigm (like ABAC for example?) 

Yes, this becomes possible. The new API exposes quite a bit of data about the user making the request and the change being accessed. A pluggable implementation could get the relevant information and make decisions on its own.

 

--
--
To unsubscribe, email repo-discuss+unsubscribe@googlegroups.com

More info at http://groups.google.com/group/repo-discuss?hl=en

---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss+unsubscribe@googlegroups.com.
Reply all
Reply to author
Forward
0 new messages