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 static api authentication with bearer token #3

Merged
merged 5 commits into from
Mar 2, 2024
Merged

Add static api authentication with bearer token #3

merged 5 commits into from
Mar 2, 2024

Conversation

BlackyDrum
Copy link
Contributor

Add Bearer Token Support to ChromaDB

Overview

This pull request introduces the capability to include a bearer token in the HTTP headers when making requests to ChromaDB. This feature enhances the createApiClient function within the existing client setup, allowing for authenticated requests to ChromaDB.

Changes

  • Added the bearerToken attribute to the Factory class
  • Modified the createApiClient method to conditionally add the Authorization header if a bearer token is provided.

Usage

Users who wish to make authenticated requests can now simply use the withBearerToken method when constructing the client. For example:

$chromaDB = ChromaDB::factory()
                ->withHost('http://localhost')
                ->withPort(8000)
                ->withDatabase('new_database')
                ->withTenant('new_tenant')
                ->withBearerToken('my-secret-token')
                ->connect(); 

Required Environment Variables for ChromaDB

To utilize the bearer token authentication feature, the following environment variables need to be set on the ChromaDB server. More information here:

CHROMA_SERVER_AUTH_CREDENTIALS="my-secret-token"
CHROMA_SERVER_AUTH_CREDENTIALS_PROVIDER="chromadb.auth.token.TokenConfigServerAuthCredentialsProvider"
CHROMA_SERVER_AUTH_PROVIDER="chromadb.auth.token.TokenAuthServerProvider"

@CodeWithKyrian CodeWithKyrian self-requested a review March 1, 2024 07:21
@CodeWithKyrian
Copy link
Owner

Hi @BlackyDrum,

Thank you for your contribution to ChromaPHP! I’ve reviewed your pull request (#3), and overall, the changes look good.

As your PR introduces static authentication in the client, it’s crucial to have tests that verify the functionality and reliability of the new feature. So I’d like to request that you include some additional tests (to ChromaDB test file) covering the following aspects:

  • Successful Authentication: Test that the correct credentials are used to establish a connection successfully.
  • Error Handling: Test that an invalid credential error is handled gracefully with an appropriate exception and message.

Remember to run the Chroma Docker client with the environment variables when testing. The default docker-compose.yml in the repo doesn’t have the environment variables.

If you need any assistance or guidance with writing the tests, feel free to reach out, and I’ll be happy to help.

Once the tests are in place and passing, I can proceed with merging into the main codebase. Thanks again for your contribution!

@BlackyDrum
Copy link
Contributor Author

Unfortunately I don't have time for that at the moment. Also, I don't really know how to write proper tests. Maybe you can take over?

@CodeWithKyrian
Copy link
Owner

Alright. That's fine. I'll leave this open and add the tests' commits here then

@CodeWithKyrian CodeWithKyrian merged commit 2beda44 into CodeWithKyrian:main Mar 2, 2024
6 checks passed
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.

2 participants