-
-
Notifications
You must be signed in to change notification settings - Fork 389
Added a local identity provider. #1333
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
base: main
Are you sure you want to change the base?
Changes from all commits
affd3f2
5234bd4
2dc7723
7fd95a9
354fea8
21d3ae8
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 |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import asyncio | ||
import getpass | ||
|
||
from jupyter_server.auth.identity import IdentityProvider, User | ||
|
||
|
||
def create_initials(username): | ||
"""Creates initials combining first 2 consonants""" | ||
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. nit: not sure we care, but generally python docstring end w/ a period |
||
|
||
username = username.lower() | ||
|
||
# Default: return first two unique consonants | ||
consonants = [c for c in username if c in "bcdfghjklmnpqrstvwxyz"] | ||
srdas marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if len(consonants) >= 2: | ||
return (consonants[0] + consonants[1]).upper() | ||
|
||
# Fallback: first two characters | ||
return username[:2].upper() | ||
|
||
|
||
class LocalIdentityProvider(IdentityProvider): | ||
"""IdentityProvider that determines username from system user.""" | ||
|
||
def get_user(self, handler): | ||
try: | ||
username = getpass.getuser() | ||
user = User( | ||
username=username, | ||
name=username, | ||
initials=create_initials(username), | ||
color=None, | ||
) | ||
return user | ||
except OSError as e: | ||
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. nit: not using variable |
||
self.log.debug( | ||
"Could not determine username from system. Falling back to anonymous" | ||
f"user." | ||
) | ||
return self._get_user(handler) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
import logging | ||
from unittest.mock import Mock, patch | ||
|
||
import pytest | ||
from jupyter_ai.auth.identity import LocalIdentityProvider, create_initials | ||
from jupyter_server.auth.identity import User | ||
|
||
|
||
@pytest.fixture | ||
def log(): | ||
log = logging.getLogger() | ||
log.addHandler(logging.NullHandler()) | ||
return log | ||
|
||
|
||
@pytest.fixture | ||
def handler(): | ||
return Mock() | ||
|
||
|
||
@patch("getpass.getuser") | ||
def test_get_user_successful(getuser, log, handler): | ||
|
||
getuser.return_value = "localuser" | ||
provider = LocalIdentityProvider(log=log) | ||
|
||
user = provider.get_user(handler) | ||
|
||
assert isinstance(user, User) | ||
assert user.username == "localuser" | ||
assert user.name == "localuser" | ||
assert user.initials == "LC" | ||
assert user.color is None | ||
|
||
|
||
@patch("getpass.getuser") | ||
@pytest.mark.asyncio | ||
async def test_get_user_with_error(getuser, log, handler): | ||
|
||
getuser.return_value = "localuser" | ||
getuser.side_effect = OSError("Could not get username") | ||
handler._jupyter_current_user = User(username="jupyteruser") | ||
|
||
provider = LocalIdentityProvider(log=log) | ||
|
||
user = provider.get_user(handler) | ||
user = await user | ||
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. nit: variable redefinition, perhaps it's okay but just flagging it. Also, why are we awaiting here? |
||
|
||
assert isinstance(user, User) | ||
assert user.username == "jupyteruser" | ||
|
||
|
||
@pytest.mark.parametrize( | ||
"username,expected_initials", | ||
[ | ||
("johndoe", "JH"), | ||
("alice", "LC"), | ||
("xy", "XY"), | ||
("a", "A"), | ||
("SARAH", "SR"), | ||
("john-smith", "JH"), | ||
("john123", "JH"), | ||
("", ""), | ||
], | ||
) | ||
def test_create_initials(username, expected_initials): | ||
"""Test various initials generation scenarios.""" | ||
assert create_initials(username) == expected_initials.upper() |
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.
how long can
username
be?if it's bounded it's fine, but line 13 is a loop within a loop so if
username
is large it could be expensive.The variable name suggests it should be a small string in which case it's not really an issue.
Otherwise, one simple way to do it is to interrupt the loop on line 13 as soon as two consonants were found.