Adds support for detecting and documenting command permissions #6705
Conversation
waldekmastykarz
force-pushed
the
commands-minimal-permissions
branch
from
dbddec9 to
e6d6357
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for detecting and documenting command permissions by introducing new Dev Proxy scripts, documentation, and configuration files for API specifications and CSOM mappings.
- Updated package.json to add new Dev Proxy commands.
- Added documentation outlining steps for detecting minimal permissions.
- Included new configuration files and OpenAPI specs for SharePoint REST and Tenant Admin APIs.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds new scripts for Dev Proxy operations. |
| docs/src/config/sidebars.ts | Registers the new documentation page in the sidebar. |
| docs/docs/contribute/document-minimal-permissions.mdx | Provides detailed guidance on detecting minimal permissions. |
| .devproxy/spo-csom-types.json | Introduces CSOM type mappings for permissions documentation. |
| .devproxy/generate-openapi-spec.json | Adds config for generating OpenAPI specs for REST APIs. |
| .devproxy/devproxyrc.json | Configures Dev Proxy with new plugins for minimal permissions. |
| .devproxy/api-specs/sharepoint.yaml | Contains the OpenAPI spec for SharePoint REST API with permissions. |
| .devproxy/api-specs/sharepoint-admin.yaml | Contains the OpenAPI spec for the SharePoint Tenant Admin API. |
Comments suppressed due to low confidence (2)
.devproxy/api-specs/sharepoint.yaml:228
- The generated tool version appears outdated compared to the new schema version referenced in the configuration. Consider updating this value to accurately reflect the current tool or schema version.
toolVersion: 0.25.0
.devproxy/api-specs/sharepoint-admin.yaml:67
- The tool version in the generated header may no longer be accurate given the updated schema versions in the other Dev Proxy configurations. Consider updating it to maintain consistency.
toolVersion: 0.25.0
There was a problem hiding this comment.
Great work on this one @waldekmastykarz! I’ve left a few remarks we should look into, though it’s possible I may have misunderstood something.
|
|
||
| ## Detect minimal permissions | ||
|
|
||
| To detect the minimal permissions for a command, we use [Dev Proxy](https://aka.ms/devproxy). Dev Proxy records the API requests that a command makes. Then, it analyzes the requests to determine the minimal permissions required for the command to run. For the analysis, it uses information about Microsoft Graph permissions from its documentation. For other APIs, such as SharePoint REST and CSOM, it uses information that we [curate together with the community](https://adoption.microsoft.com/sample-solution-gallery/sample/pnp-devproxy-sharepoint-api-minimal-permissions). |
There was a problem hiding this comment.
I initially tested this with the latest version (v0.29.2) of Dev Proxy, but it conflicted with the version set in devproxyrc.json. I think it’s better to explicitly specify the compatible version to avoid issues.
There was a problem hiding this comment.
With "conflicted" do you mean that you got the warning that the version you use differs from the version uses in the configuration? If so, it's partly caused because this PR has been open for a while and we released a new Dev Proxy version. Also, it's just a warning which doesn't prevent the setup from working. Once we get it in, we'll commit to keeping the configuration in sync with the latest Dev Proxy version.
There was a problem hiding this comment.
I do get a warning that my version doesn't match the one defined in the schema, but I also get a fail saying it can't load the GraphMinimalPermissionsPlugin, and I get this for each plugin.
And then it ends with:
There was a problem hiding this comment.
Dev Proxy v0.29 had a set of breaking changes, including renaming the plugin's DLL name. I'll update the PR to work with the latest version.
| - Press `s` in the Dev Proxy console. | ||
| - Dev Proxy stops the recording and shows the results in the console. |
There was a problem hiding this comment.
This isn't directly about this section, but more from my own experience. When testing with certain site addresses, I get a few warnings where it doesn’t even output the minimal permissions, it just says there’s no matching pattern, even though one is defined.
There was a problem hiding this comment.
If Dev Proxy doesn't find a matching pattern even though one is defined, that seems like a bug. Could you please log an issue with Dev Proxy so that we can double check all is working as intended?
There was a problem hiding this comment.
Adds support for site-relative URLs in detecting permissions
|
Thanks for the review @Jwaegebaert. I've updated the SPO API spec so that it supports variable paths for sites and subsites. I've also submitted an improvement to Dev Proxy, that introduces support for variable paths in API specs: dotnet/dev-proxy#1296. We'll try to get it merged in a beta asap so that we can retest this PR. |
|
We've just released a new beta of Dev Proxy (v1.0.0-beta.1) which now supports variable paths. I've also updated the Dev Proxy configuration to v1.0.0. Try it again please, and see if it's working as intended now. |
There was a problem hiding this comment.
Awesome work on this one @waldekmastykarz, works very smoothly and the guide is very clear!
Adds support for detecting and documenting command permissions, including