Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#61936] Can't associate storage to project via storage admin view #18177

Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<%= render(Primer::BaseComponent.new(tag: :div, data: @wrapper_data_attributes, classes: @wrapper_classes)) do %>
<%= render(Primer::BaseComponent.new(tag: :div, display: :inline_flex, direction: :row, align_items: :center)) do %>
<% if storage_oauth_access_granted? %>
<% if @storage.oauth_access_granted?(User.current) %>
<%=
render(
Primer::Beta::Button.new(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,33 @@
# frozen_string_literal: true

# -- copyright
# OpenProject is an open source project management software.
# Copyright (C) 2024 the OpenProject GmbH
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License version 3.
#
# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
# Copyright (C) 2006-2013 Jean-Philippe Lang
# Copyright (C) 2010-2013 the ChiliProject Team
#
# This program is free software; you can redistribute it and/or
# modify it under the terms of the GNU General Public License
# as published by the Free Software Foundation; either version 2
# of the License, or (at your option) any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with this program; if not, write to the Free Software
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
#
# See COPYRIGHT and LICENSE files for more details.
# ++

module Primer
module OpenProject
module Forms
Expand All @@ -12,7 +42,7 @@ def initialize(input:, project_storage:, last_project_folders: {},
super()
@input = input

@project_storage = project_storage
@storage = project_storage.storage
@last_project_folders = last_project_folders

@storage_login_button_options = storage_login_button_options
Expand All @@ -22,13 +52,6 @@ def initialize(input:, project_storage:, last_project_folders: {},
@wrapper_data_attributes = wrapper_arguments.delete(:data) { {} }
@wrapper_classes = wrapper_arguments.delete(:classes) { [] }
end

private

def storage_oauth_access_granted?
OAuthClientToken
.exists?(user: User.current, oauth_client: @project_storage.storage.oauth_client)
end
end
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,6 @@ def open_redirect_to_storage_authorization_with(callback_url:, storage:, callbac
redirect_to(storage.oauth_configuration.authorization_uri(state: nonce), allow_other_host: true)
end

def storage_oauth_access_granted?(storage:)
OAuthClientToken.exists?(user: User.current, oauth_client: storage.oauth_client)
end

def project_storage_oauth_access_grant_nudge_modal(project_storage:)
{
component: ::Storages::ProjectStorages::OAuthAccessGrantNudgeModalComponent,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ def create

if service_result.success?
flash[:notice] = I18n.t(:notice_successful_create)
redirect_to_project_storages_path_with_oauth_access_grant_confirmation
redirect_to_project_storages_path_with_oauth_access_grant_confirmation(@project_storage.storage)
else
@available_storages = available_storages
render "/storages/project_settings/new"
Expand Down Expand Up @@ -111,7 +111,7 @@ def update
if service_result.success?
@project_storage = service_result.result
flash[:notice] = I18n.t(:notice_successful_update)
redirect_to_project_storages_path_with_oauth_access_grant_confirmation
redirect_to_project_storages_path_with_oauth_access_grant_confirmation(@project_storage.storage)
else
@project_storage = @object
render "/storages/project_settings/edit"
Expand Down Expand Up @@ -156,8 +156,8 @@ def available_storages
.select(&:configured?)
end

def redirect_to_project_storages_path_with_oauth_access_grant_confirmation
if storage_oauth_access_granted?(storage: @project_storage.storage)
def redirect_to_project_storages_path_with_oauth_access_grant_confirmation(storage)
if storage.oauth_access_granted?(User.current)
redirect_to external_file_storages_project_settings_project_storages_path
else
redirect_to_project_storages_path_with_nudge_modal
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ def index; end

def new
respond_with_dialog(
if storage_oauth_access_granted?(storage: @storage)
if @project_storage.storage.oauth_access_granted?(User.current)
::Storages::Admin::Storages::ProjectsStorageModalComponent.new(
project_storage: @project_storage, last_project_folders: {}
)
Expand Down
13 changes: 13 additions & 0 deletions modules/storages/app/models/storages/storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,19 @@ def self.extract_part_from_piped_string(text, index)
end
end

def oauth_access_granted?(user)
selector = Peripherals::StorageInteraction::AuthenticationMethodSelector.new(
storage: self,
user:
)
case selector.authentication_method
when :sso
true
when :storage_oauth
OAuthClientToken.exists?(user:, oauth_client: oauth_client)
end
end

def health_notifications_should_be_sent?
# it is a fallback for already created storages without health_notifications_enabled configured.
(health_notifications_enabled.nil? && automatic_management_enabled?) || health_notifications_enabled?
Expand Down
33 changes: 33 additions & 0 deletions modules/storages/spec/models/storages/storage_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,39 @@
require_module_spec_helper

RSpec.describe Storages::Storage do
describe "#oauth_access_granted?" do
let(:storage) { build(:storage, oauth_client:) }
let(:oauth_client) { create(:oauth_client) }
let(:user) { create(:user) }
let(:selector_class) { Storages::Peripherals::StorageInteraction::AuthenticationMethodSelector }
let(:mocked_instance) { instance_double(selector_class, authentication_method:) }

before do
allow(selector_class).to receive(:new).and_return(mocked_instance)
end

context "when user is authenticated through storage oauth" do
let(:authentication_method) { :storage_oauth }

it "responds with true if oauth_client_token exists" do
create(:oauth_client_token, user: user, oauth_client:)
expect(storage.oauth_access_granted?(user)).to be true
end

it "responds with false if oauth_client_token does not exist" do
expect(storage.oauth_access_granted?(user)).to be false
end
end

context "when user is authenticated through sso" do
let(:authentication_method) { :sso }

it "responds with true" do
expect(storage.oauth_access_granted?(user)).to be true
end
end
end

describe "#health_notifications_should_be_sent?" do
let(:storage) { build(:storage, provider_fields: {}) }

Expand Down
Loading