Roles and Perms for Related Objects

The backstory is that I am working on taking our implementation to this
point and trying to ensure proper scoping of objects in the Katello
controllers (see [1] and [2]) for some reference. While everything has so
far gone smoothly I have ran into an issue that I needs addressing across
the projects. Here's the scenario:

  1. Create a new user: testuser
  2. Grant that user a new permission that allows "view" and "create" on
    subnets.
  3. As the admin, create a new Domain "example.org" with ID 1
  4. Via the API, authenticating as 'testuser' create a new subnet with that
    domain linkage, e.g:

POST: /api/v2/subnets

{
"subnet": {
"name": "TestNet",
"domain_ids": [1],
"network": "255.255.192.0 <callto:255.255.192.0>",
"mask": "255.255.255.0 <callto:255.255.255.0>"
}
}

Returned:

{
"id": 1,
"name": "TestNet",
"network_address": "255.255.192.0 <callto:255.255.192.0>/24",
"network": "255.255.192.0 <callto:255.255.192.0>",
"cidr": 24,
"mask": "255.255.255.0 <callto:255.255.255.0>",
"priority": null,
"vlanid": null,
"gateway": null,
"dns_primary": null,
"dns_secondary": null,
"from": null,
"to": null,
"created_at": "2014-07-21T15:13:00Z",
"updated_at": "2014-07-21T15:13:00Z",
"dhcp": null,
"tftp": null,
"dns": null,
"locations": [],
"organizations": [],
"domains": [
{
"id": 1,
"name": "example.org"
}
]
}

Issue 1:
As a user who is only allowed to "view" and "create" subnets, I have been
able to attach the subnet to a domain that have do not have access to.

Issue 2:
On the Katello side we attempt to verify all nested objects as accessible
by a user before allowing them to attach that object. Typically, this means
that if I am editing a Product, for example, and want to attach a GPG Key
that I must have access to that GPG Key. Our assumption is that a user must
have "view" on the GPG Key and edit/create on the Product. Currently, the
nested object API methods would automatically try to calculate and assume
that I have 'update' on both objects due to that being the current action
permission see -
https://github.com/theforeman/foreman/blob/develop/app/controllers/api/base_controller.rb#L237.
Question here is - what is the proper behavior and what do we want to
enforce?

The above are the two big issues I have run into whereby we are
inconsistent with our behavior expectations and there appears to be a
potentially large gap on the Foreman (and Katello side to a degree) around
the API (and UI) and what a user can and cannot do based on their
permission set. I wanted to reach out and see if I was missing anything, or
what your thoughts were on these issues. If you feel I need to open this
up, I am more than happy to send this out to a wider audience or put
together a meeting to discuss so that we can converge on consistency.

Thanks,
Eric

[1] https://github.com/theforeman/foreman/pull/1596
[2] https://github.com/Katello/katello/pull/4454

Hello,

sending my comments below in text

> The backstory is that I am working on taking our implementation to this
> point and trying to ensure proper scoping of objects in the Katello
> controllers (see [1] and [2]) for some reference. While everything has so
> far gone smoothly I have ran into an issue that I needs addressing across
> the projects. Here's the scenario:
>
> 1. Create a new user: testuser
> 2. Grant that user a new permission that allows "view" and "create" on
> subnets.
> 3. As the admin, create a new Domain "example.org" with ID 1
> 4. Via the API, authenticating as 'testuser' create a new subnet with that
> domain linkage, e.g:
>
> POST: /api/v2/subnets
>
> {
> "subnet": {
> "name": "TestNet",
> "domain_ids": [1],
> "network": "255.255.192.0 <callto:255.255.192.0>",
> "mask": "255.255.255.0 <callto:255.255.255.0>"
> }
> }
>
> Returned:
>
> {
> "id": 1,
> "name": "TestNet",
> "network_address": "255.255.192.0 <callto:255.255.192.0>/24",
> "network": "255.255.192.0 <callto:255.255.192.0>",
> "cidr": 24,
> "mask": "255.255.255.0 <callto:255.255.255.0>",
> "priority": null,
> "vlanid": null,
> "gateway": null,
> "dns_primary": null,
> "dns_secondary": null,
> "from": null,
> "to": null,
> "created_at": "2014-07-21T15:13:00Z",
> "updated_at": "2014-07-21T15:13:00Z",
> "dhcp": null,
> "tftp": null,
> "dns": null,
> "locations": [],
> "organizations": [],
> "domains": [
> {
> "id": 1,
> "name": "example.org"
> }
> ]
> }
>
> Issue 1:
> As a user who is only allowed to "view" and "create" subnets, I have been
> able to attach the subnet to a domain that have do not have access to.

I think that the right solution in this case is verify that domain with
particular id exists and I have view permission for it. So something like

def create
domains = Domain.authorized(:view_domains).find(:id => params[:subnet]
[:domain_ids])
@subnet = Subnet.new(params[:subnet])
@subnet.domains = domains
@subnet.save
end

We can discuss if this should be implemented in controller or rather model,
how much it should be abstracted etc. The most important thing I'm trying to
express is that I wouldn't mix it into create_subnets permissions that would
somehow enforce view_domain permission but I'd keep it as controller or
probably better model responsibility. Each model could define which
associations should be checked by which permission. Then in callback
validation it would do the check.

We may find


from unmerged PR https://github.com/theforeman/foreman/pull/1479 useful for
checking of modified associations.

> Issue 2:
> On the Katello side we attempt to verify all nested objects as accessible
> by a user before allowing them to attach that object. Typically, this means
> that if I am editing a Product, for example, and want to attach a GPG Key
> that I must have access to that GPG Key. Our assumption is that a user must
> have "view" on the GPG Key and edit/create on the Product. Currently, the
> nested object API methods would automatically try to calculate and assume
> that I have 'update' on both objects due to that being the current action
> permission see -
> https://github.com/theforeman/foreman/blob/develop/app/controllers/api/base_
> controller.rb#L237. Question here is - what is the proper behavior and what
> do we want to enforce?

I think this is more or less the same as issue 1 and same solution should
apply.

··· On Tuesday 22 of July 2014 08:25:37 Eric D Helms wrote:

The above are the two big issues I have run into whereby we are
inconsistent with our behavior expectations and there appears to be a
potentially large gap on the Foreman (and Katello side to a degree) around
the API (and UI) and what a user can and cannot do based on their
permission set. I wanted to reach out and see if I was missing anything, or
what your thoughts were on these issues. If you feel I need to open this
up, I am more than happy to send this out to a wider audience or put
together a meeting to discuss so that we can converge on consistency.

Thanks,
Eric

[1] https://github.com/theforeman/foreman/pull/1596
[2] https://github.com/Katello/katello/pull/4454


Marek

I have taken the approach of using a validator to check these
relationships. The initial work can be seen here -
https://github.com/theforeman/foreman/pull/1616. I would ask that it
receive some expedited review so I can continue forward on locking some
individual entities.

Thanks,
Eric

··· On Wed, Jul 23, 2014 at 3:33 AM, Marek Hulan wrote:

Hello,

sending my comments below in text

On Tuesday 22 of July 2014 08:25:37 Eric D Helms wrote:

The backstory is that I am working on taking our implementation to this
point and trying to ensure proper scoping of objects in the Katello
controllers (see [1] and [2]) for some reference. While everything has so
far gone smoothly I have ran into an issue that I needs addressing across
the projects. Here’s the scenario:

  1. Create a new user: testuser
  2. Grant that user a new permission that allows “view” and “create” on
    subnets.
  3. As the admin, create a new Domain “example.org” with ID 1
  4. Via the API, authenticating as ‘testuser’ create a new subnet with
    that
    domain linkage, e.g:

POST: /api/v2/subnets

{
“subnet”: {
“name”: “TestNet”,
“domain_ids”: [1],
“network”: “255.255.192.0 callto:255.255.192.0”,
“mask”: “255.255.255.0 callto:255.255.255.0
}
}

Returned:

{
“id”: 1,
“name”: “TestNet”,
“network_address”: “255.255.192.0 callto:255.255.192.0/24”,
“network”: “255.255.192.0 callto:255.255.192.0”,
“cidr”: 24,
“mask”: “255.255.255.0 callto:255.255.255.0”,
“priority”: null,
“vlanid”: null,
“gateway”: null,
“dns_primary”: null,
“dns_secondary”: null,
“from”: null,
“to”: null,
“created_at”: “2014-07-21T15:13:00Z”,
“updated_at”: “2014-07-21T15:13:00Z”,
“dhcp”: null,
“tftp”: null,
“dns”: null,
“locations”: [],
“organizations”: [],
“domains”: [
{
“id”: 1,
“name”: “example.org
}
]
}

Issue 1:
As a user who is only allowed to “view” and “create” subnets, I have been
able to attach the subnet to a domain that have do not have access to.

I think that the right solution in this case is verify that domain with
particular id exists and I have view permission for it. So something like

def create
domains = Domain.authorized(:view_domains).find(:id => params[:subnet]
[:domain_ids])
@subnet = Subnet.new(params[:subnet])
@subnet.domains = domains
@subnet.save
end

We can discuss if this should be implemented in controller or rather model,
how much it should be abstracted etc. The most important thing I’m trying
to
express is that I wouldn’t mix it into create_subnets permissions that
would
somehow enforce view_domain permission but I’d keep it as controller or
probably better model responsibility. Each model could define which
associations should be checked by which permission. Then in callback
validation it would do the check.

We may find

https://github.com/ares/foreman/blob/fix/5929/app/models/concerns/dirty_associations.rb
from unmerged PR https://github.com/theforeman/foreman/pull/1479 useful
for
checking of modified associations.

Issue 2:
On the Katello side we attempt to verify all nested objects as accessible
by a user before allowing them to attach that object. Typically, this
means
that if I am editing a Product, for example, and want to attach a GPG Key
that I must have access to that GPG Key. Our assumption is that a user
must
have “view” on the GPG Key and edit/create on the Product. Currently, the
nested object API methods would automatically try to calculate and assume
that I have ‘update’ on both objects due to that being the current action
permission see -

https://github.com/theforeman/foreman/blob/develop/app/controllers/api/base_

controller.rb#L237. Question here is - what is the proper behavior and
what
do we want to enforce?

I think this is more or less the same as issue 1 and same solution should
apply.

The above are the two big issues I have run into whereby we are
inconsistent with our behavior expectations and there appears to be a
potentially large gap on the Foreman (and Katello side to a degree)
around
the API (and UI) and what a user can and cannot do based on their
permission set. I wanted to reach out and see if I was missing anything,
or
what your thoughts were on these issues. If you feel I need to open this
up, I am more than happy to send this out to a wider audience or put
together a meeting to discuss so that we can converge on consistency.

Thanks,
Eric

[1] https://github.com/theforeman/foreman/pull/1596
[2] https://github.com/Katello/katello/pull/4454


Marek


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