From f40e87a03bdaaafa72e5afede57581d6c54dc506 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 24 Jun 2014 12:57:41 +0200 Subject: [PATCH 1/4] Return better error when account exists when attempting oauth account create. --- app/controllers/omniauth_callbacks_controller.rb | 13 +++++++++---- lib/gitlab/oauth/user.rb | 8 +++++++- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 0c87fe0d9ae..477743b34c8 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -45,14 +45,19 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController # Create user if does not exist # and allow_single_sign_on is true - if Gitlab.config.omniauth['allow_single_sign_on'] - @user ||= Gitlab::OAuth::User.create(oauth) + if Gitlab.config.omniauth['allow_single_sign_on'] && !@user + @user, errors = Gitlab::OAuth::User.create(oauth) end - if @user + if @user && !errors sign_in_and_redirect(@user) else - flash[:notice] = "There's no such user!" + if errors + error_message = errors.map{ |attribute, message| "#{attribute} #{message}" }.join(", ") + flash[:notice] = "There was a problem creating your account. #{error_message}" + else + flash[:notice] = "There's no such user!" + end redirect_to new_user_session_path end end diff --git a/lib/gitlab/oauth/user.rb b/lib/gitlab/oauth/user.rb index c5be884a895..38e33c0eee5 100644 --- a/lib/gitlab/oauth/user.rb +++ b/lib/gitlab/oauth/user.rb @@ -44,7 +44,13 @@ module Gitlab user.username = email_username.gsub("'", "") end - user.save! + begin + user.save! + rescue ActiveRecord::RecordInvalid => e + log.info "(OAuth) Email #{e.record.errors[:email]}. Username #{e.record.errors[:username]}" + return nil, e.record.errors + end + log.info "(OAuth) Creating user #{email} from login with extern_uid => #{uid}" if Gitlab.config.omniauth['block_auto_created_users'] && !ldap? From 49c9e8ec02e5219cf984da61de52352ec5775c98 Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 24 Jun 2014 14:16:52 +0200 Subject: [PATCH 2/4] Use an error page when oauth fails. --- app/controllers/omniauth_callbacks_controller.rb | 8 +++++++- config/routes.rb | 3 +++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index 477743b34c8..ef2afec52dc 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -31,6 +31,12 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController end end + def omniauth_error + @provider = params[:provider] + @error = params[:error] + render 'errors/omniauth_error', layout: "errors", status: 422 + end + private def handle_omniauth @@ -54,7 +60,7 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController else if errors error_message = errors.map{ |attribute, message| "#{attribute} #{message}" }.join(", ") - flash[:notice] = "There was a problem creating your account. #{error_message}" + redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return else flash[:notice] = "There's no such user!" end diff --git a/config/routes.rb b/config/routes.rb index 5b854ed20b9..14ff52f387a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -160,6 +160,9 @@ Gitlab::Application.routes.draw do devise_for :users, controllers: { omniauth_callbacks: :omniauth_callbacks, registrations: :registrations , passwords: :passwords, sessions: :users_sessions } + devise_scope :user do + get "/users/auth/:provider/omniauth_error" => "omniauth_callbacks#omniauth_error", as: :omniauth_error + end # # Project Area # From 308b11a5b7aca22a35fc9c4b06593be0fca5d6cc Mon Sep 17 00:00:00 2001 From: Marin Jankovski Date: Tue, 24 Jun 2014 14:19:40 +0200 Subject: [PATCH 3/4] Error page. --- app/views/errors/omniauth_error.html.haml | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 app/views/errors/omniauth_error.html.haml diff --git a/app/views/errors/omniauth_error.html.haml b/app/views/errors/omniauth_error.html.haml new file mode 100644 index 00000000000..e1eaa5a9d06 --- /dev/null +++ b/app/views/errors/omniauth_error.html.haml @@ -0,0 +1,12 @@ +%h1.http_status_code 422 +%h3.page-title Sign-in using #{@provider} auth failed +%hr +%p Sign-in failed because #{@error}. +%p There are couple of steps you can take: +1. Try logging in using your email +%br +2. Try logging in using your username +%br +3. If you have forgotten your password, try recovering it using #{ link_to "Password recovery", new_password_path(resource_name) } +%br +%p If none of the options work, try contacting the GitLab administrator. From 8307704c2dec2059089b9bf4acbd231e0fe0c8e2 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 24 Jun 2014 15:57:38 +0300 Subject: [PATCH 4/4] Improve error page layout Signed-off-by: Dmitriy Zaporozhets --- app/assets/stylesheets/generic/common.scss | 7 ------- app/assets/stylesheets/sections/errors.scss | 14 ++++++++++++++ app/views/errors/access_denied.html.haml | 4 ++-- app/views/errors/encoding.html.haml | 4 ++-- app/views/errors/git_not_found.html.haml | 4 ++-- app/views/errors/not_found.html.haml | 4 ++-- app/views/errors/omniauth_error.html.haml | 16 ++++++++-------- app/views/layouts/errors.html.haml | 7 +++---- 8 files changed, 33 insertions(+), 27 deletions(-) create mode 100644 app/assets/stylesheets/sections/errors.scss diff --git a/app/assets/stylesheets/generic/common.scss b/app/assets/stylesheets/generic/common.scss index bd2dcefce1a..b9942b5cb5d 100644 --- a/app/assets/stylesheets/generic/common.scss +++ b/app/assets/stylesheets/generic/common.scss @@ -257,13 +257,6 @@ li.note { } } -h1.http_status_code { - font-size: 56px; - line-height: 100px; - font-weight: normal; - color: #456; -} - .control-group { .controls { span { diff --git a/app/assets/stylesheets/sections/errors.scss b/app/assets/stylesheets/sections/errors.scss new file mode 100644 index 00000000000..32d2d7b1dbf --- /dev/null +++ b/app/assets/stylesheets/sections/errors.scss @@ -0,0 +1,14 @@ +.error-page { + max-width: 400px; + margin: 0 auto; + + h1, h2, h3 { + text-align: center; + } + + h1 { + font-size: 56px; + line-height: 100px; + font-weight: 300; + } +} diff --git a/app/views/errors/access_denied.html.haml b/app/views/errors/access_denied.html.haml index e005a7c4409..a1d8664c4ce 100644 --- a/app/views/errors/access_denied.html.haml +++ b/app/views/errors/access_denied.html.haml @@ -1,5 +1,5 @@ -%h1.http_status_code 403 -%h3.page-title Access Denied +%h1 403 +%h3 Access Denied %hr %p You are not allowed to access this page. %p Read more about project permissions #{link_to "here", help_page_path("permissions", "permissions"), class: "vlink"} diff --git a/app/views/errors/encoding.html.haml b/app/views/errors/encoding.html.haml index 7021f06dd7f..64c7451a8da 100644 --- a/app/views/errors/encoding.html.haml +++ b/app/views/errors/encoding.html.haml @@ -1,4 +1,4 @@ -%h1.http_status_code 500 -%h3.page-title Encoding Error +%h1 500 +%h3 Encoding Error %hr %p Page can't be loaded because of an encoding error. diff --git a/app/views/errors/git_not_found.html.haml b/app/views/errors/git_not_found.html.haml index d8ed7773207..189e53bca55 100644 --- a/app/views/errors/git_not_found.html.haml +++ b/app/views/errors/git_not_found.html.haml @@ -1,5 +1,5 @@ -%h1.http_status_code 404 -%h3.page-title Git Resource Not found +%h1 404 +%h3 Git Resource Not found %hr %p Application can't get access to some branch or commit in your repository. It diff --git a/app/views/errors/not_found.html.haml b/app/views/errors/not_found.html.haml index 4b97ddefc72..7bf88f592cf 100644 --- a/app/views/errors/not_found.html.haml +++ b/app/views/errors/not_found.html.haml @@ -1,4 +1,4 @@ -%h1.http_status_code 404 -%h3.page-title The resource you were looking for doesn't exist. +%h1 404 +%h3 The resource you were looking for doesn't exist. %hr %p You may have mistyped the address or the page may have moved. diff --git a/app/views/errors/omniauth_error.html.haml b/app/views/errors/omniauth_error.html.haml index e1eaa5a9d06..f3c8221a9d9 100644 --- a/app/views/errors/omniauth_error.html.haml +++ b/app/views/errors/omniauth_error.html.haml @@ -1,12 +1,12 @@ -%h1.http_status_code 422 -%h3.page-title Sign-in using #{@provider} auth failed +%h1 422 +%h3 Sign-in using #{@provider} auth failed %hr %p Sign-in failed because #{@error}. %p There are couple of steps you can take: -1. Try logging in using your email -%br -2. Try logging in using your username -%br -3. If you have forgotten your password, try recovering it using #{ link_to "Password recovery", new_password_path(resource_name) } -%br + +%ul + %li Try logging in using your email + %li Try logging in using your username + %li If you have forgotten your password, try recovering it using #{ link_to "Password recovery", new_password_path(resource_name) } + %p If none of the options work, try contacting the GitLab administrator. diff --git a/app/views/layouts/errors.html.haml b/app/views/layouts/errors.html.haml index 3e05746f837..d0e276d751a 100644 --- a/app/views/layouts/errors.html.haml +++ b/app/views/layouts/errors.html.haml @@ -4,7 +4,6 @@ %body{class: "#{app_theme} application"} = render "layouts/head_panel", title: "" if current_user = render "layouts/flash" - .container - .content - .center.padded.prepend-top-20 - = yield + .container.navless-container + .error-page + = yield