pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
306 stars 445 forks source link

OJS 3.3.0-9 and -10: Error if on private site password protected #7751

Closed eniocarboni closed 2 years ago

eniocarboni commented 2 years ago

Describe the bug In my OJS test environment I use password protect via Apache "Basic Authorization" (AuthType Basic). But after upgrading from 3.3.0-8 to 3.3.0-9 (or 3.3.0-10) there are working problems with OJS for every XHR call referring to /api/v1/... As an example, just click on "Submissions" to view the list of submissions or save a configuration.

The XHR return: 403 Forbidden and the message is "An unrecognized Authorization header was recived. Expected to receive "Authentication: Bearer api-token"

To Reproduce Add "AuthType Basic" to your Apache web server

What application are you using? OJS-3.3.0-9 and OJS-3.3.0-10

Additional information Screenshot_20220308_155331

The problem should have arisen after the issue #7685 and more in detail in the line https://github.com/pkp/pkp-lib/blob/77153eef0de61fe9cdb5065fd94f3a2f0a74a493/classes/security/authorization/internal/ApiTokenDecodingMiddleware.inc.php#L154

asmecher commented 2 years ago

@eniocarboni, good catch (and thanks for the detailed report). Could you try applying https://github.com/asmecher/pkp-lib/commit/bdbfc6ba5da486d3c3949782637a6513ab8d6367 in lib/pkp and see if it resolves the issue?

eniocarboni commented 2 years ago

Yes, it works perfectly.

asmecher commented 2 years ago

Thanks, I'll commit that!

asmecher commented 2 years ago

Actually this needs more work -- when BASIC auth is used and a request using Bearer authentication is sent, the header will look something like

Authorization: Bearer mF_9.B5f-4.1JqM, Basic YXNkZnNhZGZzYWRmOlZLdDVOMVhk

(See https://stackoverflow.com/questions/29282578/multiple-http-authorization-headers.)

The best solution would be to use Slim middleware rather than parsing this out manually (e.g. https://packagist.org/packages/rbdwllr/psr-jwt) but depending on the outcome of https://github.com/pkp/pkp-lib/issues/7698 we might not want to add a new Slim-based dependency.

@eniocarboni, meanwhile, you should be able to continue with the patch linked above as a temporary work-around.

asmecher commented 2 years ago

@NateWr, could you review this?

https://github.com/pkp/pkp-lib/pull/7753

I've tested it with both Basic authorization and API keys and it seems to work fine.

@eniocarboni, it would also be helpful to know if this works for your situation!

I opted not to use middleware for the moment for the reason I wrote above; it's a pretty simple fix.

eniocarboni commented 2 years ago

@asmecher Yes, it works well

I also tried simulating a double authentication via "curl":

echo -n test_user:test_pwd | openssl base64
dGVzdF91c2VyOnRlc3RfcHdk

curl --insecure -H "Authorization: Basic dGVzdF91c2VyOnRlc3RfcHdk, Bearer MY_TEST_API_KEY" "https://.../index.php/journalurl/api/v1/users"

is ok for Basic Auth, result is:

{"error":"api.500.apiSecretKeyMissing","errorMessage":"##api.500.apiSecretKeyMissing##"}

but by reversing the authorization:

curl --insecure -H "Authorization: Bearer MY_TEST_API_KEY, Basic dGVzdF91c2VyOnRlc3RfcHdk" "https://.../index.php/journalurl/api/v1/users"

the Apache Web server does not authenticate you by returning "401 Unauthorized"

asmecher commented 2 years ago

@eniocarboni, I was able to reproduce the ordering issue you encountered in your comment above, but this is an Apache (or equivalent) issue rather than an OJS one. With a .htaccess-based basic auth, Apache gets first kick at authorizing the user, and appears not to like Bearer auth coming through first. OJS doesn't even have a chance to respond. This is addressed (obliquely) in the documentation: https://httpd.apache.org/docs/2.4/howto/auth.html#beyond

So I've applied the change to stable-3_3_0 and forward-ported to main; thanks for your input and testing here!

mpbraendle commented 2 years ago

Some Apache servers use the server variable REDIRECT_HTTP_AUTHORIZATION instead of of HTTP_AUTHORIZATION, especially if redirect rules were implemented. The Slim function determineAuthorization doesn't recognise this, and subsequent methods also read HTTP_AUTHORIZATION. This always leads to a 403 HTTP status although the Bearer API Key is correct. Of course I can patch this method (and have done so for a client), but because Slim is a vendor library, where would be a better place to apply a patch?

asmecher commented 2 years ago

@mpbraendle, that was discussed within Slim but seemingly rejected here: https://github.com/slimphp/Slim/issues/831

There is a possible .htaccess configuration option to fix the Apache handling of HTTP_AUTHORIZATION: https://github.com/slimphp/Slim/issues/831#issuecomment-356990229

If that works, I'd recommend using it. It seems that Slim had an accommodation for this Apache quirk in Slim2 but that Slim3 doesn't appear to plan to re-add it. We are also at some point going to migrate from Slim to Laravel (https://github.com/pkp/pkp-lib/issues/7698).