Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions scripts/process_unasync.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

def main():
additional_replacements = {
"AsyncKBaseAuthClient": "KBaseAuthClient",
"AsyncClient": "Client",
"aclose": "close",
}
Expand Down
71 changes: 61 additions & 10 deletions src/kbase/auth/_async/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
from kbase.auth.exceptions import InvalidTokenError, InvalidUserError

# TODO PUBLISH make a pypi kbase org and publish there
# TODO RELIABILITY could add retries for these methods, tenacity looks useful
# should be safe since they're all read only
# TODO NOW CODE make a kbase/auth.py module, move other code into _auth, and import everything
# TODO NOW CODE move Token and User into a common class
# We might want to expand exceptions to include the request ID for debugging purposes


@dataclass
Expand Down Expand Up @@ -75,14 +80,14 @@ def _check_response(r: httpx.Response):
if err == 30010: # Illegal username
# The auth server does some goofy stuff when propagating errors, should be cleaned up
# at some point
raise InvalidUserError(resjson["error"]["message"].split(":", 3)[-1])
raise InvalidUserError(resjson["error"]["message"].split(":", 3)[-1].strip())
# don't really see any other error codes we need to worry about - maybe disabled?
# worry about it later.
raise IOError("Error from KBase auth server: " + resjson["error"]["message"])
return resjson


class AsyncClient:
class AsyncKBaseAuthClient:
"""
A client for the KBase Authentication service.
"""
Expand Down Expand Up @@ -111,10 +116,6 @@ async def create(
except:
await cli.close()
raise
# TODO CLIENT look through the myriad of auth clients to see what functionality we need
# TODO CLIENT cache valid user names using cachefor value from token
# TODO RELIABILITY could add retries for these methods, tenacity looks useful
# should be safe since they're all reads only
return cli

def __init__(self, base_url: str, cache_max_size: int, timer: Callable[[[]], int | float]):
Expand All @@ -123,12 +124,14 @@ def __init__(self, base_url: str, cache_max_size: int, timer: Callable[[[]], int
self._base_url = base_url
self._token_url = base_url + "api/V2/token"
self._me_url = base_url + "api/V2/me"
self._users_url = base_url + "api/V2/users/?list="
if cache_max_size < 1:
raise ValueError("cache_max_size must be > 0")
if not timer:
raise ValueError("timer is required")
self._token_cache = LRUCache(maxsize=cache_max_size, timer=timer)
self._user_cache = LRUCache(maxsize=cache_max_size, timer=timer)
self._user_name_cache = LRUCache(maxsize=cache_max_size, timer=timer)

Choose a reason for hiding this comment

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

minor thing, but it's slightly annoying to have usernames and user_name. Can we pick a way to refer to the names of users and stick to it?

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

self._cli = httpx.AsyncClient()

async def __aenter__(self):
Expand Down Expand Up @@ -168,8 +171,6 @@ async def get_token(self, token: str, on_cache_miss: Callable[[], None]=None) ->
res = await self._get(self._token_url, headers={"Authorization": token})
tk = Token(**{k: v for k, v in res.items() if k in _VALID_TOKEN_FIELDS})
# TODO TEST later may want to add tests that change the cachefor value.
# Cleanest way to do this is update the auth2 service to allow setting it
# in test mode
self._token_cache.set(token, tk, ttl=tk.cachefor / 1000)
return tk

Expand All @@ -194,7 +195,57 @@ async def get_user(self, token: str, on_cache_miss: Callable[[], None]=None) ->
res = await self._get(self._me_url, headers={"Authorization": token})
u = User(**{k: v for k, v in res.items() if k in _VALID_USER_FIELDS})
# TODO TEST later may want to add tests that change the cachefor value.
# Cleanest way to do this is update the auth2 service to allow setting it
# in test mode
self._user_cache.set(token, u, ttl=tk.cachefor / 1000)
return u

async def validate_usernames(
self,
token: str,
*usernames: str,
on_cache_miss: Callable[[str], None] = None
) -> dict[str, bool]:
"""
Validate that one or more usernames exist in the auth service.

If any of the names are illegal, an error is thrown.

token - a valid KBase token for any user.
usernames - one or more usernames to query.
on_cache_miss - a function to call if a cache miss occurs. The single argument is the
username that was not in the cache

Returns a dict of username -> boolean which is True if the username exists.
"""
_require_string(token, "token")
if not usernames:
return {}
# use a dict to preserve ordering for testing purposes
uns = {u.strip(): 1 for u in usernames if u.strip()}
to_return = {}
to_query = set()
for u in uns.keys():
if self._user_name_cache.get(u, default=False):
to_return[u] = True
else:
if on_cache_miss:
on_cache_miss(u)
to_query.add(u)
if not to_query:
return to_return
res = await self._get(
self._users_url + ",".join(to_query),
headers={"Authorization": token}
)
tk = None
for u in to_query:
exists = u in res
to_return[u] = exists
if exists:

Choose a reason for hiding this comment

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

can you just do

  to_return[u] = u in res
  if to_return[u]:

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

if not tk: # minor optimization, don't get the token until it's needed
tk = await self.get_token(token)
# Usernames are permanent but can be disabled, so we expire based on time
# Don't cache non-existant names, could be created at any time and would

Choose a reason for hiding this comment

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

existent

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

# be terrible UX for new users
# TODO TEST later may want to add tests that change the cachefor value.
self._user_name_cache.set(u, True, ttl=tk.cachefor / 1000)
return to_return
71 changes: 61 additions & 10 deletions src/kbase/auth/_sync/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@
from kbase.auth.exceptions import InvalidTokenError, InvalidUserError

# TODO PUBLISH make a pypi kbase org and publish there
# TODO RELIABILITY could add retries for these methods, tenacity looks useful
# should be safe since they're all read only
# TODO NOW CODE make a kbase/auth.py module, move other code into _auth, and import everything
# TODO NOW CODE move Token and User into a common class
# We might want to expand exceptions to include the request ID for debugging purposes


@dataclass
Expand Down Expand Up @@ -75,14 +80,14 @@ def _check_response(r: httpx.Response):
if err == 30010: # Illegal username
# The auth server does some goofy stuff when propagating errors, should be cleaned up
# at some point
raise InvalidUserError(resjson["error"]["message"].split(":", 3)[-1])
raise InvalidUserError(resjson["error"]["message"].split(":", 3)[-1].strip())
# don't really see any other error codes we need to worry about - maybe disabled?
# worry about it later.
raise IOError("Error from KBase auth server: " + resjson["error"]["message"])
return resjson


class Client:
class KBaseAuthClient:
"""
A client for the KBase Authentication service.
"""
Expand Down Expand Up @@ -111,10 +116,6 @@ def create(
except:
cli.close()
raise
# TODO CLIENT look through the myriad of auth clients to see what functionality we need
# TODO CLIENT cache valid user names using cachefor value from token
# TODO RELIABILITY could add retries for these methods, tenacity looks useful
# should be safe since they're all reads only
return cli

def __init__(self, base_url: str, cache_max_size: int, timer: Callable[[[]], int | float]):
Expand All @@ -123,12 +124,14 @@ def __init__(self, base_url: str, cache_max_size: int, timer: Callable[[[]], int
self._base_url = base_url
self._token_url = base_url + "api/V2/token"
self._me_url = base_url + "api/V2/me"
self._users_url = base_url + "api/V2/users/?list="
if cache_max_size < 1:
raise ValueError("cache_max_size must be > 0")
if not timer:
raise ValueError("timer is required")
self._token_cache = LRUCache(maxsize=cache_max_size, timer=timer)
self._user_cache = LRUCache(maxsize=cache_max_size, timer=timer)
self._user_name_cache = LRUCache(maxsize=cache_max_size, timer=timer)
self._cli = httpx.Client()

def __enter__(self):
Expand Down Expand Up @@ -168,8 +171,6 @@ def get_token(self, token: str, on_cache_miss: Callable[[], None]=None) -> Token
res = self._get(self._token_url, headers={"Authorization": token})
tk = Token(**{k: v for k, v in res.items() if k in _VALID_TOKEN_FIELDS})
# TODO TEST later may want to add tests that change the cachefor value.
# Cleanest way to do this is update the auth2 service to allow setting it
# in test mode
self._token_cache.set(token, tk, ttl=tk.cachefor / 1000)
return tk

Expand All @@ -194,7 +195,57 @@ def get_user(self, token: str, on_cache_miss: Callable[[], None]=None) -> User:
res = self._get(self._me_url, headers={"Authorization": token})
u = User(**{k: v for k, v in res.items() if k in _VALID_USER_FIELDS})
# TODO TEST later may want to add tests that change the cachefor value.
# Cleanest way to do this is update the auth2 service to allow setting it
# in test mode
self._user_cache.set(token, u, ttl=tk.cachefor / 1000)
return u

def validate_usernames(
self,
token: str,
*usernames: str,
on_cache_miss: Callable[[str], None] = None
) -> dict[str, bool]:
"""
Validate that one or more usernames exist in the auth service.

If any of the names are illegal, an error is thrown.

token - a valid KBase token for any user.
usernames - one or more usernames to query.
on_cache_miss - a function to call if a cache miss occurs. The single argument is the
username that was not in the cache

Returns a dict of username -> boolean which is True if the username exists.
"""
_require_string(token, "token")
if not usernames:
return {}
# use a dict to preserve ordering for testing purposes
uns = {u.strip(): 1 for u in usernames if u.strip()}
to_return = {}
to_query = set()
for u in uns.keys():
if self._user_name_cache.get(u, default=False):
to_return[u] = True
else:
if on_cache_miss:
on_cache_miss(u)
to_query.add(u)
if not to_query:
return to_return
res = self._get(
self._users_url + ",".join(to_query),
headers={"Authorization": token}
)
tk = None
for u in to_query:
exists = u in res
to_return[u] = exists
if exists:
if not tk: # minor optimization, don't get the token until it's needed
tk = self.get_token(token)
# Usernames are permanent but can be disabled, so we expire based on time
# Don't cache non-existant names, could be created at any time and would
# be terrible UX for new users
# TODO TEST later may want to add tests that change the cachefor value.
self._user_name_cache.set(u, True, ttl=tk.cachefor / 1000)
return to_return
4 changes: 2 additions & 2 deletions src/kbase/auth/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@
The aync and sync versions of the KBase Auth Client.
"""

from kbase.auth._async.client import AsyncClient as AsyncKBaseAuthClient # @UnusedImport
from kbase.auth._sync.client import Client as KBaseAuthClient # @UnusedImport
from kbase.auth._async.client import AsyncKBaseAuthClient # @UnusedImport
from kbase.auth._sync.client import KBaseAuthClient # @UnusedImport
Loading