diff options
author | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-13 15:05:17 +0300 |
---|---|---|
committer | Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com> | 2015-05-13 15:05:17 +0300 |
commit | 8ad91d58405296a6d0b4cec7b2f18b2a2728c717 (patch) | |
tree | 97af74c181955b5523c251c3e929400b28b57643 | |
parent | 5dcbe6f4baa0298a14592ad06763e26e12399e64 (diff) | |
parent | c5e4b443ffdc1c094450d08d29bd96e43376d6d7 (diff) |
Merge branch 'text-batch-1' into 'master'
Batch 1 of text improvements
Batch 1 of changes from my effort at !635 to walk through every piece of text in GitLab and see if it can be improved.
This batch includes:
- Improve text on error pages.
- Improve Git access error messages.
- Improve description of branch protection levels.
- Improve OAuth signup error message.
- Improve OAuth application flash messages.
cc @rspeicher
See merge request !642
-rw-r--r-- | app/controllers/omniauth_callbacks_controller.rb | 11 | ||||
-rw-r--r-- | config/locales/doorkeeper.en.yml | 10 | ||||
-rw-r--r-- | lib/api/internal.rb | 26 | ||||
-rw-r--r-- | lib/gitlab/access.rb | 6 | ||||
-rw-r--r-- | lib/gitlab/git_access.rb | 62 | ||||
-rw-r--r-- | lib/gitlab/git_access_wiki.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/o_auth/user.rb | 4 | ||||
-rw-r--r-- | public/404.html | 7 | ||||
-rw-r--r-- | public/422.html | 10 | ||||
-rw-r--r-- | public/500.html | 5 | ||||
-rw-r--r-- | public/502.html | 3 | ||||
-rw-r--r-- | public/deploy.html | 12 | ||||
-rw-r--r-- | public/static.css | 10 | ||||
-rw-r--r-- | spec/lib/gitlab/git_access_spec.rb | 14 |
14 files changed, 98 insertions, 84 deletions
diff --git a/app/controllers/omniauth_callbacks_controller.rb b/app/controllers/omniauth_callbacks_controller.rb index bb9d65c9ed6..dcd949a71de 100644 --- a/app/controllers/omniauth_callbacks_controller.rb +++ b/app/controllers/omniauth_callbacks_controller.rb @@ -65,8 +65,15 @@ class OmniauthCallbacksController < Devise::OmniauthCallbacksController redirect_to omniauth_error_path(oauth['provider'], error: error_message) and return end end - rescue Gitlab::OAuth::ForbiddenAction => e - flash[:notice] = e.message + rescue Gitlab::OAuth::SignupDisabledError => e + message = "Signing in using your #{oauth['provider']} account without a pre-existing GitLab account is not allowed." + + if current_application_settings.signup_enabled? + message << " Create a GitLab account first, and then connect it to your #{oauth['provider']} account." + end + + flash[:notice] = message + redirect_to new_user_session_path end diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml index c5b6b75e7f6..a4032a21420 100644 --- a/config/locales/doorkeeper.en.yml +++ b/config/locales/doorkeeper.en.yml @@ -31,7 +31,7 @@ en: messages: # Common error messages invalid_request: 'The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.' - invalid_redirect_uri: 'The redirect uri included is not valid.' + invalid_redirect_uri: 'The redirect URI included is not valid.' unauthorized_client: 'The client is not authorized to perform this request using this method.' access_denied: 'The resource owner or authorization server denied the request.' invalid_scope: 'The requested scope is invalid, unknown, or malformed.' @@ -63,11 +63,11 @@ en: flash: applications: create: - notice: 'Application created.' + notice: 'The application was created successfully.' destroy: - notice: 'Application deleted.' + notice: 'The application was deleted successfully.' update: - notice: 'Application updated.' + notice: 'The application was updated successfully.' authorized_applications: destroy: - notice: 'Application revoked.' + notice: 'The application was revoked access.' diff --git a/lib/api/internal.rb b/lib/api/internal.rb index f98a17773e7..e38736fc28b 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -24,10 +24,6 @@ module API User.find_by(id: params[:user_id]) end - unless actor - return Gitlab::GitAccessStatus.new(false, 'No such user or key') - end - project_path = params[:project] # Check for *.wiki repositories. @@ -39,22 +35,14 @@ module API project = Project.find_with_namespace(project_path) - if project - access = - if wiki - Gitlab::GitAccessWiki.new(actor, project) - else - Gitlab::GitAccess.new(actor, project) - end - - status = access.check(params[:action], params[:changes]) - end + access = + if wiki + Gitlab::GitAccessWiki.new(actor, project) + else + Gitlab::GitAccess.new(actor, project) + end - if project && access.can_read_project? - status - else - Gitlab::GitAccessStatus.new(false, 'No such project') - end + access.check(params[:action], params[:changes]) end # diff --git a/lib/gitlab/access.rb b/lib/gitlab/access.rb index 424541b4a04..6d0e30e916f 100644 --- a/lib/gitlab/access.rb +++ b/lib/gitlab/access.rb @@ -51,9 +51,9 @@ module Gitlab def protection_options { - "Not protected, developers and masters can (force) push and delete the branch" => PROTECTION_NONE, - "Partially protected, developers can also push but prevent all force pushes and deletion" => PROTECTION_DEV_CAN_PUSH, - "Fully protected, only masters can push and prevent all force pushes and deletion" => PROTECTION_FULL, + "Not protected: Both developers and masters can push new commits, force push, or delete the branch." => PROTECTION_NONE, + "Partially protected: Developers can push new commits, but cannot force push or delete the branch. Masters can do all of those." => PROTECTION_DEV_CAN_PUSH, + "Fully protected: Developers cannot push new commits, force push, or delete the branch. Only masters can do any of those." => PROTECTION_FULL, } end diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index bc72b7528d5..c90184d31cf 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -31,8 +31,7 @@ module Gitlab def can_push_to_branch?(ref) return false unless user - if project.protected_branch?(ref) && - !(project.developers_can_push_to_protected_branch?(ref) && project.team.developer?(user)) + if project.protected_branch?(ref) && !project.developers_can_push_to_protected_branch?(ref) user.can?(:push_code_to_protected_branches, project) else user.can?(:push_code, project) @@ -50,13 +49,25 @@ module Gitlab end def check(cmd, changes = nil) + unless actor + return build_status_object(false, "No user or key was provided.") + end + + if user && !user_allowed? + return build_status_object(false, "Your account has been blocked.") + end + + unless project && can_read_project? + return build_status_object(false, 'The project you were looking for could not be found.') + end + case cmd when *DOWNLOAD_COMMANDS download_access_check when *PUSH_COMMANDS push_access_check(changes) else - build_status_object(false, "Wrong command") + build_status_object(false, "The command you're trying to execute is not allowed.") end end @@ -64,7 +75,7 @@ module Gitlab if user user_download_access_check elsif deploy_key - deploy_key_download_access_check + build_status_object(true) else raise 'Wrong actor' end @@ -74,39 +85,27 @@ module Gitlab if user user_push_access_check(changes) elsif deploy_key - build_status_object(false, "Deploy key not allowed to push") + build_status_object(false, "Deploy keys are not allowed to push code.") else raise 'Wrong actor' end end def user_download_access_check - if user && user_allowed? && user.can?(:download_code, project) - build_status_object(true) - else - build_status_object(false, "You don't have access") + unless user.can?(:download_code, project) + return build_status_object(false, "You are not allowed to download code from this project.") end - end - def deploy_key_download_access_check - if can_read_project? - build_status_object(true) - else - build_status_object(false, "Deploy key not allowed to access this project") - end + build_status_object(true) end def user_push_access_check(changes) - unless user && user_allowed? - return build_status_object(false, "You don't have access") - end - if changes.blank? return build_status_object(true) end unless project.repository.exists? - return build_status_object(false, "Repository does not exist") + return build_status_object(false, "A repository for this project does not exist yet.") end changes = changes.lines if changes.kind_of?(String) @@ -136,11 +135,24 @@ module Gitlab :push_code end - if user.can?(action, project) - build_status_object(true) - else - build_status_object(false, "You don't have permission") + unless user.can?(action, project) + status = + case action + when :force_push_code_to_protected_branches + build_status_object(false, "You are not allowed to force push code to a protected branch on this project.") + when :remove_protected_branches + build_status_object(false, "You are not allowed to deleted protected branches from this project.") + when :push_code_to_protected_branches + build_status_object(false, "You are not allowed to push code to protected branches on this project.") + when :admin_project + build_status_object(false, "You are not allowed to change existing tags on this project.") + else # :push_code + build_status_object(false, "You are not allowed to push code to this project.") + end + return status end + + build_status_object(true) end def forced_push?(oldrev, newrev) diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb index 73d99b96202..8ba97184e69 100644 --- a/lib/gitlab/git_access_wiki.rb +++ b/lib/gitlab/git_access_wiki.rb @@ -4,7 +4,7 @@ module Gitlab if user.can?(:write_wiki, project) build_status_object(true) else - build_status_object(false, "You don't have access") + build_status_object(false, "You are not allowed to write to this project's wiki.") end end end diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index 2f5c217d764..ba5caed6131 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -5,7 +5,7 @@ # module Gitlab module OAuth - class ForbiddenAction < StandardError; end + class SignupDisabledError < StandardError; end class User attr_accessor :auth_hash, :gl_user @@ -99,7 +99,7 @@ module Gitlab end def unauthorized_to_create - raise ForbiddenAction.new("Unauthorized to create user, signup disabled for #{auth_hash.provider}") + raise SignupDisabledError end end end diff --git a/public/404.html b/public/404.html index 867f193a98f..a0106bc760d 100644 --- a/public/404.html +++ b/public/404.html @@ -1,14 +1,15 @@ <!DOCTYPE html> <html> <head> - <title>The page you were looking for doesn't exist (404)</title> + <title>The page you're looking for could not be found (404)</title> <link href="/static.css" media="screen" rel="stylesheet" type="text/css" /> </head> <body> <h1>404</h1> - <h3>The page you were looking for doesn't exist.</h3> + <h3>The page you're looking for could not be found.</h3> <hr/> - <p>You may have mistyped the address or the page may have moved.</p> + <p>Make sure the address is correct and that the page hasn't moved.</p> + <p>Please contact your GitLab administrator if you think this is a mistake.</p> </body> </html> diff --git a/public/422.html b/public/422.html index b6c37ac5386..026997b48e3 100644 --- a/public/422.html +++ b/public/422.html @@ -1,16 +1,16 @@ <!DOCTYPE html> <html> <head> - <title>The change you wanted was rejected (422)</title> + <title>The change you requested was rejected (422)</title> <link href="/static.css" media="screen" rel="stylesheet" type="text/css" /> </head> <body> <!-- This file lives in public/422.html --> <h1>422</h1> - <div> - <h2>The change you wanted was rejected.</h2> - <p>Maybe you tried to change something you didn't have access to.</p> - </div> + <h3>The change you requested was rejected.</h3> + <hr /> + <p>Make sure you have access to the thing you tried to change.</p> + <p>Please contact your GitLab administrator if you think this is a mistake.</p> </body> </html> diff --git a/public/500.html b/public/500.html index c84b9e90e4b..08c11bbd05a 100644 --- a/public/500.html +++ b/public/500.html @@ -1,13 +1,14 @@ <!DOCTYPE html> <html> <head> - <title>We're sorry, but something went wrong (500)</title> + <title>Something went wrong (500)</title> <link href="/static.css" media="screen" rel="stylesheet" type="text/css" /> </head> <body> <h1>500</h1> - <h3>We're sorry, but something went wrong.</h3> + <h3>Whoops, something went wrong on our end.</h3> <hr/> + <p>Try refreshing the page, or going back and attempting the action again.</p> <p>Please contact your GitLab administrator if this problem persists.</p> </body> </html> diff --git a/public/502.html b/public/502.html index d171eccc927..9480a928439 100644 --- a/public/502.html +++ b/public/502.html @@ -6,8 +6,9 @@ </head> <body> <h1>502</h1> - <h3>GitLab is not responding.</h3> + <h3>Whoops, GitLab is taking too much time to respond.</h3> <hr/> + <p>Try refreshing the page, or going back and attempting the action again.</p> <p>Please contact your GitLab administrator if this problem persists.</p> </body> </html> diff --git a/public/deploy.html b/public/deploy.html index e41ed76573d..1a41b772f3c 100644 --- a/public/deploy.html +++ b/public/deploy.html @@ -1,11 +1,17 @@ <!DOCTYPE html> <html> <head> - <title>Deploy in progress. Please try again in a few minutes</title> + <title>Deploy in progress</title> <link href="/static.css" media="screen" rel="stylesheet" type="text/css" /> </head> + <body> - <h1><center><img src="/gitlab_logo.png"/></center>Deploy in progress</h1> - <h3>Please try again in a few minutes or contact your administrator.</h3> + <h1> + <img src="/gitlab_logo.png" /><br /> + Deploy in progress + </h1> + <h3>Please try again in a few minutes.</h3> + <hr/> + <p>Please contact your GitLab administrator if this problem persists.</p> </body> </html> diff --git a/public/static.css b/public/static.css index c6f92ac01d9..0a2b6060d48 100644 --- a/public/static.css +++ b/public/static.css @@ -2,18 +2,24 @@ body { color: #666; text-align: center; font-family: "Helvetica Neue", Helvetica, Arial, sans-serif; - margin:0; + margin: 0; width: 800px; margin: auto; font-size: 14px; } + h1 { font-size: 56px; line-height: 100px; font-weight: normal; color: #456; } -h2 { font-size: 24px; color: #666; line-height: 1.5em; } + +h2 { + font-size: 24px; + color: #666; + line-height: 1.5em; +} h3 { color: #456; diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb index 39be9d64644..c7291689e32 100644 --- a/spec/lib/gitlab/git_access_spec.rb +++ b/spec/lib/gitlab/git_access_spec.rb @@ -115,18 +115,10 @@ describe Gitlab::GitAccess do let(:actor) { key } context 'pull code' do - context 'allowed' do - before { key.projects << project } - subject { access.download_access_check } - - it { expect(subject.allowed?).to be_truthy } - end - - context 'denied' do - subject { access.download_access_check } + before { key.projects << project } + subject { access.download_access_check } - it { expect(subject.allowed?).to be_falsey } - end + it { expect(subject.allowed?).to be_truthy } end end end |