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

CODENVY-1758; add OAuth2 authenticator for Linkedin #1791

Merged
merged 9 commits into from
Feb 22, 2017
Merged

Conversation

mshaposhnik
Copy link
Contributor

@mshaposhnik mshaposhnik commented Feb 16, 2017

What does this PR do?

Added OAuth2 authentication via LinkedIn. Configured via codenvy.env file with following variables:

CODENVY_LINKEDIN_CLIENT_ID=your_linkedin_client_id
CODENVY_LINKEDIN_SECRET=your_linkedin_secret

What issues does this PR fix or reference?

#1758

Changelog

Added oAuth2 authentication via LinkedIn.

Release Notes

We have added a new oAuth provider to Codenvy. You can now use your LinkedIn credentials to sign up or sign in.

Docs PR

codenvy/docs#81

@skabashnyuk
Copy link
Contributor

LinkedInOauthAuthenticator -> LinkedInOAuthAuthenticator

final String query = String.format("format=json");
JsonValue userValue = doRequest(new URL(userUri + ":" + values + "?" + query), params);
User user = new LinkedInUser();
user.setEmail(userValue.getElement("emailAddress").getStringValue());
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe you should check existance userValue.getElement("emailAddress") of this element?

@skabashnyuk skabashnyuk added this to the 5.4.0 milestone Feb 16, 2017
@mshaposhnik mshaposhnik changed the title CODENVY-1758; add OAuth2 authenticator for linkedin CODENVY-1758; add OAuth2 authenticator for Linkedin Feb 16, 2017
Copy link

@vkuznyetsov vkuznyetsov left a comment

Choose a reason for hiding this comment

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

icons for linkedin oauth btn will be added in https://github.com/codenvy/enterprise/issues/48

@skabashnyuk
Copy link
Contributor

@codenvy/pm please approve

@TylerJewell
Copy link
Contributor

Release notes need to be revisited. The changelog is ok but the release notes are meant for marketing. So we should talk about how to configure it within configuration, are there any logos that appear, and whether there is a difference between onprem and saas.

@bmicklea
Copy link
Contributor

To be clear:
@mshaposhnik:

  • Will this be included in SaaS and On-Prem assemblies by default?
  • Can it be turned on/off in configuration?
  • Provide a screenshot of the new signup/login screen with the LinkedIn logo for oAuth

PM will take this information and update the release note section with it.

@mshaposhnik
Copy link
Contributor Author

mshaposhnik commented Feb 21, 2017

@bmicklea
1 - yes,
2 - yes it can be switched off like any other OAuth provider - by setting CODENVY_LINKEDIN_CLIENT_ID and CODENVY_LINKEDIN_CLIENT_ID into NULL.
3 -
linked_out

@JamesDrummond
Copy link

JamesDrummond commented Feb 21, 2017

This will need docs. Not sure if I like the look and it doesn't include the other oauth links. I would think round corners like the google oauth icon below and also its not the same size/height of other information on the row. WDYT @bmicklea .

clipboard

Update: Sorry I didn't look at saas(customers) or onprem. Looks like @mshaposhnik image above.

Copy link
Contributor

@bmicklea bmicklea left a comment

Choose a reason for hiding this comment

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

You can merge

@bmicklea
Copy link
Contributor

@mshaposhnik where are those variables found (and it looks like you listed the same one twice, are there two?)

I don't see them in the codenvy.env file.

Copy link

@JamesDrummond JamesDrummond left a comment

Choose a reason for hiding this comment

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

Documentation updated.

@JamesDrummond
Copy link

@bmicklea Think @mshaposhnik meant CODENVY_LINKEDIN_SECRET for the second variable. I updated the description above a little while ago.

@bmicklea
Copy link
Contributor

Thanks, I still don't see them in the codenvy.env file so where are they changed?

@skabashnyuk
Copy link
Contributor

skabashnyuk commented Feb 21, 2017

@bmicklea
Copy link
Contributor

Thanks Sergii. I don't think we usually want people messing with the codenvy.pp so I'll skip that in the release notes. This should be on by default for Codenvy.io SaaS though.

@JamesDrummond
Copy link

@bmicklea You are looking at the wrong file on the link. The link https://github.com/codenvy/codenvy/pull/1791/files#diff-4e1211b38190902dee42a5833ea2efb5R466 highlights code for codenvy.env not codenvy.pp even though that is the filename you see at first when you first click the link. Line 466 of codenvy.env is highlighted.

@mshaposhnik mshaposhnik merged commit d353b03 into master Feb 22, 2017
@mshaposhnik mshaposhnik deleted the 1758 branch February 22, 2017 07:54
akorneta pushed a commit to sleshchenko/codenvy that referenced this pull request Feb 23, 2017
@JamesDrummond JamesDrummond mentioned this pull request Mar 8, 2017
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants