Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth2 Log-in does not work against League's Oauth2-Server #788

Closed
SimonBin opened this issue Apr 4, 2018 · 3 comments
Closed

OAuth2 Log-in does not work against League's Oauth2-Server #788

SimonBin opened this issue Apr 4, 2018 · 3 comments

Comments

@SimonBin
Copy link

SimonBin commented Apr 4, 2018

I would recommend you test your openITCOCKPIT against simplesamlphp installation, I am currently trying to make it work but running into several issues!

The error message from League is:

League\OAuth2\Server\Exception\OAuthServerException Object
(
    [httpStatusCode:League\OAuth2\Server\Exception\OAuthServerException:private] => 400
    [errorType:League\OAuth2\Server\Exception\OAuthServerException:private] => unsupported_grant_type
    [hint:League\OAuth2\Server\Exception\OAuthServerException:private] => Check the `grant_type` parameter
    [redirectUri:League\OAuth2\Server\Exception\OAuthServerException:private] => 
    [payload:League\OAuth2\Server\Exception\OAuthServerException:private] => Array
        (
            [error] => unsupported_grant_type
            [message] => The authorization grant type is not supported by the authorization server.
            [hint] => Check the `grant_type` parameter
        )

I tracked this down to this really odd and awkward code in your custom copy of AbstractProvider:

if(!empty($this->code) && empty($this->accessToken)){
$response = $this->getHttpClient()->request('POST', $this->tokenEndpoint.'?grant_type=authorization_code&code='.$this->code.
'&redirect_uri='.$this->redirectUri.
'&client_id='.$this->clientId.
'&client_secret='.$this->clientSecret);
}elseif(!empty($this->accessToken)){
$response = $this->getHttpClient()->request('POST', $this->userEndpoint.'?client_id='.$this->clientId.
'&client_secret='.$this->clientSecret.
'&access_token='.$this->accessToken);
}else{
$response = $this->getHttpClient()->send($request);
}

It was introduced in commit e4935f4#diff-62e2083e6ca37dcc429625406b8ee3abR632

It turns a POST request to a POST request with some parameters encoded in the GET parameters. According to League OAuth2 Server / https://www.php-fig.org/psr/psr-7/ / https://github.com/thephpleague/oauth2-server/blob/9fc288ce53bbd198ceb7be604ca00dc3910a7d82/src/Grant/AbstractGrant.php#L488-L496

The POST Request must have the parameters encoded in the POST Body. Thus you are sending non-compliant OAuth2 request and the SSO Login of openITCOCKPIT fails, with a 500 server error in the OAuth2 Server.

openITCOCKPIT only displays

Can not get token. Server error: `POST https://sso.example.org/module.php/oauth2/access_token.php?grant_type=authorization_code&... (truncated...)
Please perform log off from SSO Server. And then retry to login.

After I fixed that, I am now stuck at the next error: Email address not found.... still investigating this one.

@nook24
Copy link
Member

nook24 commented Apr 12, 2018

Hi @SimonBin,
the login code needs a bit love and refactoring. We have an issue in our backlog, which is addressing this issue.
I guess for now, you should go with LDAP, and wait until we have some time to refactor SSO.

@SimonBin
Copy link
Author

hi @nook24 , thanks for answer. If I fix this part of the code the SSO seems to work. The attribute mapping might need to be more flexible though.

here is the patch I made to oauth2client so it works with our install of simplesamlphp:

mail is an array of mails ->
optionally forward other attributes returned by simpleSamlPHP in the attributes dictionary ->
(the exact data you get depends on the oauth endpoint... I would prefer to match by useraccount instead of mail, since mail can be changed in our ldap)

diff --git a/app/Model/Oauth2client.php b/app/Model/Oauth2client.php
index 2dcde9e..7d96dfd 100644
--- a/app/Model/Oauth2client.php
+++ b/app/Model/Oauth2client.php
@@ -75,7 +75,7 @@ class Oauth2client
                 return ['success' => false, 'message' => 'Can not get user data: '.(ENVIRONMENT === 'production' ? '' : $userDataArr[1])];
             }
             $userArray = $userDataArr[1]->toArray();
-            return ['success' => true, 'email' => $userArray['mail']];
+            return ['success' => true, 'email' => $userArray['mail'][0], 'attributes' => $userArray['attributes']];
         }
     }
 

and here the main fix, as I said above I have no idea what the code is supposed to do but simply commenting the offending rewrite it works:

diff --git a/app/Vendor/Oauth2/league/oauth2-client/src/Provider/AbstractProvider.php b/app/Vendor/Oauth2/league/oauth2-client/src/Provider/AbstractProvider.php
index 27d8c87..b3181fd 100644
--- a/app/Vendor/Oauth2/league/oauth2-client/src/Provider/AbstractProvider.php
+++ b/app/Vendor/Oauth2/league/oauth2-client/src/Provider/AbstractProvider.php
@@ -632,18 +632,18 @@ abstract class AbstractProvider
     protected function sendRequest(RequestInterface $request)
     {
         try {
-            if(!empty($this->code) && empty($this->accessToken)){
-                $response = $this->getHttpClient()->request('POST', $this->tokenEndpoint.'?grant_type=authorization_code&code='.$this->code.
-                    '&redirect_uri='.$this->redirectUri.
-                    '&client_id='.$this->clientId.
-                    '&client_secret='.$this->clientSecret);
-            }elseif(!empty($this->accessToken)){
-                $response = $this->getHttpClient()->request('POST', $this->userEndpoint.'?client_id='.$this->clientId.
-                    '&client_secret='.$this->clientSecret.
-                    '&access_token='.$this->accessToken);
-            }else{
+#            if(!empty($this->code) && empty($this->accessToken)){
+#                $response = $this->getHttpClient()->request('POST', $this->tokenEndpoint.'?grant_type=authorization_code&code='.$this->code.
+#                    '&redirect_uri='.$this->redirectUri.
+#                    '&client_id='.$this->clientId.
+#                    '&client_secret='.$this->clientSecret);
+#            }elseif(!empty($this->accessToken)){
+#                $response = $this->getHttpClient()->request('POST', $this->userEndpoint.'?client_id='.$this->clientId.
+#                    '&client_secret='.$this->clientSecret.
+#                    '&access_token='.$this->accessToken);
+#            }else{
                 $response = $this->getHttpClient()->send($request);
-            }
+#            }
         } catch (BadResponseException $e) {
             $errorMessage = str_replace($this->clientId, '***', $e->getMessage());
             $errorMessage = str_replace($this->clientSecret, '***', $errorMessage);

@nook24
Copy link
Member

nook24 commented Nov 24, 2020

Hi @SimonBin,
the SSO implementation got rewritten from scratch with openITCOCKPIT 4. Had you already a chance to test this out?

@nook24 nook24 closed this as completed May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants