Commit 650d0bc695eb0f874561b8d4ed3fc86510573fba
Exists in
spb-stable
and in
3 other branches
Merge branch 'improve-oauth'
Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> Conflicts: CHANGELOG
Showing
9 changed files
with
35 additions
and
5 deletions
Show diff stats
CHANGELOG
| @@ -10,6 +10,7 @@ v 6.8.0 | @@ -10,6 +10,7 @@ v 6.8.0 | ||
| 10 | - Protected branch does not allow force push | 10 | - Protected branch does not allow force push |
| 11 | - Fix popen bug in `rake gitlab:satellites:create` | 11 | - Fix popen bug in `rake gitlab:satellites:create` |
| 12 | - Disable connection reaping for MySQL | 12 | - Disable connection reaping for MySQL |
| 13 | + - Allow oauth signup without email for twitter and github | ||
| 13 | 14 | ||
| 14 | v 6.7.3 | 15 | v 6.7.3 |
| 15 | - Fix the merge notification email not being sent (Pierre de La Morinerie) | 16 | - Fix the merge notification email not being sent (Pierre de La Morinerie) |
app/controllers/application_controller.rb
| @@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base | @@ -11,6 +11,7 @@ class ApplicationController < ActionController::Base | ||
| 11 | before_filter :default_headers | 11 | before_filter :default_headers |
| 12 | before_filter :add_gon_variables | 12 | before_filter :add_gon_variables |
| 13 | before_filter :configure_permitted_parameters, if: :devise_controller? | 13 | before_filter :configure_permitted_parameters, if: :devise_controller? |
| 14 | + before_filter :require_email, unless: :devise_controller? | ||
| 14 | 15 | ||
| 15 | protect_from_forgery | 16 | protect_from_forgery |
| 16 | 17 | ||
| @@ -234,4 +235,10 @@ class ApplicationController < ActionController::Base | @@ -234,4 +235,10 @@ class ApplicationController < ActionController::Base | ||
| 234 | def hexdigest(string) | 235 | def hexdigest(string) |
| 235 | Digest::SHA1.hexdigest string | 236 | Digest::SHA1.hexdigest string |
| 236 | end | 237 | end |
| 238 | + | ||
| 239 | + def require_email | ||
| 240 | + if current_user && current_user.temp_oauth_email? | ||
| 241 | + redirect_to profile_path, notice: 'Please complete your profile with email address' and return | ||
| 242 | + end | ||
| 243 | + end | ||
| 237 | end | 244 | end |
app/controllers/profiles/emails_controller.rb
| @@ -8,7 +8,7 @@ class Profiles::EmailsController < ApplicationController | @@ -8,7 +8,7 @@ class Profiles::EmailsController < ApplicationController | ||
| 8 | 8 | ||
| 9 | def create | 9 | def create |
| 10 | @email = current_user.emails.new(params[:email]) | 10 | @email = current_user.emails.new(params[:email]) |
| 11 | - | 11 | + |
| 12 | flash[:alert] = @email.errors.full_messages.first unless @email.save | 12 | flash[:alert] = @email.errors.full_messages.first unless @email.save |
| 13 | 13 | ||
| 14 | redirect_to profile_emails_url | 14 | redirect_to profile_emails_url |
app/controllers/profiles_controller.rb
| @@ -3,6 +3,7 @@ class ProfilesController < ApplicationController | @@ -3,6 +3,7 @@ class ProfilesController < ApplicationController | ||
| 3 | 3 | ||
| 4 | before_filter :user | 4 | before_filter :user |
| 5 | before_filter :authorize_change_username!, only: :update_username | 5 | before_filter :authorize_change_username!, only: :update_username |
| 6 | + skip_before_filter :require_email, only: [:show, :update] | ||
| 6 | 7 | ||
| 7 | layout 'profile' | 8 | layout 'profile' |
| 8 | 9 |
app/models/user.rb
| @@ -462,4 +462,12 @@ class User < ActiveRecord::Base | @@ -462,4 +462,12 @@ class User < ActiveRecord::Base | ||
| 462 | def all_ssh_keys | 462 | def all_ssh_keys |
| 463 | keys.map(&:key) | 463 | keys.map(&:key) |
| 464 | end | 464 | end |
| 465 | + | ||
| 466 | + def temp_oauth_email? | ||
| 467 | + email =~ /\Atemp-email-for-oauth/ | ||
| 468 | + end | ||
| 469 | + | ||
| 470 | + def generate_tmp_oauth_email | ||
| 471 | + self.email = "temp-email-for-oauth-#{username}@gitlab.localhost" | ||
| 472 | + end | ||
| 465 | end | 473 | end |
app/views/devise/sessions/_oauth_providers.html.haml
| @@ -9,5 +9,3 @@ | @@ -9,5 +9,3 @@ | ||
| 9 | = link_to authbutton(provider, 32), omniauth_authorize_path(resource_name, provider) | 9 | = link_to authbutton(provider, 32), omniauth_authorize_path(resource_name, provider) |
| 10 | - else | 10 | - else |
| 11 | = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), class: "btn" | 11 | = link_to provider.to_s.titleize, omniauth_authorize_path(resource_name, provider), class: "btn" |
| 12 | - %br | ||
| 13 | - %small * Make sure your email address is public |
app/views/profiles/show.html.haml
| @@ -30,7 +30,10 @@ | @@ -30,7 +30,10 @@ | ||
| 30 | %span.help-block.light | 30 | %span.help-block.light |
| 31 | Email is read-only for LDAP user | 31 | Email is read-only for LDAP user |
| 32 | - else | 32 | - else |
| 33 | - = f.text_field :email, class: "form-control", required: true | 33 | + - if @user.temp_oauth_email? |
| 34 | + = f.text_field :email, class: "form-control", required: true, value: nil | ||
| 35 | + - else | ||
| 36 | + = f.text_field :email, class: "form-control", required: true | ||
| 34 | - if @user.unconfirmed_email.present? | 37 | - if @user.unconfirmed_email.present? |
| 35 | %span.help-block | 38 | %span.help-block |
| 36 | Please click the link in the confirmation email before continuing, it was send to | 39 | Please click the link in the confirmation email before continuing, it was send to |
lib/gitlab/oauth/user.rb
| @@ -29,6 +29,17 @@ module Gitlab | @@ -29,6 +29,17 @@ module Gitlab | ||
| 29 | 29 | ||
| 30 | user = model.build_user(opts, as: :admin) | 30 | user = model.build_user(opts, as: :admin) |
| 31 | user.skip_confirmation! | 31 | user.skip_confirmation! |
| 32 | + | ||
| 33 | + # Services like twitter and github does not return email via oauth | ||
| 34 | + # In this case we generate temporary email and force user to fill it later | ||
| 35 | + if user.email.blank? | ||
| 36 | + user.generate_tmp_oauth_email | ||
| 37 | + else | ||
| 38 | + # Google oauth returns email but dont return nickname | ||
| 39 | + # So we use part of email as username for new user | ||
| 40 | + user.username = email.match(/^[^@]*/)[0] | ||
| 41 | + end | ||
| 42 | + | ||
| 32 | user.save! | 43 | user.save! |
| 33 | log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}" | 44 | log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}" |
| 34 | 45 | ||
| @@ -58,7 +69,7 @@ module Gitlab | @@ -58,7 +69,7 @@ module Gitlab | ||
| 58 | end | 69 | end |
| 59 | 70 | ||
| 60 | def username | 71 | def username |
| 61 | - email.match(/^[^@]*/)[0] | 72 | + auth.info.nickname.to_s.force_encoding("utf-8") |
| 62 | end | 73 | end |
| 63 | 74 | ||
| 64 | def provider | 75 | def provider |
spec/lib/oauth_spec.rb