Merge branch 'link-to-group-oauth' into 'master'

Fix link to subgroup OAuth application

See merge request gitlab-org/gitlab!60066

app/helpers/auth_helper.rb

@@ -176,6 +176,18 @@ module AuthHelper
       !current_user
   end
 
+  def auth_app_owner_text(owner)
+    return unless owner
+
+    if owner.is_a?(Group)
+      group_link = link_to(owner.name, group_path(owner))
+      _("This application was created for group %{group_link}.").html_safe % { group_link: group_link }
+    else
+      user_link = link_to(owner.name, user_path(owner))
+      _("This application was created by %{user_link}.").html_safe % { user_link: user_link }
+    end
+  end
+
   extend self
 end
 

app/views/doorkeeper/authorizations/new.html.haml

@@ -17,10 +17,8 @@
           = _("An application called %{link_to_client} is requesting access to your GitLab account.").html_safe % { link_to_client: link_to_client }
 
           - auth_app_owner = @pre_auth.client.application.owner
-          - if auth_app_owner
-            - link_to_owner = link_to(auth_app_owner.name, user_path(auth_app_owner))
-            = _("This application was created by %{link_to_owner}.").html_safe % { link_to_owner: link_to_owner }
 
+          = auth_app_owner_text(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

changelogs/unreleased/link-to-group-oauth.yml

+---
+title: Fix link to subgroup OAuth application
+merge_request: 60066
+author:
+type: fixed

locale/gitlab.pot

@@ -32588,7 +32588,10 @@ msgstr ""
 msgid "This also resolves this thread"
 msgstr ""
 
-msgid "This application was created by %{link_to_owner}."
+msgid "This application was created by %{user_link}."
+msgstr ""
+
+msgid "This application was created for group %{group_link}."
 msgstr ""
 
 msgid "This application will be able to:"

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -73,39 +73,74 @@ RSpec.describe Oauth::AuthorizationsController do
     include_examples 'OAuth Authorizations require confirmed user'
     include_examples "Implicit grant can't be used in confidential application"
 
-    context 'when the user is confirmed' do
-      let(:confirmed_at) { 1.hour.ago }
+    context 'rendering of views based on the ownership of the application' do
+      shared_examples 'render views' do
+        render_views
 
-      context 'without valid params' do
-        it 'returns 200 code and renders error view' do
-          get :new
+        it 'returns 200 and renders view with correct info', :aggregate_failures do
+          subject
 
           expect(response).to have_gitlab_http_status(:ok)
-          expect(response).to render_template('doorkeeper/authorizations/error')
+          expect(response.body).to include(application.owner.name)
+          expect(response).to render_template('doorkeeper/authorizations/new')
         end
       end
 
-      context 'with valid params' do
-        render_views
+      subject { get :new, params: params }
 
-        it 'returns 200 code and renders view' do
-          subject
+      context 'when auth app owner is a user' do
+        context 'with valid params' do
+          it_behaves_like 'render views'
+        end
+      end
 
-          expect(response).to have_gitlab_http_status(:ok)
-          expect(response).to render_template('doorkeeper/authorizations/new')
+      context 'when auth app owner is a group' do
+        let(:group) { create(:group) }
+
+        context 'when auth app owner is a root group' do
+          let(:application) { create(:oauth_application, owner_id: group.id, owner_type: 'Namespace') }
+
+          it_behaves_like 'render views'
+        end
+
+        context 'when auth app owner is a subgroup' do
+          let(:subgroup) { create(:group, parent: group) }
+          let(:application) { create(:oauth_application, owner_id: subgroup.id, owner_type: 'Namespace') }
+
+          it_behaves_like 'render views'
         end
+      end
 
-        it 'deletes session.user_return_to and redirects when skip authorization' do
-          application.update!(trusted: true)
-          request.session['user_return_to'] = 'http://example.com'
+      context 'when there is no owner associated' do
+        let(:application) { create(:oauth_application, owner_id: nil, owner_type: nil) }
 
+        it 'renders view' do
           subject
 
-          expect(request.session['user_return_to']).to be_nil
-          expect(response).to have_gitlab_http_status(:found)
+          expect(response).to have_gitlab_http_status(:ok)
+          expect(response).to render_template('doorkeeper/authorizations/new')
         end
       end
     end
+
+    context 'without valid params' do
+      it 'returns 200 code and renders error view' do
+        get :new
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(response).to render_template('doorkeeper/authorizations/error')
+      end
+    end
+
+    it 'deletes session.user_return_to and redirects when skip authorization' do
+      application.update!(trusted: true)
+      request.session['user_return_to'] = 'http://example.com'
+
+      subject
+
+      expect(request.session['user_return_to']).to be_nil
+      expect(response).to have_gitlab_http_status(:found)
+    end
   end
 
   describe 'POST #create' do

spec/helpers/auth_helper_spec.rb

@@ -313,4 +313,37 @@ RSpec.describe AuthHelper do
       it { is_expected.to be_falsey }
     end
   end
+
+  describe '#auth_app_owner_text' do
+    shared_examples 'generates text with the correct info' do
+      it 'includes the name of the application owner' do
+        auth_app_owner_text = helper.auth_app_owner_text(owner)
+
+        expect(auth_app_owner_text).to include(owner.name)
+        expect(auth_app_owner_text).to include(path_to_owner)
+      end
+    end
+
+    context 'when owner is a user' do
+      let_it_be(:owner) { create(:user) }
+
+      let(:path_to_owner) { user_path(owner) }
+
+      it_behaves_like 'generates text with the correct info'
+    end
+
+    context 'when owner is a group' do
+      let_it_be(:owner) { create(:group) }
+
+      let(:path_to_owner) { user_path(owner) }
+
+      it_behaves_like 'generates text with the correct info'
+    end
+
+    context 'when the user is missing' do
+      it 'returns nil' do
+        expect(helper.auth_app_owner_text(nil)).to be(nil)
+      end
+    end
+  end
 end