graphql-platform icon indicating copy to clipboard operation
graphql-platform copied to clipboard

WIP: Translate `Microsoft.AspNetCore.Authorization.AuthorizeAttribute` to `@authorize` directive

Open tobias-tengler opened this issue 2 years ago • 5 comments

Adds a new TypeInterceptor that is automatically registered when calling AddAuthorize to translate the Microsoft.AspNetCore.Authorization.AuthorizeAttribute to the @authorize directive.

This could be a breaking change for existing schemas, if Microsoft's AuthorizeAttribute was used by accident instead of Hot Chocolate's AuthorizeAttribute. But it will also prevent these accidental mixups in the future, by just providing the same functionality consistently. I thought about making the Hot Chocolate AuthorizeAttribute obsolete, but there are still valid use cases like AFTER_RESOLVER and also the limitation of not being able to specify the Microsoft attribute on properties.

Closes #3847

Todos

  • Infer attribute from type extensions

Outlook

I would like to add AuthenticationSchemes to the @authorize directive (and all APIs around it) to allow the use of multiple authentication schemes.

tobias-tengler avatar Mar 20 '22 11:03 tobias-tengler

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

stale[bot] avatar May 20 '22 12:05 stale[bot]

@tobias-tengler how far was this PR ? :)

PascalSenn avatar May 20 '22 14:05 PascalSenn

@tobias-tengler how far was this PR ? :)

I think it was almost done, except for some todos in the code and the missing ability to convert the Microsoft attribute, when it was applied on a type extension. I think it was tricky, because the type interceptor could not see the original CLR types where the extended field was coming from, so the attribute on the type can not be read. I think there should be a failing test.

tobias-tengler avatar May 20 '22 14:05 tobias-tengler

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Aug 17 '22 14:08 CLAassistant