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.

Sign up for GitHub

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

[a2-694] Add policy for applications service IAM v1 and v2 #462

Merged
merged 5 commits into from
Jun 5, 2019
Merged

[a2-694] Add policy for applications service IAM v1 and v2 #462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3a9da67
[a2-694] Add policy for applications service IAM v1 and v2
May 31, 2019
7c7959a
-Add v2 IAM policies to viewer and editor role
May 31, 2019
7a0b296
Split iam role migration to datamigration directory
Jun 3, 2019
ce1d770
Regenerate protos, remove END; from sql
Jun 3, 2019
43ace07
Add documents for default policies
Jun 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
180 changes: 90 additions & 90 deletions api/external/applications/applications.pb.go
View file
Open in desktop

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions api/external/applications/applications.pb.policy-v1.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions api/external/applications/applications.pb.policy-v2.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 12 additions & 14 deletions api/external/applications/applications.proto
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,44 +18,42 @@ import "api/external/common/query/parameters.proto";
service ApplicationsService {
rpc GetServiceGroups(ServiceGroupsReq) returns (ServiceGroups) {
option (google.api.http).get = "/beta/applications/service-groups";
// TODO (dan, 2/2019): need to replace this once we have resources and such
// created in the auth system
option (chef.automate.api.policy) = {
resource: "nodes"
resource: "service_groups"
action: "list"
};
option (chef.automate.api.iam.policy) = {
resource: "infra:nodes"
action: "infra:nodes:list"
resource: "applications:serviceGroups"
action: "applications:serviceGroups:list"
};
};
rpc GetServiceGroupsHealthCounts(ServiceGroupsHealthCountsReq) returns (HealthCounts) {
// TODO (afiune, 3/2019): need to replace this once we have resources and such
// created in the auth system
option (google.api.http).get = "/beta/applications/service_groups_health_counts";
option (chef.automate.api.policy).resource = "nodes";
option (chef.automate.api.policy).resource = "service_groups";
option (chef.automate.api.policy).action = "list";
option (chef.automate.api.iam.policy).resource = "infra:nodes";
option (chef.automate.api.iam.policy).action = "infra:nodes:list";
option (chef.automate.api.iam.policy).resource = "applications:serviceGroups";
option (chef.automate.api.iam.policy).action = "applications:serviceGroups:list";
Copy link
Contributor

@bcmdarroch bcmdarroch May 31, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you can GetServices, can you always get their healthcounts and get services by SG?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, at least you should be able to.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gotcha, makes sense that all the resources/actions should be the same then. 👍

};
rpc GetServices(ServicesReq) returns (ServicesRes) {
// TODO (afiune, 4/2019): need to replace this once we have resources and such
// created in the auth system
option (google.api.http).get = "/beta/applications/services";
option (chef.automate.api.policy).resource = "nodes";
option (chef.automate.api.policy).resource = "service_groups";
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this endpoint has anything to do with service-groups here. This is only listing all services so we would probably need a different resource?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For any future readers of this, we decided to stick with service_groups because this matches the labeling in the UI, so that is what users will expect to see.

option (chef.automate.api.policy).action = "list";
option (chef.automate.api.iam.policy).resource = "infra:nodes";
option (chef.automate.api.iam.policy).action = "infra:nodes:list";
option (chef.automate.api.iam.policy).resource = "applications:serviceGroups";
option (chef.automate.api.iam.policy).action = "applications:serviceGroups:list";
};

rpc GetServicesBySG(ServicesBySGReq) returns (ServicesBySGRes) {
// TODO (afiune, 4/2019): need to replace this once we have resources and such
// created in the auth system
option (google.api.http).get = "/beta/applications/service-groups/{service_group_id}";
option (chef.automate.api.policy).resource = "nodes";
option (chef.automate.api.policy).resource = "service_groups";
option (chef.automate.api.policy).action = "list";
option (chef.automate.api.iam.policy).resource = "infra:nodes";
option (chef.automate.api.iam.policy).action = "infra:nodes:list";
option (chef.automate.api.iam.policy).resource = "applications:serviceGroups";
option (chef.automate.api.iam.policy).action = "applications:serviceGroups:list";
};
rpc GetVersion (common.version.VersionInfoRequest) returns (common.version.VersionInfo) {
option (google.api.http).get = "/beta/applications/version";
Expand Down
4 changes: 4 additions & 0 deletions components/authz-service/constants/v1/constants.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,9 @@ const (
// data collector endpoint and have them proxied to A2's data collector without
// the need of a legacy data collector token
CSNginxComplianceDataCollectorPolicyID = "6e792df9-e51f-4474-9539-40ca2a2b308c"

// ApplicationsServiceGroupsPolicyID correlates to the policy applications:serviceGroups
ApplicationsServiceGroupsPolicyID = "aee14d59-da0b-4974-ba6d-1a018b024874"
)

// These are only used for testing and memstore purposes.
Expand Down Expand Up @@ -135,6 +138,7 @@ var (
ComplianceTokenSearchProfilesPolicyID,
ReadOwnUserProfilePolicyID,
LocalUserSelfPolicyID,
ApplicationsServiceGroupsPolicyID,
}

// NonDeletablePolicyIDs is an array of non-deletable policy IDs.
Expand Down
15 changes: 15 additions & 0 deletions components/authz-service/storage/postgres/datamigration/sql/07_update_v2_roles.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
BEGIN;

UPDATE iam_roles
SET
actions = actions || '{applications:*:list, applications:*:get}'
WHERE
id = 'viewer';

UPDATE iam_roles
SET
actions = actions || '{applications:*}'
WHERE
id = 'editor';

COMMIT;
13 changes: 13 additions & 0 deletions components/authz-service/storage/postgres/migration/sql/45_add_application_policies.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
BEGIN;

INSERT INTO policies
VALUES ('aee14d59-da0b-4974-ba6d-1a018b024874',
'{"action": "*", "effect": "allow", "resource": "service_groups", "subjects": ["user:*"]}',
CURRENT_TIMESTAMP,
1,
TRUE)
ON CONFLICT (id) DO UPDATE
SET policy_data='{"action": "*", "effect": "allow", "resource": "service_groups", "subjects": ["user:*"]}',
deletable=TRUE;

COMMIT;
8 changes: 8 additions & 0 deletions components/authz-service/storage/v1/storage.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,14 @@ func DefaultPolicies() (map[string]*Policy, error) {
Effect: "allow",
Version: 1,
},
constants.ApplicationsServiceGroupsPolicyID: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You've added this policy to our system policies, which are hidden policies that govern permissions that Automate needs to work correctly (i.e. every user always needs to be able to get the license status so they won't be always blocked by the license modal). These policies never change.

My guess is that we should be able to change permissions on this API. In that case, instead of adding this policy, we'll want to add the API's v2 action to the role(s) that should have access. Maybe Viewer and Editor?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually just kidding about that system policy comment! i misread the code. this is correct for adding a v1 default policy 👍

for v2 permissions:

  1. add applications:* to the Editor role
  2. add applications:serviceGroups:list to the Viewer role
  3. add a datamigration updating those two roles with their new actions (similar to this migration)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make changes to these roles please also update the iam v2 docs, thx!

ID: ids[constants.ApplicationsServiceGroupsPolicyID],
Subjects: []string{"user:*"},
Resource: "service_groups",
Action: "*",
Effect: "allow",
Version: 1,
},
}
return defaultPolicies, nil
}
34 changes: 34 additions & 0 deletions components/automate-chef-io/content/docs/default-policies.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,40 @@ EventFeed | GetEventTypeCounts | /event_type_counts | GET | events:types | count
EventFeed | GetEventTaskCounts | /event_task_counts | GET | events:tasks | count
EventFeed | GetEventStringBuckets | /eventstrings | GET | events:strings | read

## Applications (BETA)

### Applications page

> These default policies allow all users to perform any action on application page resources

```bash
{
"action": "*",
"resource": "service_groups",
"subjects": [
"user:*"
]
},
{
"action": "*",
"resource": "service_groups:*",
"subjects": [
"user:*"
]
}
```

### Applications Page

> Corresponds to "Application tab (`/applications`)

Service | Method | HTTP Endpoint | HTTP Method | Resource | Action
---|---|---|---|---|---
Applications | GetServiceGroups | /beta/applications/service-groups | GET | service_groups | list
Applications | GetServiceGroupsHealthCounts | /beta/applications/service_groups_health_counts | GET | service_groups | list
Applications | GetServices | /beta/applications/services | GET | service_groups| list
Applications | GetServicesBySG | /beta/applications/service-groups/{service_group_id} | GET | service_groups | list

## Telemetry

### TelemetryPolicies
Expand Down
4 changes: 2 additions & 2 deletions components/automate-chef-io/content/docs/iam-v2-api-reference.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ Roles you create are Custom roles.
Chef-managed Role Name | ID| Actions
-----------------------|-----|--------
Owner | owner | `*`
Viewer | viewer | `infra:*:get`, `infra:*:list`, `compliance:*:get`, `compliance:*:list`, `system:*:get`, `system:*:list`, `event:*:get`, `event:*:list`, `ingest:*:get`, `ingest:*:list`
Editor | editor | `infra:*`, `compliance:*`, `system:*`, `event:*`, `ingest:*`, `secrets:*`, `telemetry:*`
Viewer | viewer | `infra:*:get`, `infra:*:list`, `compliance:*:get`, `compliance:*:list`, `system:*:get`, `system:*:list`, `event:*:get`, `event:*:list`, `ingest:*:get`, `ingest:*:list`, `applications:*:list`, `applications:*:get`
Editor | editor | `infra:*`, `compliance:*`, `system:*`, `event:*`, `ingest:*`, `secrets:*`, `telemetry:*`, `applications:*`
Ingest | ingest | `infra:ingest:*`, `compliance:profiles:get`, `compliance:profiles:list`

### Listing Roles
Expand Down