Merge branch '32059-fix-oauth-phishing' into 'master'

Prevent OAuth phishing attack by presenting detailed wording about app to user during authorization

See merge request gitlab-org/gitlab-ce!15311

6 files changed, 51 insertions, 7 deletions

diff --git a/app/assets/stylesheets/pages/settings.scss b/app/assets/stylesheets/pages/settings.scss
index 41a6ba2023a..a3fe8a95257 100644
--- a/app/assets/stylesheets/pages/settings.scss
+++ b/app/assets/stylesheets/pages/settings.scss
@@ -246,3 +246,22 @@
     }
   }
 }
+
+.modal-doorkeepr-auth,
+.doorkeeper-app-form {
+  .scope-description {
+    color: $theme-gray-700;
+  }
+}
+
+.modal-doorkeepr-auth {
+  .modal-body {
+    padding: $gl-padding;
+  }
+}
+
+.doorkeeper-app-form {
+  .scope-description {
+    margin: 0 0 5px 17px;
+  }
+}
diff --git a/app/views/doorkeeper/applications/_form.html.haml b/app/views/doorkeeper/applications/_form.html.haml
index b3313c7c985..cf0e0de1ca4 100644
--- a/app/views/doorkeeper/applications/_form.html.haml
+++ b/app/views/doorkeeper/applications/_form.html.haml
@@ -1,4 +1,4 @@
-= form_for application, url: doorkeeper_submit_path(application), html: {role: 'form'} do |f|
+= form_for application, url: doorkeeper_submit_path(application), html: { role: 'form', class: 'doorkeeper-app-form' } do |f|
   = form_errors(application)
 
   .form-group
diff --git a/app/views/doorkeeper/authorizations/new.html.haml b/app/views/doorkeeper/authorizations/new.html.haml
index 8ba88906714..85e4170aee9 100644
--- a/app/views/doorkeeper/authorizations/new.html.haml
+++ b/app/views/doorkeeper/authorizations/new.html.haml
@@ -1,5 +1,7 @@
+- auth_app_owner = @pre_auth.client.application.owner
+
 %main{ :role => "main" }
-  .modal-no-backdrop
+  .modal-no-backdrop.modal-doorkeepr-auth
     .modal-content
       .modal-header
         %h3.page-title
@@ -16,14 +18,21 @@
               %strong= @pre_auth.client.name
               will allow them to interact with GitLab as an admin as well. Proceed with caution.
         %p
-          You are about to authorize
+          An application called
           = link_to @pre_auth.client.name, @pre_auth.redirect_uri, target: '_blank', rel: 'noopener noreferrer'
-          to use your account.
-          - if @pre_auth.scopes
+          is requesting access to your GitLab account. This application was created by
+          = succeed "." do
+            = link_to auth_app_owner.name, user_path(auth_app_owner)
+          Please note that this application is not provided by GitLab and you should verify its authenticity before
+          allowing access.
+        - if @pre_auth.scopes
+          %p
             This application will be able to:
             %ul
               - @pre_auth.scopes.each do |scope|
-                %li= t scope, scope: [:doorkeeper, :scopes]
+                %li
+                  %strong= t scope, scope: [:doorkeeper, :scopes]
+                  .scope-description= t scope, scope: [:doorkeeper, :scope_desc]
         .form-actions.text-right
           = form_tag oauth_authorization_path, method: :delete, class: 'inline'  do
             = hidden_field_tag :client_id, @pre_auth.client.uid
diff --git a/app/views/shared/tokens/_scopes_form.html.haml b/app/views/shared/tokens/_scopes_form.html.haml
index 8bbaf431536..ae437dd16d6 100644
--- a/app/views/shared/tokens/_scopes_form.html.haml
+++ b/app/views/shared/tokens/_scopes_form.html.haml
@@ -7,3 +7,4 @@
     = check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}"
     = label_tag ("#{prefix}_scopes_#{scope}"), scope
     %span= t(scope, scope: [:doorkeeper, :scopes])
+    .scope-description= t scope, scope: [:doorkeeper, :scope_desc]
diff --git a/changelogs/unreleased/32059-fix-oauth-phishing.yml b/changelogs/unreleased/32059-fix-oauth-phishing.yml
new file mode 100644
index 00000000000..1aaa7285309
--- /dev/null
+++ b/changelogs/unreleased/32059-fix-oauth-phishing.yml
@@ -0,0 +1,6 @@
+---
+title: Prevent OAuth phishing attack by presenting detailed wording about app to user
+  during authorization
+merge_request: 
+author:
+type: security
diff --git a/config/locales/doorkeeper.en.yml b/config/locales/doorkeeper.en.yml
index 14d49885fb3..768d92ffbe1 100644
--- a/config/locales/doorkeeper.en.yml
+++ b/config/locales/doorkeeper.en.yml
@@ -61,7 +61,16 @@ en:
       api: Access your API
       read_user: Read user information
       openid: Authenticate using OpenID Connect
-
+      sudo: Perform API actions as any user in the system (if the authenticated user is an admin)
+    scope_desc:
+      api:
+        Full access to GitLab as the user, including read/write on all their groups and projects
+      read_user:
+        Read-only access to the user's profile information, like username, public email and full name
+      openid:
+        The ability to authenticate using GitLab, and read-only access to the user's profile information
+      sudo:
+        Access to the Sudo feature, to perform API actions as any user in the system (only available for admins)
     flash:
       applications:
         create: