Windowsserverdocs: Shouldn't `WebAPIOBOController` be secured with an `Authorize` attribute?

Created on 8 Jul 2019  Â·  10Comments  Â·  Source: MicrosoftDocs/windowsserverdocs

It seems that the WebAPIOBOController controller should be secured with an Authorize attribute. Otherwise, the application will never validate the OAuth token when the ToDoListService calls the WebAPIOBO API, and as such, the AD FS configuration for WebAPIOBOController will never really be tested when running this locally.

[Authorize]
public class WebAPIOBOController : ApiController
{
    public IHttpActionResult Get()
    {
        return Ok("WebAPI via OBO");
    }
}

Document Details

⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

Most helpful comment

Hello, @illfated. No worries. And yes, your comment from November 19th, 2019, is correct. Sorry that I didn't see it sooner.

For additional follow-up, I think that the only pieces are to add the code from your comment. Or, to at least add the [Authorize] attribute. Because without that attribute, the person following the example won't really know for certain if their security is set up correctly because the API won't check the bearer token if that attribute isn't there.

All 10 comments

@tobyartisan : Just to make sure I have understood your code sample correctly,
are you implying that it could look like my code copy below?

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Net;
    using System.Net.Http;
    using System.Web.Http;
    namespace WebAPIOBO.Controllers
    {
        [Authorize]
        public class WebAPIOBOController : ApiController
        {
            public IHttpActionResult Get()
            {
                return Ok("WebAPI via OBO");
            }
        }
    }

My initial guess would be to have an [Authorize] attribute as well. However, that does seem to break the code. This API would need protection and besides that I would very much like to have the user info as well here. But that seems to be an issue. Could this example be extended by not just showing "WebAPI via OBO", but rather something like $"WebAPI via OBO (user: {User.Identity.Name}"?

Fair enough. Looks like I will be needing a full suggestion of what the change needs to look like, since it appears I do not know how attributes work in this context.

Feel free to post a full code sample the way you have found it to be working correctly on your side.

@illfated, yes, that is correct. Also, from my memory, I believe that the example breaks when adding the Authorize attribute because of the other issue with the documentation that's explained in issue #3000. Specifically, if you don't follow the suggestion in issue 3000, then this example fails because the JWT token won't have the correct user_impersonation scope, and AD FS won't return the OBO token that is to be sent to this WebAPIOBOController.

If you follow the suggestion in that issue, and then add the Authorize attribute as shown here, then it should work. If it still doesn't work after that, then I can go through the example again to see if there was another change that I made but did not document as an issue.

For what @riesm is suggesting, the controller would look like the following:

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Net;
    using System.Net.Http;
    using System.Web.Http;
    namespace WebAPIOBO.Controllers
    {
        [Authorize]
        public class WebAPIOBOController : ApiController
        {
            public IHttpActionResult Get()
            {
                return Ok($"WebAPI via OBO (user: {User.Identity.Name}");
            }
        }
    }

Much appreciated. Thanks a lot for the follow-up.
I will wait for a second follow-up from @riesm to see
what we should focus on before opening a PR.


edit: BTW, I presume you saw the PR I made for issue #3000 .

@illfated Yes, this is what it looks like and I can confirm it's working (had a typo in my ADFS server app settings to why it was not working at first). The [Authorize] tag protects the API and the addition of the user name helps in understanding the OBO security patterns.

OK, so it will be fair to suggest this code snippet in place of the current one in the docs page?

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Net;
    using System.Net.Http;
    using System.Web.Http;
    namespace WebAPIOBO.Controllers
    {
        [Authorize]
        public class WebAPIOBOController : ApiController
        {
            public IHttpActionResult Get()
            {
                return Ok($"WebAPI via OBO (user: {User.Identity.Name}");
            }
        }
    }

Sorry for not revisiting this issue ticket sooner. I have been preoccupied with other issues, reviews and discussions, so this one had really slipped my mind.

Would you like to see more follow-up regarding this issue?

Hello, @illfated. No worries. And yes, your comment from November 19th, 2019, is correct. Sorry that I didn't see it sooner.

For additional follow-up, I think that the only pieces are to add the code from your comment. Or, to at least add the [Authorize] attribute. Because without that attribute, the person following the example won't really know for certain if their security is set up correctly because the API won't check the bearer token if that attribute isn't there.

There you go. PR #3821 is ready for review, comments and suggestions. Thanks to the both of you for valuable input & feedback.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

wilsonnkwan picture wilsonnkwan  Â·  4Comments

chall3ng3r picture chall3ng3r  Â·  4Comments

aurelien-git picture aurelien-git  Â·  3Comments

gabrielluizbh picture gabrielluizbh  Â·  5Comments

skyflyer picture skyflyer  Â·  3Comments