Adding tenants and content apis #2561
Conversation
src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs
Outdated
Show resolved
Hide resolved
| return Unauthorized(); | ||
| } | ||
|
|
||
| if (!IsDefaultShell()) |
There was a problem hiding this comment.
its a double check, we can remove
There was a problem hiding this comment.
oh, I think you meant
if (!ModelState.IsValid)
{
return BadRequest();
}
| } | ||
|
|
||
| [HttpDelete] | ||
| [IgnoreAntiforgeryToken] |
There was a problem hiding this comment.
Out of curiosity, how is authentication managed for these API controllers?
If you simply rely on the default authentication scheme (cookies, configured by the Users module), then disabling antiforgery validation is terrible, as it will allow anyone to forge malicious requests (same-site would help block them, but it's not supported everywhere and we have to disable it completely in the OpenID module).
Jetski5822
force-pushed
the
sebros/apis
branch
from
adb058d
to
5556873
Compare
# Conflicts: # src/OrchardCore.Modules/OrchardCore.Contents/Controllers/ApiController.cs # src/OrchardCore.Modules/OrchardCore.Tenants/Controllers/ApiController.cs
| /// Provides a delegating logic for API authentication. | ||
| /// If no specific scheme handler is found it returns an anonymous user. | ||
| /// </summary> | ||
| public class ApiAuthorizationHandler : AuthenticationHandler<ApiAuthorizationOptions> |
There was a problem hiding this comment.
ApiAuthorizationHandler -> ApiAuthenticationHandler?
|
|
||
| static ApiAuthorizationHandler() | ||
| { | ||
| Anonymous = AuthenticateResult.Success( |
There was a problem hiding this comment.
Nah, this is evil. Having a "fake" (or more exactly delegating here) handler is a thing, but using fake authenticated identities is another and IMHO, we shouldn't go that far.
Using AuthenticateResult.NoResult() or better AuthenticateResult.Fail() with an error message indicating there's no handler available to process tokens is the right thing to do.
| return Anonymous; | ||
| } | ||
|
|
||
| try |
There was a problem hiding this comment.
Not sure the try/catch is useful.
| : base(options, logger, encoder, clock) | ||
| { | ||
| _authenticationOptions = authenticationOptions; | ||
| _authenticationService = authenticationService; |
|
|
||
| namespace OrchardCore.Content.Controllers | ||
| { | ||
| [Route("api/content")] |
There was a problem hiding this comment.
Curious: should API controller be decorated with [ApiController] to use MVC's default conventions for APIs?
Necessary to run unit tests which require mutations right now.