Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REST API: fine-grained authorization #621

Open
hsanjuan opened this issue Dec 17, 2018 · 21 comments

Comments

Projects
None yet
3 participants
@hsanjuan
Copy link
Collaborator

commented Dec 17, 2018

This is the initial write up for fine-grained authorization in the REST API endpoints

Background

Currently, we support Basic Authorization as configured with a map in the restapi configuration section.

However, we would like to enable advanced permissions that would allow multiple clusters to be federated (talk to each others APIs) and more flexibility for API integrations (allow pinning but not unpinning etc) and Pin Services. For this we need a way to define permissions per API endpoint.

Approach

We will need to come up with a way to make it easy to configure a permission scheme that allows:

  • Authorization for specific API endpoints for specific users
  • A way to provide read vs. write (pin) vs. admin access (peer rm etc) to specific users (in a faster way than listing each endpoint one by one).
  • We should make this a middleware+config module attachable to any HTTP API, so that we can re-use it with the IPFS Proxy API (if not too problematic).

i.e. From the top of my head:

{
  "basic_auth_credentials": {
    "user": "password",
    "user2": "pw2"
  },
  "auth_schemes": {
    "pinner":  ["Pin", "Status", "Allocation", "Recover", "Sync"],
    "admin": ["*"]
  },
  "authorizations": {
    "user": ["pinner"]
  }
}

The method names are the labels for methods in https://github.com/ipfs/ipfs-cluster/blob/master/api/rest/restapi.go#L282

However, in the case of the IPFS Proxy we would want to match routes directly so perhaps be able to do to do:

"pinner": ["POST/pins/{hash}", ...

but this has other problems. Perhaps we can do the proxy separately, I'm not sure. We can discuss here.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

I think here as well, we can have notion of trusted users. That way it would be simpler to provide API access.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

Linking federated cluster issue #602

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 13, 2019

I think It won't matter what the consensus mechanism is used underneath, so we can have a single permission policy for all. config field say authorization_enabled should work
I think we would need notion of trusted users as every cluster would have federation peers and other peers.
I will create the permission policy accordingly, may be call it something else to not confuse with rpc permission policy (say authorization scheme)

@hsanjuan @lanzafame thoughts?

kishansagathiya added a commit that referenced this issue Mar 13, 2019

@hsanjuan

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 13, 2019

@kishansagathiya I don't understand what you mean by trusted users. The issue description gives a first proposal about having users (user+pw), roles (list of endpoints that can be called) and a way to associate a user with roles. This is essentially similar RBAC mode from https://kubernetes.io/docs/reference/access-authn-authz/authorization/ (it is a good doc to figure out how advanced auth would schemes work). Another approach would be ABAC.

Unlike RPC authorization, the API deals with users (not peers). The objective here is to NOT have a single permission policy for all. This is what exists now: a valid user can access all endpoints. We want to be able to give different powers to different users. And unlike RPC, this should probably be fully configurable in the config (although it is probably useful to include default roles).

@hsanjuan

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 13, 2019

One good place to start is to perhaps to research how authorization is configured in other projects offering HTTP APIs. From googling around, there is a lot of reading about API authorization mechanisms in the first place. We are moving from User-based to role-based, although this has its limitations. For example, it would be hard to express "Allow user X to unpin only items pinned by user X", with a role-based approach, and an attribute-based approach would be more natural.

@kishansagathiya kishansagathiya self-assigned this Mar 19, 2019

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 20, 2019

Some docs that I went through

https://kubernetes.io/docs/reference/access-authn-authz/authorization/
https://docs.cloudfoundry.org/concepts/security.html#auth
https://docs.cloudfoundry.org/concepts/roles.html
https://istio.io/blog/2018/istio-authorization/

Most of them have essentially similar logic.

  1. Using ABAC, would be more fine grained and probably make sense because we don't have clear roles as K8 or CloudFoundry like admin, user etc.

  2. But RBAC, would be simpler as permissions can be set just with a role name (I think in most cases api user would need access to more than one attributes). And fine grained permissions can be achieved by allowing to add custom roles.

  3. We can also support both, as in define attributes( also support custom attributes) and define roles(support custom roles too) constituting attributes. This way it would be easier to add new roles, since it would just need to group some attributes together rather than listing api endpoints.

I am inclined to 3.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

I am inclined to 3.

Actually I am going to try the second approach. 3rd has more complexity. It would be easier to implement 3rd once 2nd is implemented.

@hsanjuan

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 21, 2019

ok, please make a more detailed proposal of what you want to implement before implementing. How does it differ from the original idea for the approach? How can it be extended to deal with the Allow user X to unpin only items pinned by user X problem etc.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

Allow user X to unpin only items pinned by user X

I am not sure if it would be possible to link pins with users. Let's say we have some way to know which user pinned certain cid. We can stop others from unpinning. But they can repin the same cid. If we don't allow repinning, the user which pinned first will have exclusive rights to pin that cid. If we do, the new user can also claim to have pinned those items and can unpin them.

@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2019

@kishansagathiya There is no need to have 'claiming'. Here is the usecase: If a user1 pins cid1 and then user2 pins cid1, they both have pinned it, but there should be only one copied actually pinned. If user2 unpins cid1, the metadata in the pinset that says they pinned cid1 is removed but it isn't actually unpinned as cid1 still exists in the pinset due to user1.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

If user2 unpins cid1, the metadata in the pinset that says they pinned cid1 is removed but it isn't actually unpinned as cid1 still exists in the pinset due to user1.

I am confused about this statement. If when user2 unpins all we do is remove the metadata, it's not really unpinning.

@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2019

Exactly, but from their view of the world, cid1 is no longer in their pinset.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 21, 2019

@lanzafame I see your point now

About adding these metadata and storing, do I do that as part of this issue or create a new one? I am guessing that should happen after this issue.

@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2019

It should be clear how that is going to work with what you are proposing.

@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 21, 2019

i.e. the work is another issue, but the design part of the discussion should happen here.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

How does this look?

REST API Authorization Proposal

The parts that user can customize

  • Add their own user groups/roles and policies for those groups

Policy is map[string request path regex]bool

func IsAuthorized(user, request) bool{
    If !AuthorizeWithPath(user, request.URL.Path){
        return false
    }

    resourceAction := getResourceAction(request)
    If !AuthorizeWithResourceAction(resourceAction){
        return false
    }
}

type ResourceAction struct{
    ResourceType string
    Resource {}interface
    Action string // GET, POST, DELETE
}

func AuthorizeWithPath(user, path) bool{
    // We could have a map that tells is a path is allowed or not, let's call it policy
    // A policy would be defined for a group/role of users
    groups, inAGroup := getGroups(user)
    if !inAnyGroup{
        return false
    }

    for group := range groups{
        If AuthorizeWithGroup(group, path){
            return true
        }
    }

    return false
}

// Resources in case of cluster would be peer and pin.
// So, resource types could be pin, peer and resourceless
// I don't think it would make sense to let user customize
// this, since the logic used to allow access to pins or
// peers will require some change to the cluster code the
// respects this logic
func AuthorizeWithResourceAction(resourceAction) bool{
    switch resourceAction.ResourceType {
        resourceless: return true
        pin: cid := resourceAction.Resource.(cid.Cid)
            // AuthorizeWithCID could talk to the cluster and
            // get relevant metadata to know if this user is 
            // allowed to perform this action or not, or 
            // whatever logic seems appropriate
            return AuthorizeWithCID(user, cid,  resourceAction.Action)
        peer: peer := resourceAction.Resouce.(Peer.ID)
            return AuthorizeWithPeer(user, peer, resourceAction.Action)
    }
}
@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2019

The policy map should be map[path regex][http method]bool. I don't get the logic of AuthorizedWithResourceAction(resourceAction) unless you are suggesting we move to a resource model similar to Kubernetes. That would be a separate issue (not opposed to the idea) but I think currently the http method concept should be addressed with introducing such a model.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Mar 22, 2019

AuthorizedWithResourceAction(resourceAction) is so that it can decide if a user is allowed to access a pin or peer.

@lanzafame

This comment has been minimized.

Copy link
Collaborator

commented Mar 22, 2019

Firstly, how would a user access a peer via the rest api? Secondly, I think this is over resourcifying how we interact with cids/peers in the current cluster system, as I mentioned above it looks very much like the Kubernetes way of dealing with resources.

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

Firstly, how would a user access a peer via the rest api?

		{
			"Peers",
			"GET",
			"/peers",
			api.peerListHandler,
		},
		{
			"PeerAdd",
			"POST",
			"/peers",
			api.peerAddHandler,
		},
		{
			"PeerRemove",
			"DELETE",
			"/peers/{peer}",
			api.peerRemoveHandler,
		},

I think this is over resourcifying how we interact with cids/peers in the current cluster system, as I mentioned above it looks very much like the Kubernetes way of dealing with resources.

I have tried to keep a generic approach. This can probably become simpler if all we care is Allow user X to unpin only items pinned by user X

@kishansagathiya

This comment has been minimized.

Copy link
Member

commented Apr 3, 2019

@hsanjuan @lanzafame
May be this is something we can discuss over a call. That would have shorter round-trips. I will make sure to post the results of the call here, if that happens.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.