Skip to content

Commit

Permalink
Enable new rules for Ruff (#507)
Browse files Browse the repository at this point in the history
  • Loading branch information
armanddidierjean authored Aug 7, 2024
1 parent 959ac7d commit 5c9118a
Show file tree
Hide file tree
Showing 44 changed files with 424 additions and 335 deletions.
10 changes: 6 additions & 4 deletions app/core/auth/cruds_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,11 @@ async def create_authorization_token(
db.add(db_authorization_code)
try:
await db.commit()
return db_authorization_code
except IntegrityError:
await db.rollback()
raise ValueError()
raise
else:
return db_authorization_code


async def delete_authorization_token_by_token(
Expand Down Expand Up @@ -72,10 +73,11 @@ async def create_refresh_token(
db.add(db_refresh_token)
try:
await db.commit()
return db_refresh_token
except IntegrityError:
await db.rollback()
raise ValueError()
raise
else:
return db_refresh_token


async def revoke_refresh_token_by_token(
Expand Down
48 changes: 23 additions & 25 deletions app/core/auth/endpoints_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,22 +261,21 @@ async def authorize_validation(
)
redirect_uri = auth_client.override_redirect_uri
# If at least one redirect_uri is hardcoded in the auth_client we will use this one. If one was provided in the request, we want to make sure they match.
elif authorizereq.redirect_uri is None:
# We use the hardcoded value
redirect_uri = auth_client.redirect_uri[0]
# If a redirect_uri is provided, it should match one specified in the auth client
elif authorizereq.redirect_uri not in auth_client.redirect_uri:
hyperion_access_logger.warning(
f"Authorize-validation: Mismatching redirect_uri, received {authorizereq.redirect_uri} but expected one of {auth_client.redirect_uri} ({request_id})",
)
return RedirectResponse(
settings.CLIENT_URL
+ calypsso.get_error_relative_url(message="Mismatching redirect_uri"),
status_code=status.HTTP_302_FOUND,
)
else:
if authorizereq.redirect_uri is None:
# We use the hardcoded value
redirect_uri = auth_client.redirect_uri[0]
# If a redirect_uri is provided, it should match one specified in the auth client
elif authorizereq.redirect_uri not in auth_client.redirect_uri:
hyperion_access_logger.warning(
f"Authorize-validation: Mismatching redirect_uri, received {authorizereq.redirect_uri} but expected one of {auth_client.redirect_uri} ({request_id})",
)
return RedirectResponse(
settings.CLIENT_URL
+ calypsso.get_error_relative_url(message="Mismatching redirect_uri"),
status_code=status.HTTP_302_FOUND,
)
else:
redirect_uri = authorizereq.redirect_uri
redirect_uri = authorizereq.redirect_uri

# Special characters like `:` or `/` may be encoded as `%3A` and `%2F`, we need to decode them before returning the redirection object
redirect_uri = urllib.parse.unquote(redirect_uri)
Expand Down Expand Up @@ -759,17 +758,16 @@ async def refresh_token_grant(
error="invalid_client",
error_description="Invalid client id or secret",
)
else:
elif tokenreq.client_secret is not None:
# We use PKCE, a client secret should not have been provided
if tokenreq.client_secret is not None:
hyperion_access_logger.warning(
f"Token authorization_code_grant: With PKCE, a client secret should not have been provided ({request_id})",
)
raise AuthHTTPException(
status_code=400,
error="invalid_client",
error_description="Invalid client id or secret",
)
hyperion_access_logger.warning(
f"Token authorization_code_grant: With PKCE, a client secret should not have been provided ({request_id})",
)
raise AuthHTTPException(
status_code=400,
error="invalid_client",
error_description="Invalid client id or secret",
)

# If everything is good we can finally create the new access/refresh tokens
# We use new refresh tokens every time as we use some client that can't store secrets (see Refresh token rotation in https://www.pingidentity.com/en/resources/blog/post/refresh-token-rotation-spa.html)
Expand Down
40 changes: 21 additions & 19 deletions app/core/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,12 @@
from pydantic import computed_field, model_validator
from pydantic_settings import BaseSettings, SettingsConfigDict

from app.types.exceptions import (
DotenvInvalidAuthClientNameInError,
DotenvInvalidVariableError,
DotenvMissingVariableError,
InvalidRSAKeyInDotenvError,
)
from app.utils.auth import providers


Expand Down Expand Up @@ -194,9 +200,7 @@ def RSA_PRIVATE_KEY(cls) -> rsa.RSAPrivateKey:
# https://cryptography.io/en/latest/hazmat/primitives/asymmetric/serialization/#module-cryptography.hazmat.primitives.serialization
private_key = load_pem_private_key(cls.RSA_PRIVATE_PEM_STRING, password=None)
if not isinstance(private_key, rsa.RSAPrivateKey):
raise TypeError(
f"RSA_PRIVATE_PEM_STRING is not an RSA key but a {private_key.__class__.__name__}",
)
raise InvalidRSAKeyInDotenvError(private_key.__class__.__name__)
return private_key

@computed_field # type: ignore[misc]
Expand Down Expand Up @@ -235,18 +239,16 @@ def KNOWN_AUTH_CLIENTS(cls) -> dict[str, providers.BaseAuthClient]:
auth_client_name,
)
except AttributeError as error:
# logger.error()
raise ValueError(
f".env AUTH_CLIENTS is invalid: {auth_client_name} is not an auth_client from app.utils.auth.providers",
raise DotenvInvalidAuthClientNameInError(
auth_client_name,
) from error
# If the secret is empty, this mean the client is expected to use PKCE
# We need to pass a None value to the auth_client_class
if not secret:
secret = None

# We can create a new instance of the auth_client_class with the client id and secret
clients[client_id] = auth_client_class(
client_id=client_id,
secret=secret,
# If the secret is empty, this mean the client is expected to use PKCE
# We need to pass a None value to the auth_client_class instead of an other falsy value
secret=secret or None,
redirect_uri=redirect_uri,
)

Expand All @@ -270,14 +272,14 @@ def check_client_urls(self) -> "Settings":
All fields are optional, but the dotenv should configure SQLITE_DB or a Postgres database
"""
if not self.CLIENT_URL[-1] == "/":
raise ValueError(
raise DotenvInvalidVariableError( # noqa: TRY003
"CLIENT_URL must contains a trailing slash",
)
if (
self.OVERRIDDEN_CLIENT_URL_FOR_OIDC
and not self.OVERRIDDEN_CLIENT_URL_FOR_OIDC[-1] == "/"
):
raise ValueError(
raise DotenvInvalidVariableError( # noqa: TRY003
"OVERRIDDEN_CLIENT_URL_FOR_OIDC must contains a trailing slash",
)

Expand All @@ -297,22 +299,22 @@ def check_database_settings(self) -> "Settings":
and self.POSTGRES_DB
)
):
raise ValueError(
"Either SQLITE_DB or POSTGRES_HOST, POSTGRES_USER, POSTGRES_PASSWORD and POSTGRES_DB should be configured in the dotenv",
raise DotenvMissingVariableError( # noqa: TRY003
"Either SQLITE_DB or POSTGRES_HOST, POSTGRES_USER, POSTGRES_PASSWORD and POSTGRES_DB",
)

return self

@model_validator(mode="after")
def check_secrets(self) -> "Settings":
if not self.ACCESS_TOKEN_SECRET_KEY:
raise ValueError(
"ACCESS_TOKEN_SECRET_KEY should be configured in the dotenv",
raise DotenvMissingVariableError(
"ACCESS_TOKEN_SECRET_KEY",
)

if not self.RSA_PRIVATE_PEM_STRING:
raise ValueError(
"RSA_PRIVATE_PEM_STRING should be configured in the dotenv",
raise DotenvMissingVariableError(
"RSA_PRIVATE_PEM_STRING",
)

return self
Expand Down
14 changes: 8 additions & 6 deletions app/core/cruds_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,11 @@ async def create_module_visibility(
db.add(module_visibility)
try:
await db.commit()
return module_visibility
except IntegrityError as error:
except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return module_visibility


async def delete_module_visibility(
Expand Down Expand Up @@ -123,10 +124,11 @@ async def add_core_data_crud(
db.add(core_data)
try:
await db.commit()
return core_data
except IntegrityError as error:
except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return core_data


async def delete_core_data_crud(
Expand Down
5 changes: 3 additions & 2 deletions app/core/groups/cruds_groups.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,11 @@ async def create_group(
db.add(group)
try:
await db.commit()
return group
except IntegrityError:
await db.rollback()
raise
else:
return group


async def delete_group(db: AsyncSession, group_id: str):
Expand All @@ -83,7 +84,7 @@ async def create_membership(
return await get_group_by_id(db, membership.group_id)
except IntegrityError:
await db.rollback()
raise ValueError("This user is already in this group")
raise


async def delete_membership_by_group_id(
Expand Down
14 changes: 8 additions & 6 deletions app/core/notification/cruds_notification.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,11 @@ async def create_firebase_devices(
db.add(firebase_device)
try:
await db.commit()
return firebase_device
except IntegrityError as error:
except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return firebase_device


async def delete_firebase_devices(
Expand Down Expand Up @@ -202,10 +203,11 @@ async def create_topic_membership(
db.add(topic_membership)
try:
await db.commit()
return topic_membership
except IntegrityError as error:
except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return topic_membership


async def delete_topic_membership(
Expand Down
15 changes: 9 additions & 6 deletions app/core/payment/cruds_payment.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ async def create_checkout(
db.add(checkout)
try:
await db.commit()
return checkout
except IntegrityError as error:

except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return checkout


async def get_checkout_by_id(
Expand Down Expand Up @@ -54,10 +56,11 @@ async def create_checkout_payment(
db.add(checkout_payment)
try:
await db.commit()
return checkout_payment
except IntegrityError as error:
except IntegrityError:
await db.rollback()
raise ValueError(error)
raise
else:
return checkout_payment


async def get_checkout_payment_by_hello_asso_payment_id(
Expand Down
12 changes: 6 additions & 6 deletions app/core/payment/endpoints_payment.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ async def webhook(
)
else:
checkout_metadata = None
except ValidationError as error:
hyperion_error_logger.error(
f"Payment: could not validate the webhook body: {await request.json()}, failed with error {error}",
except ValidationError:
hyperion_error_logger.exception(
f"Payment: could not validate the webhook body: {await request.json()}, failed",
)
raise HTTPException(
status_code=400,
Expand Down Expand Up @@ -127,7 +127,7 @@ async def webhook(
f"Payment: call to module {checkout.module} payment callback for checkout (hyperion_checkout_id: {checkout_metadata.hyperion_checkout_id}, HelloAsso checkout_id: {checkout.id}) succeeded",
)
return
except Exception as error:
hyperion_error_logger.error(
f"Payment: call to module {checkout.module} payment callback for checkout (hyperion_checkout_id: {checkout_metadata.hyperion_checkout_id}, HelloAsso checkout_id: {checkout.id}) failed with an error {error}",
except Exception:
hyperion_error_logger.exception(
f"Payment: call to module {checkout.module} payment callback for checkout (hyperion_checkout_id: {checkout_metadata.hyperion_checkout_id}, HelloAsso checkout_id: {checkout.id}) failed",
)
18 changes: 8 additions & 10 deletions app/core/payment/payment_tool.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,7 @@ async def init_checkout(
Exceptions can be imported from `helloasso_api_wrapper.exceptions`
"""
if not self.hello_asso:
raise PaymentToolCredentialsNotSetException(
"HelloAsso API credentials are not set",
)
raise PaymentToolCredentialsNotSetException

# We want to ensure that any error is logged, even if modules tries to try/except this method
# Thus we catch any exception and log it, then reraise it
Expand Down Expand Up @@ -120,12 +118,12 @@ async def init_checkout(
helloasso_slug,
init_checkout_body,
)
except ApiV5BadRequest as error:
except ApiV5BadRequest:
# We know that HelloAsso may refuse some payer infos, like using the firstname "test"
# Even when prefilling the payer infos,the user will be able to edit them on the payment page,
# so we can safely retry without the payer infos
hyperion_error_logger.error(
f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}, with error {error}. Retrying without payer infos",
hyperion_error_logger.exception(
f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}. Retrying without payer infos",
)

init_checkout_body.payer = None
Expand All @@ -149,11 +147,11 @@ async def init_checkout(
id=checkout_model_id,
payment_url=response.redirectUrl,
)
except Exception as error:
hyperion_error_logger.error(
f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}, with error {error}",
except Exception:
hyperion_error_logger.exception(
f"Payment: failed to init a checkout with HA for module {module} and name {checkout_name}",
)
raise error
raise

async def get_checkout(
self,
Expand Down
Loading

0 comments on commit 5c9118a

Please sign in to comment.