bshaffer / oauth2-server-php

A library for implementing an OAuth2 Server in php
http://bshaffer.github.io/oauth2-server-php-docs
MIT License
3.26k stars 953 forks source link

OAuth server sends a 401 response where it should be 403 #254

Open marovelo opened 10 years ago

marovelo commented 10 years ago

Hi, when I send a resource request with an insufficient scope I get the right error and error description:

{"error":"insufficient_scope","error_description":"The request requires higher privileges than provided by the access token"}

However I would expect a 403 response but the response code is 401. With some debugging I found out that everything works fine when I comment out this line in ResourceController->verifyResourceRequest(...)

$response->addHttpHeaders(array(
                'WWW-Authenticate' => sprintf('%s realm="%s", scope="%s", error="%s", error_description="%s"',
                    $this->tokenType->getTokenType(),
                    $this->config['www_realm'],
                    $scope,
                    $response->getParameter('error'),
                    $response->getParameter('error_description')
                )
            ));

In the line above this one the http code gets set to 403. So it seems that somehow the code gets set to 401 by the addHttpHeaders(...) method.

Some more information: The problem only occurs when I send a curl request. I also implemented a Silex Web Test Case and here everything works just fine. I use v0.9 of your implmentation.

Thanks in advance for your help!

bshaffer commented 10 years ago

Here is a test that verifies the 403 status code is set: https://github.com/bshaffer/oauth2-server-php/blob/v0.9/test/OAuth2/Controller/ResourceControllerTest.php#L108

It is passing. Can you provide a test case that is failing? You can fork this library and submit it in a PR. If so, I can look into the fix. Otherwise, I think it may be something broken in your implementation

bshaffer commented 10 years ago

Any word on this? Can you verify this is still taking place?

marovelo commented 10 years ago

I forked your Demo Application and wrote a test similar to the one that fails in my application. I submited this test as a Pull Request if you are interested (see above). The test doesn't fail so it seems like there is a problem in my implementation. I did some more debugging in my application and found out that your OAuth Server implementation sends out the correct HTTP Code (403) but somehow a 401 arrives as a response to my Curl Request. No idea why that happens. If I comment out the line adding the WWW-Authenticate Header (see my post above) the correct 403 HTTP Code arrives. To sum it up I'm quite sure it has nothing to do with your code. Maybe a Curl/Apache/PHP problem. I will need to do more research. Any suggestions are more than welcome.

bshaffer commented 10 years ago

This is definitely interesting, but I do not know the cause. 

Are you extending any part of the framework? Are you using any subclasses for anything, or are you using the http foundation bridge? Anything out of the standard implementation? 

— Brent Shaffer

On Fri, Nov 8, 2013 at 2:47 PM, marovelo notifications@github.com wrote:

I forked your Demo Application and wrote a test similar to the one that fails in my application. I submited this test as a Pull Request if you are interested (see above). The test doesn't fail so it seems like there is a problem in my implementation. I did some more debugging in my application and found out that your OAuth Server implementation sends out the correct HTTP Code (403) but somehow a 401 arrives as a response to my Curl Request. No idea why that happens. If I comment out the line adding the WWW-Authenticate Header (see my post above) the correct 403 HTTP Code arrives.

To sum it up I'm quite sure it has nothing to do with your code. Maybe a Curl/Apache/PHP problem. I will need to do more research. Any suggestions are more than welcome.

Reply to this email directly or view it on GitHub: https://github.com/bshaffer/oauth2-server-php/issues/254#issuecomment-28049599

marovelo commented 10 years ago

Here is a list of things that differ in my application compared to the standard implementation:

Anything here that might cause this problem?

bshaffer commented 10 years ago

Not at all! Your implementation is pretty standard. Running through silex is the only thing I can think of.. it's possible that the request object is changing the status code somewhere.

nkahnfr commented 10 years ago

Hi,

I encountered the same issue but it seems related to PHP itself since 401 status code is enforced whenever a "WWW-Authenticate" header is defined (at least from what I understood from SAPI souce code).

The only workaround I found is to force the status code using the third parameter of "header" function.

header('WWW-Authenticate: Bearer realm="Test API", scope="test", error="insufficient_scope", error_description="The request requires higher privileges than provided by the access token"', true, 403);

But to do so with your library would require some modification to the "OAuth2\Response" class since only name and value can be defined presently.

Edit1: adding a dirty fix example of the workaround ("send" method of "Response" class)

foreach ($this->getHttpHeaders() as $name => $header) {
    if ('WWW-Authenticate' === $name)
        header(sprintf('%s: %s', $name, $header), true, $this->statusCode);
    else
        header(sprintf('%s: %s', $name, $header));
}
masterzen commented 9 years ago

I also encountered the issue and IMHO I think this is a bug in the library.

The section 10.4.2 of RFC2616 and section 10.4.4, and the RFC2617 clearly states that you should not send a WWW-Authenticate header with a 403 response (it is forbidden to retry the auth on a 403), and that it should have been a 401. The authentication can be retried only with a 401.

bshaffer commented 9 years ago

Where does RFC2617 clearly state this? I only see reference to 401 and nothing referencing 403.

The OAuth2 spec unfortunately does not mention the insufficient_scope error, as far as I can tell. However, requesting a Google API with insufficient scope returns the following (abbreviated) raw HTTP response:

HTTP/1.1 403 Forbidden
Vary: X-Origin
WWW-Authenticate: Bearer realm="https://accounts.google.com/", error=insufficient_scope, scope="https://www.googleapis.com/auth/plus.me"
Content-Type: application/json; charset=UTF-8
Date: Fri, 11 Sep 2015 18:52:38 GMT
Expires: Fri, 11 Sep 2015 18:52:38 GMT

{
 "error": {
  "errors": [
   {
    "domain": "global",
    "reason": "insufficientPermissions",
    "message": "Insufficient Permission",
   }
  ],
  "code": 403,
  "message": "Insufficient Permission"
 }
}

In addition, I ran PHP's built-in web server, and used the following code to test this issue:

header('WWW-Authenticate: Basic realm="My Realm"');
header('HTTP/1.0 403 Unauthorized');

And I received the following HTTP response:

12:04 $ curl -v http://localhost:8000 
* Rebuilt URL to: http://localhost:8000/
*   Trying ::1...
* Connected to localhost (::1) port 8000 (#0)
> GET / HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.43.0
> Accept: */*
> 
* HTTP 1.0, assume close after body
< HTTP/1.0 403 Unauthorized
< Host: localhost:8000
< Connection: close
< X-Powered-By: PHP/5.5.28
< WWW-Authenticate: Basic realm="My Realm"
< Content-type: text/html
< 
* Closing connection 0

However, if I reversed the order of the header parameters:

header('HTTP/1.0 403 Unauthorized');
header('WWW-Authenticate: Basic realm="My Realm"');

I received the following HTTP response:

12:04 $ curl -v http://localhost:8000 
* Rebuilt URL to: http://localhost:8000/
*   Trying ::1...
* Connected to localhost (::1) port 8000 (#0)
> GET / HTTP/1.1
> Host: localhost:8000
> User-Agent: curl/7.43.0
> Accept: */*
> 
< HTTP/1.1 401 Unauthorized
< Host: localhost:8000
< Connection: close
< X-Powered-By: PHP/5.5.28
< WWW-Authenticate: Basic realm="My Realm"
< Content-type: text/html
< 
* Closing connection 0

So I may just need to add logic to output the status code HTTP header last, and we should be all set here.

masterzen commented 9 years ago

@bshaffer, first let me thank you for this awesome library.

The relevant part is in section 1.2 of RFC2617, which I forgot to point to in my previous comment:

If the origin server does not wish to accept the credentials sent with a request, it SHOULD return a 401 (Unauthorized) response. The response MUST include a WWW-Authenticate header field containing at least one (possibly new) challenge applicable to the requested resource.

I also noticed that you can trick PHP into sending a 403 by changing the order of headers sent. But it looks like a workaround for something that is to me the correct behavior. Also, some of your users like me, are using this library inside a framework (Slim for me) that does the gory details of sending the Response.

Anyway, sorry for resurrecting this old ticket :)

bshaffer commented 9 years ago

No, I'm glad you did. I want to fix any bugs if they exist. But I do think 403 is appropriate for the insufficient_scope response.

The difference here though is that the origin server does accept the credentials sent (i.e. client_id / client_secret, access_token, etc), but the credentials do not have the required scopes. It may be mincing words, but I believe this is the intended behavior.

Either way, it's a bug. We need to either change the order of the header (which as you say, is rather hackish), remove WWW-Authenticate from the insufficient_scope error response, or change the error response to use 401. I am not exactly sure which is the best solution, at this point.

bshaffer commented 9 years ago

Unfortunately, even if there is a non-hackish way to do #1, your point about other frameworks plugging into this library is very true, and they may not propagate the fix.

masterzen commented 9 years ago

@bshaffer, apparently other OAuth2 implementations return 403 for insufficient_scope errors, and those that I looked up don't return a WWW-Authenticate header (at least for the ruby and java implementation). So that may be the solution.