feat: add boolean permission support

[erikwrede]

Supersedes #3408

[botberry]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

---

Adds the ability to use the & and | operators on permissions to form boolean logic. For example, if you want
a field to be accessible with either the IsAdmin or IsOwner permission you
could define the field as follows:

import strawberry
from strawberry.permission import PermissionExtension, BasePermission


@strawberry.type
class Query:
    @strawberry.field(
        extensions=[
            PermissionExtension(
                permissions=[(IsAdmin() | IsOwner())], fail_silently=True
            )
        ]
    )
    def name(self) -> str:
        return "ABC"

Here's the tweet text:

🆕 Release (next) is out! Thanks to Erik Wrede for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

[sourcery-ai]

Hey @erikwrede - I've reviewed your changes and they look great!

Here's what I looked at during the review

• 🟡 General issues: 1 issue found
• 🟢 Security: all looks good
• 🟡 Testing: 4 issues found
• 🟢 Complexity: all looks good
• 🟢 Docstrings: all looks good

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.}

[sourcery-ai]

suggestion (code_refinement): Consider simplifying the return type for better maintainability.

The method now has multiple return types, which could complicate the handling of its output. Simplifying to a consistent return type, possibly encapsulated within a class or structure, might improve maintainability and readability.

Suggested change

	) -> Union[bool, Awaitable[bool], (False, dict), Awaitable[(False, dict)]]:
	) -> Union[bool, Awaitable[bool]]:

[ThirVondukr]

By the way, this looks like a mistake (a, b) is not a valid type 🤔 Perhaps you need Tuple[Literal[False], dict]?

[sourcery-ai]

suggestion (testing): Consider adding tests for mixed async and sync permission combinations.

It would be beneficial to verify the behavior when permissions are mixed (some async, some sync) in the AND and OR scenarios to ensure the system handles these mixed cases correctly.

[sourcery-ai]

suggestion (testing): Add negative test cases for async permission checks.

While there are tests for successful permission checks, adding negative test cases where async permissions fail would help ensure robust error handling and behavior verification.

[sourcery-ai]

suggestion (testing): Verify handling of exceptions thrown by permission checks.

It's important to ensure that any exceptions thrown during permission checks are handled gracefully and do not cause unexpected crashes or behavior.

Suggested change

	async def test_no_graphql_error_when_and_permission_is_allowed():
	try:
	result = schema.execute_sync(query)
	assert result.data["user"] == "patrick"
	except Exception as e:
	assert False, f"Unexpected error occurred: {str(e)}"

[sourcery-ai]

nitpick (testing): Ensure consistency in the use of 'pragma: no cover'.

The use of 'pragma: no cover' is noted in several test cases. Please ensure it is used consistently across all test cases where applicable to maintain coverage clarity.

[sourcery-ai]

suggestion (code-quality): Use the built-in function next instead of a for-loop (use-next)

Suggested change

	for permission in self.child_permissions:
	if not permission.has_permission(source, info, **kwargs):
	return False, {"failed_permissions": [permission]}
	return True
	return next(
	(
	(False, {"failed_permissions": [permission]})
	for permission in self.child_permissions
	if not permission.has_permission(source, info, **kwargs)
	),
	True,
	)

[codecov]

Codecov Report

Attention: Patch coverage is 96.86275% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 96.49%. Comparing base (0c5bc4b) to head (29fe77f).
Report is 29 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3451      +/-   ##
==========================================
+ Coverage   96.47%   96.49%   +0.02%     
==========================================
  Files         509      510       +1     
  Lines       32691    33052     +361     
  Branches     5399     5478      +79     
==========================================
+ Hits        31539    31894     +355     
+ Misses        941      921      -20     
- Partials      211      237      +26     

[codspeed-hq]

CodSpeed Performance Report

Merging #3451 will not alter performance

_{Comparing boolean-expression-permissions-erik (29fe77f) with main (17f05a7)}

Summary

✅ 12 untouched benchmarks

[ThirVondukr]

Probably get rid of dictionaries here, maybe make a dataclass instead 🤔

[erikwrede]

The idea here was to remain as flexible as possible, allowing users to pass context in the future as well. With a dataclass we would be again very static about handling additional permission error context. Maybe a TypedDict can help improve our defaults for this particular case?

[ThirVondukr]

Hm, I'm not sure how context would be used here, is it in the release noted (that I didn't read)? 😅

[erikwrede]

It's internal for now, but it allows us to pass information to on_unauthorized from has_permission. We opted to separate exception raising from permission checking back when implementing this extension due to the fail silently feature.

[erikwrede]

So basically the context contains "this permission failed out of the list of and / or permissions"

[ThirVondukr]

This is probably a mistake - with kwargs you annotate value type (e.g. int instead of dict[str, int], not sure what was intended here 🙂

[ThirVondukr]

If there are no particular operations performed on kwargs values you could annotate them as object, as it's the most narrow type, Any being worse for that use case

[patrick91]

Let's keep working in #3408 😊