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 does not work with OwnCloud #17

Open
12delta opened this issue Aug 26, 2019 · 2 comments
Open

OAuth2 does not work with OwnCloud #17

12delta opened this issue Aug 26, 2019 · 2 comments

Comments

@12delta
Copy link

12delta commented Aug 26, 2019

Actual behaviour

It is not possible to connect the OAuth2 Client Plugin to the OwnCloud OAuth2 Provider App. You get an External Authentication Error. Problem is that the OwnCloud expect the Client ID an Client Secret as Basic Authentication, but Kanboard send it as client_id and client_secret parameter.

Expected behaviour

It should work.

Steps to reproduce

  • Install the Kanboard OAuth2 Plugin
  • Install the OwnCloud OAuth2 App
  • Enable URL ReWrite (veriy importent)
  • Request Client credentials from OwnCloud by giving the Callback https://kanboard.local/oauth/callback
  • Settings in Kanboard:
    • Client ID and Client Secret from OwnCloud
    • Authorize URL: https://owncloud.local/index.php/apps/oauth2/authorize
    • Token URL: https://owncloud.local/index.php/apps/oauth2/api/v1/token
    • User API URL: https:///owncloud.local/index.php/apps/oauth2/api/v1/userinfo
    • Username Key: sub
    • Name Key: name
    • Email Key: email
    • User ID Key: sub
    • Allow Account Creation: Yes
  • Login with OAuth2 login. Account is autocreated, if it would work. (see workaround by code patch)

Workaround

On fetching the Auth2 Token OwnCloud response with an error.

https://github.com/owncloud/oauth2/blob/fc47f947de78e7180f3c73455159683fb667dc89/lib/Controller/OAuthApiController.php#L114

Just patch https://github.com/kanboard/kanboard/blob/8cee04101d351fb5321f225963d589883761d214/app/Core/Http/OAuth2.php#L116 with this:

    public function getAccessToken($code)
    {
        if (empty($this->accessToken) && ! empty($code)) {
            $params = array(
                'code' => $code,
                'client_id' => $this->clientId,
                'client_secret' => $this->secret,
                'redirect_uri' => $this->callbackUrl,
                'grant_type' => 'authorization_code',
                'state' => $this->getState(),
            );

            $authBasic = 'Authorization: Basic ' . base64_encode($this->clientId . ':' . $this->secret);
            $response = json_decode($this->httpClient->postForm($this->tokenUrl, $params, array('Accept: application/json', $authBasic)), true);

            $this->tokenType = isset($response['token_type']) ? $response['token_type'] : '';
            $this->accessToken = isset($response['access_token']) ? $response['access_token'] : '';
        }

        return $this->accessToken;
    }

This adds the client_id and client_secret also as basic authentication.

Configuration

  • Kanboard version: 1.2.11
  • Database type and version: MySQL
  • PHP version: 7.2
  • OS: Linux
  • Browser: Firefox 68
@fguillot fguillot transferred this issue from kanboard/kanboard Aug 27, 2019
@suniahk
Copy link

suniahk commented Feb 21, 2021

It would probably be better to implement something in the UI to make it selectable, i.e. Basic Authentication vs Request Body. Increasing the size of the request with duplicated information, especially sensitive information, is a bad idea.

That being said, according to the spec on transmitting the client_id and client_secret, it's heavily preferred that you send these using the HTTP Basic Auth method rather than in the request body. Ultimately it's probably a better idea to enforce sending using basic auth since, from what I can tell, most oauth authentication servers already support this anyway.

@12delta
Copy link
Author

12delta commented May 5, 2021

I never saw OAuth2 authentication with a setting for the type like BasicAuth or RequestBody. I don't know, how other software determine the right way.

But I think, this request is only done once for the session initialization. Maybe size doesn't matter so much. According to the security, I wouldn't be concerned to have it twice in the the same request. Usually request bodies are not recorded and Authorization headers are not recorded for obvious reason.

It would make it more friendly, if there wouldn't be a new setting. This is just my suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants