Implement and update readme for new env var ALLOWED_METHODS

[nitzpo]

Implement and update readme for new env var ALLOWED_METHODS that limits the methods the tool can use

[gemini-code-assist]

Summary of Changes

Hello @nitzpo, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the configurability and security of the Lokka-Microsoft tool by introducing an environment variable to explicitly control which HTTP methods it can execute. This provides administrators with finer-grained control over the tool's capabilities, limiting potential actions to only those deemed necessary.

Highlights

• New Configuration Option: I've introduced a new environment variable, ALLOWED_METHODS, which allows users to specify a comma-separated list of HTTP methods that the Lokka-Microsoft tool is permitted to use. If this variable is not set, the tool will default to allowing all standard methods (GET, POST, PUT, PATCH, DELETE).
• Dynamic Method Enforcement: The Lokka-Microsoft tool's method parameter (z.enum) now dynamically enforces the list of allowed HTTP methods based on the ALLOWED_METHODS environment variable. This ensures that only the configured methods can be used when interacting with Microsoft APIs.
• Documentation Update: The README.md has been updated to include comprehensive documentation for the new ALLOWED_METHODS environment variable, explaining its purpose, format, and default behavior.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request introduces a new environment variable ALLOWED_METHODS to restrict the HTTP methods available in the Lokka-Microsoft tool. The implementation is straightforward, but I've identified a critical type-safety issue in the TypeScript source due to an incorrect type assertion, and a high-severity robustness issue in how the environment variable is parsed in both the TypeScript and JavaScript files. My review includes suggestions to fix these issues to ensure the feature is both safe and reliable.

[merill]

@nitzpo love this, thanks for the PR.

This is something I've been thinking about and I want to think more about how we implement this.

Should we list them as get, patch, delete or instead have config options that say

Permissions: Read, Write, Delete

Permission	Maps to
Read	GET, POST
Write	PUT, PATCH
Delete	DELETE

I feel read, write, delete might be easier to understand.

Also I think we should default to Read and make Write and Delete as options to configure.

[darrenjrobinson]

What is the intent of this additional env var @nitzpo ?
To provide context to the MCP client on the capabilities?
At the end of the day you can only perform what your authZ allows. And that can change by adding/removing permissions (scopes) on the App Reg. So then you'd also need to update your env var (to inform the MCP Client).

[AKonCloud]

Permission Maps to
Read GET, POST
Write PUT, PATCH
Delete DELETE

POST is very much a WRITE operation, although batch GET requests would be a POST.

[nitzpo]

Hi all! Thanks for the comments.

The idea is to be able to limit the permissions "on the fly", without creating new entities or changing your current permissions (sometimes you don't have permissions to change the permissions...)

My original idea was to create another tool "Lokka-Readonly" that will have limited permissions; and the regular tool will have full permissions. Then limiting the usage can be done easily in the IDE :)
But - I came to realize that in MS Graph, the POST method is used for both read and write! So which methods that tool will have available?? A major issue...

That brought me to the decision to implement it by selection of the http methods; so anyone can configure the available methods to their agent according to their liking and situation.

Another implementation decision I made was to make this limitation in the input enum to the tool. The other idea was to allow the agent to request any method, but assert inside the tool and respond with an error that the tool is blocked if its not in the allowlist.

[darrenjrobinson]

How about a different approach then to list the scopes allowed.
It is all MSGraph so limit to either .Read/Read.All or also allow .ReadWrite / ReadWrite.All ?
The scopes are what the authZ is scoped too not the protocol used for the action.

[nitzpo]

I don't think there is a reasonable way to limit access by scope at the request time; and I don't want to require changing the actual permissions

[merill]

Agree blocking POST will block many read apis

Eg

https://learn.microsoft.com/en-us/graph/api/group-evaluatedynamicmembership?view=graph-rest-beta&tabs=http

Conditional access - what if api

https://learn.microsoft.com/en-us/graph/api/conditionalaccessroot-evaluate?view=graph-rest-1.0

Etc..

I like @darrenjrobinson's idea of matching the API to the permission scope.

The only catch is that it can be quite buggy to implement. There is no explicit mapping available for this. I parse the info in the doc markdown for graphpermissions.merill.net but there are many areas where docs are not accurate etc..

Maybe having a second mcp with read only permissions is the better way to go.

[nitzpo]

I think its too complicated to implement and unreliable to do the matching here

About having a second mcp - still, how do you limit it to read operations only? Using HTTP methods or some other way?
If using HTTP methods - then do you give it just GET, or GET+POST which is unreliable in the sense that it may still access some write APIs using POST....

[gauthier-bedard-ssc]

Spitballing here but if there was a way to map those graph calls to PowerShell Graph SDK cmdlet, checking for the verb would be a good starting point? GET method still have the Get verb, POST splits into New, Test, etc which can better indicate what is allowed? It's getting convoluted but I think it would cover the edge cases decently. It would be fragile though.

[AKonCloud]

Agree blocking POST will block many read apis

But leaving it as a READ ONLY is misleading, as you can create conditional access policies, compliance polices, groups, users, basically you can create almost anything and everything.

[nitzpo]

Following all the comments here
My thoughts are that its too complicated and fragile to try and match whether an endpoint action is read only or not
I think giving the user the ability to select the HTTP methods is good value for now
This could be updated in the future if/when MSFT releases some solution for this in the API itself or its preipherals