-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Add social sign in functionality (Google, Facebook, Twitter) #2155
Conversation
… into feature/social # Conflicts: # app/index.js # app/templates/_pom.xml
return 'signin/' + provider; | ||
}; | ||
|
||
socialService.getCSRF = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if it's a good way to get the CRSF to put in the form?
<%_ } _%> | ||
"font-awesome": "4.4.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding font-awesome is probably going to add a lot of resources, at the moment we only use "normal" bootstrap -> the idea is that we provide something "light" by default, but it's easy for people to extend (just do a Bower install). Can you remove this, and add the Bootstrap class instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, two options:
- I remove totally the font-awesome lib, and the icon of the social button too (because icon like Twitter, Facebook dont exist in bootstrap icon package)
- I add font awesome only when social is enabled
Wich one you prefer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally we use icons from Bootstrap, have a look at http://getbootstrap.com/components/
- can you find something good enough?
- anyway there might be copyright issues with using brand logos, I'd rather not go there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No there are no brand icon on bootstrap icon set :(. I will remove font-awesome, and add to the documentation how to add the icon on the button.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jdubois I remove the font-awesome, but there is a problem, the social button is not compatible without icon. I can just create a simple bootstrap button, but we loose the user experience. On all the website it's the same type of button (icon and color).
My opinion it's keep font-awesome, I think the ratio weight/utility is good, like you said it's add more ressource but allow user to use more icons and some special effect like loading icon. I prefer to keep it for a better user experience.
But if you prefer to not use it I can just create a simple button with the label.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 for Font Awesome, even if it's only included when someone wants social
authentication. I use it on all my client projects.
On Sat, Oct 17, 2015 at 07:38 Thibaut Mottet notifications@github.com
wrote:
In app/templates/_bower.json
#2155 (comment)
:<%_ } _%>
"font-awesome": "4.4.0",
@jdubois https://github.com/jdubois I remove the font-awesome, but
there is a problem, the social button is not compatible without icon. I can
just create a simple bootstrap button, but we loose the user experience. On
all the website it's the same type of button (icon and color).My opinion it's keep font-awesome, I think the ratio weight/utility is
good, like you said it's add more ressource but allow user to use more
icons and some special effect like loading icon. I prefer to keep it for a
better user experience.But if you prefer to not use it I can just create a simple button with the
label.—
Reply to this email directly or view it on GitHub
https://github.com/jhipster/generator-jhipster/pull/2155/files#r42309339
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it's good to use Font Awesome, that's what I use for the main JHipster website :-)
But here I don't want to add lots of CSS/images for everyone -> it should be easy to add, but not here by default
@moifort could you just use a String? Or just some specific images?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -173,6 +173,36 @@ | |||
constraintName="fk_evt_pers_audit_evt_data" | |||
referencedColumnNames="event_id" | |||
referencedTableName="jhi_persistent_audit_event"/> | |||
<% if (enableSocialSignIn) { %> | |||
<!-- Social --> | |||
<createTable tableName="jhi_UserConnection"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all our table names are now lower case, can you write this in lower case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw, where is this table configured?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I use the default one: https://github.com/spring-projects/spring-social/tree/master/spring-social-core/src/main/java/org/springframework/social/connect/jdbc. I am working to make compatible with the naming. I rewrite the class JdbcConnectionRepository.java
and JdbcUsersConnectionRepository.java
using spring Data JPA.
…into feature/social
… into feature/social # Conflicts: # app/index.js
Oups sorry I was merging this and I see you didn't put the table names in lower case:
|
@jdubois yes yes! not yet! sorry I have not finish, I change the code to make it compatible in lower case and there are some work on it. And the sign in didn't work in OAuth2 :(. |
If OAuth2 doesn't work just exclude it with some if...then -> I want a first working version, not a perfect version! |
… into feature/social # Conflicts: # app/index.js # app/templates/_pom.xml
- add package-info - add cache - add repository + domain - only available for session authentification
… into feature/social
@jdubois I finish the compatibility and add some little things like:
|
@@ -0,0 +1,116 @@ | |||
package <%=packageName%>.social.config; | |||
|
|||
import com.mycompany.myapp.social.repository.SocialUserConnectionRepository; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You need to use the package name here
@moifort I will probably rework the generator questions. We could have only one question for authentication:
And I would keep the current variables, as it's easier to use. |
@jdubois If you want me to do it? |
If the time at midday yes! Otherwise I think I'm gonna merge this after lunch :-) |
@jdubois sorry, I will don't have the time for midday :( |
@moifort no problem, I'll do it :-) |
Add social sign in functionality (Google, Facebook, Twitter)
Merged it! |
This is done on purpose, my idea it's to isolate social like a module (because it's not a really part of the project like user or entities). So if you work on social everything it's on the package (no need to find your class in all the project). That's why I put the Spring config too. If you want to change it, don't forget to remove the annotation |
Yes I understand, but the idea is that all the repos are in the same place, etc (including so that the configurations are all in one place) |
sorry for the refacto :( |
Is there anyway to enable Social sign in with oAuth based authentication. At a minimum to enable easy registration and synching relationships? |
@jvence not yet, this is needs to be coded. This would be another issue. |
@moifort Out of curiosity why doesn't social sign in work with oAuth based authentication? |
I don't know, I don't looked more than that. |
So why is it disabled when you create an oAuth app with jhipster? |
@jvence as I said, this is another issue -> at the moment we only provide the core feature, then we will improve it. |
Guys, I've just started to play with social login with jHipster and after success login in facebook app I retrieve following exception: [ERROR] pl.app.web.rest.SocialController - Exception creating social user: I believe it's related to changes of facebook API several days ago (spring-attic/spring-social-facebook#171) Are those changed already fixed and merged to jHipster repo (and I am doing something wrong) or facebook login simply does not work at the moment (due to bug in spring social)? EDIT: I've checked with some of my older facebook application which is not using latest facebook API and it is working OK, so the problem is definetally not yet solved. |
Generator:
Functionality:
login
andregister
screens.clientId
andclientSecret
keys but only for the test (http://localhost:8080/signin) so you can test directly after generation :).application.yml
to specify the url after sign in.Other:
Add the font-awesome library, more icons (https://fortawesome.github.io/Font-Awesome/icons).capitalize
(ex:{{ twitter | capitalize }}
->Twitter
).To do:
EDIT: I remove the Facebook keys because the Facebook API has been upgraded and spring-social-facebook is not working anymore, but dont worry there is a PR pending on the repository: spring-attic/spring-social-facebook#171
Close #1649