Skip to content

Commit

Permalink
Add configuration for authentication method
Browse files Browse the repository at this point in the history
Preparing for users to choose how they want to authenticate their
nextcloud storage and if using a common OAuth 2.0 IDP, what the client_id
of nextcloud is.
  • Loading branch information
NobodysNightmare committed Jan 20, 2025
1 parent c37894b commit 8486b2c
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@ class NextcloudContract < ::ModelContract
# But only do so if the validations above for URL were successful.
validates :host, secure_context_uri: true, nextcloud_compatible_host: true, unless: -> { errors.include?(:host) }

attribute :authentication_method
validates :authentication_method, presence: true, inclusion: { in: ::Storages::NextcloudStorage::AUTHENTICATION_METHODS }

attribute :nextcloud_audience
validates :nextcloud_audience, presence: true, if: :nextcloud_storage_authenticate_via_idp?

attribute :automatically_managed

attribute :username
Expand Down Expand Up @@ -68,6 +74,10 @@ def nextcloud_default_storage_username?
@model.username == @model.provider_fields_defaults[:username]
end

def nextcloud_storage_authenticate_via_idp?
nextcloud_storage? && @model.authenticate_via_idp?
end

def nextcloud_storage?
@model.is_a?(Storages::NextcloudStorage)
end
Expand Down
8 changes: 8 additions & 0 deletions modules/storages/app/models/storages/nextcloud_storage.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,15 @@ class NextcloudStorage < Storage
username: "OpenProject"
}.freeze

AUTHENTICATION_METHODS = %w[two_way_oauth2 oauth2_sso].freeze

store_attribute :provider_fields, :automatically_managed, :boolean
store_attribute :provider_fields, :username, :string
store_attribute :provider_fields, :password, :string
store_attribute :provider_fields, :group, :string
store_attribute :provider_fields, :group_folder, :string
store_attribute :provider_fields, :authentication_method, :string, default: "two_way_oauth2"
store_attribute :provider_fields, :nextcloud_audience, :string

def oauth_configuration
Peripherals::OAuthConfigurations::NextcloudConfiguration.new(self)
Expand All @@ -62,6 +66,10 @@ def available_project_folder_modes
end
end

def authenticate_via_idp?
authentication_method == "oauth2_sso"
end

def configuration_checks
{
storage_oauth_client_configured: oauth_client.present?,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# frozen_string_literal: true

class SetDefaultAuthenticationMethod < ActiveRecord::Migration[7.1]
def change
auth_json = { authentication_method: "two_way_oauth2" }.to_json
Storages::NextcloudStorage.where("NOT provider_fields ? 'authentication_method'")
.update_all("provider_fields = provider_fields || '#{auth_json}'::jsonb")
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,17 @@

RSpec.describe Storages::Storages::NextcloudContract, :storage_server_helpers, :webmock do
let(:current_user) { create(:admin) }
let(:storage_host) { "https://host1.example.com" }
let(:storage) { build(:nextcloud_storage, host: storage_host) }
let(:storage) { build(:nextcloud_storage) }
let(:mocked_host) { storage.host }

let!(:capabilities_request) { mock_server_capabilities_response(mocked_host) }
let!(:host_request) { mock_server_config_check_response(mocked_host) }

# As the NextcloudContract is selected by the BaseContract to make writable attributes available,
# the BaseContract needs to be instantiated here.
subject { Storages::Storages::BaseContract.new(storage, current_user) }

it "checks the storage url only when changed" do
capabilities_request = mock_server_capabilities_response(storage_host)
host_request = mock_server_config_check_response(storage_host)
subject.validate
expect(capabilities_request).to have_been_made.once
expect(host_request).to have_been_made.once
Expand All @@ -58,11 +59,6 @@
context "with valid credentials" do
let(:storage) { build(:nextcloud_storage, :as_automatically_managed) }

before do
mock_server_capabilities_response(storage.host)
mock_server_config_check_response(storage.host)
end

it "passes validation" do
credentials_request = mock_nextcloud_application_credentials_validation(storage.host)

Expand Down Expand Up @@ -137,6 +133,7 @@

context "when the storage host is nil" do
let(:storage) { build(:nextcloud_storage, :as_automatically_managed, host: nil) }
let(:mocked_host) { "https://example.com/unrelated" }

before do
allow(NextcloudApplicationCredentialsValidator).to receive(:new).and_call_original
Expand All @@ -149,4 +146,67 @@
end
end
end

describe "authentication_method validation" do
let(:storage) { build(:nextcloud_storage, :as_not_automatically_managed, authentication_method:) }
let(:authentication_method) { "two_way_oauth2" }

it { is_expected.to be_valid }

context "when the authentication method is oauth2_sso" do
let(:authentication_method) { "oauth2_sso" }

it { is_expected.to be_valid }
end

context "when the authentication method is unknown" do
let(:authentication_method) { "magic_unicorns" }

it { is_expected.not_to be_valid }
end

context "when the authentication method is missing" do
let(:authentication_method) { nil }

it { is_expected.not_to be_valid }
end
end

describe "nextcloud_audience validation" do
let(:storage) do
build(:nextcloud_storage, :as_not_automatically_managed, authentication_method:, nextcloud_audience:)
end

context "when authentication happens through bidirectional OAuth 2.0" do
let(:authentication_method) { "two_way_oauth2" }

context "and there is no nextcloud_audience" do
let(:nextcloud_audience) { nil }

it { is_expected.to be_valid }
end

context "and there is a nextcloud_audience" do
let(:nextcloud_audience) { "nextcloud" }

it { is_expected.to be_valid }
end
end

context "when authentication happens through a common IDP" do
let(:authentication_method) { "oauth2_sso" }

context "and there is no nextcloud_audience" do
let(:nextcloud_audience) { nil }

it { is_expected.not_to be_valid }
end

context "and there is a nextcloud_audience" do
let(:nextcloud_audience) { "nextcloud" }

it { is_expected.to be_valid }
end
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -294,8 +294,7 @@

context "when not automatically managed, no username or password" do
before do
storage.provider_fields = {}
storage.assign_attributes(automatic_management_enabled: false)
storage.assign_attributes(automatic_management_enabled: false, username: nil, password: nil)
end

it_behaves_like "contract is valid"
Expand Down
2 changes: 2 additions & 0 deletions modules/storages/spec/factories/storage_factory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@
class: "::Storages::NextcloudStorage" do
provider_type { Storages::Storage::PROVIDER_TYPE_NEXTCLOUD }
sequence(:host) { |n| "https://host#{n}.example.com/" }
authentication_method { "two_way_oauth2" }
nextcloud_audience { "nextcloud" }

trait :as_automatically_managed do
automatic_management_enabled { true }
Expand Down

0 comments on commit 8486b2c

Please sign in to comment.