Skip to content
This repository has been archived by the owner on May 13, 2024. It is now read-only.

Ruff config #1049

Merged
merged 27 commits into from
Apr 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
d8861c5
chore(linting): stricter ruff config
c0rydoras Apr 16, 2024
2061aa8
chore: linted
c0rydoras Apr 16, 2024
064a325
chore: run makemigrations
c0rydoras Apr 16, 2024
0ca6159
chore: raise from exc instead of None
c0rydoras Apr 16, 2024
f7f7f6f
chore(linting): ignore-variadic-names for ARG
c0rydoras Apr 17, 2024
86f253a
chore: make ruff happier
c0rydoras Apr 17, 2024
e2232a0
chore: alter old migrations to use tuple and drop new migration
c0rydoras Apr 17, 2024
bb42765
chore(linting): ignore more rules
c0rydoras Apr 17, 2024
a1cd7fc
fix(settings): add missing .exists()
c0rydoras Apr 17, 2024
9f58039
chore: linted
c0rydoras Apr 17, 2024
5c962b3
chore: remove unnecessary ClassVar annotations
c0rydoras Apr 17, 2024
63d5010
chore(linting): fix indentation in ruff.toml
c0rydoras Apr 17, 2024
b662cf9
chore(linting): don't require parentheses after pytest.fixture
c0rydoras Apr 17, 2024
98c99d4
docs(linting): mark ignore-fully-untyped as temporary
c0rydoras Apr 17, 2024
29fac56
refactor: update code to make ruff happier
c0rydoras Apr 17, 2024
11366b9
chore: raise PermissionDenied errors from None
c0rydoras Apr 17, 2024
28b8f45
docs(models): add __str__
c0rydoras Apr 17, 2024
da11076
chore(linting): add DJ001 to ignores
c0rydoras Apr 18, 2024
3f1c88c
chore(linting): reenable SLF001 and noqa current occurrences
c0rydoras Apr 18, 2024
cf63211
refactor(statistics-qs): adjust function signature to not allow *args
c0rydoras Apr 18, 2024
55deb9e
chore(linting): re-enable RUF012, format tuples to be one item per line
c0rydoras Apr 18, 2024
b07d07b
docs(linting): add issue link to ignore-fully-untyped=true
c0rydoras Apr 19, 2024
cb72c91
chore(linting): replace _ in args with _{argname}
c0rydoras Apr 22, 2024
04c1529
docs: move sphinx-doc to type annotations
c0rydoras Apr 22, 2024
5c54049
chore: move todo from docstring to comments
c0rydoras Apr 22, 2024
db84155
refactor: move code after except ModelDoesNotExist outside of try block
c0rydoras Apr 22, 2024
4bcb7ab
refactor: replace *args, **kwargs with _{argname} wherever possible
c0rydoras Apr 23, 2024
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
2 changes: 2 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -94,12 +94,14 @@ exclude_lines = [
"def __str__",
"def __unicode__",
"def __repr__",
"if TYPE_CHECKING",
]
omit = [
"*/migrations/*",
"*/apps.py",
"*/admin.py",
"manage.py",
"timed/redmine/management/commands/import_project_data.py",
hairmare marked this conversation as resolved.
Show resolved Hide resolved
"timed/settings_*.py",
"timed/wsgi.py",
"timed/forms.py",
Expand Down
113 changes: 96 additions & 17 deletions ruff.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,27 +11,106 @@ docstring-code-format = true
docstring-code-line-length = 88

[lint]
select = ["E", "F", "W", "I", "BLE", "T10"]
select = [
"F", # pyflakes
"E", # pycodestyle errors
"I", # isort
"C90", # mccabe
"N", # pep8-naming
"D", # pydocstyle
"UP", # pyupgrade
"ANN", # flake8-annotations
"ASYNC", # flake8-async
"S", # flake8-bandit
"BLE", # flake8-blind-exception
"FBT", # flake8-boolean-trap
"B", # flake8-bugbear
"A", # flake8-builtins
"COM", # flake8-commas
"C4", # flake8-comprehensions
"T10", # flake8-debugger
"DJ", # flake8-django
"EM", # flake8-errmsg
"EXE", # flake8-executable
"FA", # flake8-future-annotations
"ISC", # flake8-implicit-str-concat
"ICN", # flake8-import-conventions
"G", # flake8-logging-format
"INP", # flake8-no-pep420
"PIE", # flake8-pie
"T20", # flake8-print
"PYI", # flake8-pyi
"PT", # flake8-pytest-style
"Q", # flake8-quotes
"RSE", # flake8-raise
"RET", # flake8-return
"SLF", # flake8-self
"SLOT", # flake8-slots
"SIM", # flake8-simplify
"TID", # flake8-tidy-imports
"TCH", # flake8-type-checking
"INT", # flake8-gettext
"ARG", # flake8-unused-arguments
"PTH", # flake8-use-pathlib
"TD", # flake8-todos
"ERA", # eradicate
"PGH", # pygrep-hooks
"PL", # pylint
"TRY", # tryceratops
"PERF", # perflint
"RUF", # ruff specific rules
"W605", # invalid escape sequence
]
ignore = [
# https://docs.astral.sh/ruff/formatter/#conflicting-lint-rules
"W191",
"E111",
"E114",
"E117",
"E501",
"D206",
"D300",
"Q000",
"Q001",
"Q002",
"Q003",
"COM812",
"COM819",
"ISC001",
"ISC002"
"ANN101", # this is deprecated and annotating self is unnecessary
"D203", # we prefer blank-line-before-class (D211) for black compat
"D213", # we prefer multi-line-summary-first-line (D212)
"COM812", # ignore due to conflict with formatter
"ISC001", # ignore due to conflict with formatter
"E501", # managed by formatter
"TD002", # don't require author of TODO
"TD003", # don't require link to TODO
"D100", # don't enforce existance of docstrings
"D101", # don't enforce existance of docstrings
"D102", # don't enforce existance of docstrings
"D103", # don't enforce existance of docstrings
"D104", # don't enforce existance of docstrings
"D105", # don't enforce existance of docstrings
"D106", # don't enforce existance of docstrings
"D107", # don't enforce existance of docstrings
"DJ001", # null=True on string-based fields such as CharField (#1052)
]

[lint.per-file-ignores]
"**/{conftest.py,tests/*.py}" = [
"D", # pydocstyle is optional for tests
"ANN", # flake8-annotations are optional for tests
"S101", # assert is allow in tests
"S105", # tests may have hardcoded secrets
"S106", # tests may have hardcoded passwords
"S311", # tests may use pseudo-random generators
"S108", # /tmp is allowed in tests since it's expected to be mocked
"DTZ00", # tests often run in UTC
"INP001", # tests do not need a dunder init
"PLR0913", # tests can have a lot of arguments (fixtures)
"PLR2004", # tests can use magic values
]
"**/*/factories.py" = [
"S311", # factories may use pseudo-random generators
]

[lint.isort]
known-first-party = ["timed"]
known-third-party = ["pytest_factoryboy"]
combine-as-imports = true

[lint.flake8-annotations]
# TODO: drop this, its temporary
# https://github.com/adfinis/timed-backend/issues/1054
ignore-fully-untyped = true

[lint.flake8-unused-arguments]
ignore-variadic-names = true

[lint.flake8-pytest-style]
fixture-parentheses = false
55 changes: 24 additions & 31 deletions timed/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,10 @@


class TimedOIDCAuthenticationBackend(OIDCAuthenticationBackend):
def get_introspection(self, access_token, id_token, payload):
def get_introspection(self, access_token, _id_token, _payload):
Copy link
Member

Choose a reason for hiding this comment

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

Just a note (no change required): This naming pattern really only works when the method is called with positional args. We'll have to be careful to check the call sites in such cases - especially if it's implementing a protocol / defined API or overriding a base class' implementation

"""Return user details dictionary."""

basic = base64.b64encode(
f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode(
"utf-8"
)
f"{settings.OIDC_RP_INTROSPECT_CLIENT_ID}:{settings.OIDC_RP_INTROSPECT_CLIENT_SECRET}".encode()
hairmare marked this conversation as resolved.
Show resolved Hide resolved
).decode()
headers = {
"Authorization": f"Basic {basic}",
Expand All @@ -29,42 +26,39 @@ def get_introspection(self, access_token, id_token, payload):
verify=settings.OIDC_VERIFY_SSL,
headers=headers,
data={"token": access_token},
timeout=10,
hairmare marked this conversation as resolved.
Show resolved Hide resolved
)
response.raise_for_status()
return response.json()

def get_userinfo_or_introspection(self, access_token):
try:
claims = self.cached_request(
self.get_userinfo, access_token, "auth.userinfo"
)
return claims
except requests.HTTPError as e:
if e.response.status_code not in [401, 403]:
raise e
return self.cached_request(self.get_userinfo, access_token, "auth.userinfo")
except requests.HTTPError as exc:
if exc.response.status_code not in [401, 403]:
raise
if settings.OIDC_CHECK_INTROSPECT:
try:
# check introspection if userinfo fails (confidential client)
claims = self.cached_request(
self.get_introspection, access_token, "auth.introspection"
)
if "client_id" not in claims:
raise SuspiciousOperation(
"client_id not present in introspection"
)
return claims
msg = "client_id not present in introspection"
raise SuspiciousOperation(msg)
except requests.HTTPError as e:
# if the authorization fails it's not a valid client or
# the token is expired and permission is denied.
# Handing on the 401 Client Error would be transformed into
# a 500 by Django's exception handling. But that's not what we want.
if e.response.status_code not in [401, 403]: # pragma: no cover
raise e
raise AuthenticationFailed()
raise
else:
return claims
raise AuthenticationFailed from exc

def get_or_create_user(self, access_token, id_token, payload):
def get_or_create_user(self, access_token, _id_token, _payload):
"""Verify claims and return user, otherwise raise an Exception."""

claims = self.get_userinfo_or_introspection(access_token)

users = self.filter_users_by_claims(claims)
Expand All @@ -73,15 +67,14 @@ def get_or_create_user(self, access_token, id_token, payload):
user = users.get()
self.update_user_from_claims(user, claims)
return user
elif settings.OIDC_CREATE_USER:
if settings.OIDC_CREATE_USER:
return self.create_user(claims)
else:
LOGGER.debug(
"Login failed: No user with username %s found, and "
"OIDC_CREATE_USER is False",
self.get_username(claims),
)
return None
LOGGER.debug(
"Login failed: No user with username %s found, and "
"OIDC_CREATE_USER is False",
self.get_username(claims),
)
return None

def update_user_from_claims(self, user, claims):
user.email = claims.get(settings.OIDC_EMAIL_CLAIM, "")
Expand All @@ -106,7 +99,6 @@ def cached_request(self, method, token, cache_prefix):

def create_user(self, claims):
"""Return object for a newly created user account."""

username = self.get_username(claims)
email = claims.get(settings.OIDC_EMAIL_CLAIM, "")
first_name = claims.get(settings.OIDC_FIRSTNAME_CLAIM, "")
Expand All @@ -119,5 +111,6 @@ def create_user(self, claims):
def get_username(self, claims):
try:
return claims[settings.OIDC_USERNAME_CLAIM]
except KeyError:
raise SuspiciousOperation("Couldn't find username claim")
except KeyError as exc:
msg = "Couldn't find username claim"
raise SuspiciousOperation(msg) from exc
19 changes: 9 additions & 10 deletions timed/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@


def register_module(module):
for name, obj in inspect.getmembers(module):
if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract:
for _name, obj in inspect.getmembers(module):
winged marked this conversation as resolved.
Show resolved Hide resolved
if isinstance(obj, FactoryMetaClass) and not obj._meta.abstract: # noqa: SLF001
register(obj)


Expand All @@ -26,7 +26,7 @@ def register_module(module):


@pytest.fixture
def auth_user(db):
def auth_user(db): # noqa: ARG001
return get_user_model().objects.create_user(
username="user",
password="123qweasd",
Expand All @@ -38,7 +38,7 @@ def auth_user(db):


@pytest.fixture
def admin_user(db):
def admin_user(db): # noqa: ARG001
return get_user_model().objects.create_user(
username="admin",
password="123qweasd",
Expand All @@ -50,7 +50,7 @@ def admin_user(db):


@pytest.fixture
def superadmin_user(db):
def superadmin_user(db): # noqa: ARG001
return get_user_model().objects.create_user(
username="superadmin",
password="123qweasd",
Expand All @@ -62,7 +62,7 @@ def superadmin_user(db):


@pytest.fixture
def external_employee(db):
def external_employee(db): # noqa: ARG001
user = get_user_model().objects.create_user(
username="user",
password="123qweasd",
Expand All @@ -76,7 +76,7 @@ def external_employee(db):


@pytest.fixture
def internal_employee(db):
def internal_employee(db): # noqa: ARG001
user = get_user_model().objects.create_user(
username="user",
password="123qweasd",
Expand Down Expand Up @@ -140,16 +140,15 @@ def internal_employee_client(internal_employee):
return client


@pytest.fixture(scope="function", autouse=True)
@pytest.fixture(autouse=True)
def _autoclear_cache():
cache.clear()


def setup_customer_and_employment_status(
user, is_assignee, is_customer, is_employed, is_external
):
"""
Set up customer and employment status.
"""Set up customer and employment status.

Return a 2-tuple of assignee and employment, if they
were created
Expand Down
Loading