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

IAM Action Documentation #1345

Closed
2 tasks
susanev opened this issue Aug 24, 2019 · 26 comments
Closed
2 tasks

IAM Action Documentation #1345

susanev opened this issue Aug 24, 2019 · 26 comments
Labels
auth-team anything that needs to be on the auth team board documentation Anything related to the Automate docs. Epic iamv2 This issue or pull request applies to iamv2 work for Automate

Comments

@susanev
Copy link
Contributor

susanev commented Aug 24, 2019

Problem: Users do not have the information they need to define custom roles and policies.

Overview

Currently, we do not provide a reference for the many actions that are possible to use when defining a role to be used in a policy. Let's autogenerate a table of those actions that include the action identifier, and a brief optional description based on annotations. The descriptions will be important for anything that has different names then what is displayed in the ui, some examples are node manager vs node integration and event vs action.

Design Details

  • New page in the iamv2 beta docs, possibly titled Actions, need to work with docs folks on name
  • Page should contain a brief introduction, and then a table of actions with 2 columns, 1 for the identifier, and 1 for the optional description
  • Find places in the rest of the IAM v2 docs to reference and link to the actions page
  • Use Hugo Data Template
    • Data goes into a folder in automate/components/automate-chef-io/data/docs/
    • Rendered by a layout in automate/components/automate-chef-io/layouts/
    • Page structured like Automate CLI Docs
    • Reach out to @kagarmoe for related questions

Subtasks

  • autogenerate what API endpoints + methods are permissions
  • what they do manual documentation work

Reference

@susanev susanev added documentation Anything related to the Automate docs. iamv2 This issue or pull request applies to iamv2 work for Automate auth-team anything that needs to be on the auth team board labels Aug 24, 2019
@srenatus
Copy link
Contributor

💭 I believe we already have a card for this somewhere. The problem is that autogenerating what API endpoints + methods are permissions is simple, but what they do will be manual documentation work.

I wonder how this ties in with the current state of our API docs efforts... 😃 I'd think it would make sense if both "this is the action permission required to use this endpoint" and "this is what the endpoint does" was part of the API docs.

@susanev
Copy link
Contributor Author

susanev commented Aug 26, 2019

we could break it up into those two subtasks? id really like to be able to get something out there even if it doesn't have the what they do part yet.

@kagarmoe
Copy link

kagarmoe commented Aug 26, 2019

The API work is ongoing, but if you're autogenerating you'll need to follow the Hugo Data Template workflow. The data goes into a folder in automate/components/automate-chef-io/data/docs/ and gets rendered by a layout in automate/components/automate-chef-io/layouts/ to a page structured like https://github.com/chef/automate/blob/master/components/automate-chef-io/content/docs/cli-chef-automate.md

@srenatus
Copy link
Contributor

srenatus commented Aug 27, 2019

📜 some notes for backlog grooming

for the first part,

autogenerate what API endpoints + methods are permissions

desirables

  1. there is only a single source of truth for this, no separate document to keep in sync

current state

  • Right now, the information lives sprinkled all over the place in our external API proto3 definitions,
    rpc CreatePolicy (CreatePolicyReq) returns (CreatePolicyResp) {
    option (google.api.http).post = "/iam/v2beta/policies";
    option (google.api.http).body = "*";
    option (chef.automate.api.policy).resource = "auth:policies";
    option (chef.automate.api.policy).action = "create";
    option (chef.automate.api.iam.policy).resource = "iam:policies";
    option (chef.automate.api.iam.policy).action = "iam:policies:create";
    };
  • we're using a custom code generator to transform them into golang programs with init() functions transforming them into items of a map (one for v1 and v2 each)
  • these are read back
    • from automate-gateway's middleware, to figure out which permissions to ask for, when a request comes in
    • from the introspection handler code

possible approaches

  1. We could create another custom code generator, to read them into a JSON (or YAML) document, which could then be feed into a hugo data template.
  2. We could make the data document the source of truth:
    • create a YAML file containing endpoint+method mappings to resources and actions
    • bake this map into the gateway at build time
    • use this document for driving the API permission documentation (through hugo, with the doc uses as-is)

The first approach is the conservative one, do more of what we know how to do. On the other hand, the second approach will probably have us end up in a state where the system is significantly simpler, without losing anything. There's no consumers of our GRPC API (yet, or even soon?) and when when there are, the extensions we've introduced are not adhering to any standard, so -- there's no benefit from having the permissions in the proto files. Except for keeping them close to the method definiton, and making them hard to miss. If they're in an extra file, you might miss them. We would add tests that yell if a method is defined that doesn't have a mapping.

Besides being simpler, the other benefit from the second approach is that by decoupling the method mapping from the endpoint definition, we can make full use of grpc-gateway's features. Right now, for example, we cannot have patchy endpoints because they require a certain form of path that we cannot deal with in our simple method mapping logic.

Also, the second approach lets us extend the per-endpoint information more easily: if we want our own format for descriptions, examples, whatnot, adding fields to a YAML doc is much simpler than threading this into our code generation plugin. (That is, if OpenAPI (== Swagger) is not sufficient for extra metadata.)

more notes

  • For going from where we are now to the second approach, there are probably tools we could use to translate the existing mappings into a JSON/YAML doc. protoc-gen-json looks promising, but it seems like its omitting our options, but I've asked them about it: protoc-gen-json: options are omitted sourcegraph/prototools#16 Update This won't work, as grpc-gateway's, and our, method options are not JSON-serialisable (no idea why 😄). So, what we'd have to do to switch is create a one-off branch where our protoc-gen-policy generators just dump the data into a yaml file. Or just awk through this.

@srenatus
Copy link
Contributor

💭 It would be really cool if the UI gave you an idea about what's up given your permissions... I.e. if some UI elements weren't hidden when you don't have the permissions to use them, but disabled instead, with the mouseover text (or something similar) indicating which permissions your lacking...

@nataliesea
Copy link

Here's the location that I think covers AWS documentation: https://docs.aws.amazon.com/cognito/latest/developerguide/role-based-access-control.html

@susanev
Copy link
Contributor Author

susanev commented Aug 27, 2019

this is there actions table
https://docs.aws.amazon.com/IAM/latest/UserGuide/list_identityandaccessmanagement.html

they seem to have a page for every action...

@srenatus
Copy link
Contributor

@msorens since you've asked in our discussion today, this would allow us to also take the google.api.http annotations out of the proto files, too: https://grpc-ecosystem.github.io/grpc-gateway/docs/grpcapiconfiguration.html But there's currently no way to put extra openapi (swagger) information into that, see grpc-ecosystem/grpc-gateway#701 (comment)

@kagarmoe
Copy link

Let's pull in @phiggins , who is working on autogenerating the api docs.

@srenatus
Copy link
Contributor

@phiggins I've considered adding action/resource (or only action) to the swagger docs, but it seems the grpc-gateway code generator doesn't support such arbitrary key/val metadata additions on pairs of HTTP endpoint + HTTP method. (That is, I've asked around in the grpc-gateway channel of the gopher slack today...) -- This is the background for why I'd go with another YAML file instead. We could weave these together in the template, maybe...? 💭

@phiggins
Copy link
Contributor

The tools we're using to generate the swagger from proto files seem limited in their flexibility so I'm not sure how much value there is in pursuing that route. There is likely going to need to be post-processing scripting done to get all the stuff we want supported so using separate files to list the auth information seems ok to me.

@kagarmoe
Copy link

kagarmoe commented Aug 29, 2019

Before we develop something bespoke, I would like the opportunity to work with you on what you've developed. Looking at the needs that @srenatus has outlined in his discussion, we should discuss making the case for moving to the newer OpenAPI 3.0 Specification. In any event, let's make sure this work conforms to existing API documentation standards as well as the needs of user documentation. Thanks!

@srenatus
Copy link
Contributor

OK so there are a few things here:

  1. For authorization, we need a single place to put mappings between GRPC methods and authz actions.

    Right now, this place is the proto3 options shown above. The proposal is to move them into a YAML file to simplify processing them outside of their current use and expanding.

  2. There's API docs.

    They're currently generated from the swagger (openapi2) json files (created by protoc-gen-swagger), fed into a hugo template, if I understand correctly.

  3. There's IAM/Authorization docs.

They should explain what kind of access an action grants you. In our system, actions roughyl map 1-to-1 to GRPC methods, and thus HTTP endpoint+verb pairs.

What was outlined so far, would amount to _using the source of truth created in (1) for (3).

From the discussion, it looks like what's generated "for free" in (2) is not what we want for API docs, so there's OpenAPI 3. I'm not sure what to make of that -- If we had OpenAPI 3 specs, and they would allow for an arbitrary field attached to a endpoint+verb pair, we could use them as source-of-truth for our needs in (1) and (3). However, I don't know if this is something the spec allows, and I don't know how close we are in terms of having an OpenAPI 3 spec at all, so I'd lean towards a very simple solution to (1) and (3), that, even if bespoke, will be nowhere near any kind of complete API docs.

@kagarmoe @phiggins Am I misrepresenting the state of affairs wrt. API docs? What do you think about this ☝️?

@srenatus
Copy link
Contributor

ℹ️Some notes from looking around...

  • OpenAPI3 "Specification extensions" could be interesting for adding what we need for authz docs directly into our API spec
  • gnostic is an interesting framework. It's like protobuf, with different generator plugins processing stuff they're fed, but with an OpenAPI spec (version 2 or 3) as the central artifact to generate stuff from.
    • gnostic-grpc finally turns the stuff we currently do upside down: it'll generate GRPC proto files from an OpenAPI spec 😄 Including grpc-gateway, see this example

@phiggins
Copy link
Contributor

@srenatus Your assessment is accurate, AFAIK. OpenAPI 3 looks like a nice evolution of the standard but we're limited in supporting it because of the tools we're using.

@srenatus
Copy link
Contributor

srenatus commented Sep 3, 2019

ℹ️ We've opted for approach 2. Having the authz mappings in a YAML file will also not lock us in in any way when we'd later want to weave this information into some OpenAPI 3 spec.

@kagarmoe
Copy link

kagarmoe commented Sep 3, 2019

I'm not sure that I completely understand the proposed documentation. Is the plan to create a document mapping the GRPC methods and HTTP endpoint+verb pairs? Who is the audience for this documentation? What information question does it answer for our customers that wouldn't be answered by good API documentation?

@susanev
Copy link
Contributor Author

susanev commented Sep 3, 2019

@kagarmoe @srenatus would y'all be able to do a zoom call sometime soon to go over our proposal? i can set something up if yes.

@srenatus
Copy link
Contributor

srenatus commented Sep 3, 2019

@kagarmoe one by one 😃

Is the plan to create a document mapping the GRPC methods and HTTP endpoint+verb pairs?

Besides the one-to-one mapping between GRPC methods and HTTP endpoint+verb pairs, there's a third thing: IAM actions and resources. And that mapping is what we're looking to shine a light on.

Who is the audience for this documentation?

Anyone writing IAM policies who wants to understand what granting a user access to foo a bar means concretely -- in terms of API methods (GRPC or HTTP).

Update Probably more often than not, the question will be: My request to XYZ failed with a 403, which policy do I need to create to make it succeed?

What information question does it answer for our customers that wouldn't be answered by good API documentation?

The mentioned mapping of IAM action/resource pairs to HTTP endpoints+verbs. There's a table somewhere, I believe, for IAMv1, which is likely outdated and wrong.

Ideally, this would find a place in good API docs, for sure. However, from the stuff above, it seems that OpenAPI v2 doesn't allow for putting arbitrary key-val pairs anywhere.

@susanev happy to zoom if that's what everyone prefers 😃 (Time[zones] permitting.)

@kagarmoe
Copy link

kagarmoe commented Sep 3, 2019

I'm not sure that these ideas about the limitations of the OpenAPI 2.0 spec are correct. This spec can handle OAuth2 Security requirements. OpenAPI is also extensible.

Extensions & Implementations
Swagger 2.0 extensions documentation
Extension registry
AWS API Gateway
Google Cloud OpenAPI
Google Cloud Overview of API Access
Google Cloud Extensions
Microsoft OpenAPI 2.0 metadata support in Azure Functions. Also, search MS docs for OpenAPi. Azure is on 3.0
Microsoft OpenAPI.NET SDK

API Handyman blog by Arnaud Lauret on Extensions

I think that I'm having trouble visualizing what this doc needs to be. I'd love it if someone could mock up some content for me, so I can understand the information need and solution a bit better. Could you whip something up (not a full doc, just an example) so we can meet on it? Thanks!

NB: I think finally get what you are after, thanks to @susanev . I need to think about it & play with this a bit.

@susanev
Copy link
Contributor Author

susanev commented Sep 3, 2019

@srenatus
Copy link
Contributor

srenatus commented Sep 4, 2019

@kagarmoe thanks for all those resources! 😃

I'm not sure that these ideas about the limitations of the OpenAPI 2.0 spec are correct.

Ok, I might have been inexact here: I've been looking at openapi 2 through the lense of what grpc-gateway's code generator lets us use right now. This doesn't meant that we cannot either improve that one (we've done that before), or find other ways to amend our generated openapi2 docs.

It looks like @phiggins already has started doing that for custom endpoints.

This spec can handle OAuth2 Security requirements.

The example linked to looks like it could be (ab)used for our purposes:

If the security scheme is of type "oauth2" or "openIdConnect", then the value is a list of scope names required for the execution. For other security scheme types, the array MUST be empty.

So we could say that what is a scope in the spec is an action in our Automate API docs. We'd ignore that there's resources.

However, it's OpenAPI 3.0. Not sure if v2 knows of the same thing.

This, however,

Swagger 2.0 extensions documentation

looks like pretty much what we'd need 💡 -- @phiggins do you know a way to add some of these extension fields, too? Either using proto3 method options or using some post-processing step?


For this card, however, I'd uphold that unless we can use protoc-gen-swagger-supported fields today (or come up with a plan for extending the tools so that we can), we'd be better off moving our IAM action/resource mappings into a YAML file. From there, they can be mixed in in a post-processing step, etc.


Update We can misuse the oauth2 scope annotation (per-method, or "operation") in v2 using our current tooling. This is the diff, first the piece I've added to @phiggins's PR in #1412, and then how it boils down to OpenAPI v2 json:

diff --git a/components/automate-gateway/api/compliance/reporting/reporting.proto b/components/automate-gateway/api/compliance/reporting/reporting.proto
index 0a0ab94ce..68dc1c0b3 100644
--- a/components/automate-gateway/api/compliance/reporting/reporting.proto
+++ b/components/automate-gateway/api/compliance/reporting/reporting.proto
@@ -33,6 +33,14 @@ service ReportingService {
                        description: "List all reports optionally using filters";
                        summary: "List reports";
                        tags: "Compliance Reporting Reports";
+                       security: {
+                               security_requirement: {
+                                       key: "OAuth2";
+                                       value: {
+                                               scope: "compliance:reports:list";
+                                       }
+                               }
+                       }
                };
        };

diff --git a/components/automate-gateway/api/compliance/reporting/reporting.swagger.json b/components/automate-gateway/api/compliance/reporting/reporting.swagger.json
index 4e92f0985..edb400d54 100644
--- a/components/automate-gateway/api/compliance/reporting/reporting.swagger.json
+++ b/components/automate-gateway/api/compliance/reporting/reporting.swagger.json
@@ -151,6 +151,13 @@
         ],
         "tags": [
           "Compliance Reporting Reports"
+        ],
+        "security": [
+          {
+            "OAuth2": [
+              "compliance:reports:list"
+            ]
+          }
         ]
       }
     },

My only concern there is the "misuse" bit. The security scheme Automate uses for API authorization is not OAuth2, and to make things worse, it's using OpenID Connect, a sort-of specialization of OAuth2, for authenticating users. So, on the authentication+authorization peninsula of Automate-land, there is scopes, but they're different; and not used for authorization in the oauth2 sense. My fear is that misuing this field like that will foster confusion.

I think we'd be better off exposing custom extensions of the openapiv2 annotations -- i.e., contrbute something to grpc-gateway.

@srenatus
Copy link
Contributor

srenatus commented Sep 5, 2019

Update I've looked a little deeper into this, see grpc-ecosystem/grpc-gateway#1033.

@susanev susanev added this to the Auth: Sprint 8 milestone Jan 2, 2020
@susanev
Copy link
Contributor Author

susanev commented Jan 24, 2020

an additional option for this, as we need to complete something to inform folks of the actions possible is to manually document a subset of the possible actions until we can come back and autogenerate it.

@sdelano sdelano changed the title Autogenerate access management role action documentation IAM Action Documentation Jan 27, 2020
@tylercloke tylercloke assigned tylercloke and unassigned tylercloke Feb 5, 2020
@msorens msorens self-assigned this Feb 6, 2020
@msorens
Copy link
Contributor

msorens commented Feb 9, 2020

Discussion with @susanev, @tylercloke, and @msorens:
The approach that for auto-generation that had early promise turns out will not fly (per @tylercloke). Instead, we decided to manually add annotations (possibly with a one-off script) to all relevant proto files (but only the ones that have been doc-annotated already; as others get documented they will need to add the portion for actions too). May also end up writing a unit test to ensure integrity of the copied data, but not very high priority for the test, since these are not likely to change.

@susanev susanev added the Epic label Feb 10, 2020
@susanev susanev removed this from the Auth: Sprint 7 milestone Feb 10, 2020
@susanev
Copy link
Contributor Author

susanev commented Mar 10, 2020

the rest of this work has been deprioritized, so closing.

@susanev susanev closed this as completed Mar 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auth-team anything that needs to be on the auth team board documentation Anything related to the Automate docs. Epic iamv2 This issue or pull request applies to iamv2 work for Automate
Projects
None yet
Development

No branches or pull requests

7 participants