Implementation of Bearer Token Authorization Support

[amrita-shrestha]

Description

With this PR, we are enhancing our HTTP client's capability by introducing support for Bearer token-based authorization.

Related to this PR

• Improve CURLOPT_HTTPHEADER Setting Assignment http#215

Test Added

• unit tests to ensure the proper functioning of the newly introduced Bearer token authorization feature

[codecov]

Codecov Report

Merging #1483 (fdc4bc0) into master (406f688) will increase coverage by 0.00%.
Report is 2 commits behind head on master.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #1483   +/-   ##
=========================================
  Coverage     97.34%   97.34%           
- Complexity     2830     2831    +1     
=========================================
  Files           175      175           
  Lines          9418     9420    +2     
=========================================
+ Hits           9168     9170    +2     
  Misses          250      250           

Files Changed	Coverage Δ
lib/DAV/Client.php	92.74% <100.00%> (+0.11%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[phil-davis]

@staabm we are starting to use the Client part of sabre/dav for some new stuff, and it would be nice to be able to pass in an access token directly in the constructor when making a new Client class. It is another optional setting, so no backward-compatibility issues. Access tokens are a common thing these days, so maybe useful for others too.

Please review. I don't want to "just merge" this because I have been involved with Amrita in discussing this.

[saw-jan]

setting headers here will get replaced if we do requests with some headers. So, this doesn't work as we expected.
Example:

$webdav = new Client([
    "baseUri" => "https://localhost:9200/remote.php/webdav/",
    "accessToken" => "eJashdksjaausb.asdohowkawrkas.adhdiuqwiduwqgwdi"
]);

// propfind() sets headers internally
$res = $webdav->propfind("", []);  // 401 Unauthorized ('Authorization' header gets removed)

Here, the headers are replaced: https://github.com/sabre-io/http/blob/d7c809503c1cb880622659095fc73d513e9ef23c/lib/Client.php#L409-L411

This will work but can't say if the Http\Client wants to do it.

if ([] !== $nHeaders) {
    $settings[CURLOPT_HTTPHEADER] = array_merge($settings[CURLOPT_HTTPHEADER], $nHeaders);
}

CC @phil-davis

[saw-jan]

Also, we can do like this. So, I wonder if we want to have it in DAV\Client's settings but would be great if we have.

$webdav = new Client([
    "baseUri" => "https://localhost:9200/remote.php/webdav/"
]);

$webdav->addCurlSetting(CURLOPT_HTTPHEADER, ["Authorization: Bearer $accessToken"]);

[saw-jan]

If we want to have it then I found some information to set it curl way instead of in header directly

// lib/DAV/Client.php
  ...
  $this->addCurlSetting(CURLOPT_USERPWD, $userName.':'.$password);
+} else if (isset($settings['accessToken'])) {
+    $this->addCurlSetting(CURLOPT_HTTPAUTH, CURLAUTH_BEARER);
+    $this->addCurlSetting(CURLOPT_XOAUTH2_BEARER, isset($settings['accessToken']));
+}

See https://curl.se/libcurl/c/CURLOPT_HTTPAUTH.html, https://curl.se/libcurl/c/CURLOPT_XOAUTH2_BEARER.html
PHP: https://www.php.net/manual/en/function.curl-setopt.php, https://www.php.net/manual/en/curl.constants.php

[saw-jan]

Using CURLOPT_XOAUTH2_BEARER works without having to fix anything in Http\Client.

[staabm]

IMO this name is too generic. Couldn't we name it bearerToken?

Alternatively couldn't we add a method to pass any raw http header thru to curl?

[staabm]

I am not sure why we need this PR.

have a look at the examples of sabre/http client in https://github.com/sabre-io/http/blob/d7c809503c1cb880622659095fc73d513e9ef23c/examples/basicauth.php

and we have a BearerAuth class already

After discussion, we don't actually need this.

[phil-davis]

After discussion, we have realized that we can achieve this in other ways, without needing to add any feature to dav-client.
See the various comments that @saw-jan has made.
So closing.