Set Github token through workspace master Rest API. Added a force act…

[sunix]

Set Github token through workspace master Rest API. Added a force activation property variable to register Github Oauth provider even without client id/secret

Signed-off-by: Sun Seng David Tan sutan@redhat.com

What does this PR do?

• add a new REST operation to set a Oauth token to an existing provider
• add a property che.oauth.github.forceactivation to force registration of Github Oauth provider, even without client id/secret (actually set NULL string for these value)

What issues does this PR fix or reference?

https://issues.jboss.org/browse/CHE-151

Changelog

Set Github token through workspace master Rest API. Added a force activation property variable to register Github Oauth provider even without client id/secret

Release Notes

Set Github token through workspace master Rest API. Added a force activation property variable to register Github Oauth provider even without client id/secret

Docs PR

[sunix]

will remove produces

[JamesDrummond]

We should add this to git documentation https://github.com/eclipse/che-docs/blob/master/src/main/_docs/use-che-as-an-ide/ide-git-svn.md .

[codenvy-ci]

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2184/

[sunix]

https://youtu.be/5D6XZlDyPKE

[skabashnyuk]

I don't like the idea to change existed OAuthAuthenticationService.java it's potential security hole. Can you do
OpenshiftOAuthTokenSetterService with single method POST and OpenshiftOAuthTokenProvider that connected to OpenshiftOAuthTokenSetterService ?

[codenvy-ci]

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2186/

[sunix]

GET token is a potential security hole, POST isn't. It is not secured to get its token stolen, but setting a token ... I don't see how it can be a security hole

[skabashnyuk]

I don't see it either (now). But my butt told be that this is too risky to have it on non-openshift specific environment. And I would like to do that by moving such code in openshift specific REST service and custom OAuthTokenProvider.

[sunix]

@skabashnyuk sorry I don't get why you think this is risky. Please provide good reasons not to approve it. This is a very generic use case that may be used not only by openshift: a third party application that want to integrate Che and not recreate another clientid/secret to perform another oauth flow. We could even add a UI and let the user set his personal token created in Github.

[l0rd]

@skabashnyuk I don't see why this should be related to OpenShift

[skabashnyuk]

The reason is that the method you want to add is not a part of general OAuth flow and is made only for some specific rare use case.

[skabashnyuk]

BTW how are you going to protect the system from attack of such kind
while(true){
curl post token
}
I think it will take less when a minute before OOM
?

[garagatyi]

I agree with Sergii that this functionality is not part of OAuth flow, so it should not be integrated into OAuth service.
Here is my view on why it doesn't fit:
Our app uses tokens to access Github API.
OAuth specifies how to retrieve these tokens.
Setting token from personal application tokens list/from file/from another source is not part of OAuth flow.
This looks like part of single sign on process, which is not part of OAuth.
So generally I'm +1 in addition possibility to provide API token in a way that is not supposed by OAuth flow. But it should be done separately from our OAuth service.

[garagatyi]

BTW aliases file is for renaming of properties. So you should not modify it when you add new property.

[sunix]

Thanks, will remove it in a fixup

[sunix]

I'm going to create a new PR to master and make few fixup ( the one from @garagatyi , + use createAndStoreCredential method) so we can discuss about that for a merge to master. I'm going to merge that in our branch for the time being as we need this

About the flow. If it's not in the flow, then we should ask Google not make createAndStoreCredential public in their google-oauth-java-client. https://github.com/google/google-oauth-java-client/blob/dev/google-oauth-client/src/main/java/com/google/api/client/auth/oauth2/AuthorizationCodeFlow.java#L221
This is the OAuth flow, but the first part is done by another microservice or process.

I guess if you loop curl post ... it will store in a map in the same entry so no OOM

[sunix]

#4444 for a merge to master, I'm merging this one to openshift-connector

[codenvy-ci]

Build success. https://ci.codenvycorp.com/job/che-pullrequests-build/2192/

[sunix]

@JamesDrummond this will not be part of 5.6.0

[JamesDrummond]

@sunix I have a lot of pr's to go through so I can check to the hour which ones are part of current release. The ones that are merged same day of release I try to get right but again there are many to go through. Be sure to put a milestone label, anytime you merge, to the next release milestone which in this case 5.7.0 .