Adds a new Provider for self-hosted atlassian #35

heiglandreas · 2017-03-27T15:38:44Z

This commit adds a new provider for self-hosted attlasian-instances. It has been tested against a self-hosted confluence version 5.9.8.

Due to missing API-versioning there might be adaptions needed for general usage. Currently I only have this instance available for testing

There currently also are no tests!

In raising this pull request, I confirm the following (please check boxes):

I have read and understood the Contributing Guidelines.
I have checked that another pull request for this purpose does not exist.
I wrote some tests for this PR.

Small description of change:

Thanks 😺

heiglandreas · 2017-03-28T18:56:19Z

Bugs fixed. Though no new tests… As I coudln't find some examples for other OAuth1-Providers I didn't add some…

ovr · 2017-03-31T14:28:27Z

src/OAuth1/Provider/Atlassian.php

+     */
+    public function oauthRequest($uri, $method = Client::GET, $parameters = [])
+    {
+        error_log($uri);


Debug things should be removed

ouch! Right! :(

That method shouldn't be there in the first place as it is obsoleted by #36

ovr · 2017-03-31T14:28:42Z

src/OAuth1/Signature/MethodRSASHA1.php

+
+    public function __construct($private_key)
+    {
+        if (! is_readable($private_key)) {


unexpected spaces

ovr · 2017-03-31T14:28:46Z

src/OAuth1/Signature/MethodRSASHA1.php

+
+class MethodRSASHA1 extends AbstractSignatureMethod
+{
+    private $private_key;


not camelCase

ovr · 2017-04-08T05:44:36Z

src/OAuth1/AbstractProvider.php

+     */
+    protected $signature;
+
+


unneeded blank line

ovr · 2017-04-08T05:44:45Z

src/OAuth1/Provider/Atlassian.php

+<?php
+/**
+ * SocialConnect project
+ * @author: Patsura Dmitry https://github.com/ovr <talk@dmtry.me>


you @author

ovr · 2017-04-08T05:45:05Z

src/OAuth1/Provider/Atlassian.php

+     */
+    public function __construct(ClientInterface $httpClient, SessionInterface $session, Consumer $consumer, array $parameters)
+    {
+        if (! isset($parameters['baseUri'])) {


if (! i -> if (!i - please remove unneeded space

ovr · 2017-04-08T05:45:31Z

src/OAuth1/Provider/Atlassian.php

+
+class Atlassian extends AbstractProvider
+{
+    private $baseUri;


blank line after private $baseUri;
and please doc block that it's a string
Thank

ovr · 2017-04-08T05:45:59Z

src/OAuth1/Provider/Atlassian.php

+            );
+        }
+
+        $headers = $response->getHeader('X-AUSERNAME');


$headers is unused, please remove it, thanks

ovr · 2017-04-08T05:46:51Z

src/OAuth1/Provider/Atlassian.php

+            Client::GET,
+            $parameters
+        );
+        while ($response->hasHeader('Location')) {


plank line before while
and btw, this while can be an infinity loop
if you are sure that attlasin can lag, maybe add retryCount, and retryMax? Thank

ovr · 2017-04-08T05:46:59Z

src/OAuth1/Signature/MethodRSASHA1.php

+<?php
+/**
+ * SocialConnect project
+ * @author: Patsura Dmitry https://github.com/ovr <talk@dmtry.me>


You @author

ovr · 2017-04-08T05:47:16Z

src/OAuth1/Signature/MethodRSASHA1.php

+
+    public function __construct($privateKey)
+    {
+        if (! is_readable($privateKey)) {


if (! is_readable($privateKey)) - please remove uneeded space
Thank

ovr · 2017-04-08T05:47:26Z

src/OAuth1/Signature/MethodRSASHA1.php

+
+class MethodRSASHA1 extends AbstractSignatureMethod
+{
+    private $privateKey;


Docblock that it's string?

ovr · 2017-04-08T05:47:47Z

src/OAuth1/Signature/MethodRSASHA1.php

+            throw new \InvalidArgumentException('The private key is not readable');
+        }
+
+        if (! function_exists('openssl_pkey_get_private')) {


if (! function_exists('openssl_pkey_get_private')) { - please remove unneeded space
Thank

ovr · 2017-04-08T05:48:18Z

tests/Test/OAuth1/Provider/AtlassianTest.php

@@ -0,0 +1,75 @@
+<?php
+/**
+ * Copyright (c) Andreas Heigl<andreas@heigl.org>


It's not a SocialConnect LICENSE header

ovr · 2017-04-08T05:49:17Z

tests/Test/OAuth1/Signature/MethodRSASHA1Test.php

@@ -0,0 +1,83 @@
+<?php
+/**
+ * Copyright (c) Andreas Heigl<andreas@heigl.org>


Again, not SociallConnect LICENSE header

ovr · 2017-04-08T05:49:50Z

@heiglandreas I added small comments, if you can fix this will be cool
and after it this can merged!
Thank

This commit adds a new provider for self-hosted attlasian-instances. It has been tested against a self-hosted confluence version 5.9.8. Due to missing API-versioning there might be adaptions needed for general usage. Currently I only have this instance available for testing Ther ecurrently also are *no* tests!

These mainly include removing whitespace and adding dockblocks

heiglandreas · 2017-04-21T06:04:50Z

All your comments should be addressed by now. Thanks for the feedback!

ADmad · 2017-04-21T06:20:35Z

src/OAuth1/Provider/Atlassian.php

+     *
+     * Required configuration parameters are:
+     *
+     * * base_uri The base URI of your self-hosted atlassian product


In code the key is baseUri not base_uri.

ADmad · 2017-04-21T06:24:06Z

src/OAuth1/Provider/Atlassian.php

+     * Required configuration parameters are:
+     *
+     * * base_uri The base URI of your self-hosted atlassian product
+     * * private_key The path to the private key file used for signing.


Is this correct? A quick search doesn't show any use of $parameters['private_key'] in the code.

Thanks for spotting!!! I'll check that!!

This commit fixes the docblock describing the config-parameters

ovr · 2017-04-24T23:47:27Z

BIG Thank @heiglandreas Awesome! Take it 🍺
Thank @ADmad for review

ovr reviewed Mar 31, 2017

View reviewed changes

ovr reviewed Apr 8, 2017

View reviewed changes

src/OAuth1/AbstractProvider.php Outdated

*/

protected $signature;

Copy link

Member

ovr Apr 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unneeded blank line

ovr reviewed Apr 8, 2017

View reviewed changes

heiglandreas added 6 commits April 21, 2017 07:49

Fixes a PSR-2 violation

82d04c8

Fixes camelCase and obsolete parameter

c1f8328

Adds Test for RSA-Signature method

49864fb

Adds tests for atlassian-provider

62212c5

fixes CS issues

544bf4f

heiglandreas force-pushed the feature/addAtlassianProvider branch from 8f925bd to 46d77af Compare April 21, 2017 06:02

Fixes issues found during code review

f50b9a7

These mainly include removing whitespace and adding dockblocks

heiglandreas force-pushed the feature/addAtlassianProvider branch from 46d77af to f50b9a7 Compare April 21, 2017 06:03

ADmad requested changes Apr 21, 2017

View reviewed changes

Fixes docblock

f68d863

This commit fixes the docblock describing the config-parameters

ovr merged commit 1f0db0f into SocialConnect:master Apr 24, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds a new Provider for self-hosted atlassian #35

Adds a new Provider for self-hosted atlassian #35

heiglandreas commented Mar 27, 2017 •

edited

Loading

heiglandreas commented Mar 28, 2017

ovr Mar 31, 2017

heiglandreas Mar 31, 2017

heiglandreas Mar 31, 2017

ovr Mar 31, 2017

heiglandreas Mar 31, 2017

ovr Mar 31, 2017

heiglandreas Mar 31, 2017 •

edited

Loading

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr Apr 8, 2017

ovr commented Apr 8, 2017 •

edited

Loading

heiglandreas commented Apr 21, 2017

ADmad Apr 21, 2017

ADmad Apr 21, 2017

heiglandreas Apr 21, 2017

ovr commented Apr 24, 2017

Adds a new Provider for self-hosted atlassian #35

Adds a new Provider for self-hosted atlassian #35

Conversation

heiglandreas commented Mar 27, 2017 • edited Loading

heiglandreas commented Mar 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heiglandreas Mar 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ovr commented Apr 8, 2017 • edited Loading

heiglandreas commented Apr 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ovr commented Apr 24, 2017

heiglandreas commented Mar 27, 2017 •

edited

Loading

heiglandreas Mar 31, 2017 •

edited

Loading

ovr commented Apr 8, 2017 •

edited

Loading