From a76215ff051fe1c0c1c73854c4a7d6e91a5e6906 Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 00:24:11 -0700 Subject: [PATCH 1/7] add typehints to accounts/model.py --- openlibrary/accounts/model.py | 74 +++++++++++++++++------------------ 1 file changed, 37 insertions(+), 37 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index 73d8c006530..e639516171c 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -41,15 +41,15 @@ class OLAuthenticationError(Exception): pass -def append_random_suffix(text, limit=9999): +def append_random_suffix(text: str, limit: int = 9999) -> str: return f'{text}{random.randint(0, limit)}' -def valid_email(email): +def valid_email(email: str) -> bool: return validate_email(email) -def sendmail(to, msg, cc=None): +def sendmail(to: str, msg, cc=None) -> None: cc = cc or [] if config.get('dummy_sendmail'): message = ( @@ -70,13 +70,13 @@ def sendmail(to, msg, cc=None): ) -def verify_hash(secret_key, text, hash): +def verify_hash(secret_key, text, hash) -> bool: """Verifies if the hash is generated""" salt = hash.split('$', 1)[0] return generate_hash(secret_key, text, salt) == hash -def generate_hash(secret_key, text, salt=None): +def generate_hash(secret_key, text, salt=None) -> str: if not isinstance(secret_key, bytes): secret_key = secret_key.encode('utf-8') salt = ( @@ -95,11 +95,11 @@ def get_secret_key(): return config.infobase['secret_key'] -def generate_uuid(): +def generate_uuid() -> str: return str(uuid.uuid4()).replace("-", "") -def send_verification_email(username, email): +def send_verification_email(username: str, email: str) -> None: """Sends account verification email.""" key = f"account/{username}/verify" @@ -113,7 +113,7 @@ def send_verification_email(username, email): sendmail(email, msg) -def create_link_doc(key, username, email): +def create_link_doc(key: str, username: str, email: str) -> dict: """Creates doc required for generating verification link email. The doc contains username, email and a generated code. @@ -135,35 +135,35 @@ def create_link_doc(key, username, email): } -def clear_cookies(): +def clear_cookies() -> None: web.setcookie('pd', "", expires=-1) web.setcookie('sfw', "", expires=-1) class Link(web.storage): - def get_expiration_time(self): + def get_expiration_time(self) -> datetime.datetime: d = self['expires_on'].split(".")[0] return datetime.datetime.strptime(d, "%Y-%m-%dT%H:%M:%S") - def get_creation_time(self): + def get_creation_time(self) -> datetime.datetime: d = self['created_on'].split(".")[0] return datetime.datetime.strptime(d, "%Y-%m-%dT%H:%M:%S") - def delete(self): + def delete(self) -> None: del web.ctx.site.store[self['_key']] class Account(web.storage): @property - def username(self): + def username(self) -> str: return self._key.split("/")[-1] - def get_edit_count(self): + def get_edit_count(self) -> int: user = self.get_user() return (user and user.get_edit_count()) or 0 @property - def registered_on(self): + def registered_on(self) -> datetime.datetime | None: """Returns the registration time.""" t = self.get("created_on") return t and helpers.parse_datetime(t) @@ -174,7 +174,7 @@ def activated_on(self): return user and user.created @property - def displayname(self): + def displayname(self) -> str: if doc := self.get_user(): return doc.displayname or self.username elif "data" in self: @@ -182,34 +182,34 @@ def displayname(self): else: return self.username - def creation_time(self): + def creation_time(self) -> datetime.datetime: d = self['created_on'].split(".")[0] return datetime.datetime.strptime(d, "%Y-%m-%dT%H:%M:%S") - def get_recentchanges(self, limit=100, offset=0): + def get_recentchanges(self, limit: int = 100, offset: int = 0): q = {"author": self.get_user().key, "limit": limit, "offset": offset} return web.ctx.site.recentchanges(q) - def verify_password(self, password): + def verify_password(self, password) -> bool: return verify_hash(get_secret_key(), password, self.enc_password) - def update_password(self, new_password): + def update_password(self, new_password) -> None: web.ctx.site.update_account(self.username, password=new_password) - def update_email(self, email): + def update_email(self, email) -> None: web.ctx.site.update_account(self.username, email=email) - def send_verification_email(self): + def send_verification_email(self) -> None: send_verification_email(self.username, self.email) - def activate(self): + def activate(self) -> None: web.ctx.site.activate_account(username=self.username) - def block(self): + def block(self) -> None: """Blocks this account.""" web.ctx.site.update_account(self.username, status="blocked") - def unblock(self): + def unblock(self) -> None: """Unblocks this account.""" web.ctx.site.update_account(self.username, status="active") @@ -243,25 +243,25 @@ def login(self, password): return "ok" @classmethod - def generate_random_password(cls, n=12): + def generate_random_password(cls, n: int = 12) -> str: return ''.join( random.SystemRandom().choice(string.ascii_uppercase + string.digits) for _ in range(n) ) - def generate_login_code(self): + def generate_login_code(self) -> str: """Returns a string that can be set as login cookie to log in as this user.""" user_key = "/people/" + self.username t = datetime.datetime(*time.gmtime()[:6]).isoformat() text = f"{user_key},{t}" return text + "," + generate_hash(get_secret_key(), text) - def _save(self): + def _save(self) -> None: """Saves this account in store.""" web.ctx.site.store[self._key] = self @property - def last_login(self): + def last_login(self) -> datetime.datetime: """Returns the last_login time of the user, if available. The `last_login` will not be available for accounts, who haven't @@ -313,21 +313,21 @@ def get_tags(self) -> list[str]: def has_tag(self, tag: str) -> bool: return tag in self.get_tags() - def add_tag(self, tag): + def add_tag(self, tag) -> None: tags = self.get_tags() if tag not in tags: tags.append(tag) self['tags'] = tags self._save() - def remove_tag(self, tag): + def remove_tag(self, tag) -> None: tags = self.get_tags() if tag in tags: tags.remove(tag) self['tags'] = tags self._save() - def set_bot_flag(self, flag): + def set_bot_flag(self, flag) -> None: """Enables/disables the bot flag.""" self.bot = flag self._save() @@ -564,14 +564,14 @@ def get_by_email( return None @property - def verified(self): + def verified(self) -> bool: return getattr(self, 'status', '') != 'pending' @property - def blocked(self): + def blocked(self) -> bool: return getattr(self, 'status', '') == 'blocked' - def unlink(self): + def unlink(self) -> None: """Careful, this will save any other changes to the ol user object as well """ @@ -581,7 +581,7 @@ def unlink(self): self.internetarchive_itemname = None stats.increment('ol.account.xauth.unlinked') - def link(self, itemname): + def link(self, itemname) -> None: """Careful, this will save any other changes to the ol user object as well """ @@ -593,7 +593,7 @@ def link(self, itemname): self.internetarchive_itemname = itemname stats.increment('ol.account.xauth.linked') - def save_s3_keys(self, s3_keys): + def save_s3_keys(self, s3_keys) -> None: _ol_account = web.ctx.site.store.get(self._key) _ol_account['s3_keys'] = s3_keys web.ctx.site.store[self._key] = _ol_account From a8bb978daebd87562f116be0d4dd49f8668ab940 Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 00:28:42 -0700 Subject: [PATCH 2/7] add many more typehints --- openlibrary/accounts/model.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index e639516171c..b47c63378e3 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -281,30 +281,33 @@ def get_user(self) -> 'User': key = "/people/" + self.username return web.ctx.site.get(key) - def get_creation_info(self): + def get_creation_info(self) -> dict: key = "/people/" + self.username doc = web.ctx.site.get(key) return doc.get_creation_info() - def get_activation_link(self): + def get_activation_link(self) -> Link | bool: key = f"account/{self.username}/verify" if doc := web.ctx.site.store.get(key): return Link(doc) else: return False - def get_password_reset_link(self): + def get_password_reset_link(self) -> Link | bool: key = f"account/{self.username}/password" if doc := web.ctx.site.store.get(key): return Link(doc) else: return False - def get_links(self): + def get_links(self) -> list[Link]: """Returns all the verification links present in the database.""" - return web.ctx.site.store.values( - type="account-link", name="username", value=self.username - ) + return [ + Link(doc) + for doc in web.ctx.site.store.values( + type="account-link", name="username", value=self.username + ) + ] def get_tags(self) -> list[str]: """Returns list of tags that this user has.""" @@ -396,11 +399,12 @@ def itemname(self) -> str | None: """ return getattr(self, 'internetarchive_itemname', None) - def get_linked_ia_account(self): + def get_linked_ia_account(self) -> 'InternetArchiveAccount | None': if self.itemname: act = InternetArchiveAccount.xauth('info', itemname=self.itemname) if 'values' in act and 'email' in act['values']: return InternetArchiveAccount.get(email=act['values']['email']) + return None def render_link(self): return f'{web.net.htmlquote(self.displayname)}' From cb26723fae360c735520fb5e8756823a1ca9f97f Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 00:48:12 -0700 Subject: [PATCH 3/7] add typehints to accounts/model.py --- openlibrary/accounts/model.py | 192 ++++++++++++++++++++++++---------- 1 file changed, 137 insertions(+), 55 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index b47c63378e3..48cfd405104 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -401,12 +401,18 @@ def itemname(self) -> str | None: def get_linked_ia_account(self) -> 'InternetArchiveAccount | None': if self.itemname: - act = InternetArchiveAccount.xauth('info', itemname=self.itemname) - if 'values' in act and 'email' in act['values']: - return InternetArchiveAccount.get(email=act['values']['email']) + act = InternetArchiveAccount.xauth('info', itemname=str(self.itemname)) + if ( + isinstance(act, dict) + and 'values' in act + and isinstance(act['values'], dict) + ): + values = act['values'] + if 'email' in values and isinstance(values['email'], str): + return InternetArchiveAccount.get(email=values['email']) return None - def render_link(self): + def render_link(self) -> str: return f'{web.net.htmlquote(self.displayname)}' @@ -597,21 +603,38 @@ def link(self, itemname) -> None: self.internetarchive_itemname = itemname stats.increment('ol.account.xauth.linked') - def save_s3_keys(self, s3_keys) -> None: + def save_s3_keys(self, s3_keys: dict) -> None: _ol_account = web.ctx.site.store.get(self._key) _ol_account['s3_keys'] = s3_keys web.ctx.site.store[self._key] = _ol_account self.s3_keys = s3_keys - def update_last_login(self): + def update_last_login(self) -> None: _ol_account = web.ctx.site.store.get(self._key) - last_login = datetime.datetime.utcnow().isoformat() - _ol_account['last_login'] = last_login + if _ol_account is None: + return + last_login = datetime.datetime.utcnow() + _ol_account['last_login'] = last_login.isoformat() web.ctx.site.store[self._key] = _ol_account - self.last_login = last_login + # Use _set method to bypass the read-only property + self._set('last_login', last_login) @classmethod - def authenticate(cls, email, password, test=False): + def authenticate(cls, email: str, password: str, test: bool = False) -> str: + """Authenticate a user with email and password. + + Args: + email: The user's email address + password: The user's password + test: Whether this is a test account + + Returns: + str: Status code indicating authentication result: + - "ok" if authentication succeeded + - "account_not_found" if no account exists with the given email + - "account_blocked" if the account is blocked + - Other error codes from the login attempt + """ ol_account = cls.get(email=email, test=test) if not ol_account: return "account_not_found" @@ -621,27 +644,26 @@ def authenticate(cls, email, password, test=False): web.ctx.site.login(ol_account.username, password) except ClientException as e: code = e.get_data().get("code") - return code - else: - return "ok" + return code if code else "authentication_failed" + return "ok" class InternetArchiveAccount(web.storage): - def __init__(self, **kwargs): + def __init__(self, **kwargs) -> None: for k, v in kwargs.items(): setattr(self, k, v) @classmethod def create( cls, - screenname, - email, - password, - notifications=None, - retries=0, - verified=False, - test=None, - ): + screenname: str, + email: str, + password: str, + notifications: list[str] | None = None, + retries: int = 0, + verified: bool = False, + test: bool | None = None, + ) -> 'InternetArchiveAccount': """ :param unicode screenname: changeable human readable archive.org username. The slug / itemname is generated automatically from this value. @@ -673,13 +695,13 @@ def create( while True: try: response = cls.xauth( - 'create', + op='create', email=email, password=password, screenname=_screenname, - notifications=notifications, - test=test, - verified=verified, + notifications=notifications or [], + test=str(test) if test is not None else None, + verified=str(verified).lower(), service='openlibrary', ) except requests.HTTPError as err: @@ -689,41 +711,58 @@ def create( raise OLAuthenticationError("undefined_error") if response.get('success'): - ia_account = cls.get(email=email) + account = cls.get(email=email) + if not account: + raise OLAuthenticationError("account_creation_failed") if test: - ia_account.test = True - return ia_account + account.test = True # type: ignore[attr-defined] + return account # Response has returned "failure" with reasons in "values" failures = response.get('values', {}) + if not isinstance(failures, dict): + failures = {} if 'screenname' not in failures: - for field in failures: + for field in failures.keys() if failures else []: # raise the first error if multiple # e.g. bad_email, bad_password error = OLAuthenticationError(f'bad_{field}') - error.response = response + # Store response as an attribute for error handling + error.response = response # type: ignore[attr-defined] raise error elif attempt < retries: _screenname = append_random_suffix(screenname) attempt += 1 else: e = OLAuthenticationError('username_registered') - e.value = _screenname + e.value = _screenname # type: ignore[attr-defined] raise e @classmethod - def xauth(cls, op, test=None, s3_key=None, s3_secret=None, xauth_url=None, **data): + def xauth( + cls, + op: str, + test: str | None = None, + s3_key: str | None = None, + s3_secret: str | None = None, + xauth_url: str | None = None, + **kwargs: object, + ) -> dict[str, object]: """ See https://git.archive.org/ia/petabox/tree/master/www/sf/services/xauthn """ from openlibrary.core import lending - url = xauth_url or lending.config_ia_xauth_api_url + url = xauth_url or str(getattr(lending, 'config_ia_xauth_api_url', '')) params = {'op': op} + + # Create a copy of kwargs to avoid modifying the original + data = dict(kwargs) + config = getattr(lending, 'config_ia_ol_xauth_s3', {}) or {} data.update( { - 'access': s3_key or lending.config_ia_ol_xauth_s3.get('s3_key'), - 'secret': s3_secret or lending.config_ia_ol_xauth_s3.get('s3_secret'), + 'access': s3_key or config.get('s3_key'), + 'secret': s3_secret or config.get('s3_secret'), } ) @@ -754,42 +793,85 @@ def xauth(cls, op, test=None, s3_key=None, s3_secret=None, xauth_url=None, **dat return {'error': response.text, 'code': response.status_code} @classmethod - def s3auth(cls, access_key, secret_key): + def s3auth(cls, access_key: str, secret_key: str) -> dict[str, object]: """Authenticates an Archive.org user based on s3 keys""" from openlibrary.core import lending - url = lending.config_ia_s3_auth_url + base_url = getattr(lending, 'config_ia_xauth_api_url', '') or '' + url = f"{base_url}/s3" if base_url else "" + + if not url: + return {'error': 'missing_config', 'code': 500} + + response = requests.get( + url, params={"op": "s3"}, auth=(access_key, secret_key), timeout=30 + ) + + if response.status_code == 403: + raise OLAuthenticationError("security_error") + try: - response = requests.get( - url, - headers={ - 'Content-Type': 'application/json', - 'authorization': f'LOW {access_key}:{secret_key}', - }, - ) - response.raise_for_status() return response.json() - except requests.HTTPError as e: - return {'error': e.response.text, 'code': e.response.status_code} + except ValueError: + return {'error': response.text, 'code': response.status_code} except JSONDecodeError as e: return {'error': str(e), 'code': response.status_code} @classmethod def get( - cls, email, test=False, _json=False, s3_key=None, s3_secret=None, xauth_url=None - ): + cls, + email: str, + test: bool = False, + _json: bool = False, + s3_key: str | None = None, + s3_secret: str | None = None, + xauth_url: str | None = None, + ) -> 'InternetArchiveAccount | None': + """Get an Internet Archive account by email. + + Args: + email: The email address of the account + test: Whether this is a test account + _json: If True, return the raw JSON response + s3_key: Optional S3 key for authentication + s3_secret: Optional S3 secret for authentication + xauth_url: Optional custom xauth URL + + Returns: + InternetArchiveAccount if account exists and _json is False + dict if _json is True + None if account doesn't exist + """ email = email.strip().lower() response = cls.xauth( + op='info', email=email, - test=test, - op="info", + test=str(test).lower() if test else None, s3_key=s3_key, s3_secret=s3_secret, xauth_url=xauth_url, ) - if 'success' in response: - values = response.get('values', {}) - return values if _json else cls(**values) + if not response.get('success'): + return None + + values = response.get('values', {}) + if not isinstance(values, dict): + return None + + if _json: + # If _json is True, raise an error since we can't return a dict + # This maintains type safety while matching the method's return type + raise ValueError("Cannot return raw JSON when _json=False is not set") + + account = cls() + for k, v in values.items(): + if isinstance(k, str): + setattr(account, k, v) + + if test is not None: + account.test = test # type: ignore[attr-defined] + + return account @classmethod def authenticate(cls, email, password, test=False): From 6a65ceec1110d04be6c0ec4097d67216d159b04d Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 00:57:25 -0700 Subject: [PATCH 4/7] simplify changes --- openlibrary/accounts/model.py | 83 ++++++++++------------------------- 1 file changed, 23 insertions(+), 60 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index 48cfd405104..0c03ae09056 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -793,85 +793,48 @@ def xauth( return {'error': response.text, 'code': response.status_code} @classmethod - def s3auth(cls, access_key: str, secret_key: str) -> dict[str, object]: + def s3auth(cls, access_key: str, secret_key: str): """Authenticates an Archive.org user based on s3 keys""" from openlibrary.core import lending - base_url = getattr(lending, 'config_ia_xauth_api_url', '') or '' - url = f"{base_url}/s3" if base_url else "" + url = lending.config_ia_s3_auth_url - if not url: - return {'error': 'missing_config', 'code': 500} - - response = requests.get( - url, params={"op": "s3"}, auth=(access_key, secret_key), timeout=30 - ) - - if response.status_code == 403: - raise OLAuthenticationError("security_error") + if not isinstance(url, str): + raise TypeError( + f"Expected 'url' to be a string, got {type(url).__name__} instead" + ) try: + response = requests.get( + url, + headers={ + 'Content-Type': 'application/json', + 'authorization': f'LOW {access_key}:{secret_key}', + }, + ) + response.raise_for_status() return response.json() - except ValueError: - return {'error': response.text, 'code': response.status_code} + except requests.HTTPError as e: + return {'error': e.response.text, 'code': e.response.status_code} except JSONDecodeError as e: return {'error': str(e), 'code': response.status_code} @classmethod def get( - cls, - email: str, - test: bool = False, - _json: bool = False, - s3_key: str | None = None, - s3_secret: str | None = None, - xauth_url: str | None = None, - ) -> 'InternetArchiveAccount | None': - """Get an Internet Archive account by email. - - Args: - email: The email address of the account - test: Whether this is a test account - _json: If True, return the raw JSON response - s3_key: Optional S3 key for authentication - s3_secret: Optional S3 secret for authentication - xauth_url: Optional custom xauth URL - - Returns: - InternetArchiveAccount if account exists and _json is False - dict if _json is True - None if account doesn't exist - """ + cls, email, test=False, _json=False, s3_key=None, s3_secret=None, xauth_url=None + ): email = email.strip().lower() response = cls.xauth( - op='info', email=email, - test=str(test).lower() if test else None, + test=test, + op="info", s3_key=s3_key, s3_secret=s3_secret, xauth_url=xauth_url, ) - if not response.get('success'): - return None - - values = response.get('values', {}) - if not isinstance(values, dict): - return None - - if _json: - # If _json is True, raise an error since we can't return a dict - # This maintains type safety while matching the method's return type - raise ValueError("Cannot return raw JSON when _json=False is not set") - - account = cls() - for k, v in values.items(): - if isinstance(k, str): - setattr(account, k, v) - - if test is not None: - account.test = test # type: ignore[attr-defined] - - return account + if 'success' in response: + values = response.get('values', {}) + return values if _json else cls(**values) @classmethod def authenticate(cls, email, password, test=False): From 56f0c1ea3966f772ae8515bf3f50904d8bff32f4 Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 01:03:48 -0700 Subject: [PATCH 5/7] simplify changes 2 --- openlibrary/accounts/model.py | 42 ++++++++--------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index 0c03ae09056..e7f9ee9186b 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -399,18 +399,11 @@ def itemname(self) -> str | None: """ return getattr(self, 'internetarchive_itemname', None) - def get_linked_ia_account(self) -> 'InternetArchiveAccount | None': + def get_linked_ia_account(self): if self.itemname: - act = InternetArchiveAccount.xauth('info', itemname=str(self.itemname)) - if ( - isinstance(act, dict) - and 'values' in act - and isinstance(act['values'], dict) - ): - values = act['values'] - if 'email' in values and isinstance(values['email'], str): - return InternetArchiveAccount.get(email=values['email']) - return None + act = InternetArchiveAccount.xauth('info', itemname=self.itemname) + if 'values' in act and 'email' in act['values']: + return InternetArchiveAccount.get(email=act['values']['email']) def render_link(self) -> str: return f'{web.net.htmlquote(self.displayname)}' @@ -609,32 +602,15 @@ def save_s3_keys(self, s3_keys: dict) -> None: web.ctx.site.store[self._key] = _ol_account self.s3_keys = s3_keys - def update_last_login(self) -> None: + def update_last_login(self): _ol_account = web.ctx.site.store.get(self._key) - if _ol_account is None: - return - last_login = datetime.datetime.utcnow() - _ol_account['last_login'] = last_login.isoformat() + last_login = datetime.datetime.utcnow().isoformat() + _ol_account['last_login'] = last_login web.ctx.site.store[self._key] = _ol_account - # Use _set method to bypass the read-only property - self._set('last_login', last_login) + self.last_login = last_login @classmethod def authenticate(cls, email: str, password: str, test: bool = False) -> str: - """Authenticate a user with email and password. - - Args: - email: The user's email address - password: The user's password - test: Whether this is a test account - - Returns: - str: Status code indicating authentication result: - - "ok" if authentication succeeded - - "account_not_found" if no account exists with the given email - - "account_blocked" if the account is blocked - - Other error codes from the login attempt - """ ol_account = cls.get(email=email, test=test) if not ol_account: return "account_not_found" @@ -644,7 +620,7 @@ def authenticate(cls, email: str, password: str, test: bool = False) -> str: web.ctx.site.login(ol_account.username, password) except ClientException as e: code = e.get_data().get("code") - return code if code else "authentication_failed" + return code return "ok" From c9a171fba7f631f5b435b9e925119a717099d399 Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 01:07:14 -0700 Subject: [PATCH 6/7] simplify changes 3 --- openlibrary/accounts/model.py | 61 +++++++++++++---------------------- 1 file changed, 22 insertions(+), 39 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index e7f9ee9186b..a7081f5a88a 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -632,14 +632,14 @@ def __init__(self, **kwargs) -> None: @classmethod def create( cls, - screenname: str, - email: str, - password: str, - notifications: list[str] | None = None, - retries: int = 0, - verified: bool = False, - test: bool | None = None, - ) -> 'InternetArchiveAccount': + screenname, + email, + password, + notifications=None, + retries=0, + verified=False, + test=None, + ): """ :param unicode screenname: changeable human readable archive.org username. The slug / itemname is generated automatically from this value. @@ -671,13 +671,13 @@ def create( while True: try: response = cls.xauth( - op='create', + 'create', email=email, password=password, screenname=_screenname, - notifications=notifications or [], - test=str(test) if test is not None else None, - verified=str(verified).lower(), + notifications=notifications, + test=test, + verified=verified, service='openlibrary', ) except requests.HTTPError as err: @@ -687,58 +687,41 @@ def create( raise OLAuthenticationError("undefined_error") if response.get('success'): - account = cls.get(email=email) - if not account: - raise OLAuthenticationError("account_creation_failed") + ia_account = cls.get(email=email) if test: - account.test = True # type: ignore[attr-defined] - return account + ia_account.test = True + return ia_account # Response has returned "failure" with reasons in "values" failures = response.get('values', {}) - if not isinstance(failures, dict): - failures = {} if 'screenname' not in failures: - for field in failures.keys() if failures else []: + for field in failures: # raise the first error if multiple # e.g. bad_email, bad_password error = OLAuthenticationError(f'bad_{field}') - # Store response as an attribute for error handling - error.response = response # type: ignore[attr-defined] + error.response = response raise error elif attempt < retries: _screenname = append_random_suffix(screenname) attempt += 1 else: e = OLAuthenticationError('username_registered') - e.value = _screenname # type: ignore[attr-defined] + e.value = _screenname raise e @classmethod - def xauth( - cls, - op: str, - test: str | None = None, - s3_key: str | None = None, - s3_secret: str | None = None, - xauth_url: str | None = None, - **kwargs: object, - ) -> dict[str, object]: + def xauth(cls, op, test=None, s3_key=None, s3_secret=None, xauth_url=None, **data): """ See https://git.archive.org/ia/petabox/tree/master/www/sf/services/xauthn """ from openlibrary.core import lending - url = xauth_url or str(getattr(lending, 'config_ia_xauth_api_url', '')) + url = xauth_url or lending.config_ia_xauth_api_url params = {'op': op} - - # Create a copy of kwargs to avoid modifying the original - data = dict(kwargs) - config = getattr(lending, 'config_ia_ol_xauth_s3', {}) or {} data.update( { - 'access': s3_key or config.get('s3_key'), - 'secret': s3_secret or config.get('s3_secret'), + 'access': s3_key or lending.config_ia_ol_xauth_s3.get('s3_key'), + 'secret': s3_secret or lending.config_ia_ol_xauth_s3.get('s3_secret'), } ) From 16a61c36ef150bde2a471a9370123fabd2f6755f Mon Sep 17 00:00:00 2001 From: RayBB Date: Sun, 6 Jul 2025 01:08:17 -0700 Subject: [PATCH 7/7] simplify changes 4 --- openlibrary/accounts/model.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/openlibrary/accounts/model.py b/openlibrary/accounts/model.py index a7081f5a88a..ce0372e2b9b 100644 --- a/openlibrary/accounts/model.py +++ b/openlibrary/accounts/model.py @@ -302,12 +302,9 @@ def get_password_reset_link(self) -> Link | bool: def get_links(self) -> list[Link]: """Returns all the verification links present in the database.""" - return [ - Link(doc) - for doc in web.ctx.site.store.values( - type="account-link", name="username", value=self.username - ) - ] + return web.ctx.site.store.values( + type="account-link", name="username", value=self.username + ) def get_tags(self) -> list[str]: """Returns list of tags that this user has."""