thecodingmachine / graphqlite

Use PHP Attributes/Annotations to declare your GraphQL API
https://graphqlite.thecodingmachine.io
MIT License
557 stars 97 forks source link

Improve #[InjectUser] to throw authentication errors #572

Closed oprypkhantc closed 1 year ago

oprypkhantc commented 1 year ago

#[InjectUser] can be improved further to throw a MissingAuthorizationException when a user is not authenticated, to avoid adding #[Logged] when user is required anyway, i.e.:

#[Query]
public function someQuery(
    #[InjectUser] User $user, // <-- here a user is required (because the type isn't nullable), so it doesn't make sense to fail with a 500 internal error when a query isn't annotated with #[Logged]
): void {}

Obviously this should not apply to nullable or optional parameters:

#[Query]
public function someQuery(
    #[InjectUser] ?User $user, // <-- user is not required, so it's assumed to be optional
    #[InjectUser] User $user2 = new User(), // same here, it has a default value so it's assumed optional
): void {}

This improvement will be easy to implement:

I'll PR this improvement if it's desirable. Thoughts?

Lappihuan commented 1 year ago

sounds good, i'd probably not rely on it to replace logged but we manage that in our custom middleware anyway.