Support for sign-in & sign-up via OAuth resource owner #354
Conversation
| @@ -5,17 +5,17 @@ | |||
| <h1>Login <small>Sylius store</small></h1> | |||
| </div> | |||
|
|
|||
| {% if error %} | |||
| <div class="alert alert-error"> | |||
| {{ error }} | |||
There was a problem hiding this comment.
This message should be translated, right?
|
@headrevision @stloyd is there a reason for not having added Facebook in this PR? (different mechanism?) |
|
@winzou No, there is no difference. I simply was not interested in adding Facebook. |
|
OK, this is great. If you or @stloyd want to add it, it would be even greater :p There is an error on the build: https://travis-ci.org/stloyd/Sylius/jobs/11815773#L735 Composer is weirdly not installing OAuthBundle. |
|
@stloyd I think it would be super-awesome to have Google, Facebook, Amazon initially, possible? :P (with my help) |
|
@stloyd Can we help you with this somehow? :) |
|
@pjedrzejewski I guess that's is good, maybe just some behat test would require some polishing/extending, but I don't have time for this right now. |
|
@stloyd Let's face it: The Behat scenarios for the OAuth sign-in/-up won't never run successfully. First they come with an extraordinary complexity (third-party websites, switching back and forth to Sylius, SSL, Javascript, etc.). Second (and much worse) they are critical to security (resource owner setup, actual user account for each resource owner). The latter makes it virtually impossible to have a completely pre-configured reference implementation for testing. Even Travis CI's secure environment variables are not a solution. The credentials would remain insecure in respect of account hijacking and Google, Facebook, Amazon, etc. don't offer user accounts (+ terms of use) just for testing. The only solution is to use your own OAuth server such that you could at least test a reference implementation for that "dummy" resource owner. |
|
Yeah, that was my concern too... thanks for clarifying that @headrevision, I never worked with OAuth through Behat. What's our best option here? I think removing these scenarios is acceptable. |
…ign-in provider account (incomplete) added step to initially grant access for app in OAuth login scenarios disabled cache clearing removed obsolete code restored cache clearing added given step to OAuth login scenarios set placeholder values for resource owner client id + secret execute scenarios with saucelabs on travic ci apache added phpspec for oauth user provider used local selenium server for travis ci instead of sauce labs
Add missing facebook logo, rework some html stuff with new design
… successfully with pre-configured resource owner
| @@ -0,0 +1,5 @@ | |||
| {% for owner in hwi_oauth_resource_owners() %} | |||
| <a class="oauth-login oauth-login-{{ owner|trans({}, 'HWIOAuthBundle') }}" href="{{ hwi_oauth_login_url(owner) }}" title="{{ owner|trans({}, 'HWIOAuthBundle') }}"> | |||
| <img src="/assets/img/icon/64/{{ owner|trans({}, 'HWIOAuthBundle') }}.png" alt="{{ owner|trans({}, 'HWIOAuthBundle') }}"> | |||
There was a problem hiding this comment.
This img should be in WebBundle/assets I think.
Support for sign-in & sign-up via OAuth resource owner
|
Good step forward, thanks both Józef and Fabian! |
|
Cool, I always wondered how others integrated this bundle. Seems I wasn't too wrong :D |
|
|
||
| // set default values taken from OAuth sign-in provider account | ||
| if (null !== $email = $response->getEmail()) { | ||
| $user->setEmail($email); |
There was a problem hiding this comment.
isn't this insecure? after OAuth authentication my email could be anyone
There was a problem hiding this comment.
I guess, yes & no =)
Yes - because in theory it's possible,
No - because most of oauth providers have built-in email verification systems, so you can't have email you want, but only that one you can confirm.
@inmarelibero Can you open new issue, so we could disscuss more clearly? Thanks!
[Locale] Documentation of Locale component
Rebased version of #163.