Add support of BasicAuthentication Authentication to Git #1940

i053322 · 2016-07-26T07:49:27Z

What does this PR do?

Add support of Basic Authenticationt git.
It will support in operation of import,clone,fetch,rebase,push,pull.
The user will provide the user and password for the request and it will create credentials provider

What issues does this PR fix or reference?

Add support to clone and work with repository that support basic authentication

Previous Behavior

Remove this section if not relevant
It was not possible to work with repository that need basic Authentication

New Behavior

This content may also be included in the release notes.

Tests written?

Yes/No
It exist for our scenario

Docs requirements?

Include the content for all the docs changes required.

API changes
User docs changes

Please review Che's Contributing Guide for best practices.

Signed-off-by: i053322 [email protected]

codenvy-ci · 2016-07-26T07:54:02Z

Can one of the admins verify this patch?

skabashnyuk · 2016-07-26T07:59:19Z

Where this user and password is come from. How they are stored?

i053322 · 2016-07-26T08:02:45Z

It should come from the client. for example in the POST call it will be added to the oData in format of for example
"source": {
"runners": {},
"project": {
"location": sGitUrl,
"type": "git-ba",
"parameters": {
"userName": sGitSshUsername,
"password": sGitSshPassword
}
}
}

codenvy-ci · 2016-08-05T13:14:01Z

Can one of the admins verify this patch?

skabashnyuk · 2016-08-15T06:39:22Z

ok. @vinokurig ?

vinokurig · 2016-08-15T06:49:26Z

wsagent/che-core-api-git/src/main/java/org/eclipse/che/api/git/GitProjectImporter.java

+                final String pass = parameters.get("password");
+                if (user != null && pass != null) {
+                    setCredentials = true;
+                    GitBasicAuthenticationCredentialsProvider.setCurrentCredentials(user, pass);


Please add static import

i053322 · 2016-08-15T10:31:23Z

@vinokurig - I fixed the comment

vinokurig · 2016-08-15T11:21:03Z

...api-git/src/main/java/org/eclipse/che/api/git/GitBasicAuthenticationCredentialsProvider.java

+    private static final String GIT_PROJECT_ID = "git-project-id";
+
+    @Override
+    public UserCredential getUserCredential() throws GitException {


This method never throws GitException, I think you should remove this 'throws'

@vinokurig - Its is how the interfce CredentialsProvider
UserCredential getUserCredential() throws GitException;
also GitHubOAuthCredentialProvider has the same logic

Yes, but you are not obligated to add throws GitException in overridden method.
In this case you will get rid of of try-catch in canProvideCredentials() method

vinokurig · 2016-08-15T13:34:28Z

...api-git/src/main/java/org/eclipse/che/api/git/GitBasicAuthenticationCredentialsProvider.java

+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *   Codenvy, S.A. - API


Wrong license: * Codenvy, S.A. - initial API and implementation

vinokurig · 2016-08-15T13:36:03Z

Please fix JGitConnectionTest according to your changes

i053322 · 2016-08-16T06:11:59Z

@vinokurig - Fixed the comment

vinokurig · 2016-08-16T06:49:55Z

Please remove new line (1593) in JGitConnection other ok for me

i053322 · 2016-08-16T12:20:55Z

@vinokurig - fixed the last issue

vinokurig · 2016-08-16T13:06:18Z

ok for me

sleshchenko · 2016-08-16T13:09:19Z

...api-git/src/main/java/org/eclipse/che/api/git/GitBasicAuthenticationCredentialsProvider.java

+
+    private static final Logger LOG = LoggerFactory.getLogger(GitBasicAuthenticationCredentialsProvider.class);
+    private static ThreadLocal<UserCredential> currRequestCredentials = new ThreadLocal<>();
+    private static final String GIT_PROJECT_ID = "git-project-id";


Are you sure that git-project-id is good name for CredentialsProvider? Mb git-basic or just basic would be better?

I will fix it

sleshchenko · 2016-08-18T07:14:01Z

wsagent/che-core-api-git-shared/src/main/java/org/eclipse/che/api/git/shared/ProviderInfo.java

+    public ProviderInfo(@NotNull String providerName) {
+        info.put(PROVIDER_NAME, providerName);
+    }
+


also please fix java doc of getAuthenticateUrl method and describe that it can return null value and add org.eclipse.che.commons.annotation.Nullable for it

i053322 · 2016-08-18T12:07:09Z

@sleshchenko - fixed the comments

sleshchenko · 2016-08-18T14:56:00Z

@i053322 What about #1940 (comment) ?

sleshchenko · 2016-08-18T15:00:19Z

wsagent/che-core-api-git-shared/src/main/java/org/eclipse/che/api/git/shared/ProviderInfo.java

-        return info.get(AUTHENTICATE_URL);
-    }
+    /**
+     * @return authenticate URL. It could retrun null value.


typo retrun

i053322 · 2016-08-21T06:41:20Z

@sleshchenko - fixed the comments

sleshchenko · 2016-08-22T07:19:30Z

...api-git/src/main/java/org/eclipse/che/api/git/GitBasicAuthenticationCredentialsProvider.java

+    @Override
+    public UserCredential getUserCredential() {
+        UserCredential credentials = currRequestCredentials.get();
+        return credentials;


just return currRequestCredentials.get(); ?

sleshchenko · 2016-08-22T07:19:43Z

LGTM

skabashnyuk · 2016-08-22T08:22:04Z

@vinokurig wdyt?

i053322 · 2016-08-23T05:44:36Z

@sleshchenko - fixed the last comment

vinokurig · 2016-08-23T07:04:54Z

Ok for me

skabashnyuk · 2016-08-23T07:06:22Z

@vinokurig please continue merging this pr in appropriate moment of our release cycle

i053322 · 2016-09-04T19:12:42Z

@skabashnyuk - can I create PR for 3.x ?

vinokurig · 2016-09-05T07:30:03Z

@i053322 Yes, you can create a PR to 3.x. We have merged big changes to 4.x and it became unstable. This PR will be merged when 4.x master will become stable.

vinokurig · 2016-09-05T12:05:01Z

...api-git/src/main/java/org/eclipse/che/api/git/GitBasicAuthenticationCredentialsProvider.java

+@Singleton
+public class GitBasicAuthenticationCredentialsProvider implements CredentialsProvider {
+
+    private static final Logger LOG = LoggerFactory.getLogger(GitBasicAuthenticationCredentialsProvider.class);


this is unused here, please remove it

Signed-off-by: i053322 <[email protected]>

…#1940) Signed-off-by: i053322 <[email protected]>

i053322 force-pushed the add_basic_Authentication_to_git branch from 0b9f6d3 to 34eeb7d Compare August 14, 2016 06:37

vinokurig reviewed Aug 15, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from 34eeb7d to 4c52b09 Compare August 15, 2016 10:30

vinokurig reviewed Aug 15, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from 4c52b09 to c4735b2 Compare August 15, 2016 11:53

vinokurig reviewed Aug 15, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from c4735b2 to e769fb8 Compare August 16, 2016 06:11

i053322 force-pushed the add_basic_Authentication_to_git branch from e769fb8 to 5ab7896 Compare August 16, 2016 08:55

sleshchenko reviewed Aug 16, 2016
View reviewed changes

sleshchenko reviewed Aug 18, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from da69e82 to 2db28d8 Compare August 18, 2016 12:06

sleshchenko reviewed Aug 18, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from 2db28d8 to 20ffbb0 Compare August 21, 2016 06:41

sleshchenko reviewed Aug 22, 2016
View reviewed changes

i053322 force-pushed the add_basic_Authentication_to_git branch from 20ffbb0 to b3e54e0 Compare August 23, 2016 05:43

bmicklea added the sprint/next label Aug 23, 2016

vinokurig reviewed Sep 5, 2016
View reviewed changes

Add support of BasicAuthentication Authentication to Git

2047a5b

Signed-off-by: i053322 <[email protected]>

i053322 force-pushed the add_basic_Authentication_to_git branch from b3e54e0 to 2047a5b Compare September 5, 2016 12:16

vkuznyetsov added this to the 5.0.0-M2 milestone Sep 14, 2016

vkuznyetsov added the status/pending-merge label Sep 14, 2016

riuvshin modified the milestones: 5.0.0-M3, 5.0.0-M2 Sep 20, 2016

vinokurig mentioned this pull request Sep 22, 2016

Add dependencie #2553

Merged

vinokurig merged commit 4595c9d into eclipse-che:master Sep 22, 2016

bmicklea removed the sprint/next label Sep 28, 2016

bmicklea mentioned this pull request Sep 28, 2016

5.0.0-M3 Milestone Overview #2641

Closed

63 tasks

JPinkney pushed a commit to JPinkney/che that referenced this pull request Aug 17, 2017

Add support of BasicAuthentication Authentication to Git (eclipse-che…

363ea44

…#1940) Signed-off-by: i053322 <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support of BasicAuthentication Authentication to Git #1940

Add support of BasicAuthentication Authentication to Git #1940

i053322 commented Jul 26, 2016

codenvy-ci commented Jul 26, 2016

skabashnyuk commented Jul 26, 2016

i053322 commented Jul 26, 2016

codenvy-ci commented Aug 5, 2016

skabashnyuk commented Aug 15, 2016

vinokurig Aug 15, 2016

i053322 commented Aug 15, 2016

vinokurig Aug 15, 2016

i053322 Aug 15, 2016 •

edited

Loading

vinokurig Aug 15, 2016

vinokurig Aug 15, 2016

vinokurig commented Aug 15, 2016

i053322 commented Aug 16, 2016

vinokurig commented Aug 16, 2016

i053322 commented Aug 16, 2016

vinokurig commented Aug 16, 2016

sleshchenko Aug 16, 2016

i053322 Aug 16, 2016

sleshchenko Aug 18, 2016

i053322 commented Aug 18, 2016

sleshchenko commented Aug 18, 2016

sleshchenko Aug 18, 2016 •

edited

Loading

i053322 commented Aug 21, 2016

sleshchenko Aug 22, 2016

sleshchenko commented Aug 22, 2016

skabashnyuk commented Aug 22, 2016

i053322 commented Aug 23, 2016

vinokurig commented Aug 23, 2016

skabashnyuk commented Aug 23, 2016

i053322 commented Sep 4, 2016

vinokurig commented Sep 5, 2016

vinokurig Sep 5, 2016

Add support of BasicAuthentication Authentication to Git #1940

Add support of BasicAuthentication Authentication to Git #1940

Conversation

i053322 commented Jul 26, 2016

What does this PR do?

What issues does this PR fix or reference?

Previous Behavior

New Behavior

Tests written?

Docs requirements?

codenvy-ci commented Jul 26, 2016

skabashnyuk commented Jul 26, 2016

i053322 commented Jul 26, 2016

codenvy-ci commented Aug 5, 2016

skabashnyuk commented Aug 15, 2016

Choose a reason for hiding this comment

i053322 commented Aug 15, 2016

Choose a reason for hiding this comment

i053322 Aug 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinokurig commented Aug 15, 2016

i053322 commented Aug 16, 2016

vinokurig commented Aug 16, 2016

i053322 commented Aug 16, 2016

vinokurig commented Aug 16, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

i053322 commented Aug 18, 2016

sleshchenko commented Aug 18, 2016

sleshchenko Aug 18, 2016 • edited Loading

Choose a reason for hiding this comment

i053322 commented Aug 21, 2016

Choose a reason for hiding this comment

sleshchenko commented Aug 22, 2016

skabashnyuk commented Aug 22, 2016

i053322 commented Aug 23, 2016

vinokurig commented Aug 23, 2016

skabashnyuk commented Aug 23, 2016

i053322 commented Sep 4, 2016

vinokurig commented Sep 5, 2016

Choose a reason for hiding this comment

i053322 Aug 15, 2016 •

edited

Loading

sleshchenko Aug 18, 2016 •

edited

Loading