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

draft feat: roles for api keys #2321

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

dave-gray101
Copy link
Collaborator

@dave-gray101 dave-gray101 commented May 14, 2024

Roles

This was prepared as a quick option in case we want a demo.
Currently working on an improved version using casbin for permanent version - will replace this PR when finished.


initial version of a role system for api keys, allowing users to be granted more finely grained access to specific parts of localai

Some notes:

  • adds a new configuration file that is shipped by default named roles.json - exists to give sane combinations of endpoints easy names, though technically this can be skipped.

  • changes how api_keys are interpreted. Now it's a map of api key ==> slice of endpoints
    whenever api_keys.json is evaluated (including at the initial program load) we shake out the endpoint slice - reducing roles to raw endpoints and deduplicating

  • the old format for api_keys.json still works... and is interpreted as if all keys are ui user

  • a fake api key of "_" represents the set of endpoints that should be authorized for unauthenticated users. Currently this also expands the access of all authenticated users - so "_" serves as the single default set of allowed to all endpoints. Let me know if anyone can think of a reason to have the complication of two distinct lists

I'd appreciate help testing every possible use case... as well as ideas for ways to improve roles.json such as different groupings or things I've missed

Docs will need to be updated to match this, but I want to do that in a separate PR for organizational reasons.

…ranted more finely grained access to specific parts of localai

Signed-off-by: Dave Lee <dave@gray101.com>
Signed-off-by: Dave Lee <dave@gray101.com>
Copy link

netlify bot commented May 14, 2024

Deploy Preview for localai canceled.

Name Link
🔨 Latest commit 6b91703
🔍 Latest deploy log https://app.netlify.com/sites/localai/deploys/664426ba4ddcec0008112af8

@dave-gray101 dave-gray101 enabled auto-merge (squash) May 14, 2024 06:47
@lunamidori5 lunamidori5 requested review from cryptk and mudler May 14, 2024 07:26
@mudler
Copy link
Owner

mudler commented May 14, 2024

I really think this adds a complexity layer that should be out of LocalAI - ideally there are reverse proxy that can work with this. What is the use-case at hand here?

I'm also ok to merge it - but it should at least use another argument to not break current API_KEY usage - let's keep it as is and map it to have access to all the API endpoints.

Copy link
Owner

@mudler mudler left a comment

Choose a reason for hiding this comment

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

I think we shouldn't break user's configuration by changing API_KEYS - let's have another argument for api keys with roles or support both notations in API_KEYS

@cryptk
Copy link
Collaborator

cryptk commented May 14, 2024

I agree with Mudler. I don't like the idea of breaking existing API_KEYS for this.

@dave-gray101 dave-gray101 requested a review from mudler May 14, 2024 16:11
@dave-gray101 dave-gray101 changed the title feat: roles for api keys draft feat: roles for api keys May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants