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

Add documentation for authorization on the REST layer #4544

Merged
merged 11 commits into from
Sep 29, 2023

Conversation

cwillum
Copy link
Contributor

@cwillum cwillum commented Jul 12, 2023

Description

To support extensions, authorization of API actions can now be carried out on the REST layer, necessarily for extensions and optionally for new and existing plugins. This PR documents details surrounding the enhancement.

Issues Resolved

Fixes #4381

Checklist

  • By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and subject to the Developers Certificate of Origin.
    For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum added 2 - In progress Issue/PR: The issue or PR is in progress. security release-notes PR: Include this PR in the automated release notes v2.9.0 labels Jul 12, 2023
@cwillum cwillum self-assigned this Jul 12, 2023
@cwillum cwillum marked this pull request as draft July 12, 2023 03:16
@cwillum
Copy link
Contributor Author

cwillum commented Jul 12, 2023

Just collecting the dots at this point. Thus the draft status.

Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum added v2.10.0 1 - Backlog Issue: The issue is unassigned or assigned but not started and removed 2 - In progress Issue/PR: The issue or PR is in progress. v2.9.0 labels Jul 12, 2023
@cwillum
Copy link
Contributor Author

cwillum commented Jul 12, 2023

Received news that this will most likely get pushed to 2.10.

Signed-off-by: cwillum <cwmmoore@amazon.com>
Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum marked this pull request as ready for review August 29, 2023 00:33
Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum
Copy link
Contributor Author

cwillum commented Aug 29, 2023

@DarshitChanpura Could you have a look at this for technical accuracy, language, and completeness? I'm not sure if I got some of the language-concept agreement right in some cases. Thanks.

Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum removed the 1 - Backlog Issue: The issue is unassigned or assigned but not started label Aug 30, 2023
@cwillum cwillum added the 3 - Tech Review PR: Tech review in progress label Aug 30, 2023
@hdhalter hdhalter added v2.11.0 and removed v2.10.0 labels Sep 8, 2023
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

Some minor changes requested. Thank you @cwillum for putting this together.


# REST layer authorization

Authorization on the REST layer provides an added level of security for plugin and extension API requests by offering a mechanism for authorization checks on the REST layer. This level of security sits atop the transport layer and provides a complementary method of authorization without replacing, modifying, or in any way changing the same process on the transport layer. REST layer authorization was initially created to address the need for an authorization check for extensions, which do not communicate on the transport layer. However, the feature is also supported for existing plugins and will be available for future plugins created to operate with OpenSearch.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Authorization on the REST layer provides an added level of security for plugin and extension API requests by offering a mechanism for authorization checks on the REST layer. This level of security sits atop the transport layer and provides a complementary method of authorization without replacing, modifying, or in any way changing the same process on the transport layer. REST layer authorization was initially created to address the need for an authorization check for extensions, which do not communicate on the transport layer. However, the feature is also supported for existing plugins and will be available for future plugins created to operate with OpenSearch.
Authorization on the REST layer provides an added level of security for plugin and extension API requests by offering a mechanism for authorization checks on the REST layer. This level of security sits atop the transport layer and provides a complementary method of authorization without replacing, modifying, or in any way changing the same process on the transport layer. REST layer authorization was initially created to address the need for an authorization check for extensions, which do not communicate on the transport layer. However, the feature is also available for developers to utilize while creating future plugins to operate with OpenSearch.

Current plugins have not been migrated to utilize this feature. But, the feature is available for current plugins to migrate their APIs to support authorization.


For users that work with REST layer authorization, the methods of assigning roles and mapping users and roles, and the general usage of plugins and extensions, remain the same: the only additional requirement being that users become familiar with a new scheme for permissions. Developers, on the other hand, will need to understand the ideas behind `NamedRoute` and how the new route scheme is constructed. For detailed information, see [Authorization at REST Layer for plugins](https://github.com/opensearch-project/security/blob/main/REST_AUTHZ_FOR_PLUGINS.md).

The benefits to developers when using the REST layer for authorization mean that they do not need to build transport layer actions and get authorization for them to adhere to security procedures. As a result, this decreases the code-writing burden and time invested in creating a single action. As an alternative, they can create REST API actions and authorize them on the REST layer.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The benefits to developers when using the REST layer for authorization mean that they do not need to build transport layer actions and get authorization for them to adhere to security procedures. As a result, this decreases the code-writing burden and time invested in creating a single action. As an alternative, they can create REST API actions and authorize them on the REST layer.
The benefits to developers when using the REST layer for authorization mean that they can authorize requests at REST layer and filter out unauthorized requests. As a result, this decreases the processing burden on transport layer while allowing granular control over access to APIs.

We are not replacing transport authz, we are just adding a layer on top.


`_/detectors/<detectorId>/profile`

To create a NamedRoute from this, the `routeNamePrefix` value in the `settings.yml` file for the resource `ad` is added to the route to complete a unique name. The result is shown in the following example:
Copy link
Member

Choose a reason for hiding this comment

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

This is true only for extensions. For plugins, developers will have to declare the permission names.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DarshitChanpura (1) What exact part of "this" is true for creating a permission name for extensions? Is it this whole piece: "... the routeNamePrefix value in the settings.yml file for the resource ad is added to the route to complete a unique name."?
(2) Is there any guidance for declaring a permission name for plugins? Are there required elements in the name like that for a permission name for an extension? Or do you just invent anything you want?

Copy link
Member

Choose a reason for hiding this comment

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

Apologies for confusion. By "this" I mean the routeNamePrefix is a setting only configured in extension. This setting doesn't exist in plugins.

There is no guidance as of now in defining permissions. But we can make a suggestion to have it something like: <plugin/extension-name>:<route> (i.e. ad:detectors/get)

Let me know if this helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DarshitChanpura Thanks for the clarification. I've adjusted that wording to include current understanding for extensions and plugins. I've also addressed the other two comments/suggestions. I think this should be ready for documentation team review when 2.11 comes along. I'll comment that it's on hold for that. And if there are any changes between now and then, we can always revise the current draft before the release.

Signed-off-by: cwillum <cwmmoore@amazon.com>
Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum added 6 - Done but waiting to merge PR: The work is done and ready to merge and removed 6 - Done but waiting to merge PR: The work is done and ready to merge labels Sep 21, 2023
@cwillum
Copy link
Contributor Author

cwillum commented Sep 21, 2023

The work is essentially done for this PR. I considered adding the status tag "Done but waiting to merge". But it will need documentation team review and editorial review before merging for 2.11. I'm leaving it alone for now in case the Security team wants any additional changes before then.

Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

LGTM!

@cwillum cwillum added 4 - Doc Review PR: Doc review in progress and removed 3 - Tech Review PR: Tech review in progress labels Sep 21, 2023
@cwillum
Copy link
Contributor Author

cwillum commented Sep 21, 2023

Stepping this up to doc review.

Copy link
Collaborator

@Naarcha-AWS Naarcha-AWS left a comment

Choose a reason for hiding this comment

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

Two small fixes, otherwise LGTM.

_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
Signed-off-by: cwillum <cwmmoore@amazon.com>
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

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

LGTM. @Naarcha-AWS Would you please re-review this?

Copy link
Collaborator

@natebower natebower left a comment

Choose a reason for hiding this comment

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

@cwillum Just a few comments/changes. Thanks for your advance work on this!

_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved

For users that work with REST layer authorization, the methods of assigning roles and mapping users and roles, and the general usage of plugins and extensions, remain the same: the only additional requirement being that users become familiar with a new scheme for permissions.

Developers, on the other hand, will need to understand the ideas behind `NamedRoute` and how the new route scheme is constructed. For detailed information, see [Authorization at REST Layer for plugins](https://github.com/opensearch-project/security/blob/main/REST_AUTHZ_FOR_PLUGINS.md).
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's probably worth noting that up to this point, we used "on" the REST layer as opposed to "at". We should probably be consistent throughout.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

RE: Authorization at REST Layer for plugins
Yeah, I can't do anything about the name of that resource. It's created by Security team developers. But point taken.

_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
_security/access-control/rest-layer-authz.md Outdated Show resolved Hide resolved
Signed-off-by: cwillum <cwmmoore@amazon.com>
@cwillum cwillum merged commit 39c0358 into main Sep 29, 2023
4 checks passed
@cwillum cwillum deleted the fix#4381-authz-rest-layer branch September 29, 2023 20:37
@hdhalter hdhalter removed the 4 - Doc Review PR: Doc review in progress label Oct 12, 2023
vagimeli pushed a commit that referenced this pull request Oct 13, 2023
* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

---------

Signed-off-by: cwillum <cwmmoore@amazon.com>
Signed-off-by: Melissa Vagi <vagimeli@amazon.com>
harshavamsi pushed a commit to harshavamsi/documentation-website that referenced this pull request Oct 31, 2023
…ject#4544)

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

---------

Signed-off-by: cwillum <cwmmoore@amazon.com>
vagimeli pushed a commit that referenced this pull request Dec 21, 2023
* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

* fix#4381 authz on rest layer

Signed-off-by: cwillum <cwmmoore@amazon.com>

---------

Signed-off-by: cwillum <cwmmoore@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes PR: Include this PR in the automated release notes security v2.11.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DOC] Add doc for Authorization in REST Layer
5 participants