-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 option for custom auth #1280
base: main
Are you sure you want to change the base?
Changes from all commits
65a6453
477d81d
d568c64
d084f58
deea68d
7be90b4
1c760fe
fa98dd9
fdc0d82
e12bec4
5c52f65
8338774
5bbb45b
0c396ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,7 +8,7 @@ async def is_thread_author(username: str, thread_id: str): | |
raise HTTPException(status_code=400, detail="Data layer not initialized") | ||
|
||
thread_author = await data_layer.get_thread_author(thread_id) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Please make sure this is removed from the PR, it is conducive to merge conflicts. |
||
if not thread_author: | ||
raise HTTPException(status_code=404, detail="Thread not found") | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -107,6 +107,65 @@ async def auth_func( | |
assert result is None | ||
|
||
|
||
async def test_custom_authenticate_user(test_config): | ||
from unittest.mock import patch | ||
|
||
from chainlit.callbacks import custom_authenticate_user | ||
from chainlit.user import User | ||
|
||
# Mock the get_configured_oauth_providers function | ||
with patch( | ||
"chainlit.callbacks.get_configured_oauth_providers", | ||
return_value=["custom_provider"], | ||
): | ||
|
||
@custom_authenticate_user | ||
async def auth_func( | ||
provider_id: str, | ||
token: str, | ||
raw_user_data: dict, | ||
default_app_user: User, | ||
id_token: str | None = None, | ||
) -> User | None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason to add this callback over using What does the value of |
||
if ( | ||
provider_id == "custom_provider" and token == "valid_token" | ||
): # nosec B105 | ||
return User(identifier="oauth_user") | ||
return None | ||
|
||
# Test that the callback is properly registered as custom one | ||
assert test_config.code.custom_authenticate_user is not None | ||
|
||
# Test the wrapped function with valid data | ||
result = await test_config.code.custom_authenticate_user( | ||
"custom_provider", "valid_token", {}, User(identifier="default_user") | ||
) | ||
assert isinstance(result, User) | ||
assert result.identifier == "oauth_user" | ||
|
||
# Test with invalid data | ||
result = await test_config.code.custom_authenticate_user( | ||
"google", "invalid_token", {}, User(identifier="default_user") | ||
) | ||
assert result is None | ||
|
||
|
||
async def test_custom_oauth_provider(test_config): | ||
from unittest.mock import Mock, patch | ||
|
||
from chainlit.callbacks import custom_oauth_provider | ||
|
||
custom_provider = Mock() | ||
|
||
with patch("chainlit.callbacks.providers") as providers: | ||
|
||
# Add custom provider to providers | ||
custom_oauth_provider(custom_provider) | ||
|
||
# Custom provider should be added to providers | ||
providers.append.assert_called_once_with(custom_provider()) | ||
|
||
|
||
async def test_on_message(mock_chainlit_context, test_config): | ||
from chainlit.callbacks import on_message | ||
from chainlit.message import Message | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure whether
custom_
is the right name here. As we'll be moving to having less and less oauth providers built in, I feel it's more likely that most if not all oauth providers will be pluggable. Additionally, I want to move towards using an externally maintained OAuth client framework instead (see #1240).So perhaps, this could become the default way of setting up oauth providers -- in that case something like
@register_oauth_provider
or merely@oauth_provider
might be a more appropriate name.