From 002074eb39932a1116aa742f11ba77a2a0c8dadc Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:18:03 +0200 Subject: [PATCH 01/31] Add MDS env config and host-matching helper. Scaffold settings for the embedded Metadata Store protocol (MDS_ENABLED, MDS_HOSTS, MDS_URL, MDS_REALM) used by host-constrained routes. Enable by default in development and test only. --- .env.example | 9 ++++++++- config/application.rb | 15 +++++++++++++++ config/environments/development.rb | 4 ++++ config/environments/test.rb | 4 ++++ lib/mds.rb | 28 ++++++++++++++++++++++++++++ 5 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 lib/mds.rb diff --git a/.env.example b/.env.example index 198ef19c0..14665afe1 100644 --- a/.env.example +++ b/.env.example @@ -37,10 +37,17 @@ ES_HOST=elasticsearch:9200 MAILGUN_API_KEY= MG_DOMAIN=mg.datacite.org -# Needed for tests +# Needed for tests (seed client credentials, not the MDS protocol server) MDS_USERNAME= MDS_PASSWORD= +# MDS protocol (embedded former Poodle API). Host-constrained routes under Mds:: +# Set MDS_ENABLED=true and point mds.* hosts at this app to serve classic MDS paths. +MDS_ENABLED=false +MDS_HOSTS=mds.datacite.org,mds.test.datacite.org,mds.stage.datacite.org,mds.local +MDS_URL=https://mds.test.datacite.org +MDS_REALM=mds.datacite.org + CONCURRENCY=25 INTEGRATION=1 # Set this environment variable to run the integration tests REFRESH=true diff --git a/config/application.rb b/config/application.rb index 1ea978eac..06b7b8324 100644 --- a/config/application.rb +++ b/config/application.rb @@ -72,11 +72,26 @@ ENV["MONTHLY_DATAFILE_ACCESS_ROLE"] ||= "" ENV["ENRICHMENTS_INGESTION_FILES_BUCKET_NAME"] ||= "" +# MDS (legacy Metadata Store protocol, formerly Poodle) +# When MDS_ENABLED is true, hostnames in MDS_HOSTS serve classic MDS routes in-process. +ENV["MDS_ENABLED"] ||= "false" +ENV["MDS_HOSTS"] ||= "mds.datacite.org,mds.test.datacite.org,mds.stage.datacite.org,mds.local" +ENV["MDS_URL"] ||= + if Rails.env.production? + "https://mds.datacite.org" + else + "https://mds.test.datacite.org" + end +ENV["MDS_REALM"] ||= ENV["MDS_HOSTS"].to_s.split(",").first.to_s.strip.presence || "mds.datacite.org" + module Lupo class Application < Rails::Application # Initialize configuration defaults for originally generated Rails version. config.load_defaults 8.1 + # MDS protocol helpers (lib/mds.rb) + require Rails.root.join("lib/mds") + # include graphql config.paths.add Rails.root.join("app", "graphql", "types").to_s, eager_load: true diff --git a/config/environments/development.rb b/config/environments/development.rb index 76bfe24a4..894ec7685 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -59,6 +59,10 @@ ENV["SLACK_WEBHOOK_URL"] ||= "" # Keep empty if not sending Slack notifications in tests ENV["MDS_USERNAME"] ||= "DATACITE.TESTUSER" ENV["MDS_PASSWORD"] ||= "test_mds_password" + ENV["MDS_ENABLED"] ||= "true" + ENV["MDS_HOSTS"] ||= "mds.local,localhost,127.0.0.1" + ENV["MDS_URL"] ||= "http://mds.local:8065" + ENV["MDS_REALM"] ||= "mds.local" ENV["ADMIN_USERNAME"] ||= "DATACITE.TESTADMIN" ENV["ADMIN_PASSWORD"] ||= "test_admin_password" ENV["PRIVATE_IP"] ||= "127.0.0.1" # Placeholder for local testing diff --git a/config/environments/test.rb b/config/environments/test.rb index 5a4ad6e65..1f85f2a8d 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -79,6 +79,10 @@ ENV["SLACK_WEBHOOK_URL"] ||= "" # Keep empty if not sending Slack notifications in tests ENV["MDS_USERNAME"] ||= "DATACITE.TESTUSER" ENV["MDS_PASSWORD"] ||= "test_mds_password" + ENV["MDS_ENABLED"] ||= "true" + ENV["MDS_HOSTS"] ||= "mds.local,www.example.com,example.org" + ENV["MDS_URL"] ||= "https://mds.test.datacite.org" + ENV["MDS_REALM"] ||= "mds.local" ENV["ADMIN_USERNAME"] ||= "DATACITE.TESTADMIN" ENV["ADMIN_PASSWORD"] ||= "test_admin_password" ENV["PRIVATE_IP"] ||= "127.0.0.1" # Placeholder for local testing diff --git a/lib/mds.rb b/lib/mds.rb new file mode 100644 index 000000000..e741bf001 --- /dev/null +++ b/lib/mds.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +# Helpers for the embedded MDS (legacy Metadata Store) protocol surface. +module Mds + module_function + + def enabled? + ActiveModel::Type::Boolean.new.cast(ENV.fetch("MDS_ENABLED", "false")) + end + + def hosts + ENV.fetch("MDS_HOSTS", "").split(",").map { |h| h.strip.downcase }.reject(&:blank?) + end + + def host_match?(request) + return false unless enabled? + + hosts.include?(request.host.to_s.downcase) + end + + def url + ENV.fetch("MDS_URL", "https://mds.test.datacite.org") + end + + def realm + ENV.fetch("MDS_REALM", "mds.datacite.org") + end +end From 264a64fc0bb6d474d5b64a5bdc9a6902f58ee5e5 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:18:45 +0200 Subject: [PATCH 02/31] Add MDS application layer, DOI operations, and host routes. Introduce Mds:: controllers for auth/heartbeat/login, DoiOperations for list/get/put/delete of classic /doi paths, and host-constrained routes matching the former Poodle MDS surface. --- app/controllers/mds/application_controller.rb | 114 ++++++++++++ app/controllers/mds/dois_controller.rb | 61 +++++++ app/controllers/mds/heartbeat_controller.rb | 9 + app/controllers/mds/index_controller.rb | 9 + app/services/mds/doi_operations.rb | 169 ++++++++++++++++++ app/services/mds/result.rb | 35 ++++ config/routes.rb | 30 ++++ 7 files changed, 427 insertions(+) create mode 100644 app/controllers/mds/application_controller.rb create mode 100644 app/controllers/mds/dois_controller.rb create mode 100644 app/controllers/mds/heartbeat_controller.rb create mode 100644 app/controllers/mds/index_controller.rb create mode 100644 app/services/mds/doi_operations.rb create mode 100644 app/services/mds/result.rb diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb new file mode 100644 index 000000000..3903534e6 --- /dev/null +++ b/app/controllers/mds/application_controller.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +module Mds + class ApplicationController < ActionController::API + include ActionController::HttpAuthentication::Basic::ControllerMethods + include CanCan::ControllerAdditions + include Bolognese::DoiUtils + include Bolognese::Utils + + attr_accessor :current_user + + after_action :set_consumer_header + + def route_not_found + render plain: "Resource not found", status: :not_found + end + + protected + + # Authenticate via HTTP Basic (classic MDS) or Bearer token. + def authenticate_mds_user! + type, credentials = type_and_credentials_from_request_headers + + if credentials.blank? + request_http_basic_authentication(Mds.realm, "An Authentication object was not found in the SecurityContext") + return false + end + + if type.to_s.casecmp("basic").zero? + @current_user = User.new(credentials, type: "basic") + else + # Bearer / raw JWT — same path as REST API + @current_user = User.new(credentials) + end + + if @current_user.blank? || @current_user.errors.present? || @current_user.role_id == "anonymous" + response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" + response.headers.delete("X-Credential-Username") + render plain: "Bad credentials", status: :unauthorized + return false + end + + true + end + + def current_ability + @current_ability ||= Ability.new(current_user) + end + + def type_and_credentials_from_request_headers + header = request.headers["Authorization"] + return [nil, nil] if header.blank? + + type, credentials = header.split(" ", 2) + return [nil, nil] if credentials.blank? + + [type, credentials] + end + + def set_consumer_header + if current_user&.uid.present? + response.headers["X-Credential-Username"] = current_user.uid + else + response.headers["X-Anonymous-Consumer"] = true + end + end + + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end + + unless Rails.env.development? + rescue_from(*RESCUABLE_EXCEPTIONS, IdentifierError) do |exception| + status = + case exception.class.to_s + when "CanCan::AuthorizationNotPerformed", "JWT::DecodeError", "JWT::VerificationError" + 401 + when "CanCan::AccessDenied" + 403 + when "ActionController::RoutingError", "AbstractController::ActionNotFound", + "ActiveRecord::RecordNotFound" + 404 + when "ActiveModel::ForbiddenAttributesError", "ActionController::UnpermittedParameters", + "NoMethodError" + 422 + when "NotImplementedError" + 501 + when "IdentifierError" + 400 + else + 400 + end + + if status == 401 + response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" + response.headers.delete("X-Credential-Username") + message = "Bad credentials" + elsif status == 403 + message = "Access is denied" + elsif status == 404 + message = "DOI not found" + elsif status == 501 + message = "Not Implemented" + else + Sentry.capture_exception(exception) unless exception.class.to_s == "IdentifierError" + message = exception.message + end + + logger.error "[MDS #{status}]: #{message}" + render plain: message, status: status + end + end + end +end diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb new file mode 100644 index 000000000..3c1038515 --- /dev/null +++ b/app/controllers/mds/dois_controller.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module Mds + class DoisController < Mds::ApplicationController + prepend_before_action :authenticate_mds_user! + before_action :set_doi, only: %i[show destroy] + + def index + result = Mds::DoiOperations.new(current_user: current_user).list + render_result(result) + end + + def show + result = Mds::DoiOperations.new(current_user: current_user).get_url(@doi) + render_result(result) + end + + def update + doi, url = parse_doi_and_url + return head :bad_request if doi.blank? || url.blank? + + result = Mds::DoiOperations.new(current_user: current_user).put_url(doi, url: url) + render_result(result) + end + + def destroy + result = Mds::DoiOperations.new(current_user: current_user).destroy(@doi) + render_result(result) + end + + private + + def set_doi + @doi = validate_doi(params[:id]) + fail AbstractController::ActionNotFound if @doi.blank? + end + + def parse_doi_and_url + if (params[:id].present? || params[:doi].present?) && params[:url].present? + [params[:id].presence || params[:doi], params[:url]] + elsif request.raw_post.present? + Mds::DoiOperations.extract_url( + doi: validate_doi(params[:id]), + data: request.raw_post, + ) + else + [nil, nil] + end + end + + def render_result(result) + result.headers.each { |k, v| response.headers[k] = v } + + if result.status == 204 + head :no_content + else + render plain: result.body.to_s, status: result.status + end + end + end +end diff --git a/app/controllers/mds/heartbeat_controller.rb b/app/controllers/mds/heartbeat_controller.rb new file mode 100644 index 000000000..0c9f46c45 --- /dev/null +++ b/app/controllers/mds/heartbeat_controller.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Mds + class HeartbeatController < Mds::ApplicationController + def index + render plain: "OK", status: :ok + end + end +end diff --git a/app/controllers/mds/index_controller.rb b/app/controllers/mds/index_controller.rb new file mode 100644 index 000000000..f3b9fa9c6 --- /dev/null +++ b/app/controllers/mds/index_controller.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module Mds + class IndexController < Mds::ApplicationController + def login + render plain: "session cookies not supported", status: :not_implemented + end + end +end diff --git a/app/services/mds/doi_operations.rb b/app/services/mds/doi_operations.rb new file mode 100644 index 000000000..eff7a6a38 --- /dev/null +++ b/app/services/mds/doi_operations.rb @@ -0,0 +1,169 @@ +# frozen_string_literal: true + +module Mds + # In-process DOI operations for the classic MDS /doi surface. + class DoiOperations + include Bolognese::DoiUtils + + attr_reader :current_user, :current_ability + + def initialize(current_user:) + @current_user = current_user + @current_ability = Ability.new(current_user) + end + + def list + client = + Client.where("datacentre.symbol = ?", current_user.uid.upcase).first + return Result.no_content if client.blank? + + client_prefix = client.prefixes.first + return Result.no_content if client_prefix.blank? + + unless current_ability.can?(:get_urls, Doi) + return Result.error(403, "Access is denied") + end + + dois = + DataciteDoi.get_dois( + prefix: client_prefix.uid, + username: current_user.uid.upcase, + password: current_user.password, + ) + + if dois.blank? || !dois.is_a?(Array) || dois.empty? + return Result.no_content + end + + Result.ok(dois.join("\n")) + end + + def get_url(doi_string) + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI not found") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + return Result.error(404, "DOI not found") if doi.blank? + + unless current_ability.can?(:get_url, doi) + return Result.error(403, "Access is denied") + end + + url = resolve_url(doi) + return Result.no_content if url.blank? + + Result.ok(url) + end + + def put_url(doi_string, url:) + return Result.error(400, "Not a valid HTTP(S) or FTP URL") unless valid_landing_url?(url) + + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI not found") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + exists = doi.present? + + attrs = { + url: url, + should_validate: true, + source: "mds", + event: "publish", + client_id: client_symbol, + } + + if exists + unless current_ability.can?(:update, doi) + return Result.error(403, "Access is denied") + end + + doi.current_user = current_user + doi.assign_attributes(attrs.except(:client_id)) + else + doi = DataciteDoi.new(attrs.merge(doi: doi_id)) + doi.current_user = current_user + unless current_ability.can?(:new, doi) + return Result.error(403, "Access is denied") + end + end + + if doi.save + Result.created("OK") + else + message = doi.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + rescue ActiveRecord::RecordNotFound + Result.error(404, "DOI not found") + end + + def destroy(doi_string) + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI not found") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + return Result.error(404, "DOI not found") if doi.blank? + + unless current_ability.can?(:destroy, doi) + return Result.error(403, "Access is denied") + end + + unless doi.draft? + return Result.error(405, "Method not allowed") + end + + if doi.destroy + Result.ok("OK") + else + message = doi.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + end + + # Parse classic MDS body: "doi=...\nurl=..." lines. + def self.extract_url(doi: nil, data: nil) + hsh = + data.to_s.split("\n").map do |line| + arr = line.to_s.split("=", 2) + arr << "value" if arr.length < 2 + arr + end.to_h + + fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? + + body_doi = CGI.unescape(hsh["doi"].strip) + if doi.present? && body_doi.casecmp(doi) != 0 + fail IdentifierError, "doi parameter does not match doi of resource" + end + + fail IdentifierError, "param 'url' required" unless hsh["url"].present? + + [body_doi, CGI.unescape(hsh["url"].strip)] + end + + private + + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end + + def valid_landing_url?(url) + url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) + end + + def resolve_url(doi) + if !doi.is_registered_or_findable? || + %w[europ].include?(doi.provider_id) || + doi.type == "OtherDoi" + return doi.url + end + + response = doi.get_url + if response.status == 200 + response.body.dig("data", "values", 0, "data", "value") || doi.url + else + doi.url + end + end + end +end diff --git a/app/services/mds/result.rb b/app/services/mds/result.rb new file mode 100644 index 000000000..150970e36 --- /dev/null +++ b/app/services/mds/result.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Mds + # Simple result object for MDS operations (status + body/message + optional headers). + class Result + attr_reader :status, :body, :headers, :error + + def initialize(status:, body: nil, headers: {}, error: nil) + @status = status + @body = body + @headers = headers + @error = error + end + + def success? + status.to_i.between?(200, 299) + end + + def self.ok(body = "OK", status: 200, headers: {}) + new(status: status, body: body, headers: headers) + end + + def self.created(body = "OK", headers: {}) + new(status: 201, body: body, headers: headers) + end + + def self.no_content + new(status: 204, body: nil) + end + + def self.error(status, message) + new(status: status, body: message, error: message) + end + end +end diff --git a/config/routes.rb b/config/routes.rb index 083987e2e..71b9a911b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,6 +1,36 @@ # frozen_string_literal: true Rails.application.routes.draw do + # Classic MDS protocol (formerly Poodle). Only active when MDS_ENABLED and Host ∈ MDS_HOSTS. + # Must be declared before the REST catch-all so mds.* hosts never fall into content negotiation. + constraints(->(req) { Mds.host_match?(req) }) do + scope module: :mds do + resources :heartbeat, only: %i[index] + get "login", to: "index#login" + + # update doi (body form without id in path) + post "doi", to: "dois#update" + + # media (flat MDS paths) + post "media/:doi_id", to: "media#create", constraints: { doi_id: /.+/ } + get "media/:doi_id", to: "media#index", constraints: { doi_id: /.+/ } + + # metadata + post "metadata", to: "metadata#create" + post "metadata/:doi_id", to: "metadata#create", constraints: { doi_id: /.+/ } + put "metadata/:doi_id", to: "metadata#create", constraints: { doi_id: /.+/ } + get "metadata/:doi_id", to: "metadata#show", constraints: { doi_id: /.+/ } + get "metadata", to: "metadata#show" + delete "metadata/:doi_id", to: "metadata#destroy", constraints: { doi_id: /.+/ } + + resources :dois, path: "/doi", constraints: { id: /.+/ } do + resources :media + end + + match "*path", to: "application#route_not_found", via: :all + end + end + post "/client-api/graphql", to: "graphql#execute" get "/client-api/graphql", to: "index#method_not_allowed" From 5350e370808a12f3ba317ff46b8860f4f529ce11 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:19:19 +0200 Subject: [PATCH 03/31] Add MDS metadata and media protocol handlers. Port classic /metadata and /media MDS behavior in-process: format detection and auto-mint for metadata, line-oriented media bodies, mapped to DataciteDoi and Media models with source=mds events. --- app/controllers/mds/media_controller.rb | 59 +++++++ app/controllers/mds/metadata_controller.rb | 54 +++++++ app/services/mds/media_operations.rb | 104 ++++++++++++ app/services/mds/metadata_operations.rb | 180 +++++++++++++++++++++ 4 files changed, 397 insertions(+) create mode 100644 app/controllers/mds/media_controller.rb create mode 100644 app/controllers/mds/metadata_controller.rb create mode 100644 app/services/mds/media_operations.rb create mode 100644 app/services/mds/metadata_operations.rb diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb new file mode 100644 index 000000000..654429ef9 --- /dev/null +++ b/app/controllers/mds/media_controller.rb @@ -0,0 +1,59 @@ +# frozen_string_literal: true + +module Mds + class MediaController < Mds::ApplicationController + prepend_before_action :authenticate_mds_user! + before_action :set_doi + before_action :set_media, only: %i[show destroy] + + def index + result = Mds::MediaOperations.new(current_user: current_user).list(@doi) + render_result(result) + end + + def show + result = + Mds::MediaOperations.new(current_user: current_user).show(@doi, @id) + render_result(result) + end + + def create + result = + Mds::MediaOperations.new(current_user: current_user).create( + @doi, + data: request.raw_post, + ) + render_result(result) + end + + def destroy + result = + Mds::MediaOperations.new(current_user: current_user).destroy(@doi, @id) + render_result(result) + end + + private + + def set_doi + # Flat /media/:doi_id and nested /doi/:doi_id/media both expose :doi_id. + raw = params[:doi_id] + fail AbstractController::ActionNotFound if raw.blank? + + @doi = validate_doi(raw) + fail AbstractController::ActionNotFound if @doi.blank? + end + + def set_media + @id = params[:id] + fail AbstractController::ActionNotFound if @id.blank? + end + + def render_result(result) + if result.status == 204 + head :no_content + else + render plain: result.body.to_s, status: result.status + end + end + end +end diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb new file mode 100644 index 000000000..88d82a421 --- /dev/null +++ b/app/controllers/mds/metadata_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module Mds + class MetadataController < Mds::ApplicationController + prepend_before_action :authenticate_mds_user! + before_action :set_doi, only: %i[destroy] + + def show + @doi = validate_doi(params[:doi_id]) + fail AbstractController::ActionNotFound unless @doi.present? + + result = Mds::MetadataOperations.new(current_user: current_user).get(@doi) + + if result.status == 204 + head :no_content + elsif result.success? + render xml: result.body, status: :ok + else + render plain: result.body.to_s, status: result.status + end + end + + def create + if request.content_type.to_s.include?("application/x-www-form-urlencoded") + render plain: "Content type application/x-www-form-urlencoded is not supported", + status: :unsupported_media_type + return + end + + data = request.raw_post + result = + Mds::MetadataOperations.new(current_user: current_user).create( + doi_string: params[:doi_id], + data: data, + number: params[:number], + ) + + result.headers.each { |k, v| response.headers[k] = v } + render plain: result.body.to_s, status: result.status + end + + def destroy + result = Mds::MetadataOperations.new(current_user: current_user).destroy(@doi) + render plain: result.body.to_s, status: result.status + end + + private + + def set_doi + @doi = validate_doi(params[:doi_id]) + fail AbstractController::ActionNotFound unless @doi.present? + end + end +end diff --git a/app/services/mds/media_operations.rb b/app/services/mds/media_operations.rb new file mode 100644 index 000000000..b4c857b20 --- /dev/null +++ b/app/services/mds/media_operations.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +module Mds + # In-process media operations for classic MDS /media surface. + class MediaOperations + include Bolognese::DoiUtils + + attr_reader :current_user, :current_ability + + def initialize(current_user:) + @current_user = current_user + @current_ability = Ability.new(current_user) + end + + def list(doi_string) + doi = find_doi(doi_string) + return doi if doi.is_a?(Result) + + unless current_ability.can?(:read, doi) + return Result.error(403, "Access is denied") + end + + media = doi.media.to_a + return Result.error(404, "No media for the DOI") if media.blank? + + body = + media.map { |m| "#{m.media_type}=#{m.url}" }.join("\n") + Result.ok(body) + end + + def show(doi_string, media_id) + doi = find_doi(doi_string) + return doi if doi.is_a?(Result) + + unless current_ability.can?(:read, doi) + return Result.error(403, "Access is denied") + end + + media = find_media(doi, media_id) + return Result.error(404, "No media for the DOI") if media.blank? + + Result.ok("#{media.media_type}=#{media.url}") + end + + def create(doi_string, data:) + return Result.error(400, "Media type and URL missing") if data.blank? + + doi = find_doi(doi_string) + return doi if doi.is_a?(Result) + + unless current_ability.can?(:update, doi) + return Result.error(403, "Access is denied") + end + + media_type, url = data.to_s.split("=", 2) + media = Media.new(doi: doi, media_type: media_type, url: url) + + if media.save + Result.ok("OK") + else + message = media.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + end + + def destroy(doi_string, media_id) + doi = find_doi(doi_string) + return doi if doi.is_a?(Result) + + unless current_ability.can?(:update, doi) + return Result.error(403, "Access is denied") + end + + media = find_media(doi, media_id) + return Result.error(404, "No media for the DOI") if media.blank? + + if media.destroy + Result.ok("OK") + else + message = media.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + end + + private + + def find_doi(doi_string) + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + return Result.error(404, "DOI is unknown to MDS") if doi.blank? + + doi + end + + def find_media(doi, media_id) + id = Base32::URL.decode(CGI.unescape(media_id.to_s)) + return nil if id.blank? + + doi.media.where(id: id.to_i).first + end + end +end diff --git a/app/services/mds/metadata_operations.rb b/app/services/mds/metadata_operations.rb new file mode 100644 index 000000000..1396e57fb --- /dev/null +++ b/app/services/mds/metadata_operations.rb @@ -0,0 +1,180 @@ +# frozen_string_literal: true + +module Mds + # In-process metadata operations for classic MDS /metadata surface. + class MetadataOperations + include Bolognese::DoiUtils + include Bolognese::Utils + include Helpable + + UPPER_LIMIT = 1_073_741_823 + + attr_reader :current_user, :current_ability + + def initialize(current_user:) + @current_user = current_user + @current_ability = Ability.new(current_user) + end + + def get(doi_string) + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + return Result.error(404, "DOI is unknown to MDS") if doi.blank? + + unless current_ability.can?(:read, doi) + return Result.error(403, "Access is denied") + end + + xml = doi.xml + return Result.error(204, nil) if xml.blank? + + Result.ok(xml, status: 200) + end + + def create(doi_string: nil, data:, number: nil) + from = data.blank? ? "datacite" : find_from_format_by_string(data) + return Result.error(415, "Metadata format not recognized") if from.blank? + + doi_id = extract_doi(doi_string, data: data, from: from, number: number) + return Result.error(404, "DOI not found") if doi_id.blank? + + xml_b64 = data.present? ? Base64.strict_encode64(data) : nil + raw_attrs = { + doi: doi_id, + xml: xml_b64, + should_validate: true, + source: "mds", + event: "show", + client_id: client_symbol, + }.compact + + attrs = ParamsSanitizer.new(raw_attrs).cleanse + + doi = DataciteDoi.where(doi: doi_id).first + exists = doi.present? + + if exists + unless current_ability.can?(:update, doi) + return Result.error(403, "Access is denied") + end + + doi.current_user = current_user + doi.assign_attributes(attrs.except(:doi, :client_id)) + else + doi = DataciteDoi.new(attrs.merge(doi: doi_id)) + doi.current_user = current_user + unless current_ability.can?(:new, doi) + return Result.error(403, "Access is denied") + end + end + + if doi.save + minted = doi.doi.to_s.upcase + Result.created( + "OK (#{minted})", + headers: { "Location" => "#{Mds.url}/metadata/#{doi.doi}" }, + ) + else + message = doi.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + rescue ActiveRecord::RecordNotFound + Result.error(404, "DOI not found") + end + + def destroy(doi_string) + doi_id = validate_doi(doi_string) + return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + return Result.error(404, "DOI is unknown to MDS") if doi.blank? + + unless current_ability.can?(:update, doi) + return Result.error(403, "Access is denied") + end + + doi.current_user = current_user + doi.assign_attributes(event: "hide") + + if doi.save(validate: false) + Result.ok("OK") + else + message = doi.errors.full_messages.first || "Unprocessable entity" + Result.error(422, message) + end + end + + def find_from_format_by_string(string) + if Maremma.from_xml(string).to_h.dig("doi_records", "doi_record", "crossref").present? + "crossref" + elsif Nokogiri::XML(string, nil, "UTF-8", &:noblanks).collect_namespaces.detect { |_, v| v.to_s.start_with?("http://datacite.org/schema/kernel") } + "datacite" + elsif Maremma.from_json(string).to_h.dig("@context").to_s.start_with?("http://schema.org", "https://schema.org") + "schema_org" + elsif Maremma.from_json(string).to_h.dig("@context") == "https://raw.githubusercontent.com/codemeta/codemeta/master/codemeta.jsonld" + "codemeta" + elsif Maremma.from_json(string).to_h.dig("schema-version").to_s.start_with?("http://datacite.org/schema/kernel") + "datacite_json" + elsif Maremma.from_json(string).to_h.dig("types").present? + "crosscite" + elsif Maremma.from_json(string).to_h.dig("issued", "date-parts").present? + "citeproc" + elsif string.start_with?("TY - ") + "ris" + elsif begin + BibTeX.parse(string).first + rescue StandardError + nil + end + "bibtex" + end + rescue StandardError + nil + end + + def extract_doi(str, options = {}) + doi = validate_doi(str) + return doi if doi.present? + + if options[:from] == "datacite" + doi = doi_from_xml(str, options) + return doi if doi.present? + end + + generate_unique_doi(str, options) + end + + private + + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end + + def doi_from_xml(str, options = {}) + doc = Nokogiri::XML(str || options[:data], nil, "UTF-8", &:noblanks) + doc.remove_namespaces! + identifier = doc.at_css("identifier") + identifier = identifier.content if identifier.present? + validate_doi(identifier) + end + + def generate_unique_doi(str, options = {}) + if options[:number].present? + doi = generate_random_dois(str, number: options[:number]).first + existing = DataciteDoi.where(doi: doi).exists? + fail IdentifierError, "doi:#{doi} has already been registered" if existing + else + doi = nil + duplicate = true + while duplicate + doi = generate_random_dois(str, options).first + duplicate = !Rails.env.test? && DataciteDoi.where(doi: doi).exists? + end + end + + doi + end + end +end From aea545b1f0b6d27a8de5719188cdca91fbb5073b Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:19:45 +0200 Subject: [PATCH 04/31] Document MDS embed and dual-host nginx routing. Add MDS API notes to the README and a dedicated nginx server_name block for mds.* hosts that redirects / to the MDS support guide. --- README.md | 11 +++++++++ app/services/mds/metadata_operations.rb | 4 ++-- vendor/docker/webapp.conf.template | 30 ++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 66728d6d0..79c2a298b 100644 --- a/README.md +++ b/README.md @@ -13,6 +13,17 @@ Rails API application for managing DataCite providers, clients, prefixes and DOI * **DOI**: Digital Object Identifier, the core entity managed by the system. * **Prefix**: DOIs are assigned within a prefix (e.g., 10.1234). +## MDS API (classic Metadata Store protocol) + +Lupo can serve the [classic DataCite MDS API](https://support.datacite.org/docs/mds-api-guide) in-process (paths such as `/doi`, `/metadata`, `/media`), replacing the former standalone [Poodle](https://github.com/datacite/poodle) service. + +* Enable with `MDS_ENABLED=true`. +* Requests are accepted only when the HTTP `Host` is listed in `MDS_HOSTS` (e.g. `mds.datacite.org`, `mds.test.datacite.org`, `mds.local`). +* Public MDS URLs stay the same; only the backend behind those hosts changes. +* Non-MDS hosts continue to serve the JSON:API / GraphQL REST surface only. + +See `.env.example` for `MDS_ENABLED`, `MDS_HOSTS`, `MDS_URL`, and `MDS_REALM`. + ## Tech Stack * Ruby on Rails diff --git a/app/services/mds/metadata_operations.rb b/app/services/mds/metadata_operations.rb index 1396e57fb..e4d5fb757 100644 --- a/app/services/mds/metadata_operations.rb +++ b/app/services/mds/metadata_operations.rb @@ -28,9 +28,9 @@ def get(doi_string) end xml = doi.xml - return Result.error(204, nil) if xml.blank? + return Result.no_content if xml.blank? - Result.ok(xml, status: 200) + Result.ok(xml) end def create(doi_string: nil, data:, number: nil) diff --git a/vendor/docker/webapp.conf.template b/vendor/docker/webapp.conf.template index 4f602348a..8525ba2ee 100644 --- a/vendor/docker/webapp.conf.template +++ b/vendor/docker/webapp.conf.template @@ -1,5 +1,33 @@ passenger_max_pool_size ${PASSENGER_MAX_POOL_SIZE}; +# Classic MDS protocol hosts (embedded former Poodle surface). +# Host must also be listed in Rails ENV MDS_HOSTS with MDS_ENABLED=true. +server { + listen 80; + server_name mds.datacite.org mds.test.datacite.org mds.stage.datacite.org mds.local; + root /home/app/webapp/public; + + passenger_enabled on; + passenger_user app; + passenger_min_instances ${PASSENGER_MIN_INSTANCES}; + passenger_ruby /usr/bin/ruby; + passenger_preload_bundler on; + + merge_slashes off; + client_max_body_size 10M; + + error_log stderr; + + gzip on; + gzip_types text/plain application/xml; + gzip_proxied no-cache no-store private expired auth; + + location = / { + return 301 https://support.datacite.org/docs/mds-api-guide; + } +} + +# Default: DataCite REST API (api.datacite.org and catch-all). server { listen 80 default_server; server_name _; @@ -66,4 +94,4 @@ server { } } -passenger_pre_start http://localhost/heartbeat; \ No newline at end of file +passenger_pre_start http://localhost/heartbeat; From 9e564947a392039532507f44eac91b03fa66d3d5 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:20:42 +0200 Subject: [PATCH 05/31] Add MDS routing and request specs without running them. Cover host-constrained routing, DOI/metadata/media contract behaviors, auth failures, heartbeat/login, and Mds helper unit tests. --- spec/lib/mds_spec.rb | 34 ++++++ spec/requests/mds/dois_spec.rb | 161 +++++++++++++++++++++++++++++ spec/requests/mds/media_spec.rb | 106 +++++++++++++++++++ spec/requests/mds/metadata_spec.rb | 98 ++++++++++++++++++ spec/requests/mds/misc_spec.rb | 34 ++++++ spec/routing/mds_routing_spec.rb | 95 +++++++++++++++++ 6 files changed, 528 insertions(+) create mode 100644 spec/lib/mds_spec.rb create mode 100644 spec/requests/mds/dois_spec.rb create mode 100644 spec/requests/mds/media_spec.rb create mode 100644 spec/requests/mds/metadata_spec.rb create mode 100644 spec/requests/mds/misc_spec.rb create mode 100644 spec/routing/mds_routing_spec.rb diff --git a/spec/lib/mds_spec.rb b/spec/lib/mds_spec.rb new file mode 100644 index 000000000..e79a26be3 --- /dev/null +++ b/spec/lib/mds_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Mds do + describe ".enabled?" do + it "is true in the test environment by default" do + expect(Mds.enabled?).to be(true) + end + end + + describe ".host_match?" do + it "matches configured MDS hosts" do + request = double(host: "mds.local") + expect(Mds.host_match?(request)).to be(true) + end + + it "does not match unrelated hosts" do + request = double(host: "api.datacite.org") + expect(Mds.host_match?(request)).to be(false) + end + + it "is case-insensitive" do + request = double(host: "MDS.LOCAL") + expect(Mds.host_match?(request)).to be(true) + end + end + + describe ".hosts" do + it "returns a list of downcased hosts" do + expect(Mds.hosts).to include("mds.local") + end + end +end diff --git a/spec/requests/mds/dois_spec.rb b/spec/requests/mds/dois_spec.rb new file mode 100644 index 000000000..2add4acef --- /dev/null +++ b/spec/requests/mds/dois_spec.rb @@ -0,0 +1,161 @@ +# frozen_string_literal: true + +require "rails_helper" +include Passwordable + +describe "MDS Dois API", type: :request, vcr: true, prefix_pool_size: 1 do + let(:provider) do + create( + :provider, + symbol: "DATACITE", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let(:client) do + create( + :client, + provider: provider, + symbol: ENV["MDS_USERNAME"], + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:prefix) { create(:prefix, uid: "10.14454") } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let(:doi) do + create( + :doi, + client: client, + doi: "10.14454/mds-doi-1", + aasm_state: "draft", + url: nil, + ) + end + let(:findable_doi) do + create( + :doi, + client: client, + doi: "10.14454/mds-findable-1", + aasm_state: "findable", + url: "https://example.org/landing", + ) + end + + let(:mds_host) { { "HTTP_HOST" => "mds.local" } } + let(:basic_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + ENV["MDS_PASSWORD"], + ), + ) + end + + describe "authentication" do + it "returns 401 without credentials" do + get "/doi/#{doi.doi}", nil, mds_host + + expect(last_response.status).to eq(401) + expect(last_response.body).to match(/Bad credentials|Authentication/i) + end + + it "returns 401 with wrong password" do + headers = + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + "wrong-password", + ), + ) + get "/doi/#{doi.doi}", nil, headers + + expect(last_response.status).to eq(401) + expect(last_response.body).to eq("Bad credentials") + end + end + + describe "GET /doi/:id" do + it "returns the URL for a DOI with a known url attribute" do + findable_doi + get "/doi/#{findable_doi.doi}", nil, basic_headers + + # May be 200 with URL from attribute/handle, or 204 if handle lookup empty in test + expect([200, 204]).to include(last_response.status) + if last_response.status == 200 + expect(last_response.body).to be_present + end + end + + it "returns 404 for unknown DOI" do + get "/doi/10.14454/does-not-exist", nil, basic_headers + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("DOI not found") + end + end + + describe "PUT /doi/:id" do + it "publishes a URL for an existing draft DOI" do + doi + body = "doi=#{doi.doi}\nurl=https://example.org/new-landing" + put "/doi/#{doi.doi}", + body, + basic_headers.merge("CONTENT_TYPE" => "text/plain;charset=UTF-8") + + expect(last_response.status).to eq(201) + expect(last_response.body).to eq("OK") + expect(doi.reload.url).to eq("https://example.org/new-landing") + expect(doi).to be_findable + end + + it "rejects invalid URLs" do + doi + body = "doi=#{doi.doi}\nurl=not-a-url" + put "/doi/#{doi.doi}", + body, + basic_headers.merge("CONTENT_TYPE" => "text/plain;charset=UTF-8") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("Not a valid HTTP(S) or FTP URL") + end + end + + describe "DELETE /doi/:id" do + it "deletes a draft DOI" do + doi + delete "/doi/#{doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("OK") + expect(DataciteDoi.where(doi: doi.doi)).to be_empty + end + + it "does not delete a findable DOI" do + findable_doi + delete "/doi/#{findable_doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(405) + expect(DataciteDoi.where(doi: findable_doi.doi)).to exist + end + end + + describe "host isolation" do + it "does not serve MDS /doi on a non-MDS host" do + get "/doi/#{doi.doi}", + nil, + { + "HTTP_HOST" => "api.example.org", + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + ENV["MDS_PASSWORD"], + ), + } + + # Falls through to REST catch-all / content negotiation, not MDS plain-text contract + expect(last_response.status).not_to eq(200) if last_response.body == "OK" + expect(last_response.headers["Content-Type"].to_s).not_to eq("text/plain") if last_response.status == 404 + end + end +end diff --git a/spec/requests/mds/media_spec.rb b/spec/requests/mds/media_spec.rb new file mode 100644 index 000000000..826d9fe09 --- /dev/null +++ b/spec/requests/mds/media_spec.rb @@ -0,0 +1,106 @@ +# frozen_string_literal: true + +require "rails_helper" +include Passwordable + +describe "MDS Media API", type: :request, vcr: true, prefix_pool_size: 1 do + let(:provider) do + create( + :provider, + symbol: "DATACITE", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let(:client) do + create( + :client, + provider: provider, + symbol: ENV["MDS_USERNAME"], + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:prefix) { create(:prefix, uid: "10.14454") } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:doi) do + create( + :doi, + client: client, + doi: "10.14454/mds-media-1", + aasm_state: "findable", + ) + end + + let(:mds_host) { { "HTTP_HOST" => "mds.local" } } + let(:basic_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + ENV["MDS_PASSWORD"], + ), + ) + end + + describe "POST /media/:doi_id" do + it "creates media from mediaType=url body" do + post "/media/#{doi.doi}", + "application/pdf=https://example.org/file.pdf", + basic_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("OK") + expect(doi.media.count).to eq(1) + expect(doi.media.first.media_type).to eq("application/pdf") + expect(doi.media.first.url).to eq("https://example.org/file.pdf") + end + end + + describe "GET /media/:doi_id" do + it "lists media as mediaType=url lines" do + create( + :media, + doi: doi, + media_type: "application/pdf", + url: "https://example.org/a.pdf", + ) + create( + :media, + doi: doi, + media_type: "text/plain", + url: "https://example.org/a.txt", + ) + + get "/media/#{doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(200) + lines = last_response.body.split("\n") + expect(lines).to include("application/pdf=https://example.org/a.pdf") + expect(lines).to include("text/plain=https://example.org/a.txt") + end + + it "returns 404 when no media exist" do + get "/media/#{doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("No media for the DOI") + end + end + + describe "nested /doi/:doi_id/media" do + it "lists media via nested path" do + create( + :media, + doi: doi, + media_type: "application/json", + url: "https://example.org/data.json", + ) + + get "/doi/#{doi.doi}/media", nil, basic_headers + + expect(last_response.status).to eq(200) + expect(last_response.body).to include( + "application/json=https://example.org/data.json", + ) + end + end +end diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb new file mode 100644 index 000000000..69413a0a4 --- /dev/null +++ b/spec/requests/mds/metadata_spec.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require "rails_helper" +include Passwordable + +describe "MDS Metadata API", type: :request, vcr: true, prefix_pool_size: 1 do + let(:provider) do + create( + :provider, + symbol: "DATACITE", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let(:client) do + create( + :client, + provider: provider, + symbol: ENV["MDS_USERNAME"], + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:prefix) { create(:prefix, uid: "10.14454") } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let(:xml) { file_fixture("datacite.xml").read } + let(:doi_string) { "10.14454/4K3M-NYVG" } + + let(:mds_host) { { "HTTP_HOST" => "mds.local" } } + let(:basic_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + ENV["MDS_PASSWORD"], + ), + "CONTENT_TYPE" => "application/xml;charset=UTF-8", + ) + end + + describe "PUT /metadata/:doi_id" do + it "registers metadata and returns OK with DOI" do + put "/metadata/#{doi_string}", xml, basic_headers + + expect(last_response.status).to eq(201) + expect(last_response.body).to match(%r{\AOK \(10\.14454/4K3M-NYVG\)\z}i) + expect(last_response.headers["Location"]).to include("/metadata/") + + doi = DataciteDoi.where(doi: doi_string.downcase).first + expect(doi).to be_present + expect(doi.source).to eq("mds") + expect(doi.xml).to be_present + end + + it "rejects application/x-www-form-urlencoded" do + put "/metadata/#{doi_string}", + xml, + basic_headers.merge( + "CONTENT_TYPE" => "application/x-www-form-urlencoded", + ) + + expect(last_response.status).to eq(415) + expect(last_response.body).to include("not supported") + end + end + + describe "GET /metadata/:doi_id" do + it "returns XML for an existing DOI" do + put "/metadata/#{doi_string}", xml, basic_headers + get "/metadata/#{doi_string}", nil, basic_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to include("resource") + expect(last_response.body).to include("Eating your own Dog Food") + end + + it "returns 404 for unknown DOI" do + get "/metadata/10.14454/unknown-doi", + nil, + basic_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("DOI is unknown to MDS") + end + end + + describe "DELETE /metadata/:doi_id" do + it "hides a findable DOI (registered state)" do + put "/metadata/#{doi_string}", xml, basic_headers + doi = DataciteDoi.where(doi: doi_string.downcase).first + doi.update_columns(aasm_state: "findable") if doi.draft? || doi.registered? + + delete "/metadata/#{doi_string}", nil, basic_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("OK") + expect(doi.reload).to be_registered + end + end +end diff --git a/spec/requests/mds/misc_spec.rb b/spec/requests/mds/misc_spec.rb new file mode 100644 index 000000000..66f0fd2f5 --- /dev/null +++ b/spec/requests/mds/misc_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe "MDS misc endpoints", type: :request do + let(:mds_host) { { "HTTP_HOST" => "mds.local" } } + + describe "GET /heartbeat" do + it "returns OK without authentication" do + get "/heartbeat", nil, mds_host + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("OK") + end + end + + describe "GET /login" do + it "returns 501" do + get "/login", nil, mds_host + + expect(last_response.status).to eq(501) + expect(last_response.body).to include("session cookies not supported") + end + end + + describe "unknown path on MDS host" do + it "returns MDS-style not found" do + get "/not-a-real-mds-path", nil, mds_host + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("Resource not found") + end + end +end diff --git a/spec/routing/mds_routing_spec.rb b/spec/routing/mds_routing_spec.rb new file mode 100644 index 000000000..3d7e4c0a8 --- /dev/null +++ b/spec/routing/mds_routing_spec.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe "MDS routing", type: :routing do + let(:mds_host) { "mds.local" } + + def mds_url(path) + "http://#{mds_host}#{path}" + end + + describe "when Host is an MDS host" do + it "routes GET /doi to mds/dois#index" do + expect(get: mds_url("/doi")).to route_to("mds/dois#index") + end + + it "routes GET /doi/:id with slashes to mds/dois#show" do + expect(get: mds_url("/doi/10.14454/abc")).to route_to( + "mds/dois#show", id: "10.14454/abc", + ) + end + + it "routes PUT /doi/:id to mds/dois#update" do + expect(put: mds_url("/doi/10.14454/abc")).to route_to( + "mds/dois#update", id: "10.14454/abc", + ) + end + + it "routes DELETE /doi/:id to mds/dois#destroy" do + expect(delete: mds_url("/doi/10.14454/abc")).to route_to( + "mds/dois#destroy", id: "10.14454/abc", + ) + end + + it "routes POST /doi to mds/dois#update" do + expect(post: mds_url("/doi")).to route_to("mds/dois#update") + end + + it "routes PUT /metadata/:doi_id to mds/metadata#create" do + expect(put: mds_url("/metadata/10.14454/abc")).to route_to( + "mds/metadata#create", doi_id: "10.14454/abc", + ) + end + + it "routes GET /metadata/:doi_id to mds/metadata#show" do + expect(get: mds_url("/metadata/10.14454/abc")).to route_to( + "mds/metadata#show", doi_id: "10.14454/abc", + ) + end + + it "routes DELETE /metadata/:doi_id to mds/metadata#destroy" do + expect(delete: mds_url("/metadata/10.14454/abc")).to route_to( + "mds/metadata#destroy", doi_id: "10.14454/abc", + ) + end + + it "routes GET /media/:doi_id to mds/media#index" do + expect(get: mds_url("/media/10.14454/abc")).to route_to( + "mds/media#index", doi_id: "10.14454/abc", + ) + end + + it "routes POST /media/:doi_id to mds/media#create" do + expect(post: mds_url("/media/10.14454/abc")).to route_to( + "mds/media#create", doi_id: "10.14454/abc", + ) + end + + it "routes nested media under /doi" do + expect(get: mds_url("/doi/10.14454/abc/media")).to route_to( + "mds/media#index", doi_id: "10.14454/abc", + ) + end + + it "routes GET /heartbeat to mds/heartbeat#index" do + expect(get: mds_url("/heartbeat")).to route_to("mds/heartbeat#index") + end + + it "routes GET /login to mds/index#login" do + expect(get: mds_url("/login")).to route_to("mds/index#login") + end + end + + describe "when Host is not an MDS host" do + it "does not route classic /doi to MDS controllers" do + expect(get: "/doi/10.14454/abc").not_to route_to( + "mds/dois#show", id: "10.14454/abc", + ) + end + + it "still routes REST /dois" do + expect(get: "/dois").to route_to("datacite_dois#index") + end + end +end From 393919798ec3488957677fe0937526a22a002e0f Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:30:33 +0200 Subject: [PATCH 06/31] Fix MDS env bootstrap so test/dev enable safely. Stop setting MDS_ENABLED/MDS_HOSTS in application.rb before env files load. Restrict test/dev hosts to mds.local only so REST on default Rack hosts is not swallowed by the MDS catch-all. Tighten isolation specs. --- .env.example | 1 + config/application.rb | 14 ++++------ config/environments/development.rb | 5 ++-- config/environments/test.rb | 6 +++-- lib/mds.rb | 19 +++++++++++++- spec/lib/mds_spec.rb | 42 +++++++++++++++++++++++++++--- spec/requests/mds/dois_spec.rb | 29 +++++++++++++++++---- spec/routing/mds_routing_spec.rb | 13 ++++++--- 8 files changed, 102 insertions(+), 27 deletions(-) diff --git a/.env.example b/.env.example index 14665afe1..1f2e85867 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,7 @@ MDS_PASSWORD= # MDS protocol (embedded former Poodle API). Host-constrained routes under Mds:: # Set MDS_ENABLED=true and point mds.* hosts at this app to serve classic MDS paths. +# Only list true MDS hostnames — never api/localhost/example.org or REST will hit MDS catch-all. MDS_ENABLED=false MDS_HOSTS=mds.datacite.org,mds.test.datacite.org,mds.stage.datacite.org,mds.local MDS_URL=https://mds.test.datacite.org diff --git a/config/application.rb b/config/application.rb index 06b7b8324..000bbee29 100644 --- a/config/application.rb +++ b/config/application.rb @@ -74,15 +74,11 @@ # MDS (legacy Metadata Store protocol, formerly Poodle) # When MDS_ENABLED is true, hostnames in MDS_HOSTS serve classic MDS routes in-process. -ENV["MDS_ENABLED"] ||= "false" -ENV["MDS_HOSTS"] ||= "mds.datacite.org,mds.test.datacite.org,mds.stage.datacite.org,mds.local" -ENV["MDS_URL"] ||= - if Rails.env.production? - "https://mds.datacite.org" - else - "https://mds.test.datacite.org" - end -ENV["MDS_REALM"] ||= ENV["MDS_HOSTS"].to_s.split(",").first.to_s.strip.presence || "mds.datacite.org" +# Do NOT set MDS_ENABLED / MDS_HOSTS here — environment files and process env own those so +# test/dev can enable MDS without fighting a premature default, and production stays off +# unless explicitly enabled. Safe fallbacks are applied in lib/mds.rb and after_initialize. +ENV["MDS_URL"] ||= "https://mds.test.datacite.org" +ENV["MDS_REALM"] ||= "mds.datacite.org" module Lupo class Application < Rails::Application diff --git a/config/environments/development.rb b/config/environments/development.rb index 894ec7685..3a03c387d 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -59,8 +59,9 @@ ENV["SLACK_WEBHOOK_URL"] ||= "" # Keep empty if not sending Slack notifications in tests ENV["MDS_USERNAME"] ||= "DATACITE.TESTUSER" ENV["MDS_PASSWORD"] ||= "test_mds_password" - ENV["MDS_ENABLED"] ||= "true" - ENV["MDS_HOSTS"] ||= "mds.local,localhost,127.0.0.1" + # MDS only on mds.local so localhost REST (api) coexists. Map mds.local in /etc/hosts. + ENV["MDS_ENABLED"] = "true" if ENV["MDS_ENABLED"].nil? + ENV["MDS_HOSTS"] = "mds.local" if ENV["MDS_HOSTS"].nil? ENV["MDS_URL"] ||= "http://mds.local:8065" ENV["MDS_REALM"] ||= "mds.local" ENV["ADMIN_USERNAME"] ||= "DATACITE.TESTADMIN" diff --git a/config/environments/test.rb b/config/environments/test.rb index 1f85f2a8d..c60c7e42b 100644 --- a/config/environments/test.rb +++ b/config/environments/test.rb @@ -79,8 +79,10 @@ ENV["SLACK_WEBHOOK_URL"] ||= "" # Keep empty if not sending Slack notifications in tests ENV["MDS_USERNAME"] ||= "DATACITE.TESTUSER" ENV["MDS_PASSWORD"] ||= "test_mds_password" - ENV["MDS_ENABLED"] ||= "true" - ENV["MDS_HOSTS"] ||= "mds.local,www.example.com,example.org" + # Dedicated MDS host only — never Rack/Rails defaults (example.org / www.example.com) + # or REST request specs would hit the MDS catch-all. + ENV["MDS_ENABLED"] = "true" if ENV["MDS_ENABLED"].nil? + ENV["MDS_HOSTS"] = "mds.local" if ENV["MDS_HOSTS"].nil? ENV["MDS_URL"] ||= "https://mds.test.datacite.org" ENV["MDS_REALM"] ||= "mds.local" ENV["ADMIN_USERNAME"] ||= "DATACITE.TESTADMIN" diff --git a/lib/mds.rb b/lib/mds.rb index e741bf001..034fe41e4 100644 --- a/lib/mds.rb +++ b/lib/mds.rb @@ -2,14 +2,30 @@ # Helpers for the embedded MDS (legacy Metadata Store) protocol surface. module Mds + # Production-like hosts when MDS_HOSTS is unset (e.g. production with only MDS_ENABLED=true). + DEFAULT_HOSTS = %w[ + mds.datacite.org + mds.test.datacite.org + mds.stage.datacite.org + mds.local + ].freeze + module_function def enabled? + # Default off unless explicitly enabled (production-safe). ActiveModel::Type::Boolean.new.cast(ENV.fetch("MDS_ENABLED", "false")) end def hosts - ENV.fetch("MDS_HOSTS", "").split(",").map { |h| h.strip.downcase }.reject(&:blank?) + raw = ENV["MDS_HOSTS"].to_s + list = + if raw.blank? + DEFAULT_HOSTS + else + raw.split(",").map { |h| h.strip.downcase }.reject(&:blank?) + end + list end def host_match?(request) @@ -26,3 +42,4 @@ def realm ENV.fetch("MDS_REALM", "mds.datacite.org") end end + diff --git a/spec/lib/mds_spec.rb b/spec/lib/mds_spec.rb index e79a26be3..d31b4e372 100644 --- a/spec/lib/mds_spec.rb +++ b/spec/lib/mds_spec.rb @@ -3,10 +3,33 @@ require "rails_helper" describe Mds do + def with_env(key, value) + previous = ENV[key] + if value.nil? + ENV.delete(key) + else + ENV[key] = value + end + yield + ensure + if previous.nil? + ENV.delete(key) + else + ENV[key] = previous + end + end + describe ".enabled?" do it "is true in the test environment by default" do + # config/environments/test.rb enables MDS for contract specs on mds.local only expect(Mds.enabled?).to be(true) end + + it "is false when MDS_ENABLED is explicitly false" do + with_env("MDS_ENABLED", "false") do + expect(Mds.enabled?).to be(false) + end + end end describe ".host_match?" do @@ -15,20 +38,31 @@ expect(Mds.host_match?(request)).to be(true) end - it "does not match unrelated hosts" do + it "does not match Rack/Rails default hosts used by REST specs" do + expect(Mds.host_match?(double(host: "www.example.com"))).to be(false) + expect(Mds.host_match?(double(host: "example.org"))).to be(false) + end + + it "does not match unrelated API hosts" do request = double(host: "api.datacite.org") expect(Mds.host_match?(request)).to be(false) end - it "is case-insensitive" do + it "is case-insensitive for MDS hosts" do request = double(host: "MDS.LOCAL") expect(Mds.host_match?(request)).to be(true) end + + it "is false for all hosts when MDS is disabled" do + with_env("MDS_ENABLED", "false") do + expect(Mds.host_match?(double(host: "mds.local"))).to be(false) + end + end end describe ".hosts" do - it "returns a list of downcased hosts" do - expect(Mds.hosts).to include("mds.local") + it "returns the dedicated test MDS host only" do + expect(Mds.hosts).to eq(["mds.local"]) end end end diff --git a/spec/requests/mds/dois_spec.rb b/spec/requests/mds/dois_spec.rb index 2add4acef..0f137b171 100644 --- a/spec/requests/mds/dois_spec.rb +++ b/spec/requests/mds/dois_spec.rb @@ -141,11 +141,11 @@ end describe "host isolation" do - it "does not serve MDS /doi on a non-MDS host" do + it "does not serve MDS plain-text /doi on a non-MDS host" do get "/doi/#{doi.doi}", nil, { - "HTTP_HOST" => "api.example.org", + "HTTP_HOST" => "www.example.com", "HTTP_AUTHORIZATION" => ActionController::HttpAuthentication::Basic.encode_credentials( client.symbol, @@ -153,9 +153,28 @@ ), } - # Falls through to REST catch-all / content negotiation, not MDS plain-text contract - expect(last_response.status).not_to eq(200) if last_response.body == "OK" - expect(last_response.headers["Content-Type"].to_s).not_to eq("text/plain") if last_response.status == 404 + # Non-MDS host must not hit Mds::DoisController (plain "DOI not found" / URL body). + # Falls through to REST index/content-negotiation or routing error instead. + expect(last_response.body).not_to eq("DOI not found") + expect(last_response.headers["X-Credential-Username"]).not_to eq(client.symbol.downcase) if last_response.status == 401 + end + + it "still serves REST /dois on the default host" do + get "/dois", + nil, + { + "HTTP_HOST" => "www.example.com", + "HTTP_ACCEPT" => "application/vnd.api+json", + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + ENV["MDS_PASSWORD"], + ), + } + + expect(last_response.status).to eq(200) + expect(json).to have_key("data") end end end + diff --git a/spec/routing/mds_routing_spec.rb b/spec/routing/mds_routing_spec.rb index 3d7e4c0a8..2d52179ef 100644 --- a/spec/routing/mds_routing_spec.rb +++ b/spec/routing/mds_routing_spec.rb @@ -82,14 +82,19 @@ def mds_url(path) end describe "when Host is not an MDS host" do - it "does not route classic /doi to MDS controllers" do - expect(get: "/doi/10.14454/abc").not_to route_to( + it "does not route classic /doi on default host to MDS controllers" do + expect(get: "http://www.example.com/doi/10.14454/abc").not_to route_to( "mds/dois#show", id: "10.14454/abc", ) end - it "still routes REST /dois" do - expect(get: "/dois").to route_to("datacite_dois#index") + it "still routes REST /dois on the default host" do + expect(get: "http://www.example.com/dois").to route_to("datacite_dois#index") + end + + it "does not treat example.org as an MDS host" do + expect(get: "http://example.org/doi").not_to route_to("mds/dois#index") end end end + From 2f7a789a88e175a180b0c0e9cf3188dab3342c9f Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:35:11 +0200 Subject: [PATCH 07/31] Clarify MDS config comment after env bootstrap fix. Point to lib/mds.rb DEFAULT_HOSTS instead of a non-existent after_initialize. --- config/application.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/application.rb b/config/application.rb index 000bbee29..810e0a007 100644 --- a/config/application.rb +++ b/config/application.rb @@ -76,7 +76,7 @@ # When MDS_ENABLED is true, hostnames in MDS_HOSTS serve classic MDS routes in-process. # Do NOT set MDS_ENABLED / MDS_HOSTS here — environment files and process env own those so # test/dev can enable MDS without fighting a premature default, and production stays off -# unless explicitly enabled. Safe fallbacks are applied in lib/mds.rb and after_initialize. +# unless explicitly enabled. Safe fallbacks for blank hosts live in lib/mds.rb (DEFAULT_HOSTS). ENV["MDS_URL"] ||= "https://mds.test.datacite.org" ENV["MDS_REALM"] ||= "mds.datacite.org" From e105f5dec1d826b4841917e11903c242f3621433 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:59:11 +0200 Subject: [PATCH 08/31] Add Mds::Error and shared DOI support for MDS protocol layer. Introduce a single exception-based error model and one upsert/find/URL helper concern so MDS can stop forking domain logic per resource. --- app/controllers/concerns/mds/doi_support.rb | 137 ++++++++++++++++++++ lib/mds.rb | 3 +- lib/mds/error.rb | 14 ++ 3 files changed, 153 insertions(+), 1 deletion(-) create mode 100644 app/controllers/concerns/mds/doi_support.rb create mode 100644 lib/mds/error.rb diff --git a/app/controllers/concerns/mds/doi_support.rb b/app/controllers/concerns/mds/doi_support.rb new file mode 100644 index 000000000..1b8c16d8b --- /dev/null +++ b/app/controllers/concerns/mds/doi_support.rb @@ -0,0 +1,137 @@ +# frozen_string_literal: true + +module Mds + # Shared DOI domain helpers for MDS protocol controllers. + # Thin protocol adapters call these instead of a parallel *Operations stack. + module DoiSupport + extend ActiveSupport::Concern + + included do + include Bolognese::DoiUtils + include Bolognese::Utils + include Bolognese::MetadataUtils + include Helpable + end + + private + + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end + + # Load a DataciteDoi by validated DOI string or raise Mds::Error. + def find_datacite_doi!(doi_string, not_found: "DOI not found") + doi_id = validate_doi(doi_string) + fail Mds::Error.new(not_found, status: 404) if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + fail Mds::Error.new(not_found, status: 404) if doi.blank? + + doi + end + + # Single write path for MDS create/update of DataciteDoi records. + # Used by both PUT /doi (publish URL) and PUT /metadata (register metadata). + # Callers supply already-shaped attributes (metadata runs ParamsSanitizer first). + def upsert_datacite_doi!(doi_id, attributes) + attrs = attributes.to_h.compact.with_indifferent_access + doi = DataciteDoi.where(doi: doi_id).first + + if doi + fail Mds::Error.new("Access is denied", status: 403) unless can?(:update, doi) + + doi.current_user = current_user + doi.assign_attributes(attrs.except(:doi, :client_id)) + else + doi = DataciteDoi.new(attrs.merge(doi: doi_id)) + doi.current_user = current_user + fail Mds::Error.new("Access is denied", status: 403) unless can?(:new, doi) + end + + return doi if doi.save + + message = doi.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + + # Resolve landing URL the same way as DataciteDoisController#get_url domain logic: + # use stored url for draft/other/special providers; otherwise ask Handle via doi.get_url. + def resolve_landing_url(doi) + if !doi.is_registered_or_findable? || + %w[europ].include?(doi.provider_id) || + doi.type == "OtherDoi" + return doi.url + end + + response = doi.get_url + if response.status == 200 + response.body.dig("data", "values", 0, "data", "value") || doi.url + else + doi.url + end + end + + def valid_landing_url?(url) + url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) + end + + # Classic MDS body: "doi=...\nurl=..." lines (also used when path lacks url param). + def extract_doi_and_url_from_body(data, path_doi: nil) + hsh = + data.to_s.split("\n").map do |line| + arr = line.to_s.split("=", 2) + arr << "value" if arr.length < 2 + arr + end.to_h + + fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? + + body_doi = CGI.unescape(hsh["doi"].strip) + if path_doi.present? && body_doi.casecmp(path_doi) != 0 + fail IdentifierError, "doi parameter does not match doi of resource" + end + + fail IdentifierError, "param 'url' required" unless hsh["url"].present? + + [body_doi, CGI.unescape(hsh["url"].strip)] + end + + # Resolve DOI for metadata registration: path param, XML identifier, or mint. + def resolve_metadata_doi_id(str, data:, from:, number: nil) + doi = validate_doi(str) + return doi if doi.present? + + if from == "datacite" + doi = doi_from_xml_identifier(data) + return doi if doi.present? + end + + mint_unique_doi(str, number: number) + end + + def doi_from_xml_identifier(string) + doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) + doc.remove_namespaces! + identifier = doc.at_css("identifier") + identifier = identifier.content if identifier.present? + validate_doi(identifier) + end + + def mint_unique_doi(str, number: nil) + if number.present? + doi = generate_random_dois(str, number: number).first + existing = DataciteDoi.where(doi: doi).exists? + fail IdentifierError, "doi:#{doi} has already been registered" if existing + else + doi = nil + duplicate = true + while duplicate + doi = generate_random_dois(str, number: number).first + duplicate = !Rails.env.test? && DataciteDoi.where(doi: doi).exists? + end + end + + doi + end + end +end diff --git a/lib/mds.rb b/lib/mds.rb index 034fe41e4..d89b57be3 100644 --- a/lib/mds.rb +++ b/lib/mds.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "mds/error" + # Helpers for the embedded MDS (legacy Metadata Store) protocol surface. module Mds # Production-like hosts when MDS_HOSTS is unset (e.g. production with only MDS_ENABLED=true). @@ -42,4 +44,3 @@ def realm ENV.fetch("MDS_REALM", "mds.datacite.org") end end - diff --git a/lib/mds/error.rb b/lib/mds/error.rb new file mode 100644 index 000000000..7394f4a5f --- /dev/null +++ b/lib/mds/error.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Mds + # Protocol-level error for the embedded MDS surface. + # Controllers raise this; ApplicationController maps status + plain-text body. + class Error < StandardError + attr_reader :status + + def initialize(message, status:) + super(message) + @status = status.to_i + end + end +end From 48644106c75a58646b6bd5bd0cb5f17d247adf8f Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 12:59:14 +0200 Subject: [PATCH 09/31] Collapse MDS into thin protocol controllers over Lupo domain. Delete the parallel *Operations/Result stack. Controllers raise Mds::Error, share one DOI upsert, reuse REST-style auth parsing, Bolognese format detection, and Handle get_url resolution. Stop mapping NoMethodError to 422. --- app/controllers/mds/application_controller.rb | 131 +++++++------ app/controllers/mds/dois_controller.rb | 82 +++++--- app/controllers/mds/media_controller.rb | 71 ++++--- app/controllers/mds/metadata_controller.rb | 74 ++++--- app/services/mds/doi_operations.rb | 169 ---------------- app/services/mds/media_operations.rb | 104 ---------- app/services/mds/metadata_operations.rb | 180 ------------------ app/services/mds/result.rb | 35 ---- 8 files changed, 224 insertions(+), 622 deletions(-) delete mode 100644 app/services/mds/doi_operations.rb delete mode 100644 app/services/mds/media_operations.rb delete mode 100644 app/services/mds/metadata_operations.rb delete mode 100644 app/services/mds/result.rb diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 3903534e6..5ff4ce237 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -4,39 +4,61 @@ module Mds class ApplicationController < ActionController::API include ActionController::HttpAuthentication::Basic::ControllerMethods include CanCan::ControllerAdditions - include Bolognese::DoiUtils - include Bolognese::Utils + include Mds::DoiSupport attr_accessor :current_user after_action :set_consumer_header + # Protocol-facing errors always map to plain-text MDS responses. + rescue_from Mds::Error do |exception| + render_mds_error(exception.message, exception.status) + end + + rescue_from IdentifierError do |exception| + render_mds_error(exception.message, 400) + end + + rescue_from CanCan::AccessDenied do |_exception| + render_mds_error("Access is denied", 403) + end + + rescue_from CanCan::AuthorizationNotPerformed, + JWT::DecodeError, + JWT::VerificationError do |_exception| + render_mds_error("Bad credentials", 401) + end + + rescue_from ActiveRecord::RecordNotFound, + AbstractController::ActionNotFound do |_exception| + render_mds_error("DOI not found", 404) + end + def route_not_found render plain: "Resource not found", status: :not_found end protected - # Authenticate via HTTP Basic (classic MDS) or Bearer token. + # Align with REST ApplicationController#authenticate_user! credential parsing: + # Authorization header split + User.new(credentials, type: type). + # MDS still challenges with Basic realm and plain-text bodies. def authenticate_mds_user! type, credentials = type_and_credentials_from_request_headers if credentials.blank? - request_http_basic_authentication(Mds.realm, "An Authentication object was not found in the SecurityContext") + request_http_basic_authentication( + Mds.realm, + "An Authentication object was not found in the SecurityContext", + ) return false end - if type.to_s.casecmp("basic").zero? - @current_user = User.new(credentials, type: "basic") - else - # Bearer / raw JWT — same path as REST API - @current_user = User.new(credentials) - end + @current_user = User.new(credentials, type: type) - if @current_user.blank? || @current_user.errors.present? || @current_user.role_id == "anonymous" - response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" - response.headers.delete("X-Credential-Username") - render plain: "Bad credentials", status: :unauthorized + if @current_user.blank? || @current_user.errors.present? || + @current_user.role_id == "anonymous" + render_mds_error("Bad credentials", 401) return false end @@ -47,14 +69,9 @@ def current_ability @current_ability ||= Ability.new(current_user) end + # Same split as REST ApplicationController (Knock-style). def type_and_credentials_from_request_headers - header = request.headers["Authorization"] - return [nil, nil] if header.blank? - - type, credentials = header.split(" ", 2) - return [nil, nil] if credentials.blank? - - [type, credentials] + request.headers["Authorization"]&.split end def set_consumer_header @@ -65,49 +82,41 @@ def set_consumer_header end end - def client_symbol - (current_user.client_id.presence || current_user.uid).to_s + def render_mds(body = "OK", status: 200, headers: {}) + headers.each { |k, v| response.headers[k] = v } + + if status.to_i == 204 + head :no_content + else + render plain: body.to_s, status: status + end + end + + def render_mds_error(message, status) + if status.to_i == 401 + response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" + response.headers.delete("X-Credential-Username") + end + + logger.error "[MDS #{status}]: #{message}" + render plain: message.to_s, status: status end + # Unexpected framework errors — do not map NoMethodError to 422. unless Rails.env.development? - rescue_from(*RESCUABLE_EXCEPTIONS, IdentifierError) do |exception| - status = - case exception.class.to_s - when "CanCan::AuthorizationNotPerformed", "JWT::DecodeError", "JWT::VerificationError" - 401 - when "CanCan::AccessDenied" - 403 - when "ActionController::RoutingError", "AbstractController::ActionNotFound", - "ActiveRecord::RecordNotFound" - 404 - when "ActiveModel::ForbiddenAttributesError", "ActionController::UnpermittedParameters", - "NoMethodError" - 422 - when "NotImplementedError" - 501 - when "IdentifierError" - 400 - else - 400 - end - - if status == 401 - response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" - response.headers.delete("X-Credential-Username") - message = "Bad credentials" - elsif status == 403 - message = "Access is denied" - elsif status == 404 - message = "DOI not found" - elsif status == 501 - message = "Not Implemented" - else - Sentry.capture_exception(exception) unless exception.class.to_s == "IdentifierError" - message = exception.message - end - - logger.error "[MDS #{status}]: #{message}" - render plain: message, status: status + rescue_from ActionController::RoutingError do |_exception| + render_mds_error("DOI not found", 404) + end + + rescue_from ActiveModel::ForbiddenAttributesError, + ActionController::UnpermittedParameters, + ActionController::ParameterMissing do |exception| + Sentry.capture_exception(exception) + render_mds_error(exception.message, 422) + end + + rescue_from NotImplementedError do |_exception| + render_mds_error("Not Implemented", 501) end end end diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index 3c1038515..45cd100a5 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -1,61 +1,93 @@ # frozen_string_literal: true module Mds + # Classic MDS /doi surface — thin protocol adapter over DataciteDoi domain. class DoisController < Mds::ApplicationController prepend_before_action :authenticate_mds_user! before_action :set_doi, only: %i[show destroy] def index - result = Mds::DoiOperations.new(current_user: current_user).list - render_result(result) + authorize! :get_urls, Doi + + client = + Client.where("datacentre.symbol = ?", current_user.uid.upcase).first + client_prefix = client&.prefixes&.first + return head :no_content if client_prefix.blank? + + dois = + DataciteDoi.get_dois( + prefix: client_prefix.uid, + username: current_user.uid.upcase, + password: current_user.password, + ) + + return head :no_content if dois.blank? || !dois.is_a?(Array) || dois.empty? + + render_mds(dois.join("\n")) end def show - result = Mds::DoiOperations.new(current_user: current_user).get_url(@doi) - render_result(result) + authorize! :get_url, @doi + + url = resolve_landing_url(@doi) + return head :no_content if url.blank? + + render_mds(url) end def update - doi, url = parse_doi_and_url - return head :bad_request if doi.blank? || url.blank? + doi_string, url = parse_doi_and_url + return head :bad_request if doi_string.blank? || url.blank? + + fail Mds::Error.new("Not a valid HTTP(S) or FTP URL", status: 400) unless valid_landing_url?(url) + + doi_id = validate_doi(doi_string) + fail Mds::Error.new("DOI not found", status: 404) if doi_id.blank? + + upsert_datacite_doi!( + doi_id, + url: url, + should_validate: true, + source: "mds", + event: "publish", + client_id: client_symbol, + ) - result = Mds::DoiOperations.new(current_user: current_user).put_url(doi, url: url) - render_result(result) + render_mds("OK", status: 201) end def destroy - result = Mds::DoiOperations.new(current_user: current_user).destroy(@doi) - render_result(result) + authorize! :destroy, @doi + + unless @doi.draft? + fail Mds::Error.new("Method not allowed", status: 405) + end + + unless @doi.destroy + message = @doi.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + + render_mds("OK") end private def set_doi - @doi = validate_doi(params[:id]) - fail AbstractController::ActionNotFound if @doi.blank? + @doi = find_datacite_doi!(params[:id], not_found: "DOI not found") end def parse_doi_and_url if (params[:id].present? || params[:doi].present?) && params[:url].present? [params[:id].presence || params[:doi], params[:url]] elsif request.raw_post.present? - Mds::DoiOperations.extract_url( - doi: validate_doi(params[:id]), - data: request.raw_post, + extract_doi_and_url_from_body( + request.raw_post, + path_doi: validate_doi(params[:id]), ) else [nil, nil] end end - - def render_result(result) - result.headers.each { |k, v| response.headers[k] = v } - - if result.status == 204 - head :no_content - else - render plain: result.body.to_s, status: result.status - end - end end end diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index 654429ef9..7acb06f90 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -1,35 +1,58 @@ # frozen_string_literal: true module Mds + # Classic MDS /media surface — thin protocol adapter over Media AR association. class MediaController < Mds::ApplicationController prepend_before_action :authenticate_mds_user! before_action :set_doi before_action :set_media, only: %i[show destroy] def index - result = Mds::MediaOperations.new(current_user: current_user).list(@doi) - render_result(result) + authorize! :read, @doi + + media = @doi.media.to_a + fail Mds::Error.new("No media for the DOI", status: 404) if media.blank? + + body = media.map { |m| "#{m.media_type}=#{m.url}" }.join("\n") + render_mds(body) end def show - result = - Mds::MediaOperations.new(current_user: current_user).show(@doi, @id) - render_result(result) + authorize! :read, @doi + + fail Mds::Error.new("No media for the DOI", status: 404) if @media.blank? + + render_mds("#{@media.media_type}=#{@media.url}") end def create - result = - Mds::MediaOperations.new(current_user: current_user).create( - @doi, - data: request.raw_post, - ) - render_result(result) + authorize! :update, @doi + + data = request.raw_post + fail Mds::Error.new("Media type and URL missing", status: 400) if data.blank? + + media_type, url = data.to_s.split("=", 2) + media = Media.new(doi: @doi, media_type: media_type, url: url) + + unless media.save + message = media.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + + render_mds("OK") end def destroy - result = - Mds::MediaOperations.new(current_user: current_user).destroy(@doi, @id) - render_result(result) + authorize! :update, @doi + + fail Mds::Error.new("No media for the DOI", status: 404) if @media.blank? + + unless @media.destroy + message = @media.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + + render_mds("OK") end private @@ -37,23 +60,19 @@ def destroy def set_doi # Flat /media/:doi_id and nested /doi/:doi_id/media both expose :doi_id. raw = params[:doi_id] - fail AbstractController::ActionNotFound if raw.blank? + fail Mds::Error.new("DOI is unknown to MDS", status: 404) if raw.blank? - @doi = validate_doi(raw) - fail AbstractController::ActionNotFound if @doi.blank? + @doi = find_datacite_doi!(raw, not_found: "DOI is unknown to MDS") end def set_media - @id = params[:id] - fail AbstractController::ActionNotFound if @id.blank? - end + encoded = params[:id] + fail Mds::Error.new("No media for the DOI", status: 404) if encoded.blank? - def render_result(result) - if result.status == 204 - head :no_content - else - render plain: result.body.to_s, status: result.status - end + id = Base32::URL.decode(CGI.unescape(encoded.to_s)) + fail Mds::Error.new("No media for the DOI", status: 404) if id.blank? + + @media = @doi.media.where(id: id.to_i).first end end end diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index 88d82a421..abc58258b 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -1,54 +1,84 @@ # frozen_string_literal: true module Mds + # Classic MDS /metadata surface — thin protocol adapter over DataciteDoi domain. class MetadataController < Mds::ApplicationController prepend_before_action :authenticate_mds_user! before_action :set_doi, only: %i[destroy] def show - @doi = validate_doi(params[:doi_id]) - fail AbstractController::ActionNotFound unless @doi.present? + doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") + authorize! :read, doi - result = Mds::MetadataOperations.new(current_user: current_user).get(@doi) + xml = doi.xml + return head :no_content if xml.blank? - if result.status == 204 - head :no_content - elsif result.success? - render xml: result.body, status: :ok - else - render plain: result.body.to_s, status: result.status - end + render xml: xml, status: :ok end def create if request.content_type.to_s.include?("application/x-www-form-urlencoded") - render plain: "Content type application/x-www-form-urlencoded is not supported", - status: :unsupported_media_type - return + fail Mds::Error.new( + "Content type application/x-www-form-urlencoded is not supported", + status: 415, + ) end data = request.raw_post - result = - Mds::MetadataOperations.new(current_user: current_user).create( - doi_string: params[:doi_id], + # Reuse Bolognese format detection (via MetadataUtils), not a forked copy. + from = data.blank? ? "datacite" : find_from_format(string: data) + fail Mds::Error.new("Metadata format not recognized", status: 415) if from.blank? + + doi_id = + resolve_metadata_doi_id( + params[:doi_id], data: data, + from: from, number: params[:number], ) + fail Mds::Error.new("DOI not found", status: 404) if doi_id.blank? + + xml_b64 = data.present? ? Base64.strict_encode64(data) : nil + attrs = + ParamsSanitizer.new( + { + doi: doi_id, + xml: xml_b64, + should_validate: true, + source: "mds", + event: "show", + client_id: client_symbol, + }.compact, + ).cleanse + + doi = upsert_datacite_doi!(doi_id, attrs) - result.headers.each { |k, v| response.headers[k] = v } - render plain: result.body.to_s, status: result.status + minted = doi.doi.to_s.upcase + render_mds( + "OK (#{minted})", + status: 201, + headers: { "Location" => "#{Mds.url}/metadata/#{doi.doi}" }, + ) end def destroy - result = Mds::MetadataOperations.new(current_user: current_user).destroy(@doi) - render plain: result.body.to_s, status: result.status + authorize! :update, @doi + + @doi.current_user = current_user + @doi.assign_attributes(event: "hide") + + unless @doi.save(validate: false) + message = @doi.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + + render_mds("OK") end private def set_doi - @doi = validate_doi(params[:doi_id]) - fail AbstractController::ActionNotFound unless @doi.present? + @doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") end end end diff --git a/app/services/mds/doi_operations.rb b/app/services/mds/doi_operations.rb deleted file mode 100644 index eff7a6a38..000000000 --- a/app/services/mds/doi_operations.rb +++ /dev/null @@ -1,169 +0,0 @@ -# frozen_string_literal: true - -module Mds - # In-process DOI operations for the classic MDS /doi surface. - class DoiOperations - include Bolognese::DoiUtils - - attr_reader :current_user, :current_ability - - def initialize(current_user:) - @current_user = current_user - @current_ability = Ability.new(current_user) - end - - def list - client = - Client.where("datacentre.symbol = ?", current_user.uid.upcase).first - return Result.no_content if client.blank? - - client_prefix = client.prefixes.first - return Result.no_content if client_prefix.blank? - - unless current_ability.can?(:get_urls, Doi) - return Result.error(403, "Access is denied") - end - - dois = - DataciteDoi.get_dois( - prefix: client_prefix.uid, - username: current_user.uid.upcase, - password: current_user.password, - ) - - if dois.blank? || !dois.is_a?(Array) || dois.empty? - return Result.no_content - end - - Result.ok(dois.join("\n")) - end - - def get_url(doi_string) - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI not found") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - return Result.error(404, "DOI not found") if doi.blank? - - unless current_ability.can?(:get_url, doi) - return Result.error(403, "Access is denied") - end - - url = resolve_url(doi) - return Result.no_content if url.blank? - - Result.ok(url) - end - - def put_url(doi_string, url:) - return Result.error(400, "Not a valid HTTP(S) or FTP URL") unless valid_landing_url?(url) - - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI not found") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - exists = doi.present? - - attrs = { - url: url, - should_validate: true, - source: "mds", - event: "publish", - client_id: client_symbol, - } - - if exists - unless current_ability.can?(:update, doi) - return Result.error(403, "Access is denied") - end - - doi.current_user = current_user - doi.assign_attributes(attrs.except(:client_id)) - else - doi = DataciteDoi.new(attrs.merge(doi: doi_id)) - doi.current_user = current_user - unless current_ability.can?(:new, doi) - return Result.error(403, "Access is denied") - end - end - - if doi.save - Result.created("OK") - else - message = doi.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - rescue ActiveRecord::RecordNotFound - Result.error(404, "DOI not found") - end - - def destroy(doi_string) - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI not found") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - return Result.error(404, "DOI not found") if doi.blank? - - unless current_ability.can?(:destroy, doi) - return Result.error(403, "Access is denied") - end - - unless doi.draft? - return Result.error(405, "Method not allowed") - end - - if doi.destroy - Result.ok("OK") - else - message = doi.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - end - - # Parse classic MDS body: "doi=...\nurl=..." lines. - def self.extract_url(doi: nil, data: nil) - hsh = - data.to_s.split("\n").map do |line| - arr = line.to_s.split("=", 2) - arr << "value" if arr.length < 2 - arr - end.to_h - - fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? - - body_doi = CGI.unescape(hsh["doi"].strip) - if doi.present? && body_doi.casecmp(doi) != 0 - fail IdentifierError, "doi parameter does not match doi of resource" - end - - fail IdentifierError, "param 'url' required" unless hsh["url"].present? - - [body_doi, CGI.unescape(hsh["url"].strip)] - end - - private - - def client_symbol - (current_user.client_id.presence || current_user.uid).to_s - end - - def valid_landing_url?(url) - url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) - end - - def resolve_url(doi) - if !doi.is_registered_or_findable? || - %w[europ].include?(doi.provider_id) || - doi.type == "OtherDoi" - return doi.url - end - - response = doi.get_url - if response.status == 200 - response.body.dig("data", "values", 0, "data", "value") || doi.url - else - doi.url - end - end - end -end diff --git a/app/services/mds/media_operations.rb b/app/services/mds/media_operations.rb deleted file mode 100644 index b4c857b20..000000000 --- a/app/services/mds/media_operations.rb +++ /dev/null @@ -1,104 +0,0 @@ -# frozen_string_literal: true - -module Mds - # In-process media operations for classic MDS /media surface. - class MediaOperations - include Bolognese::DoiUtils - - attr_reader :current_user, :current_ability - - def initialize(current_user:) - @current_user = current_user - @current_ability = Ability.new(current_user) - end - - def list(doi_string) - doi = find_doi(doi_string) - return doi if doi.is_a?(Result) - - unless current_ability.can?(:read, doi) - return Result.error(403, "Access is denied") - end - - media = doi.media.to_a - return Result.error(404, "No media for the DOI") if media.blank? - - body = - media.map { |m| "#{m.media_type}=#{m.url}" }.join("\n") - Result.ok(body) - end - - def show(doi_string, media_id) - doi = find_doi(doi_string) - return doi if doi.is_a?(Result) - - unless current_ability.can?(:read, doi) - return Result.error(403, "Access is denied") - end - - media = find_media(doi, media_id) - return Result.error(404, "No media for the DOI") if media.blank? - - Result.ok("#{media.media_type}=#{media.url}") - end - - def create(doi_string, data:) - return Result.error(400, "Media type and URL missing") if data.blank? - - doi = find_doi(doi_string) - return doi if doi.is_a?(Result) - - unless current_ability.can?(:update, doi) - return Result.error(403, "Access is denied") - end - - media_type, url = data.to_s.split("=", 2) - media = Media.new(doi: doi, media_type: media_type, url: url) - - if media.save - Result.ok("OK") - else - message = media.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - end - - def destroy(doi_string, media_id) - doi = find_doi(doi_string) - return doi if doi.is_a?(Result) - - unless current_ability.can?(:update, doi) - return Result.error(403, "Access is denied") - end - - media = find_media(doi, media_id) - return Result.error(404, "No media for the DOI") if media.blank? - - if media.destroy - Result.ok("OK") - else - message = media.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - end - - private - - def find_doi(doi_string) - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - return Result.error(404, "DOI is unknown to MDS") if doi.blank? - - doi - end - - def find_media(doi, media_id) - id = Base32::URL.decode(CGI.unescape(media_id.to_s)) - return nil if id.blank? - - doi.media.where(id: id.to_i).first - end - end -end diff --git a/app/services/mds/metadata_operations.rb b/app/services/mds/metadata_operations.rb deleted file mode 100644 index e4d5fb757..000000000 --- a/app/services/mds/metadata_operations.rb +++ /dev/null @@ -1,180 +0,0 @@ -# frozen_string_literal: true - -module Mds - # In-process metadata operations for classic MDS /metadata surface. - class MetadataOperations - include Bolognese::DoiUtils - include Bolognese::Utils - include Helpable - - UPPER_LIMIT = 1_073_741_823 - - attr_reader :current_user, :current_ability - - def initialize(current_user:) - @current_user = current_user - @current_ability = Ability.new(current_user) - end - - def get(doi_string) - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - return Result.error(404, "DOI is unknown to MDS") if doi.blank? - - unless current_ability.can?(:read, doi) - return Result.error(403, "Access is denied") - end - - xml = doi.xml - return Result.no_content if xml.blank? - - Result.ok(xml) - end - - def create(doi_string: nil, data:, number: nil) - from = data.blank? ? "datacite" : find_from_format_by_string(data) - return Result.error(415, "Metadata format not recognized") if from.blank? - - doi_id = extract_doi(doi_string, data: data, from: from, number: number) - return Result.error(404, "DOI not found") if doi_id.blank? - - xml_b64 = data.present? ? Base64.strict_encode64(data) : nil - raw_attrs = { - doi: doi_id, - xml: xml_b64, - should_validate: true, - source: "mds", - event: "show", - client_id: client_symbol, - }.compact - - attrs = ParamsSanitizer.new(raw_attrs).cleanse - - doi = DataciteDoi.where(doi: doi_id).first - exists = doi.present? - - if exists - unless current_ability.can?(:update, doi) - return Result.error(403, "Access is denied") - end - - doi.current_user = current_user - doi.assign_attributes(attrs.except(:doi, :client_id)) - else - doi = DataciteDoi.new(attrs.merge(doi: doi_id)) - doi.current_user = current_user - unless current_ability.can?(:new, doi) - return Result.error(403, "Access is denied") - end - end - - if doi.save - minted = doi.doi.to_s.upcase - Result.created( - "OK (#{minted})", - headers: { "Location" => "#{Mds.url}/metadata/#{doi.doi}" }, - ) - else - message = doi.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - rescue ActiveRecord::RecordNotFound - Result.error(404, "DOI not found") - end - - def destroy(doi_string) - doi_id = validate_doi(doi_string) - return Result.error(404, "DOI is unknown to MDS") if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - return Result.error(404, "DOI is unknown to MDS") if doi.blank? - - unless current_ability.can?(:update, doi) - return Result.error(403, "Access is denied") - end - - doi.current_user = current_user - doi.assign_attributes(event: "hide") - - if doi.save(validate: false) - Result.ok("OK") - else - message = doi.errors.full_messages.first || "Unprocessable entity" - Result.error(422, message) - end - end - - def find_from_format_by_string(string) - if Maremma.from_xml(string).to_h.dig("doi_records", "doi_record", "crossref").present? - "crossref" - elsif Nokogiri::XML(string, nil, "UTF-8", &:noblanks).collect_namespaces.detect { |_, v| v.to_s.start_with?("http://datacite.org/schema/kernel") } - "datacite" - elsif Maremma.from_json(string).to_h.dig("@context").to_s.start_with?("http://schema.org", "https://schema.org") - "schema_org" - elsif Maremma.from_json(string).to_h.dig("@context") == "https://raw.githubusercontent.com/codemeta/codemeta/master/codemeta.jsonld" - "codemeta" - elsif Maremma.from_json(string).to_h.dig("schema-version").to_s.start_with?("http://datacite.org/schema/kernel") - "datacite_json" - elsif Maremma.from_json(string).to_h.dig("types").present? - "crosscite" - elsif Maremma.from_json(string).to_h.dig("issued", "date-parts").present? - "citeproc" - elsif string.start_with?("TY - ") - "ris" - elsif begin - BibTeX.parse(string).first - rescue StandardError - nil - end - "bibtex" - end - rescue StandardError - nil - end - - def extract_doi(str, options = {}) - doi = validate_doi(str) - return doi if doi.present? - - if options[:from] == "datacite" - doi = doi_from_xml(str, options) - return doi if doi.present? - end - - generate_unique_doi(str, options) - end - - private - - def client_symbol - (current_user.client_id.presence || current_user.uid).to_s - end - - def doi_from_xml(str, options = {}) - doc = Nokogiri::XML(str || options[:data], nil, "UTF-8", &:noblanks) - doc.remove_namespaces! - identifier = doc.at_css("identifier") - identifier = identifier.content if identifier.present? - validate_doi(identifier) - end - - def generate_unique_doi(str, options = {}) - if options[:number].present? - doi = generate_random_dois(str, number: options[:number]).first - existing = DataciteDoi.where(doi: doi).exists? - fail IdentifierError, "doi:#{doi} has already been registered" if existing - else - doi = nil - duplicate = true - while duplicate - doi = generate_random_dois(str, options).first - duplicate = !Rails.env.test? && DataciteDoi.where(doi: doi).exists? - end - end - - doi - end - end -end diff --git a/app/services/mds/result.rb b/app/services/mds/result.rb deleted file mode 100644 index 150970e36..000000000 --- a/app/services/mds/result.rb +++ /dev/null @@ -1,35 +0,0 @@ -# frozen_string_literal: true - -module Mds - # Simple result object for MDS operations (status + body/message + optional headers). - class Result - attr_reader :status, :body, :headers, :error - - def initialize(status:, body: nil, headers: {}, error: nil) - @status = status - @body = body - @headers = headers - @error = error - end - - def success? - status.to_i.between?(200, 299) - end - - def self.ok(body = "OK", status: 200, headers: {}) - new(status: status, body: body, headers: headers) - end - - def self.created(body = "OK", headers: {}) - new(status: 201, body: body, headers: headers) - end - - def self.no_content - new(status: 204, body: nil) - end - - def self.error(status, message) - new(status: status, body: message, error: message) - end - end -end From 51f4ba977984e31fea0f309a85bfbd9142642dcf Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:08:08 +0200 Subject: [PATCH 10/31] Move landing URL policy onto DataciteDoi domain helpers. Add uses_stored_landing_url? and resolved_landing_url on Helpable so REST and MDS share one stored-vs-Handle decision instead of forked controller logic. --- app/controllers/datacite_dois_controller.rb | 53 ++++++++++----------- app/models/concerns/helpable.rb | 18 +++++++ 2 files changed, 43 insertions(+), 28 deletions(-) diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb index c7c319dd9..7007f8001 100644 --- a/app/controllers/datacite_dois_controller.rb +++ b/app/controllers/datacite_dois_controller.rb @@ -721,38 +721,35 @@ def get_url authorize! :get_url, @doi - if !@doi.is_registered_or_findable? || - %w[europ].include?(@doi.provider_id) || - @doi.type == "OtherDoi" + # Domain owns stored-vs-Handle policy via uses_stored_landing_url? / resolved_landing_url. + if @doi.uses_stored_landing_url? url = @doi.url head :no_content && return if url.blank? - else - response = @doi.get_url - - if response.status == 200 - url = response.body.dig("data", "values", 0, "data", "value") - elsif response.status == 400 && - response.body.dig("errors", 0, "title", "responseCode") == 301 - response = - OpenStruct.new( - status: 403, - body: { - "errors" => [ - { - "status" => 403, - "title" => "SERVER NOT RESPONSIBLE FOR HANDLE", - }, - ], - }, - ) - url = nil - else - url = nil - end + render json: { url: url }.to_json, status: :ok + return end - if url.present? - render json: { url: url }.to_json, status: :ok + response = @doi.get_url + + if response.status == 200 + url = response.body.dig("data", "values", 0, "data", "value") + if url.present? + render json: { url: url }.to_json, status: :ok + else + render json: response.body.to_json, + status: response.status || :bad_request + end + elsif response.status == 400 && + response.body.dig("errors", 0, "title", "responseCode") == 301 + render json: { + "errors" => [ + { + "status" => 403, + "title" => "SERVER NOT RESPONSIBLE FOR HANDLE", + }, + ], + }.to_json, + status: :forbidden else render json: response.body.to_json, status: response.status || :bad_request diff --git a/app/models/concerns/helpable.rb b/app/models/concerns/helpable.rb index 4038f3926..77b212cfc 100644 --- a/app/models/concerns/helpable.rb +++ b/app/models/concerns/helpable.rb @@ -106,6 +106,24 @@ def get_url response end + # When true, the stored `url` attribute is authoritative (draft/other/special providers). + # When false, resolve via Handle (`get_url`). Shared by REST and MDS protocol surfaces. + def uses_stored_landing_url? + !is_registered_or_findable? || + %w[europ].include?(provider_id) || + type == "OtherDoi" + end + + # Landing URL for protocol responses. Nil when unknown or Handle has no value. + def resolved_landing_url + return url if uses_stored_landing_url? + + response = get_url + return nil unless response.status == 200 + + response.body.dig("data", "values", 0, "data", "value") + end + def generate_random_provider_symbol "4:X".gen end From 99d21ef4157e8eff03f69f5eca2f002b366aa969 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:08:08 +0200 Subject: [PATCH 11/31] Share request credential parsing between REST and MDS. Extract RequestCredentials so both ApplicationController and MDS auth use the same Authorization header split, User construction, and JWT blacklist check. --- app/controllers/application_controller.rb | 17 +++---------- .../concerns/request_credentials.rb | 25 +++++++++++++++++++ app/controllers/mds/application_controller.rb | 19 ++++++-------- 3 files changed, 35 insertions(+), 26 deletions(-) create mode 100644 app/controllers/concerns/request_credentials.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 6babf971d..953ab8d07 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -3,6 +3,7 @@ class ApplicationController < ActionController::API include ActionController::HttpAuthentication::Basic::ControllerMethods include Authenticable + include RequestCredentials include CanCan::ControllerAdditions include ErrorSerializable require "facets/string/snakecase" @@ -71,15 +72,8 @@ def authenticate_user_with_basic_auth! end def authenticate_user! - type, credentials = type_and_credentials_from_request_headers - - return false if credentials.blank? - - if (ENV["JWT_BLACKLISTED"] || "").split(",").include?(credentials) - raise JWT::VerificationError - end - - @current_user = User.new(credentials, type: type) + @current_user = user_from_request_credentials + return false if @current_user.nil? fail CanCan::AuthorizationNotPerformed if @current_user.errors.present? @@ -90,11 +84,6 @@ def current_ability @current_ability ||= Ability.new(current_user) end - # based on https://github.com/nsarno/knock/blob/master/lib/knock/authenticable.rb - def type_and_credentials_from_request_headers - request.headers["Authorization"]&.split - end - def authenticated_user current_user.try(:uid) end diff --git a/app/controllers/concerns/request_credentials.rb b/app/controllers/concerns/request_credentials.rb new file mode 100644 index 000000000..40a861a55 --- /dev/null +++ b/app/controllers/concerns/request_credentials.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +# Shared Authorization-header credential parsing for REST and MDS controllers. +module RequestCredentials + extend ActiveSupport::Concern + + # based on https://github.com/nsarno/knock/blob/master/lib/knock/authenticable.rb + def type_and_credentials_from_request_headers + request.headers["Authorization"]&.split + end + + # Build a User from the request Authorization header. + # Returns nil when credentials are missing. + # Raises JWT::VerificationError when the token is blacklisted. + def user_from_request_credentials + type, credentials = type_and_credentials_from_request_headers + return if credentials.blank? + + if (ENV["JWT_BLACKLISTED"] || "").split(",").include?(credentials) + raise JWT::VerificationError + end + + User.new(credentials, type: type) + end +end diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 5ff4ce237..14f1eaac7 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -1,10 +1,12 @@ # frozen_string_literal: true module Mds + # Protocol base: auth challenge, plain-text errors, consumer headers. + # Domain helpers (lookup/write/mint) are included only on controllers that need them. class ApplicationController < ActionController::API include ActionController::HttpAuthentication::Basic::ControllerMethods include CanCan::ControllerAdditions - include Mds::DoiSupport + include RequestCredentials attr_accessor :current_user @@ -40,13 +42,11 @@ def route_not_found protected - # Align with REST ApplicationController#authenticate_user! credential parsing: - # Authorization header split + User.new(credentials, type: type). - # MDS still challenges with Basic realm and plain-text bodies. + # MDS-specific challenge and plain-text failure bodies; credentials via RequestCredentials. def authenticate_mds_user! - type, credentials = type_and_credentials_from_request_headers + user = user_from_request_credentials - if credentials.blank? + if user.nil? request_http_basic_authentication( Mds.realm, "An Authentication object was not found in the SecurityContext", @@ -54,7 +54,7 @@ def authenticate_mds_user! return false end - @current_user = User.new(credentials, type: type) + @current_user = user if @current_user.blank? || @current_user.errors.present? || @current_user.role_id == "anonymous" @@ -69,11 +69,6 @@ def current_ability @current_ability ||= Ability.new(current_user) end - # Same split as REST ApplicationController (Knock-style). - def type_and_credentials_from_request_headers - request.headers["Authorization"]&.split - end - def set_consumer_header if current_user&.uid.present? response.headers["X-Credential-Username"] = current_user.uid From 504fc61e161092e15385345e9616321dfb8cbce0 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:08:08 +0200 Subject: [PATCH 12/31] Split MDS DOI helpers off the protocol base controller. Replace kitchen-sink DoiSupport with DoiLookup/DoiWriter includes only where needed, move minting to Mds::DoiMinter, use authorize! only in upsert, and drop the Rails.env.test? uniqueness bypass. --- app/controllers/concerns/mds/doi_lookup.rb | 29 +++++ app/controllers/concerns/mds/doi_support.rb | 137 -------------------- app/controllers/concerns/mds/doi_writer.rb | 35 +++++ app/controllers/mds/dois_controller.rb | 29 ++++- app/controllers/mds/media_controller.rb | 2 + app/controllers/mds/metadata_controller.rb | 6 +- app/services/mds/doi_minter.rb | 50 +++++++ 7 files changed, 148 insertions(+), 140 deletions(-) create mode 100644 app/controllers/concerns/mds/doi_lookup.rb delete mode 100644 app/controllers/concerns/mds/doi_support.rb create mode 100644 app/controllers/concerns/mds/doi_writer.rb create mode 100644 app/services/mds/doi_minter.rb diff --git a/app/controllers/concerns/mds/doi_lookup.rb b/app/controllers/concerns/mds/doi_lookup.rb new file mode 100644 index 000000000..b17c5e536 --- /dev/null +++ b/app/controllers/concerns/mds/doi_lookup.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module Mds + # Minimal DOI lookup for MDS controllers that need a DataciteDoi record. + # Intentionally does not include model concerns (Helpable, MetadataUtils). + module DoiLookup + extend ActiveSupport::Concern + + included do + include Bolognese::DoiUtils + end + + private + + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end + + def find_datacite_doi!(doi_string, not_found: "DOI not found") + doi_id = validate_doi(doi_string) + fail Mds::Error.new(not_found, status: 404) if doi_id.blank? + + doi = DataciteDoi.where(doi: doi_id).first + fail Mds::Error.new(not_found, status: 404) if doi.blank? + + doi + end + end +end diff --git a/app/controllers/concerns/mds/doi_support.rb b/app/controllers/concerns/mds/doi_support.rb deleted file mode 100644 index 1b8c16d8b..000000000 --- a/app/controllers/concerns/mds/doi_support.rb +++ /dev/null @@ -1,137 +0,0 @@ -# frozen_string_literal: true - -module Mds - # Shared DOI domain helpers for MDS protocol controllers. - # Thin protocol adapters call these instead of a parallel *Operations stack. - module DoiSupport - extend ActiveSupport::Concern - - included do - include Bolognese::DoiUtils - include Bolognese::Utils - include Bolognese::MetadataUtils - include Helpable - end - - private - - def client_symbol - (current_user.client_id.presence || current_user.uid).to_s - end - - # Load a DataciteDoi by validated DOI string or raise Mds::Error. - def find_datacite_doi!(doi_string, not_found: "DOI not found") - doi_id = validate_doi(doi_string) - fail Mds::Error.new(not_found, status: 404) if doi_id.blank? - - doi = DataciteDoi.where(doi: doi_id).first - fail Mds::Error.new(not_found, status: 404) if doi.blank? - - doi - end - - # Single write path for MDS create/update of DataciteDoi records. - # Used by both PUT /doi (publish URL) and PUT /metadata (register metadata). - # Callers supply already-shaped attributes (metadata runs ParamsSanitizer first). - def upsert_datacite_doi!(doi_id, attributes) - attrs = attributes.to_h.compact.with_indifferent_access - doi = DataciteDoi.where(doi: doi_id).first - - if doi - fail Mds::Error.new("Access is denied", status: 403) unless can?(:update, doi) - - doi.current_user = current_user - doi.assign_attributes(attrs.except(:doi, :client_id)) - else - doi = DataciteDoi.new(attrs.merge(doi: doi_id)) - doi.current_user = current_user - fail Mds::Error.new("Access is denied", status: 403) unless can?(:new, doi) - end - - return doi if doi.save - - message = doi.errors.full_messages.first || "Unprocessable entity" - fail Mds::Error.new(message, status: 422) - end - - # Resolve landing URL the same way as DataciteDoisController#get_url domain logic: - # use stored url for draft/other/special providers; otherwise ask Handle via doi.get_url. - def resolve_landing_url(doi) - if !doi.is_registered_or_findable? || - %w[europ].include?(doi.provider_id) || - doi.type == "OtherDoi" - return doi.url - end - - response = doi.get_url - if response.status == 200 - response.body.dig("data", "values", 0, "data", "value") || doi.url - else - doi.url - end - end - - def valid_landing_url?(url) - url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) - end - - # Classic MDS body: "doi=...\nurl=..." lines (also used when path lacks url param). - def extract_doi_and_url_from_body(data, path_doi: nil) - hsh = - data.to_s.split("\n").map do |line| - arr = line.to_s.split("=", 2) - arr << "value" if arr.length < 2 - arr - end.to_h - - fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? - - body_doi = CGI.unescape(hsh["doi"].strip) - if path_doi.present? && body_doi.casecmp(path_doi) != 0 - fail IdentifierError, "doi parameter does not match doi of resource" - end - - fail IdentifierError, "param 'url' required" unless hsh["url"].present? - - [body_doi, CGI.unescape(hsh["url"].strip)] - end - - # Resolve DOI for metadata registration: path param, XML identifier, or mint. - def resolve_metadata_doi_id(str, data:, from:, number: nil) - doi = validate_doi(str) - return doi if doi.present? - - if from == "datacite" - doi = doi_from_xml_identifier(data) - return doi if doi.present? - end - - mint_unique_doi(str, number: number) - end - - def doi_from_xml_identifier(string) - doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) - doc.remove_namespaces! - identifier = doc.at_css("identifier") - identifier = identifier.content if identifier.present? - validate_doi(identifier) - end - - def mint_unique_doi(str, number: nil) - if number.present? - doi = generate_random_dois(str, number: number).first - existing = DataciteDoi.where(doi: doi).exists? - fail IdentifierError, "doi:#{doi} has already been registered" if existing - else - doi = nil - duplicate = true - while duplicate - doi = generate_random_dois(str, number: number).first - duplicate = !Rails.env.test? && DataciteDoi.where(doi: doi).exists? - end - end - - doi - end - end -end diff --git a/app/controllers/concerns/mds/doi_writer.rb b/app/controllers/concerns/mds/doi_writer.rb new file mode 100644 index 000000000..b7b3fbd38 --- /dev/null +++ b/app/controllers/concerns/mds/doi_writer.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Mds + # Shared DataciteDoi create/update path for MDS PUT /doi and PUT /metadata. + # Authorization uses CanCan authorize! only (same path as other controller actions). + module DoiWriter + extend ActiveSupport::Concern + + included do + include Mds::DoiLookup + end + + private + + def upsert_datacite_doi!(doi_id, attributes) + attrs = attributes.to_h.compact.with_indifferent_access + doi = DataciteDoi.where(doi: doi_id).first + + if doi + authorize! :update, doi + doi.current_user = current_user + doi.assign_attributes(attrs.except(:doi, :client_id)) + else + doi = DataciteDoi.new(attrs.merge(doi: doi_id)) + doi.current_user = current_user + authorize! :new, doi + end + + return doi if doi.save + + message = doi.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end + end +end diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index 45cd100a5..8fb18fed2 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -3,6 +3,8 @@ module Mds # Classic MDS /doi surface — thin protocol adapter over DataciteDoi domain. class DoisController < Mds::ApplicationController + include Mds::DoiWriter + prepend_before_action :authenticate_mds_user! before_action :set_doi, only: %i[show destroy] @@ -29,7 +31,7 @@ def index def show authorize! :get_url, @doi - url = resolve_landing_url(@doi) + url = @doi.resolved_landing_url return head :no_content if url.blank? render_mds(url) @@ -77,6 +79,10 @@ def set_doi @doi = find_datacite_doi!(params[:id], not_found: "DOI not found") end + def valid_landing_url?(url) + url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) + end + def parse_doi_and_url if (params[:id].present? || params[:doi].present?) && params[:url].present? [params[:id].presence || params[:doi], params[:url]] @@ -89,5 +95,26 @@ def parse_doi_and_url [nil, nil] end end + + # Classic MDS body: "doi=...\nurl=..." lines. + def extract_doi_and_url_from_body(data, path_doi: nil) + hsh = + data.to_s.split("\n").map do |line| + arr = line.to_s.split("=", 2) + arr << "value" if arr.length < 2 + arr + end.to_h + + fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? + + body_doi = CGI.unescape(hsh["doi"].strip) + if path_doi.present? && body_doi.casecmp(path_doi) != 0 + fail IdentifierError, "doi parameter does not match doi of resource" + end + + fail IdentifierError, "param 'url' required" unless hsh["url"].present? + + [body_doi, CGI.unescape(hsh["url"].strip)] + end end end diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index 7acb06f90..a77d99fa8 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -3,6 +3,8 @@ module Mds # Classic MDS /media surface — thin protocol adapter over Media AR association. class MediaController < Mds::ApplicationController + include Mds::DoiLookup + prepend_before_action :authenticate_mds_user! before_action :set_doi before_action :set_media, only: %i[show destroy] diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index abc58258b..a2c51afdd 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -3,6 +3,9 @@ module Mds # Classic MDS /metadata surface — thin protocol adapter over DataciteDoi domain. class MetadataController < Mds::ApplicationController + include Mds::DoiWriter + include Bolognese::MetadataUtils + prepend_before_action :authenticate_mds_user! before_action :set_doi, only: %i[destroy] @@ -25,12 +28,11 @@ def create end data = request.raw_post - # Reuse Bolognese format detection (via MetadataUtils), not a forked copy. from = data.blank? ? "datacite" : find_from_format(string: data) fail Mds::Error.new("Metadata format not recognized", status: 415) if from.blank? doi_id = - resolve_metadata_doi_id( + Mds::DoiMinter.new.resolve_doi_id( params[:doi_id], data: data, from: from, diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb new file mode 100644 index 000000000..45a1f2da3 --- /dev/null +++ b/app/services/mds/doi_minter.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Mds + # DOI identity resolution for MDS metadata registration (path, XML identifier, or mint). + # Lives outside controllers so Helpable stays off the protocol base class. + class DoiMinter + include Bolognese::DoiUtils + include Bolognese::Utils + include Helpable + + def resolve_doi_id(str, data:, from:, number: nil) + doi = validate_doi(str) + return doi if doi.present? + + if from == "datacite" + doi = doi_from_xml_identifier(data) + return doi if doi.present? + end + + mint_unique_doi(str, number: number) + end + + private + + def doi_from_xml_identifier(string) + doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) + doc.remove_namespaces! + identifier = doc.at_css("identifier") + identifier = identifier.content if identifier.present? + validate_doi(identifier) + end + + def mint_unique_doi(str, number: nil) + if number.present? + doi = generate_random_dois(str, number: number).first + existing = DataciteDoi.where(doi: doi).exists? + fail IdentifierError, "doi:#{doi} has already been registered" if existing + + return doi + end + + doi = nil + loop do + doi = generate_random_dois(str).first + break unless DataciteDoi.where(doi: doi).exists? + end + doi + end + end +end From 3d65c8fa694356a8c8e3fbf3f1455614295ba38c Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:20:15 +0200 Subject: [PATCH 13/31] Fix RuboCop layout on MDS controllers and specs. Apply indented_internal_methods style for private/protected sections and drop trailing blank lines so CI rubocop is clean for the MDS embed. --- app/controllers/concerns/mds/doi_lookup.rb | 21 ++-- app/controllers/concerns/mds/doi_writer.rb | 33 +++-- app/controllers/mds/application_controller.rb | 119 +++++++++--------- app/controllers/mds/dois_controller.rb | 67 +++++----- app/controllers/mds/media_controller.rb | 27 ++-- app/controllers/mds/metadata_controller.rb | 7 +- app/services/mds/doi_minter.rb | 43 ++++--- spec/requests/mds/dois_spec.rb | 1 - spec/routing/mds_routing_spec.rb | 1 - 9 files changed, 155 insertions(+), 164 deletions(-) diff --git a/app/controllers/concerns/mds/doi_lookup.rb b/app/controllers/concerns/mds/doi_lookup.rb index b17c5e536..66687d2a8 100644 --- a/app/controllers/concerns/mds/doi_lookup.rb +++ b/app/controllers/concerns/mds/doi_lookup.rb @@ -11,19 +11,18 @@ module DoiLookup end private + def client_symbol + (current_user.client_id.presence || current_user.uid).to_s + end - def client_symbol - (current_user.client_id.presence || current_user.uid).to_s - end - - def find_datacite_doi!(doi_string, not_found: "DOI not found") - doi_id = validate_doi(doi_string) - fail Mds::Error.new(not_found, status: 404) if doi_id.blank? + def find_datacite_doi!(doi_string, not_found: "DOI not found") + doi_id = validate_doi(doi_string) + fail Mds::Error.new(not_found, status: 404) if doi_id.blank? - doi = DataciteDoi.where(doi: doi_id).first - fail Mds::Error.new(not_found, status: 404) if doi.blank? + doi = DataciteDoi.where(doi: doi_id).first + fail Mds::Error.new(not_found, status: 404) if doi.blank? - doi - end + doi + end end end diff --git a/app/controllers/concerns/mds/doi_writer.rb b/app/controllers/concerns/mds/doi_writer.rb index b7b3fbd38..4a94fe7ce 100644 --- a/app/controllers/concerns/mds/doi_writer.rb +++ b/app/controllers/concerns/mds/doi_writer.rb @@ -11,25 +11,24 @@ module DoiWriter end private + def upsert_datacite_doi!(doi_id, attributes) + attrs = attributes.to_h.compact.with_indifferent_access + doi = DataciteDoi.where(doi: doi_id).first - def upsert_datacite_doi!(doi_id, attributes) - attrs = attributes.to_h.compact.with_indifferent_access - doi = DataciteDoi.where(doi: doi_id).first + if doi + authorize! :update, doi + doi.current_user = current_user + doi.assign_attributes(attrs.except(:doi, :client_id)) + else + doi = DataciteDoi.new(attrs.merge(doi: doi_id)) + doi.current_user = current_user + authorize! :new, doi + end - if doi - authorize! :update, doi - doi.current_user = current_user - doi.assign_attributes(attrs.except(:doi, :client_id)) - else - doi = DataciteDoi.new(attrs.merge(doi: doi_id)) - doi.current_user = current_user - authorize! :new, doi - end - - return doi if doi.save + return doi if doi.save - message = doi.errors.full_messages.first || "Unprocessable entity" - fail Mds::Error.new(message, status: 422) - end + message = doi.errors.full_messages.first || "Unprocessable entity" + fail Mds::Error.new(message, status: 422) + end end end diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 14f1eaac7..184c72ed3 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -41,78 +41,77 @@ def route_not_found end protected - - # MDS-specific challenge and plain-text failure bodies; credentials via RequestCredentials. - def authenticate_mds_user! - user = user_from_request_credentials - - if user.nil? - request_http_basic_authentication( - Mds.realm, - "An Authentication object was not found in the SecurityContext", - ) - return false + # MDS-specific challenge and plain-text failure bodies; credentials via RequestCredentials. + def authenticate_mds_user! + user = user_from_request_credentials + + if user.nil? + request_http_basic_authentication( + Mds.realm, + "An Authentication object was not found in the SecurityContext", + ) + return false + end + + @current_user = user + + if @current_user.blank? || @current_user.errors.present? || + @current_user.role_id == "anonymous" + render_mds_error("Bad credentials", 401) + return false + end + + true end - @current_user = user - - if @current_user.blank? || @current_user.errors.present? || - @current_user.role_id == "anonymous" - render_mds_error("Bad credentials", 401) - return false + def current_ability + @current_ability ||= Ability.new(current_user) end - true - end - - def current_ability - @current_ability ||= Ability.new(current_user) - end - - def set_consumer_header - if current_user&.uid.present? - response.headers["X-Credential-Username"] = current_user.uid - else - response.headers["X-Anonymous-Consumer"] = true + def set_consumer_header + if current_user&.uid.present? + response.headers["X-Credential-Username"] = current_user.uid + else + response.headers["X-Anonymous-Consumer"] = true + end end - end - def render_mds(body = "OK", status: 200, headers: {}) - headers.each { |k, v| response.headers[k] = v } + def render_mds(body = "OK", status: 200, headers: {}) + headers.each { |k, v| response.headers[k] = v } - if status.to_i == 204 - head :no_content - else - render plain: body.to_s, status: status - end - end - - def render_mds_error(message, status) - if status.to_i == 401 - response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" - response.headers.delete("X-Credential-Username") + if status.to_i == 204 + head :no_content + else + render plain: body.to_s, status: status + end end - logger.error "[MDS #{status}]: #{message}" - render plain: message.to_s, status: status - end + def render_mds_error(message, status) + if status.to_i == 401 + response.headers["WWW-Authenticate"] = "Basic realm=\"#{Mds.realm}\"" + response.headers.delete("X-Credential-Username") + end - # Unexpected framework errors — do not map NoMethodError to 422. - unless Rails.env.development? - rescue_from ActionController::RoutingError do |_exception| - render_mds_error("DOI not found", 404) + logger.error "[MDS #{status}]: #{message}" + render plain: message.to_s, status: status end - rescue_from ActiveModel::ForbiddenAttributesError, - ActionController::UnpermittedParameters, - ActionController::ParameterMissing do |exception| - Sentry.capture_exception(exception) - render_mds_error(exception.message, 422) + # Unexpected framework errors — do not map NoMethodError to 422. + unless Rails.env.development? + rescue_from ActionController::RoutingError do |_exception| + render_mds_error("DOI not found", 404) + end + + rescue_from ActiveModel::ForbiddenAttributesError, + ActionController::UnpermittedParameters, + ActionController::ParameterMissing do |exception| + Sentry.capture_exception(exception) + render_mds_error(exception.message, 422) + end + + rescue_from NotImplementedError do |_exception| + render_mds_error("Not Implemented", 501) + end end - - rescue_from NotImplementedError do |_exception| - render_mds_error("Not Implemented", 501) - end - end end end diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index 8fb18fed2..d828c64e7 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -74,47 +74,46 @@ def destroy end private + def set_doi + @doi = find_datacite_doi!(params[:id], not_found: "DOI not found") + end - def set_doi - @doi = find_datacite_doi!(params[:id], not_found: "DOI not found") - end - - def valid_landing_url?(url) - url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) - end + def valid_landing_url?(url) + url.to_s.match?(%r{\A(http|https|ftp)://\S+\z}) + end - def parse_doi_and_url - if (params[:id].present? || params[:doi].present?) && params[:url].present? - [params[:id].presence || params[:doi], params[:url]] - elsif request.raw_post.present? - extract_doi_and_url_from_body( - request.raw_post, - path_doi: validate_doi(params[:id]), - ) - else - [nil, nil] + def parse_doi_and_url + if (params[:id].present? || params[:doi].present?) && params[:url].present? + [params[:id].presence || params[:doi], params[:url]] + elsif request.raw_post.present? + extract_doi_and_url_from_body( + request.raw_post, + path_doi: validate_doi(params[:id]), + ) + else + [nil, nil] + end end - end - # Classic MDS body: "doi=...\nurl=..." lines. - def extract_doi_and_url_from_body(data, path_doi: nil) - hsh = - data.to_s.split("\n").map do |line| - arr = line.to_s.split("=", 2) - arr << "value" if arr.length < 2 - arr - end.to_h + # Classic MDS body: "doi=...\nurl=..." lines. + def extract_doi_and_url_from_body(data, path_doi: nil) + hsh = + data.to_s.split("\n").map do |line| + arr = line.to_s.split("=", 2) + arr << "value" if arr.length < 2 + arr + end.to_h - fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? + fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? - body_doi = CGI.unescape(hsh["doi"].strip) - if path_doi.present? && body_doi.casecmp(path_doi) != 0 - fail IdentifierError, "doi parameter does not match doi of resource" - end + body_doi = CGI.unescape(hsh["doi"].strip) + if path_doi.present? && body_doi.casecmp(path_doi) != 0 + fail IdentifierError, "doi parameter does not match doi of resource" + end - fail IdentifierError, "param 'url' required" unless hsh["url"].present? + fail IdentifierError, "param 'url' required" unless hsh["url"].present? - [body_doi, CGI.unescape(hsh["url"].strip)] - end + [body_doi, CGI.unescape(hsh["url"].strip)] + end end end diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index a77d99fa8..683c05eb4 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -58,23 +58,22 @@ def destroy end private + def set_doi + # Flat /media/:doi_id and nested /doi/:doi_id/media both expose :doi_id. + raw = params[:doi_id] + fail Mds::Error.new("DOI is unknown to MDS", status: 404) if raw.blank? - def set_doi - # Flat /media/:doi_id and nested /doi/:doi_id/media both expose :doi_id. - raw = params[:doi_id] - fail Mds::Error.new("DOI is unknown to MDS", status: 404) if raw.blank? - - @doi = find_datacite_doi!(raw, not_found: "DOI is unknown to MDS") - end + @doi = find_datacite_doi!(raw, not_found: "DOI is unknown to MDS") + end - def set_media - encoded = params[:id] - fail Mds::Error.new("No media for the DOI", status: 404) if encoded.blank? + def set_media + encoded = params[:id] + fail Mds::Error.new("No media for the DOI", status: 404) if encoded.blank? - id = Base32::URL.decode(CGI.unescape(encoded.to_s)) - fail Mds::Error.new("No media for the DOI", status: 404) if id.blank? + id = Base32::URL.decode(CGI.unescape(encoded.to_s)) + fail Mds::Error.new("No media for the DOI", status: 404) if id.blank? - @media = @doi.media.where(id: id.to_i).first - end + @media = @doi.media.where(id: id.to_i).first + end end end diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index a2c51afdd..d8605c0d8 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -78,9 +78,8 @@ def destroy end private - - def set_doi - @doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") - end + def set_doi + @doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") + end end end diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index 45a1f2da3..89cda502d 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -21,30 +21,29 @@ def resolve_doi_id(str, data:, from:, number: nil) end private - - def doi_from_xml_identifier(string) - doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) - doc.remove_namespaces! - identifier = doc.at_css("identifier") - identifier = identifier.content if identifier.present? - validate_doi(identifier) - end - - def mint_unique_doi(str, number: nil) - if number.present? - doi = generate_random_dois(str, number: number).first - existing = DataciteDoi.where(doi: doi).exists? - fail IdentifierError, "doi:#{doi} has already been registered" if existing - - return doi + def doi_from_xml_identifier(string) + doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) + doc.remove_namespaces! + identifier = doc.at_css("identifier") + identifier = identifier.content if identifier.present? + validate_doi(identifier) end - doi = nil - loop do - doi = generate_random_dois(str).first - break unless DataciteDoi.where(doi: doi).exists? + def mint_unique_doi(str, number: nil) + if number.present? + doi = generate_random_dois(str, number: number).first + existing = DataciteDoi.where(doi: doi).exists? + fail IdentifierError, "doi:#{doi} has already been registered" if existing + + return doi + end + + doi = nil + loop do + doi = generate_random_dois(str).first + break unless DataciteDoi.where(doi: doi).exists? + end + doi end - doi - end end end diff --git a/spec/requests/mds/dois_spec.rb b/spec/requests/mds/dois_spec.rb index 0f137b171..757ac67a0 100644 --- a/spec/requests/mds/dois_spec.rb +++ b/spec/requests/mds/dois_spec.rb @@ -177,4 +177,3 @@ end end end - diff --git a/spec/routing/mds_routing_spec.rb b/spec/routing/mds_routing_spec.rb index 2d52179ef..120dc703c 100644 --- a/spec/routing/mds_routing_spec.rb +++ b/spec/routing/mds_routing_spec.rb @@ -97,4 +97,3 @@ def mds_url(path) end end end - From 52ba467336cd307b22b203540dd5d768d5419975 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:23:20 +0200 Subject: [PATCH 14/31] Bump crass, css_parser, and msgpack for bundler-audit. Update Gemfile.lock to patched versions resolving GHSA/CVE findings reported by bundler-audit (crass 1.0.7, css_parser 3.0.0, msgpack 1.8.3). --- Gemfile.lock | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index a3b9150f4..0d5122fad 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -183,7 +183,7 @@ GEM crack (1.0.1) bigdecimal rexml - crass (1.0.6) + crass (1.0.7) crawler_detect (1.2.11) qonfig (>= 0.24) csl (2.2.1) @@ -196,8 +196,9 @@ GEM time (< 1.0) csl-styles (2.0.2) csl (~> 2.0) - css_parser (2.2.0) + css_parser (3.0.0) addressable + ssrf_filter (~> 1.5) csv (3.3.5) dalli (5.0.2) logger @@ -476,7 +477,7 @@ GEM minitest (6.0.6) drb (~> 2.0) prism (~> 1.5) - msgpack (1.8.0) + msgpack (1.8.3) multi_json (1.21.1) multipart-post (2.4.1) mysql2 (0.5.7) @@ -729,6 +730,7 @@ GEM sparql-client (3.3.0) net-http-persistent (~> 4.0, >= 4.0.2) rdf (~> 3.3) + ssrf_filter (1.5.0) string_pattern (2.4.0) regexp_parser (~> 2.5, >= 2.5.0) stringio (3.2.0) @@ -771,6 +773,7 @@ GEM PLATFORMS aarch64-linux arm64-darwin-23 + arm64-darwin-25 universal-darwin-21 x86_64-darwin-20 x86_64-linux From f30437087fa5ff9c0d19378c84a14892e95d2f5b Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:35:39 +0200 Subject: [PATCH 15/31] Fix MDS metadata GET/DELETE 500s from CI. Render metadata XML with an explicit application/xml content type because Lupo unregisters the default :xml MIME type. Seed a landing URL when forcing findable state so hide's handle update_url callback does not raise on a blank URL. --- app/controllers/mds/metadata_controller.rb | 3 ++- spec/requests/mds/metadata_spec.rb | 8 +++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index d8605c0d8..a16eb1047 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -16,7 +16,8 @@ def show xml = doi.xml return head :no_content if xml.blank? - render xml: xml, status: :ok + # Lupo unregisters the default :xml MIME type; return raw MDS payload explicitly. + render body: xml, content_type: "application/xml", status: :ok end def create diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb index 69413a0a4..61b55d960 100644 --- a/spec/requests/mds/metadata_spec.rb +++ b/spec/requests/mds/metadata_spec.rb @@ -86,7 +86,13 @@ it "hides a findable DOI (registered state)" do put "/metadata/#{doi_string}", xml, basic_headers doi = DataciteDoi.where(doi: doi_string.downcase).first - doi.update_columns(aasm_state: "findable") if doi.draft? || doi.registered? + # findable DOIs always have a landing URL; update_url/register_url requires it + if doi.draft? || doi.registered? + doi.update_columns( + aasm_state: "findable", + url: "https://example.org/mds-metadata-hide", + ) + end delete "/metadata/#{doi_string}", nil, basic_headers.except("CONTENT_TYPE") From 03322426f0cc70343c53a03026264091dc890591 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 13:59:22 +0200 Subject: [PATCH 16/31] Address MDS review blockers for protocol and minting. Map ActionController::BadRequest to plain-text MDS 400 responses so handle/url invariant failures do not 500. Extract DoiMinting from Helpable and use it from Mds::DoiMinter without the full model concern. Fix REST head :no_content early returns, nil-safe get_dois client lookup, drop unused render_mds 204 branch, and tighten MDS GET /doi specs to deterministic stored-URL cases. --- app/controllers/datacite_dois_controller.rb | 7 ++-- app/controllers/mds/application_controller.rb | 12 +++--- app/models/concerns/doi_minting.rb | 40 +++++++++++++++++++ app/models/concerns/helpable.rb | 30 +------------- app/services/mds/doi_minter.rb | 5 +-- spec/requests/mds/dois_spec.rb | 28 +++++++++---- 6 files changed, 74 insertions(+), 48 deletions(-) create mode 100644 app/models/concerns/doi_minting.rb diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb index 7007f8001..7d907acf2 100644 --- a/app/controllers/datacite_dois_controller.rb +++ b/app/controllers/datacite_dois_controller.rb @@ -724,7 +724,8 @@ def get_url # Domain owns stored-vs-Handle policy via uses_stored_landing_url? / resolved_landing_url. if @doi.uses_stored_landing_url? url = @doi.url - head :no_content && return if url.blank? + return head :no_content if url.blank? + render json: { url: url }.to_json, status: :ok return end @@ -761,8 +762,8 @@ def get_dois client = Client.where("datacentre.symbol = ?", current_user.uid.upcase).first - client_prefix = client.prefixes.first - head :no_content && return if client_prefix.blank? + client_prefix = client&.prefixes&.first + return head :no_content if client_prefix.blank? dois = DataciteDoi.get_dois( diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 184c72ed3..ff64344d9 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -21,6 +21,11 @@ class ApplicationController < ActionController::API render_mds_error(exception.message, 400) end + # Domain/handle code (Helpable#register_url) raises this for missing url/client, etc. + rescue_from ActionController::BadRequest do |exception| + render_mds_error(exception.message.presence || "Bad Request", 400) + end + rescue_from CanCan::AccessDenied do |_exception| render_mds_error("Access is denied", 403) end @@ -78,12 +83,7 @@ def set_consumer_header def render_mds(body = "OK", status: 200, headers: {}) headers.each { |k, v| response.headers[k] = v } - - if status.to_i == 204 - head :no_content - else - render plain: body.to_s, status: status - end + render plain: body.to_s, status: status end def render_mds_error(message, status) diff --git a/app/models/concerns/doi_minting.rb b/app/models/concerns/doi_minting.rb new file mode 100644 index 000000000..97e02bb40 --- /dev/null +++ b/app/models/concerns/doi_minting.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +# Random DOI generation (prefix + shoulder + Base32). Shared by domain models and MDS minting. +# Depends on Bolognese::DoiUtils (#validate_prefix) being available on the including class. +module DoiMinting + extend ActiveSupport::Concern + + require "securerandom" + require "base32/url" + + UPPER_LIMIT = 1_073_741_823 + + def generate_random_dois(str, options = {}) + prefix = validate_prefix(str) + fail IdentifierError, "No valid prefix found" if prefix.blank? + + shoulder = str.split("/", 2)[1].to_s + encode_doi( + prefix, + shoulder: shoulder, number: options[:number], size: options[:size], + ) + end + + def encode_doi(prefix, options = {}) + return nil if prefix.blank? + + number = options[:number].to_s.scan(/\d+/).join("").to_i + shoulder = options[:shoulder].to_s + shoulder += "-" if shoulder.present? + length = 8 + split = 4 + size = (options[:size] || 1).to_i + + Array.new(size).map do |_a| + n = number.positive? ? number : SecureRandom.random_number(UPPER_LIMIT) + prefix.to_s + "/" + shoulder + + Base32::URL.encode(n, split: split, length: length, checksum: true) + end.uniq + end +end diff --git a/app/models/concerns/helpable.rb b/app/models/concerns/helpable.rb index 77b212cfc..0f36681fe 100644 --- a/app/models/concerns/helpable.rb +++ b/app/models/concerns/helpable.rb @@ -8,11 +8,13 @@ module Helpable require "securerandom" require "base32/url" + # Kept for existing call sites; mint algorithm lives in DoiMinting. UPPER_LIMIT = 1_073_741_823 included do include Bolognese::Utils include Bolognese::DoiUtils + include DoiMinting def register_url if url.blank? @@ -132,34 +134,6 @@ def generate_random_repository_symbol "6:X".gen end - def generate_random_dois(str, options = {}) - prefix = validate_prefix(str) - fail IdentifierError, "No valid prefix found" if prefix.blank? - - shoulder = str.split("/", 2)[1].to_s - encode_doi( - prefix, - shoulder: shoulder, number: options[:number], size: options[:size], - ) - end - - def encode_doi(prefix, options = {}) - return nil if prefix.blank? - - number = options[:number].to_s.scan(/\d+/).join("").to_i - shoulder = options[:shoulder].to_s - shoulder += "-" if shoulder.present? - length = 8 - split = 4 - size = (options[:size] || 1).to_i - - Array.new(size).map do |_a| - n = number.positive? ? number : SecureRandom.random_number(UPPER_LIMIT) - prefix.to_s + "/" + shoulder + - Base32::URL.encode(n, split: split, length: length, checksum: true) - end.uniq - end - def epoch_to_utc(epoch) Time.at(epoch).to_datetime.utc.iso8601 end diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index 89cda502d..9dbd03767 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -2,11 +2,10 @@ module Mds # DOI identity resolution for MDS metadata registration (path, XML identifier, or mint). - # Lives outside controllers so Helpable stays off the protocol base class. + # Uses DoiMinting only — not the full Helpable model concern (handle, landing URL, etc.). class DoiMinter include Bolognese::DoiUtils - include Bolognese::Utils - include Helpable + include DoiMinting def resolve_doi_id(str, data:, from:, number: nil) doi = validate_doi(str) diff --git a/spec/requests/mds/dois_spec.rb b/spec/requests/mds/dois_spec.rb index 757ac67a0..186575f4f 100644 --- a/spec/requests/mds/dois_spec.rb +++ b/spec/requests/mds/dois_spec.rb @@ -76,15 +76,27 @@ end describe "GET /doi/:id" do - it "returns the URL for a DOI with a known url attribute" do - findable_doi - get "/doi/#{findable_doi.doi}", nil, basic_headers + it "returns the stored landing URL for a draft DOI" do + draft = + create( + :doi, + client: client, + doi: "10.14454/mds-draft-url", + aasm_state: "draft", + url: "https://example.org/draft-landing", + ) + get "/doi/#{draft.doi}", nil, basic_headers + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("https://example.org/draft-landing") + end + + it "returns 204 when a draft DOI has no landing URL" do + doi + get "/doi/#{doi.doi}", nil, basic_headers - # May be 200 with URL from attribute/handle, or 204 if handle lookup empty in test - expect([200, 204]).to include(last_response.status) - if last_response.status == 200 - expect(last_response.body).to be_present - end + expect(last_response.status).to eq(204) + expect(last_response.body).to be_blank end it "returns 404 for unknown DOI" do From fa8f2dfd9a691795ce3e116735a1e5e6c86ea174 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 14:18:12 +0200 Subject: [PATCH 17/31] Route MDS heartbeat to Lupo memcached probe; drop login. Reuse HeartbeatController on MDS hosts instead of an always-OK stub, and remove the unused session-cookie login endpoint. --- app/controllers/mds/heartbeat_controller.rb | 9 --------- app/controllers/mds/index_controller.rb | 9 --------- config/routes.rb | 7 ++++--- spec/requests/mds/misc_spec.rb | 20 +++++++++++++------- spec/routing/mds_routing_spec.rb | 8 ++------ 5 files changed, 19 insertions(+), 34 deletions(-) delete mode 100644 app/controllers/mds/heartbeat_controller.rb delete mode 100644 app/controllers/mds/index_controller.rb diff --git a/app/controllers/mds/heartbeat_controller.rb b/app/controllers/mds/heartbeat_controller.rb deleted file mode 100644 index 0c9f46c45..000000000 --- a/app/controllers/mds/heartbeat_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Mds - class HeartbeatController < Mds::ApplicationController - def index - render plain: "OK", status: :ok - end - end -end diff --git a/app/controllers/mds/index_controller.rb b/app/controllers/mds/index_controller.rb deleted file mode 100644 index f3b9fa9c6..000000000 --- a/app/controllers/mds/index_controller.rb +++ /dev/null @@ -1,9 +0,0 @@ -# frozen_string_literal: true - -module Mds - class IndexController < Mds::ApplicationController - def login - render plain: "session cookies not supported", status: :not_implemented - end - end -end diff --git a/config/routes.rb b/config/routes.rb index 71b9a911b..acc67a6e0 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -4,10 +4,11 @@ # Classic MDS protocol (formerly Poodle). Only active when MDS_ENABLED and Host ∈ MDS_HOSTS. # Must be declared before the REST catch-all so mds.* hosts never fall into content negotiation. constraints(->(req) { Mds.host_match?(req) }) do - scope module: :mds do - resources :heartbeat, only: %i[index] - get "login", to: "index#login" + # Same memcached probe as REST /heartbeat (HeartbeatController), not an always-OK MDS stub. + # Declared outside module: :mds so it does not resolve to Mds::HeartbeatController. + resources :heartbeat, only: %i[index] + scope module: :mds do # update doi (body form without id in path) post "doi", to: "dois#update" diff --git a/spec/requests/mds/misc_spec.rb b/spec/requests/mds/misc_spec.rb index 66f0fd2f5..acb6558f8 100644 --- a/spec/requests/mds/misc_spec.rb +++ b/spec/requests/mds/misc_spec.rb @@ -6,20 +6,26 @@ let(:mds_host) { { "HTTP_HOST" => "mds.local" } } describe "GET /heartbeat" do - it "returns OK without authentication" do + it "uses Lupo Heartbeat (memcached probe) without authentication" do + allow(Heartbeat).to receive(:new).and_return( + instance_double(Heartbeat, string: "OK", status: 200), + ) + get "/heartbeat", nil, mds_host expect(last_response.status).to eq(200) expect(last_response.body).to eq("OK") end - end - describe "GET /login" do - it "returns 501" do - get "/login", nil, mds_host + it "returns 500 when the shared heartbeat reports failure" do + allow(Heartbeat).to receive(:new).and_return( + instance_double(Heartbeat, string: "failed", status: 500), + ) + + get "/heartbeat", nil, mds_host - expect(last_response.status).to eq(501) - expect(last_response.body).to include("session cookies not supported") + expect(last_response.status).to eq(500) + expect(last_response.body).to eq("failed") end end diff --git a/spec/routing/mds_routing_spec.rb b/spec/routing/mds_routing_spec.rb index 120dc703c..c15e2b2bd 100644 --- a/spec/routing/mds_routing_spec.rb +++ b/spec/routing/mds_routing_spec.rb @@ -72,12 +72,8 @@ def mds_url(path) ) end - it "routes GET /heartbeat to mds/heartbeat#index" do - expect(get: mds_url("/heartbeat")).to route_to("mds/heartbeat#index") - end - - it "routes GET /login to mds/index#login" do - expect(get: mds_url("/login")).to route_to("mds/index#login") + it "routes GET /heartbeat to shared heartbeat#index (not MDS stub)" do + expect(get: mds_url("/heartbeat")).to route_to("heartbeat#index") end end From e4ed6685b762127e3f8a6022eb904b6eccd06b3a Mon Sep 17 00:00:00 2001 From: kaysiz Date: Tue, 7 Jul 2026 14:23:19 +0200 Subject: [PATCH 18/31] Bump actions/checkout to v5 for Node 24 runtime. Silences the Node.js 20 deprecation annotation on GitHub Actions runners. --- .github/workflows/_update_terraform.yml | 2 +- .github/workflows/build.yml | 2 +- .github/workflows/changelog.yml | 2 +- .github/workflows/parallel_ci.yml | 2 +- .github/workflows/rubocop.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/_update_terraform.yml b/.github/workflows/_update_terraform.yml index 76f280750..cf10a6ae8 100644 --- a/.github/workflows/_update_terraform.yml +++ b/.github/workflows/_update_terraform.yml @@ -22,7 +22,7 @@ jobs: GIT_TAG: ${{ inputs.image_tag }} steps: - name: Checkout terraform config repo - uses: actions/checkout@v4 + uses: actions/checkout@v5 with: # public repo with terraform configuration repository: 'datacite/mastino' diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 85b0031bc..e419aded9 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -20,7 +20,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Checkout - uses: actions/checkout@v4 + uses: actions/checkout@v5 - name: Set up Docker Buildx uses: docker/setup-buildx-action@v3 - name: Cache Docker layers diff --git a/.github/workflows/changelog.yml b/.github/workflows/changelog.yml index f5b54ef38..f4e57f951 100644 --- a/.github/workflows/changelog.yml +++ b/.github/workflows/changelog.yml @@ -15,7 +15,7 @@ jobs: runs-on: ubuntu-latest name: Generate changelog steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Generate changelog uses: charmixer/auto-changelog-action@v1.1 diff --git a/.github/workflows/parallel_ci.yml b/.github/workflows/parallel_ci.yml index 87e324e60..8fbf8500b 100644 --- a/.github/workflows/parallel_ci.yml +++ b/.github/workflows/parallel_ci.yml @@ -60,7 +60,7 @@ jobs: MYSQL_HOST: "127.0.0.1" MYSQL_USER: root steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Sync time run: | diff --git a/.github/workflows/rubocop.yml b/.github/workflows/rubocop.yml index 73cd31782..d41985ec8 100644 --- a/.github/workflows/rubocop.yml +++ b/.github/workflows/rubocop.yml @@ -16,7 +16,7 @@ jobs: BUNDLE_WITHOUT: "default doc job cable storage ujs test db" BUNDLE_PATH: "vendor/bundle" steps: - - uses: actions/checkout@v4 + - uses: actions/checkout@v5 - name: Set up Ruby uses: ruby/setup-ruby@v1 with: From 57ba84b698d731328c796fa7b1f37fad2e2d4d6b Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 24 Jul 2026 14:50:46 +0200 Subject: [PATCH 19/31] cleaup some comments and authenticate mds with api key --- app/controllers/application_controller.rb | 15 +- app/controllers/concerns/mds/doi_lookup.rb | 2 - app/controllers/concerns/mds/doi_writer.rb | 2 - .../concerns/request_credentials.rb | 24 ++- app/controllers/mds/application_controller.rb | 15 +- app/controllers/mds/dois_controller.rb | 3 - app/controllers/mds/media_controller.rb | 2 - app/controllers/mds/metadata_controller.rb | 2 - app/models/ability.rb | 1 + config/routes.rb | 8 +- spec/models/ability_spec.rb | 4 +- spec/requests/mds/api_keys_spec.rb | 155 ++++++++++++++++++ 12 files changed, 184 insertions(+), 49 deletions(-) create mode 100644 spec/requests/mds/api_keys_spec.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index ce8d68308..b77b18cd6 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -72,13 +72,7 @@ def authenticate_user_with_basic_auth! end def authenticate_user! - @current_user = user_from_request_credentials - return false if @current_user.nil? - - fail CanCan::AuthorizationNotPerformed if @current_user.errors.present? - - set_api_key_sentry_tags - @current_user + authenticate_request! end def current_ability @@ -186,11 +180,6 @@ def set_raven_context end def set_api_key_sentry_tags - return unless current_user.try(:api_key_authenticated?) - - Sentry.set_tags( - auth_method: current_user.auth_method, - api_key_prefix: current_user.api_key_prefix, - ) + tag_api_key_observability! end end diff --git a/app/controllers/concerns/mds/doi_lookup.rb b/app/controllers/concerns/mds/doi_lookup.rb index 66687d2a8..f58c19120 100644 --- a/app/controllers/concerns/mds/doi_lookup.rb +++ b/app/controllers/concerns/mds/doi_lookup.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true module Mds - # Minimal DOI lookup for MDS controllers that need a DataciteDoi record. - # Intentionally does not include model concerns (Helpable, MetadataUtils). module DoiLookup extend ActiveSupport::Concern diff --git a/app/controllers/concerns/mds/doi_writer.rb b/app/controllers/concerns/mds/doi_writer.rb index 4a94fe7ce..b067726aa 100644 --- a/app/controllers/concerns/mds/doi_writer.rb +++ b/app/controllers/concerns/mds/doi_writer.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true module Mds - # Shared DataciteDoi create/update path for MDS PUT /doi and PUT /metadata. - # Authorization uses CanCan authorize! only (same path as other controller actions). module DoiWriter extend ActiveSupport::Concern diff --git a/app/controllers/concerns/request_credentials.rb b/app/controllers/concerns/request_credentials.rb index 40a861a55..b64f3cdb1 100644 --- a/app/controllers/concerns/request_credentials.rb +++ b/app/controllers/concerns/request_credentials.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -# Shared Authorization-header credential parsing for REST and MDS controllers. module RequestCredentials extend ActiveSupport::Concern @@ -9,9 +8,6 @@ def type_and_credentials_from_request_headers request.headers["Authorization"]&.split end - # Build a User from the request Authorization header. - # Returns nil when credentials are missing. - # Raises JWT::VerificationError when the token is blacklisted. def user_from_request_credentials type, credentials = type_and_credentials_from_request_headers return if credentials.blank? @@ -22,4 +18,24 @@ def user_from_request_credentials User.new(credentials, type: type) end + + def authenticate_request! + @current_user = user_from_request_credentials + return false if @current_user.nil? + + fail CanCan::AuthorizationNotPerformed if @current_user.errors.present? + + tag_api_key_observability! + @current_user + end + + def tag_api_key_observability! + return unless @current_user.try(:api_key_authenticated?) + return unless defined?(Sentry) + + Sentry.set_tags( + auth_method: @current_user.auth_method, + api_key_prefix: @current_user.api_key_prefix, + ) + end end diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index ff64344d9..77aa3070e 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -1,8 +1,6 @@ # frozen_string_literal: true module Mds - # Protocol base: auth challenge, plain-text errors, consumer headers. - # Domain helpers (lookup/write/mint) are included only on controllers that need them. class ApplicationController < ActionController::API include ActionController::HttpAuthentication::Basic::ControllerMethods include CanCan::ControllerAdditions @@ -12,7 +10,6 @@ class ApplicationController < ActionController::API after_action :set_consumer_header - # Protocol-facing errors always map to plain-text MDS responses. rescue_from Mds::Error do |exception| render_mds_error(exception.message, exception.status) end @@ -21,7 +18,6 @@ class ApplicationController < ActionController::API render_mds_error(exception.message, 400) end - # Domain/handle code (Helpable#register_url) raises this for missing url/client, etc. rescue_from ActionController::BadRequest do |exception| render_mds_error(exception.message.presence || "Bad Request", 400) end @@ -46,11 +42,10 @@ def route_not_found end protected - # MDS-specific challenge and plain-text failure bodies; credentials via RequestCredentials. def authenticate_mds_user! - user = user_from_request_credentials + user = authenticate_request! - if user.nil? + if user == false || user.nil? request_http_basic_authentication( Mds.realm, "An Authentication object was not found in the SecurityContext", @@ -58,10 +53,7 @@ def authenticate_mds_user! return false end - @current_user = user - - if @current_user.blank? || @current_user.errors.present? || - @current_user.role_id == "anonymous" + if user.role_id == "anonymous" render_mds_error("Bad credentials", 401) return false end @@ -96,7 +88,6 @@ def render_mds_error(message, status) render plain: message.to_s, status: status end - # Unexpected framework errors — do not map NoMethodError to 422. unless Rails.env.development? rescue_from ActionController::RoutingError do |_exception| render_mds_error("DOI not found", 404) diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index d828c64e7..d6640fa19 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module Mds - # Classic MDS /doi surface — thin protocol adapter over DataciteDoi domain. class DoisController < Mds::ApplicationController include Mds::DoiWriter @@ -20,7 +19,6 @@ def index DataciteDoi.get_dois( prefix: client_prefix.uid, username: current_user.uid.upcase, - password: current_user.password, ) return head :no_content if dois.blank? || !dois.is_a?(Array) || dois.empty? @@ -95,7 +93,6 @@ def parse_doi_and_url end end - # Classic MDS body: "doi=...\nurl=..." lines. def extract_doi_and_url_from_body(data, path_doi: nil) hsh = data.to_s.split("\n").map do |line| diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index 683c05eb4..d802fda2c 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module Mds - # Classic MDS /media surface — thin protocol adapter over Media AR association. class MediaController < Mds::ApplicationController include Mds::DoiLookup @@ -59,7 +58,6 @@ def destroy private def set_doi - # Flat /media/:doi_id and nested /doi/:doi_id/media both expose :doi_id. raw = params[:doi_id] fail Mds::Error.new("DOI is unknown to MDS", status: 404) if raw.blank? diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index a16eb1047..8f2364e3e 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true module Mds - # Classic MDS /metadata surface — thin protocol adapter over DataciteDoi domain. class MetadataController < Mds::ApplicationController include Mds::DoiWriter include Bolognese::MetadataUtils @@ -16,7 +15,6 @@ def show xml = doi.xml return head :no_content if xml.blank? - # Lupo unregisters the default :xml MIME type; return raw MDS payload explicitly. render body: xml, content_type: "application/xml", status: :ok end diff --git a/app/models/ability.rb b/app/models/ability.rb index 0a567be59..4ca570b2c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -165,6 +165,7 @@ def initialize(user) validate undo get_url + get_urls read_landing_page_results ], Doi, diff --git a/config/routes.rb b/config/routes.rb index 10ea822bd..b7047768f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -1,22 +1,16 @@ # frozen_string_literal: true Rails.application.routes.draw do - # Classic MDS protocol (formerly Poodle). Only active when MDS_ENABLED and Host ∈ MDS_HOSTS. - # Must be declared before the REST catch-all so mds.* hosts never fall into content negotiation. + # Classic MDS protocol (formerly Poodle). Only active when MDS_ENABLED. constraints(->(req) { Mds.host_match?(req) }) do - # Same memcached probe as REST /heartbeat (HeartbeatController), not an always-OK MDS stub. - # Declared outside module: :mds so it does not resolve to Mds::HeartbeatController. resources :heartbeat, only: %i[index] scope module: :mds do - # update doi (body form without id in path) post "doi", to: "dois#update" - # media (flat MDS paths) post "media/:doi_id", to: "media#create", constraints: { doi_id: /.+/ } get "media/:doi_id", to: "media#index", constraints: { doi_id: /.+/ } - # metadata post "metadata", to: "metadata#create" post "metadata/:doi_id", to: "metadata#create", constraints: { doi_id: /.+/ } put "metadata/:doi_id", to: "metadata#create", constraints: { doi_id: /.+/ } diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb index aafd05073..1b9a20555 100644 --- a/spec/models/ability_spec.rb +++ b/spec/models/ability_spec.rb @@ -198,13 +198,13 @@ is_expected.not_to be_able_to(:create, api_key) end - it "can read/create/update/destroy dois but not get_urls or transfer" do + it "can read/create/update/destroy dois and get_url(s) but not transfer" do is_expected.to be_able_to(:read, doi) is_expected.to be_able_to(:create, doi) is_expected.to be_able_to(:update, doi) is_expected.to be_able_to(:destroy, doi) is_expected.to be_able_to(:get_url, doi) - is_expected.not_to be_able_to(:get_urls, Doi) + is_expected.to be_able_to(:get_urls, Doi) is_expected.not_to be_able_to(:transfer, doi) end diff --git a/spec/requests/mds/api_keys_spec.rb b/spec/requests/mds/api_keys_spec.rb new file mode 100644 index 000000000..bd3e95c2e --- /dev/null +++ b/spec/requests/mds/api_keys_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require "rails_helper" +include Passwordable + +describe "MDS API key authentication", type: :request, vcr: true, prefix_pool_size: 1 do + let(:provider) do + create( + :provider, + symbol: "DATACITE", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let(:client) do + create( + :client, + provider: provider, + symbol: ENV["MDS_USERNAME"], + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:prefix) { create(:prefix, uid: "10.14454") } + let!(:client_prefix) { create(:client_prefix, client: client, prefix: prefix) } + let!(:api_key_record) { client.api_keys.create!(name: "mds automation key") } + let(:plain_key) { api_key_record.key } + let(:xml) { file_fixture("datacite.xml").read } + let(:doi_string) { "10.14454/4K3M-NYVG" } + + let(:mds_host) { { "HTTP_HOST" => "mds.local" } } + let(:bearer_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => "Bearer #{plain_key}", + "CONTENT_TYPE" => "application/xml;charset=UTF-8", + ) + end + let(:basic_key_as_username_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + plain_key, + "ignored", + ), + "CONTENT_TYPE" => "application/xml;charset=UTF-8", + ) + end + let(:basic_key_as_password_headers) do + mds_host.merge( + "HTTP_AUTHORIZATION" => + ActionController::HttpAuthentication::Basic.encode_credentials( + client.symbol, + plain_key, + ), + "CONTENT_TYPE" => "application/xml;charset=UTF-8", + ) + end + + describe "Bearer DC.* API key" do + it "registers metadata with Bearer API key" do + put "/metadata/#{doi_string}", xml, bearer_headers + + expect(last_response.status).to eq(201) + expect(last_response.body).to match(%r{\AOK \(10\.14454/4K3M-NYVG\)\z}i) + expect(last_response.headers["X-Credential-Username"]).to eq( + client.symbol.downcase, + ) + end + + it "returns metadata with Bearer API key" do + put "/metadata/#{doi_string}", xml, bearer_headers + get "/metadata/#{doi_string}", nil, bearer_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to include("Eating your own Dog Food") + end + + it "returns landing URL with Bearer API key" do + doi = + create( + :doi, + client: client, + doi: "10.14454/mds-api-key-url", + aasm_state: "draft", + url: "https://example.org/api-key-landing", + ) + + get "/doi/#{doi.doi}", nil, bearer_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("https://example.org/api-key-landing") + end + + it "authorizes GET /doi list (get_urls) with Bearer API key" do + get "/doi", nil, bearer_headers.except("CONTENT_TYPE") + + expect(last_response.status).to be_in([200, 204]) + expect(last_response.body).not_to eq("Access is denied") + end + + it "creates media with Bearer API key" do + doi = + create( + :doi, + client: client, + doi: "10.14454/mds-api-key-media", + aasm_state: "findable", + ) + + post "/media/#{doi.doi}", + "application/pdf=https://example.org/file.pdf", + bearer_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("OK") + expect(doi.media.count).to eq(1) + end + + it "returns 401 for an invalid Bearer API key" do + headers = + mds_host.merge("HTTP_AUTHORIZATION" => "Bearer DC.invalidkey000000000000000000000") + get "/doi/10.14454/anything", nil, headers + + expect(last_response.status).to eq(401) + expect(last_response.body).to eq("Bad credentials") + end + end + + describe "Basic with API key as username" do + it "registers metadata when DC.* is the Basic username" do + put "/metadata/#{doi_string}", xml, basic_key_as_username_headers + + expect(last_response.status).to eq(201) + expect(last_response.body).to match(%r{\AOK \(10\.14454/4K3M-NYVG\)\z}i) + end + end + + describe "Basic with API key as password" do + it "returns DOI URL when DC.* is the Basic password for the client" do + doi = + create( + :doi, + client: client, + doi: "10.14454/mds-api-key-basic-pw", + aasm_state: "draft", + url: "https://example.org/basic-key", + ) + + get "/doi/#{doi.doi}", + nil, + basic_key_as_password_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to eq("https://example.org/basic-key") + end + end +end From 909288e30b55a340f127a0b1c5750cccd747a5c0 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 7 Aug 2026 12:22:46 +0200 Subject: [PATCH 20/31] Prefer DC.* API keys over JWT and fail closed on invalid keys. --- .../concerns/request_credentials.rb | 11 ++++++++-- app/models/concerns/authenticable.rb | 22 +++++++++---------- app/models/user.rb | 18 ++++++--------- spec/concerns/authenticable_spec.rb | 14 ++++++++++-- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/app/controllers/concerns/request_credentials.rb b/app/controllers/concerns/request_credentials.rb index b64f3cdb1..b8da122ca 100644 --- a/app/controllers/concerns/request_credentials.rb +++ b/app/controllers/concerns/request_credentials.rb @@ -5,20 +5,27 @@ module RequestCredentials # based on https://github.com/nsarno/knock/blob/master/lib/knock/authenticable.rb def type_and_credentials_from_request_headers - request.headers["Authorization"]&.split + # Limit 2 keeps JWT/API-key material intact if it ever contained spaces. + request.headers["Authorization"]&.split(" ", 2) end def user_from_request_credentials type, credentials = type_and_credentials_from_request_headers return if credentials.blank? - if (ENV["JWT_BLACKLISTED"] || "").split(",").include?(credentials) + # Only JWT access tokens belong on the blacklist — never DC.* API keys. + if !api_key_token?(credentials) && + (ENV["JWT_BLACKLISTED"] || "").split(",").include?(credentials) raise JWT::VerificationError end User.new(credentials, type: type) end + def api_key_token?(token) + token.present? && token.length > 20 && token.to_s.match?(/\ADC\./i) + end + def authenticate_request! @current_user = user_from_request_credentials return false if @current_user.nil? diff --git a/app/models/concerns/authenticable.rb b/app/models/concerns/authenticable.rb index d37a830c3..cf9981cd2 100644 --- a/app/models/concerns/authenticable.rb +++ b/app/models/concerns/authenticable.rb @@ -153,16 +153,10 @@ def encode_auth_param(username: nil, password: nil) # basic auth def decode_auth_param(username: nil, password: nil) - if username.present? && username.length > 20 && username.match?(/\ADC\./i) - api_key = ApiKey.authenticate(username) - if api_key - client = api_key.client - if client - touch_api_key_last_used(api_key) - return payload_for_api_key(api_key, client) - end - end - return {} + # API key as Basic username (password ignored). Fail hard so callers get 401 + # instead of silently falling through to anonymous on public REST endpoints. + if api_key_token?(username) + return decode_api_key(username) end return {} unless username.present? && password.present? @@ -180,13 +174,15 @@ def decode_auth_param(username: nil, password: nil) return get_payload(uid: uid, user: user, password: password.to_s) end - if username.include?(".") && user + # API key as Basic password for a known client symbol. + if username.include?(".") && user && api_key_token?(password) api_key = ApiKey.authenticate(password) if api_key && api_key.client&.symbol&.downcase == user.symbol.downcase touch_api_key_last_used(api_key) # Do not pass the API key secret through as password (handle system). return payload_for_api_key(api_key, user) end + return { errors: "Invalid API key." } end {} @@ -204,6 +200,10 @@ def decode_api_key(token) payload_for_api_key(api_key, client) end + def api_key_token?(token) + token.present? && token.length > 20 && token.match?(/\ADC\./i) + end + def get_payload(uid: nil, user: nil, password: nil) roles = { "ROLE_ADMIN" => "staff_admin", diff --git a/app/models/user.rb b/app/models/user.rb index 6179cac88..7531afd5c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -70,18 +70,14 @@ def initialize(credentials, options = {}) ) end elsif credentials.present? - payload = decode_token(credentials) - if payload.blank? || payload[:errors] - # Try as API key (drop-in Bearer support, non-JWT) - ak_payload = decode_api_key(credentials) - if ak_payload.present? && !ak_payload[:errors] - payload = ak_payload - @jwt = nil # keys are not JWTs - else - @jwt = credentials - end + # Prefer API-key detection before JWT. DC.* tokens are never valid JWTs + # (they have only two "."-segments), and JWT blacklist must not block keys. + if api_key_token?(credentials) + payload = decode_api_key(credentials) + @jwt = nil else - @jwt = credentials + payload = decode_token(credentials) + @jwt = credentials if payload.present? && !payload[:errors] end end diff --git a/spec/concerns/authenticable_spec.rb b/spec/concerns/authenticable_spec.rb index c7c1c6b52..589077768 100644 --- a/spec/concerns/authenticable_spec.rb +++ b/spec/concerns/authenticable_spec.rb @@ -491,12 +491,12 @@ expect(payload["api_key_prefix"]).to be_nil end - it "rejects wrong key" do + it "rejects wrong key with an explicit error" do payload = client.decode_auth_param( username: client.symbol, password: "DC.wrongkeyvalue1234567890abcdef", ) - expect(payload).to eq({}) + expect(payload[:errors]).to eq("Invalid API key.") end end @@ -514,4 +514,14 @@ expect(payload[:errors]).to be_present end end + + describe "decode_auth_param with invalid DC.* username" do + it "returns Invalid API key error instead of empty anonymous payload" do + payload = client.decode_auth_param( + username: "DC.invalidkey000000000000000000000", + password: "ignored", + ) + expect(payload[:errors]).to eq("Invalid API key.") + end + end end From 26a2c691f532b9598bc5294382de8e6f943f14ef Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 7 Aug 2026 12:28:58 +0200 Subject: [PATCH 21/31] upgrade graphql --- Gemfile | 2 +- Gemfile.lock | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Gemfile b/Gemfile index 9413fe952..208debcb9 100644 --- a/Gemfile +++ b/Gemfile @@ -35,7 +35,7 @@ gem "flipper", "~> 1.4", ">= 1.4.1" gem "flipper-active_support_cache_store", "~> 1.4", ">= 1.4.1" gem "gender_detector", "~> 2.1" gem "google-protobuf", "~> 4.34", ">= 4.34.1" -gem "graphql", "~> 2.5", ">= 2.5.26" +gem "graphql", ">= 2.6.7" gem "graphql-batch", "~> 0.6.1" gem "hashid-rails", "~> 1.4", ">= 1.4.1" gem "iso-639", "~> 0.3.8" diff --git a/Gemfile.lock b/Gemfile.lock index ea8ad42fd..90efc51c2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -336,7 +336,7 @@ GEM google-protobuf (4.34.1-x86_64-linux-gnu) bigdecimal rake (~> 13.3) - graphql (2.6.3) + graphql (2.6.7) base64 fiber-storage logger @@ -817,7 +817,7 @@ DEPENDENCIES flipper-active_support_cache_store (~> 1.4, >= 1.4.1) gender_detector (~> 2.1) google-protobuf (~> 4.34, >= 4.34.1) - graphql (~> 2.5, >= 2.5.26) + graphql (>= 2.6.7) graphql-batch (~> 0.6.1) hashdiff (~> 1.2, >= 1.2.1) hashid-rails (~> 1.4, >= 1.4.1) From 5a3bc63c9f809f1f482b8b1a4e1aea273b3eeb3d Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 7 Aug 2026 20:47:58 +0200 Subject: [PATCH 22/31] Reject blank media type or URL on MDS media create. Parse mediaType=url before persist so malformed bodies do not fall through to Media defaults or partial records. Cover blank type, blank URL, and missing pair in request specs. --- app/controllers/mds/media_controller.rb | 16 ++++++++++++- spec/requests/mds/media_spec.rb | 30 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index d802fda2c..e2e300174 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -32,7 +32,7 @@ def create data = request.raw_post fail Mds::Error.new("Media type and URL missing", status: 400) if data.blank? - media_type, url = data.to_s.split("=", 2) + media_type, url = parse_media_body(data) media = Media.new(doi: @doi, media_type: media_type, url: url) unless media.save @@ -73,5 +73,19 @@ def set_media @media = @doi.media.where(id: id.to_i).first end + + # MDS media body is mediaType=url. Both parts are required; blank type must not + # fall through to Media#set_defaults (text/plain). + def parse_media_body(data) + media_type, url = data.to_s.split("=", 2) + media_type = media_type.to_s.strip + url = url.to_s.strip + + if media_type.blank? || url.blank? + fail Mds::Error.new("Media type and URL missing", status: 400) + end + + [media_type, url] + end end end diff --git a/spec/requests/mds/media_spec.rb b/spec/requests/mds/media_spec.rb index 826d9fe09..27e671af2 100644 --- a/spec/requests/mds/media_spec.rb +++ b/spec/requests/mds/media_spec.rb @@ -53,6 +53,36 @@ expect(doi.media.first.media_type).to eq("application/pdf") expect(doi.media.first.url).to eq("https://example.org/file.pdf") end + + it "rejects blank URL" do + post "/media/#{doi.doi}", + "application/pdf=", + basic_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("Media type and URL missing") + expect(doi.media.count).to eq(0) + end + + it "rejects blank media type" do + post "/media/#{doi.doi}", + "=https://example.org/file.pdf", + basic_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("Media type and URL missing") + expect(doi.media.count).to eq(0) + end + + it "rejects body without media type and URL pair" do + post "/media/#{doi.doi}", + "application/pdf", + basic_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("Media type and URL missing") + expect(doi.media.count).to eq(0) + end end describe "GET /media/:doi_id" do From 5dae62c93527c6cc7db17e0665f842ca5836f48c Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 7 Aug 2026 20:48:27 +0200 Subject: [PATCH 23/31] Validate MDS metadata path DOI against body identifier. DoiMinter now compares path and DataCite XML identifier before upsert, matching the PUT /doi consistency rule, so mismatched metadata cannot be stored under the wrong DOI. --- app/services/mds/doi_minter.rb | 20 +++++++++++++++----- spec/requests/mds/metadata_spec.rb | 11 +++++++++++ 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index 9dbd03767..aeb81b9fd 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -8,14 +8,16 @@ class DoiMinter include DoiMinting def resolve_doi_id(str, data:, from:, number: nil) - doi = validate_doi(str) - return doi if doi.present? + path_doi = validate_doi(str) + body_doi = from == "datacite" ? doi_from_xml_identifier(data) : nil - if from == "datacite" - doi = doi_from_xml_identifier(data) - return doi if doi.present? + if path_doi.present? + ensure_path_matches_body!(path_doi, body_doi) + return path_doi end + return body_doi if body_doi.present? + mint_unique_doi(str, number: number) end @@ -28,6 +30,14 @@ def doi_from_xml_identifier(string) validate_doi(identifier) end + # Same consistency rule as MDS PUT /doi path vs body doi parameter. + def ensure_path_matches_body!(path_doi, body_doi) + return if body_doi.blank? + return if body_doi.casecmp(path_doi).zero? + + fail IdentifierError, "doi parameter does not match doi of resource" + end + def mint_unique_doi(str, number: nil) if number.present? doi = generate_random_dois(str, number: number).first diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb index 61b55d960..cb4ca1df0 100644 --- a/spec/requests/mds/metadata_spec.rb +++ b/spec/requests/mds/metadata_spec.rb @@ -60,6 +60,17 @@ expect(last_response.status).to eq(415) expect(last_response.body).to include("not supported") end + + it "rejects path DOI that does not match metadata identifier" do + put "/metadata/10.14454/other-doi", xml, basic_headers + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq( + "doi parameter does not match doi of resource", + ) + expect(DataciteDoi.where(doi: "10.14454/other-doi").count).to eq(0) + expect(DataciteDoi.where(doi: doi_string.downcase).count).to eq(0) + end end describe "GET /metadata/:doi_id" do From de4fd9a624e9018d2a6d81310678c61da9e0eadc Mon Sep 17 00:00:00 2001 From: kaysiz Date: Fri, 7 Aug 2026 20:48:58 +0200 Subject: [PATCH 24/31] Validate MDS mint prefix before generating random DOIs. Reject empty or malformed mint input with a deterministic IdentifierError instead of relying on generate_random_dois internals. Cover blank and invalid mint requests on POST /metadata. --- app/services/mds/doi_minter.rb | 5 +++++ spec/requests/mds/metadata_spec.rb | 34 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index aeb81b9fd..44c88c512 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -39,6 +39,11 @@ def ensure_path_matches_body!(path_doi, body_doi) end def mint_unique_doi(str, number: nil) + # Fail closed before generate_random_dois so blank/malformed mint + # input returns a deterministic MDS client error (IdentifierError → 400). + prefix = validate_prefix(str) + fail IdentifierError, "No valid prefix found" if prefix.blank? + if number.present? doi = generate_random_dois(str, number: number).first existing = DataciteDoi.where(doi: doi).exists? diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb index cb4ca1df0..892e52b40 100644 --- a/spec/requests/mds/metadata_spec.rb +++ b/spec/requests/mds/metadata_spec.rb @@ -73,6 +73,40 @@ end end + describe "POST /metadata" do + it "rejects minting when path and body provide no valid prefix" do + expect { + post "/metadata", "", basic_headers + }.not_to change(DataciteDoi, :count) + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("No valid prefix found") + end + + it "rejects minting when path is not a valid prefix" do + expect { + post "/metadata/not-a-prefix", "", basic_headers + }.not_to change(DataciteDoi, :count) + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("No valid prefix found") + end + + it "rejects minting when metadata has no identifier and path is blank" do + body = xml.sub( + %r{]*>.*?}m, + "", + ) + + expect { + post "/metadata", body, basic_headers + }.not_to change(DataciteDoi, :count) + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("No valid prefix found") + end + end + describe "GET /metadata/:doi_id" do it "returns XML for an existing DOI" do put "/metadata/#{doi_string}", xml, basic_headers From a26656035fb6ba4cbf8bd9d26bd2145b9c82947b Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 15:45:07 +0200 Subject: [PATCH 25/31] Harden prefix cache and client assign_prefix against stale ids. Cache only prefix primary keys and re-verify uid on load so destroyed and recreated prefixes cannot leave orphan FKs. Make assign_prefix use create! and the known Prefix uid instead of provider_prefix.prefix.uid, which flaked shard 14 (repository_type_spec) on this PR. --- app/models/client.rb | 36 ++++++++++++++++++++------------ app/models/concerns/cacheable.rb | 24 +++++++++++++++------ 2 files changed, 41 insertions(+), 19 deletions(-) diff --git a/app/models/client.rb b/app/models/client.rb index f998347d5..d40b59930 100644 --- a/app/models/client.rb +++ b/app/models/client.rb @@ -957,27 +957,37 @@ def check_prefix def assign_prefix available_prefix = get_prefix - if !available_prefix + if available_prefix.blank? errors.add( :base, "No prefixes available. Created repository, but a prefix was not assigned. Contact support to get a prefix.", ) - else - prefix, provider_prefix = nil - available_prefix.class.name == "Prefix" ? prefix = available_prefix : provider_prefix = available_prefix + return + end - if !provider_prefix.present? - provider_prefix = ProviderPrefix.create( - provider_id: provider.symbol, prefix_id: prefix.uid + if available_prefix.is_a?(Prefix) + prefix = available_prefix + provider_prefix = ProviderPrefix.create!( + provider_id: provider.symbol, + prefix_id: prefix.uid, + ) + else + provider_prefix = available_prefix + prefix = provider_prefix.prefix + if prefix.blank? + errors.add( + :base, + "No prefixes available. Created repository, but a prefix was not assigned. Contact support to get a prefix.", ) + return end - - ClientPrefix.create( - client_id: symbol, - provider_prefix_id: provider_prefix.uid, - prefix_id: provider_prefix.prefix.uid - ) end + + ClientPrefix.create!( + client_id: symbol, + provider_prefix_id: provider_prefix.uid, + prefix_id: prefix.uid, + ) end def user_url diff --git a/app/models/concerns/cacheable.rb b/app/models/concerns/cacheable.rb index b82bd0e7c..1264520e5 100644 --- a/app/models/concerns/cacheable.rb +++ b/app/models/concerns/cacheable.rb @@ -56,13 +56,25 @@ def cached_prefixes_totals(params = {}) end def cached_prefix_response(prefix, _options = {}) - if Rails.application.config.action_controller.perform_caching - Rails.cache.fetch("prefix_response/#{prefix}", expires_in: 24.hours) do - Prefix.where(uid: prefix).first - end - else - Prefix.where(uid: prefix).first + uid = prefix.to_s + return Prefix.where(uid: uid).first unless Rails.application.config.action_controller.perform_caching + + # Cache only the numeric id as a hint. Always re-load and verify uid so a + # destroyed/recreated prefix cannot yield an orphan FK (common in tests that + # reuse fixed uids like 10.17616 across before/after :all blocks). + cached_id = Rails.cache.fetch("prefix_response/#{uid}", expires_in: 24.hours) do + Prefix.where(uid: uid).pick(:id) + end + + record = cached_id.present? ? Prefix.find_by(id: cached_id) : nil + return record if record&.uid.to_s == uid + + Rails.cache.delete("prefix_response/#{uid}") + found = Prefix.where(uid: uid).first + if found + Rails.cache.write("prefix_response/#{uid}", found.id, expires_in: 24.hours) end + found end def cached_resource_type_response(id) From 97bbb1de1d960a7cf02293ad0deb4e2e4d1ad03d Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 16:11:35 +0200 Subject: [PATCH 26/31] Consolidate MDS protocol helpers and error handling. Share path/body DOI consistency via Mds.assert_path_matches_body!, centralize classic MDS 404 copy constants, list DOIs through DoiLookup, always register rescue_from handlers, and return plain-text MDS errors for blank DOI/URL on PUT /doi. --- app/controllers/concerns/mds/doi_lookup.rb | 20 ++++++++++- app/controllers/mds/application_controller.rb | 33 ++++++++----------- app/controllers/mds/dois_controller.rb | 26 +++++---------- app/controllers/mds/media_controller.rb | 4 +-- app/controllers/mds/metadata_controller.rb | 6 ++-- app/services/mds/doi_minter.rb | 10 +----- lib/mds.rb | 13 ++++++++ spec/lib/mds_spec.rb | 20 +++++++++++ 8 files changed, 80 insertions(+), 52 deletions(-) diff --git a/app/controllers/concerns/mds/doi_lookup.rb b/app/controllers/concerns/mds/doi_lookup.rb index f58c19120..231fa89c7 100644 --- a/app/controllers/concerns/mds/doi_lookup.rb +++ b/app/controllers/concerns/mds/doi_lookup.rb @@ -13,7 +13,7 @@ def client_symbol (current_user.client_id.presence || current_user.uid).to_s end - def find_datacite_doi!(doi_string, not_found: "DOI not found") + def find_datacite_doi!(doi_string, not_found: Mds::DOI_NOT_FOUND) doi_id = validate_doi(doi_string) fail Mds::Error.new(not_found, status: 404) if doi_id.blank? @@ -22,5 +22,23 @@ def find_datacite_doi!(doi_string, not_found: "DOI not found") doi end + + # DOIs for the authenticated repository's first prefix (MDS GET /doi list). + # Returns nil when there is nothing to list (caller should 204). + def listed_dois_for_current_user + client = + Client.where("datacentre.symbol = ?", current_user.uid.upcase).first + client_prefix = client&.prefixes&.first + return if client_prefix.blank? + + dois = + DataciteDoi.get_dois( + prefix: client_prefix.uid, + username: current_user.uid.upcase, + ) + return if dois.blank? || !dois.is_a?(Array) || dois.empty? + + dois + end end end diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 77aa3070e..06841049f 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -33,8 +33,20 @@ class ApplicationController < ActionController::API end rescue_from ActiveRecord::RecordNotFound, - AbstractController::ActionNotFound do |_exception| - render_mds_error("DOI not found", 404) + AbstractController::ActionNotFound, + ActionController::RoutingError do |_exception| + render_mds_error(Mds::DOI_NOT_FOUND, 404) + end + + rescue_from ActiveModel::ForbiddenAttributesError, + ActionController::UnpermittedParameters, + ActionController::ParameterMissing do |exception| + Sentry.capture_exception(exception) if defined?(Sentry) + render_mds_error(exception.message, 422) + end + + rescue_from NotImplementedError do |_exception| + render_mds_error("Not Implemented", 501) end def route_not_found @@ -87,22 +99,5 @@ def render_mds_error(message, status) logger.error "[MDS #{status}]: #{message}" render plain: message.to_s, status: status end - - unless Rails.env.development? - rescue_from ActionController::RoutingError do |_exception| - render_mds_error("DOI not found", 404) - end - - rescue_from ActiveModel::ForbiddenAttributesError, - ActionController::UnpermittedParameters, - ActionController::ParameterMissing do |exception| - Sentry.capture_exception(exception) - render_mds_error(exception.message, 422) - end - - rescue_from NotImplementedError do |_exception| - render_mds_error("Not Implemented", 501) - end - end end end diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index d6640fa19..4aaf47a4b 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -10,18 +10,8 @@ class DoisController < Mds::ApplicationController def index authorize! :get_urls, Doi - client = - Client.where("datacentre.symbol = ?", current_user.uid.upcase).first - client_prefix = client&.prefixes&.first - return head :no_content if client_prefix.blank? - - dois = - DataciteDoi.get_dois( - prefix: client_prefix.uid, - username: current_user.uid.upcase, - ) - - return head :no_content if dois.blank? || !dois.is_a?(Array) || dois.empty? + dois = listed_dois_for_current_user + return head :no_content if dois.blank? render_mds(dois.join("\n")) end @@ -37,12 +27,14 @@ def show def update doi_string, url = parse_doi_and_url - return head :bad_request if doi_string.blank? || url.blank? + if doi_string.blank? || url.blank? + fail Mds::Error.new("DOI and URL required", status: 400) + end fail Mds::Error.new("Not a valid HTTP(S) or FTP URL", status: 400) unless valid_landing_url?(url) doi_id = validate_doi(doi_string) - fail Mds::Error.new("DOI not found", status: 404) if doi_id.blank? + fail Mds::Error.new(Mds::DOI_NOT_FOUND, status: 404) if doi_id.blank? upsert_datacite_doi!( doi_id, @@ -73,7 +65,7 @@ def destroy private def set_doi - @doi = find_datacite_doi!(params[:id], not_found: "DOI not found") + @doi = find_datacite_doi!(params[:id], not_found: Mds::DOI_NOT_FOUND) end def valid_landing_url?(url) @@ -104,9 +96,7 @@ def extract_doi_and_url_from_body(data, path_doi: nil) fail IdentifierError, "param 'doi' required" unless hsh["doi"].present? body_doi = CGI.unescape(hsh["doi"].strip) - if path_doi.present? && body_doi.casecmp(path_doi) != 0 - fail IdentifierError, "doi parameter does not match doi of resource" - end + Mds.assert_path_matches_body!(path_doi, body_doi) fail IdentifierError, "param 'url' required" unless hsh["url"].present? diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index e2e300174..938581c88 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -59,9 +59,9 @@ def destroy private def set_doi raw = params[:doi_id] - fail Mds::Error.new("DOI is unknown to MDS", status: 404) if raw.blank? + fail Mds::Error.new(Mds::DOI_UNKNOWN_TO_MDS, status: 404) if raw.blank? - @doi = find_datacite_doi!(raw, not_found: "DOI is unknown to MDS") + @doi = find_datacite_doi!(raw, not_found: Mds::DOI_UNKNOWN_TO_MDS) end def set_media diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index 8f2364e3e..fd37479d7 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -9,7 +9,7 @@ class MetadataController < Mds::ApplicationController before_action :set_doi, only: %i[destroy] def show - doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") + doi = find_datacite_doi!(params[:doi_id], not_found: Mds::DOI_UNKNOWN_TO_MDS) authorize! :read, doi xml = doi.xml @@ -37,7 +37,7 @@ def create from: from, number: params[:number], ) - fail Mds::Error.new("DOI not found", status: 404) if doi_id.blank? + fail Mds::Error.new(Mds::DOI_NOT_FOUND, status: 404) if doi_id.blank? xml_b64 = data.present? ? Base64.strict_encode64(data) : nil attrs = @@ -78,7 +78,7 @@ def destroy private def set_doi - @doi = find_datacite_doi!(params[:doi_id], not_found: "DOI is unknown to MDS") + @doi = find_datacite_doi!(params[:doi_id], not_found: Mds::DOI_UNKNOWN_TO_MDS) end end end diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index 44c88c512..d7534a016 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -12,7 +12,7 @@ def resolve_doi_id(str, data:, from:, number: nil) body_doi = from == "datacite" ? doi_from_xml_identifier(data) : nil if path_doi.present? - ensure_path_matches_body!(path_doi, body_doi) + Mds.assert_path_matches_body!(path_doi, body_doi) return path_doi end @@ -30,14 +30,6 @@ def doi_from_xml_identifier(string) validate_doi(identifier) end - # Same consistency rule as MDS PUT /doi path vs body doi parameter. - def ensure_path_matches_body!(path_doi, body_doi) - return if body_doi.blank? - return if body_doi.casecmp(path_doi).zero? - - fail IdentifierError, "doi parameter does not match doi of resource" - end - def mint_unique_doi(str, number: nil) # Fail closed before generate_random_dois so blank/malformed mint # input returns a deterministic MDS client error (IdentifierError → 400). diff --git a/lib/mds.rb b/lib/mds.rb index d89b57be3..a8bf54666 100644 --- a/lib/mds.rb +++ b/lib/mds.rb @@ -12,6 +12,11 @@ module Mds mds.local ].freeze + # Classic MDS plain-text 404 copy differs by resource family. + DOI_NOT_FOUND = "DOI not found" + DOI_UNKNOWN_TO_MDS = "DOI is unknown to MDS" + PATH_BODY_MISMATCH = "doi parameter does not match doi of resource" + module_function def enabled? @@ -43,4 +48,12 @@ def url def realm ENV.fetch("MDS_REALM", "mds.datacite.org") end + + # Shared path vs body DOI consistency for PUT /doi and metadata registration. + def assert_path_matches_body!(path_doi, body_doi) + return if path_doi.blank? || body_doi.blank? + return if body_doi.to_s.casecmp(path_doi.to_s).zero? + + raise IdentifierError, PATH_BODY_MISMATCH + end end diff --git a/spec/lib/mds_spec.rb b/spec/lib/mds_spec.rb index d31b4e372..3f3ae5ab2 100644 --- a/spec/lib/mds_spec.rb +++ b/spec/lib/mds_spec.rb @@ -65,4 +65,24 @@ def with_env(key, value) expect(Mds.hosts).to eq(["mds.local"]) end end + + describe ".assert_path_matches_body!" do + it "allows matching path and body DOIs case-insensitively" do + expect { + Mds.assert_path_matches_body!("10.14454/abc", "10.14454/ABC") + }.not_to raise_error + end + + it "allows when body DOI is blank" do + expect { + Mds.assert_path_matches_body!("10.14454/abc", nil) + }.not_to raise_error + end + + it "raises IdentifierError on mismatch" do + expect { + Mds.assert_path_matches_body!("10.14454/a", "10.14454/b") + }.to raise_error(IdentifierError, Mds::PATH_BODY_MISMATCH) + end + end end From 87ce85a20d8d49cdcfa66dda2f4c320df58d6249 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 16:15:56 +0200 Subject: [PATCH 27/31] Finish shared landing-URL and API-key boundaries. Move full Handle vs stored URL policy into Doi#resolve_landing_url via LandingUrlResolution so REST and MDS map the same domain outcomes. Define Authenticable.api_key_token? once and reuse it from RequestCredentials. Align metadata create blank-DOI 404 copy with DOI_UNKNOWN_TO_MDS. --- .../concerns/request_credentials.rb | 2 +- app/controllers/datacite_dois_controller.rb | 34 ++++--------- app/controllers/mds/metadata_controller.rb | 2 +- app/models/concerns/authenticable.rb | 7 ++- app/models/concerns/helpable.rb | 37 +++++++++++--- app/models/landing_url_resolution.rb | 50 +++++++++++++++++++ 6 files changed, 99 insertions(+), 33 deletions(-) create mode 100644 app/models/landing_url_resolution.rb diff --git a/app/controllers/concerns/request_credentials.rb b/app/controllers/concerns/request_credentials.rb index b8da122ca..145f2fa57 100644 --- a/app/controllers/concerns/request_credentials.rb +++ b/app/controllers/concerns/request_credentials.rb @@ -23,7 +23,7 @@ def user_from_request_credentials end def api_key_token?(token) - token.present? && token.length > 20 && token.to_s.match?(/\ADC\./i) + Authenticable.api_key_token?(token) end def authenticate_request! diff --git a/app/controllers/datacite_dois_controller.rb b/app/controllers/datacite_dois_controller.rb index e4df5843b..c265bc3fe 100644 --- a/app/controllers/datacite_dois_controller.rb +++ b/app/controllers/datacite_dois_controller.rb @@ -728,27 +728,13 @@ def get_url authorize! :get_url, @doi - # Domain owns stored-vs-Handle policy via uses_stored_landing_url? / resolved_landing_url. - if @doi.uses_stored_landing_url? - url = @doi.url - return head :no_content if url.blank? - - render json: { url: url }.to_json, status: :ok - return - end - - response = @doi.get_url - - if response.status == 200 - url = response.body.dig("data", "values", 0, "data", "value") - if url.present? - render json: { url: url }.to_json, status: :ok - else - render json: response.body.to_json, - status: response.status || :bad_request - end - elsif response.status == 400 && - response.body.dig("errors", 0, "title", "responseCode") == 301 + result = @doi.resolve_landing_url + case result.kind + when :ok + render json: { url: result.url }.to_json, status: :ok + when :no_content + head :no_content + when :forbidden_handle render json: { "errors" => [ { @@ -758,9 +744,9 @@ def get_url ], }.to_json, status: :forbidden - else - render json: response.body.to_json, - status: response.status || :bad_request + when :upstream + render json: result.body.to_json, + status: result.status || :bad_request end end diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index fd37479d7..ff3ea1c4d 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -37,7 +37,7 @@ def create from: from, number: params[:number], ) - fail Mds::Error.new(Mds::DOI_NOT_FOUND, status: 404) if doi_id.blank? + fail Mds::Error.new(Mds::DOI_UNKNOWN_TO_MDS, status: 404) if doi_id.blank? xml_b64 = data.present? ? Base64.strict_encode64(data) : nil attrs = diff --git a/app/models/concerns/authenticable.rb b/app/models/concerns/authenticable.rb index cf9981cd2..8e5979dba 100644 --- a/app/models/concerns/authenticable.rb +++ b/app/models/concerns/authenticable.rb @@ -200,8 +200,13 @@ def decode_api_key(token) payload_for_api_key(api_key, client) end + # Single source of truth for DC.* API key material (also used by RequestCredentials). + def self.api_key_token?(token) + token.present? && token.to_s.length > 20 && token.to_s.match?(/\ADC\./i) + end + def api_key_token?(token) - token.present? && token.length > 20 && token.match?(/\ADC\./i) + Authenticable.api_key_token?(token) end def get_payload(uid: nil, user: nil, password: nil) diff --git a/app/models/concerns/helpable.rb b/app/models/concerns/helpable.rb index 0f36681fe..c818086d6 100644 --- a/app/models/concerns/helpable.rb +++ b/app/models/concerns/helpable.rb @@ -109,21 +109,46 @@ def get_url end # When true, the stored `url` attribute is authoritative (draft/other/special providers). - # When false, resolve via Handle (`get_url`). Shared by REST and MDS protocol surfaces. + # When false, resolve via Handle (`get_url`). def uses_stored_landing_url? !is_registered_or_findable? || %w[europ].include?(provider_id) || type == "OtherDoi" end - # Landing URL for protocol responses. Nil when unknown or Handle has no value. - def resolved_landing_url - return url if uses_stored_landing_url? + # Full landing-URL policy for REST and MDS. Controllers only map the result. + def resolve_landing_url + if uses_stored_landing_url? + return url.present? ? LandingUrlResolution.ok(url) : LandingUrlResolution.no_content + end response = get_url - return nil unless response.status == 200 - response.body.dig("data", "values", 0, "data", "value") + if response.status == 200 + value = response.body.dig("data", "values", 0, "data", "value") + if value.present? + LandingUrlResolution.ok(value) + else + LandingUrlResolution.upstream( + status: response.status || 400, + body: response.body, + ) + end + elsif response.status == 400 && + response.body.dig("errors", 0, "title", "responseCode") == 301 + LandingUrlResolution.forbidden_handle + else + LandingUrlResolution.upstream( + status: response.status || 400, + body: response.body, + ) + end + end + + # Convenience for plain-text surfaces (MDS): URL string or nil → 204. + def resolved_landing_url + result = resolve_landing_url + result.ok? ? result.url : nil end def generate_random_provider_symbol diff --git a/app/models/landing_url_resolution.rb b/app/models/landing_url_resolution.rb new file mode 100644 index 000000000..07d364c9b --- /dev/null +++ b/app/models/landing_url_resolution.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +# Domain outcome for DOI landing-URL resolution (stored attribute vs Handle). +# Controllers map these to REST JSON or MDS plain-text; they do not re-encode policy. +class LandingUrlResolution + attr_reader :kind, :url, :status, :body + + KINDS = %i[ok no_content forbidden_handle upstream].freeze + + def initialize(kind, url: nil, status: nil, body: nil) + @kind = kind.to_sym + fail ArgumentError, "unknown kind #{kind.inspect}" unless KINDS.include?(@kind) + + @url = url + @status = status + @body = body + end + + def self.ok(url) + new(:ok, url: url) + end + + def self.no_content + new(:no_content) + end + + def self.forbidden_handle + new(:forbidden_handle) + end + + def self.upstream(status:, body:) + new(:upstream, status: status, body: body) + end + + def ok? + kind == :ok + end + + def no_content? + kind == :no_content + end + + def forbidden_handle? + kind == :forbidden_handle + end + + def upstream? + kind == :upstream + end +end From db459a17734b65b7bba58c09a26b76c92bcd1560 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 16:16:16 +0200 Subject: [PATCH 28/31] Define Authenticable.api_key_token? on the module itself. The class method was nested inside included, so it attached to User instead of Authenticable and RequestCredentials could not call it. --- app/models/concerns/authenticable.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/app/models/concerns/authenticable.rb b/app/models/concerns/authenticable.rb index 8e5979dba..eee64d20d 100644 --- a/app/models/concerns/authenticable.rb +++ b/app/models/concerns/authenticable.rb @@ -6,6 +6,12 @@ module Authenticable require "jwt" require "base64" + # Single source of truth for DC.* API key material (controllers via RequestCredentials, + # User via instance method below). Must live on the module, not inside `included`. + def self.api_key_token?(token) + token.present? && token.to_s.length > 20 && token.to_s.match?(/\ADC\./i) + end + included do # encode JWT token using SHA-256 hash algorithm def encode_token(payload) @@ -200,11 +206,6 @@ def decode_api_key(token) payload_for_api_key(api_key, client) end - # Single source of truth for DC.* API key material (also used by RequestCredentials). - def self.api_key_token?(token) - token.present? && token.to_s.length > 20 && token.to_s.match?(/\ADC\./i) - end - def api_key_token?(token) Authenticable.api_key_token?(token) end From 2985cdbe5d0984af83eec5730b6482f7ac94fe2e Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 17:04:03 +0200 Subject: [PATCH 29/31] Fix MDS draft read leak and tighten protocol consistency. Gate GET metadata/media with not_allowed_by_doi_and_user (404, not Ability :read) so clients cannot read another repository's drafts. Extend path/body DOI checks to non-DataCite bodies via Bolognese, surface Handle failures on GET /doi instead of collapsing to 204, align media URL schemes with landing URLs, limit DOI/media routes, and restore a pessimistic graphql constraint. --- Gemfile | 2 +- Gemfile.lock | 2 +- app/controllers/concerns/mds/doi_lookup.rb | 10 +++ app/controllers/mds/application_controller.rb | 2 + app/controllers/mds/dois_controller.rb | 19 ++++-- app/controllers/mds/media_controller.rb | 11 +++- app/controllers/mds/metadata_controller.rb | 2 +- app/models/concerns/helpable.rb | 4 +- app/services/mds/doi_minter.rb | 21 +++++- config/routes.rb | 5 +- spec/requests/mds/dois_spec.rb | 34 ++++++++++ spec/requests/mds/media_spec.rb | 46 +++++++++++++ spec/requests/mds/metadata_spec.rb | 65 +++++++++++++++++++ spec/services/mds/doi_minter_spec.rb | 34 ++++++++++ 14 files changed, 243 insertions(+), 14 deletions(-) create mode 100644 spec/services/mds/doi_minter_spec.rb diff --git a/Gemfile b/Gemfile index 0f4db0d32..96ecfbedf 100644 --- a/Gemfile +++ b/Gemfile @@ -35,7 +35,7 @@ gem "flipper", "~> 1.4", ">= 1.4.1" gem "flipper-active_support_cache_store", "~> 1.4", ">= 1.4.1" gem "gender_detector", "~> 2.1" gem "google-protobuf", "~> 4.34", ">= 4.34.1" -gem "graphql", ">= 2.6.7" +gem "graphql", "~> 2.5", ">= 2.6.7" gem "graphql-batch", "~> 0.6.1" gem "hashid-rails", "~> 1.4", ">= 1.4.1" gem "iso-639", "~> 0.3.8" diff --git a/Gemfile.lock b/Gemfile.lock index 928466489..d5fa483e8 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -818,7 +818,7 @@ DEPENDENCIES flipper-active_support_cache_store (~> 1.4, >= 1.4.1) gender_detector (~> 2.1) google-protobuf (~> 4.34, >= 4.34.1) - graphql (>= 2.6.7) + graphql (~> 2.5, >= 2.6.7) graphql-batch (~> 0.6.1) hashdiff (~> 1.2, >= 1.2.1) hashid-rails (~> 1.4, >= 1.4.1) diff --git a/app/controllers/concerns/mds/doi_lookup.rb b/app/controllers/concerns/mds/doi_lookup.rb index 231fa89c7..aae8f81a5 100644 --- a/app/controllers/concerns/mds/doi_lookup.rb +++ b/app/controllers/concerns/mds/doi_lookup.rb @@ -23,6 +23,16 @@ def find_datacite_doi!(doi_string, not_found: Mds::DOI_NOT_FOUND) doi end + # Same visibility rules as REST DataciteDoisController#show: findable is + # public; draft/registered only for the owning client (or provider/staff). + # Use 404 (not 403) so existence of another repository's draft is not confirmed. + # Do not use authorize! :read — Ability has a global can :read, Doi. + def authorize_mds_doi_read!(doi, not_found: Mds::DOI_UNKNOWN_TO_MDS) + if not_allowed_by_doi_and_user(doi: doi, user: current_user) + fail Mds::Error.new(not_found, status: 404) + end + end + # DOIs for the authenticated repository's first prefix (MDS GET /doi list). # Returns nil when there is nothing to list (caller should 204). def listed_dois_for_current_user diff --git a/app/controllers/mds/application_controller.rb b/app/controllers/mds/application_controller.rb index 06841049f..47a180c86 100644 --- a/app/controllers/mds/application_controller.rb +++ b/app/controllers/mds/application_controller.rb @@ -4,6 +4,8 @@ module Mds class ApplicationController < ActionController::API include ActionController::HttpAuthentication::Basic::ControllerMethods include CanCan::ControllerAdditions + # not_allowed_by_doi_and_user — same draft/registered gate as REST show + include Authenticable include RequestCredentials attr_accessor :current_user diff --git a/app/controllers/mds/dois_controller.rb b/app/controllers/mds/dois_controller.rb index 4aaf47a4b..deded091d 100644 --- a/app/controllers/mds/dois_controller.rb +++ b/app/controllers/mds/dois_controller.rb @@ -19,10 +19,21 @@ def index def show authorize! :get_url, @doi - url = @doi.resolved_landing_url - return head :no_content if url.blank? - - render_mds(url) + # Map full LandingUrlResolution so Handle failures are not collapsed to 204 + # (REST get_url keeps 403/upstream; MDS uses plain-text status codes). + result = @doi.resolve_landing_url + case result.kind + when :ok + render_mds(result.url) + when :no_content + head :no_content + when :forbidden_handle + fail Mds::Error.new("SERVER NOT RESPONSIBLE FOR HANDLE", status: 403) + when :upstream + status = result.status.to_i + status = 502 if status < 400 + fail Mds::Error.new("Handle service error", status: status) + end end def update diff --git a/app/controllers/mds/media_controller.rb b/app/controllers/mds/media_controller.rb index 938581c88..f91a49371 100644 --- a/app/controllers/mds/media_controller.rb +++ b/app/controllers/mds/media_controller.rb @@ -9,7 +9,7 @@ class MediaController < Mds::ApplicationController before_action :set_media, only: %i[show destroy] def index - authorize! :read, @doi + authorize_mds_doi_read!(@doi) media = @doi.media.to_a fail Mds::Error.new("No media for the DOI", status: 404) if media.blank? @@ -19,7 +19,7 @@ def index end def show - authorize! :read, @doi + authorize_mds_doi_read!(@doi) fail Mds::Error.new("No media for the DOI", status: 404) if @media.blank? @@ -75,7 +75,8 @@ def set_media end # MDS media body is mediaType=url. Both parts are required; blank type must not - # fall through to Media#set_defaults (text/plain). + # fall through to Media#set_defaults (text/plain). URL schemes match PUT /doi + # landing URLs (http/https/ftp) for a consistent MDS surface. def parse_media_body(data) media_type, url = data.to_s.split("=", 2) media_type = media_type.to_s.strip @@ -85,6 +86,10 @@ def parse_media_body(data) fail Mds::Error.new("Media type and URL missing", status: 400) end + unless url.match?(%r{\A(http|https|ftp)://\S+\z}) + fail Mds::Error.new("Not a valid HTTP(S) or FTP URL", status: 400) + end + [media_type, url] end end diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index ff3ea1c4d..82ec1108e 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -10,7 +10,7 @@ class MetadataController < Mds::ApplicationController def show doi = find_datacite_doi!(params[:doi_id], not_found: Mds::DOI_UNKNOWN_TO_MDS) - authorize! :read, doi + authorize_mds_doi_read!(doi, not_found: Mds::DOI_UNKNOWN_TO_MDS) xml = doi.xml return head :no_content if xml.blank? diff --git a/app/models/concerns/helpable.rb b/app/models/concerns/helpable.rb index c818086d6..e18d85287 100644 --- a/app/models/concerns/helpable.rb +++ b/app/models/concerns/helpable.rb @@ -145,7 +145,9 @@ def resolve_landing_url end end - # Convenience for plain-text surfaces (MDS): URL string or nil → 204. + # URL string only when resolution is :ok. Non-ok outcomes (no URL, Handle + # 403, upstream errors) yield nil — callers that need distinct Handle errors + # (REST get_url, MDS GET /doi) should use #resolve_landing_url instead. def resolved_landing_url result = resolve_landing_url result.ok? ? result.url : nil diff --git a/app/services/mds/doi_minter.rb b/app/services/mds/doi_minter.rb index d7534a016..46a52d954 100644 --- a/app/services/mds/doi_minter.rb +++ b/app/services/mds/doi_minter.rb @@ -9,7 +9,7 @@ class DoiMinter def resolve_doi_id(str, data:, from:, number: nil) path_doi = validate_doi(str) - body_doi = from == "datacite" ? doi_from_xml_identifier(data) : nil + body_doi = doi_from_metadata_body(data, from) if path_doi.present? Mds.assert_path_matches_body!(path_doi, body_doi) @@ -22,6 +22,18 @@ def resolve_doi_id(str, data:, from:, number: nil) end private + # Extract a DOI identifier from the metadata body for any recognized format. + # DataCite XML uses a direct parse; other formats go through Bolognese. + def doi_from_metadata_body(data, from) + return if data.blank? || from.blank? + + if from == "datacite" + return doi_from_xml_identifier(data) + end + + doi_from_bolognese(data, from) + end + def doi_from_xml_identifier(string) doc = Nokogiri::XML(string, nil, "UTF-8", &:noblanks) doc.remove_namespaces! @@ -30,6 +42,13 @@ def doi_from_xml_identifier(string) validate_doi(identifier) end + def doi_from_bolognese(string, from) + meta = Bolognese::Metadata.new(input: string, from: from) + validate_doi(meta.doi.presence || meta.id) + rescue StandardError + nil + end + def mint_unique_doi(str, number: nil) # Fail closed before generate_random_dois so blank/malformed mint # input returns a deterministic MDS client error (IdentifierError → 400). diff --git a/config/routes.rb b/config/routes.rb index b7047768f..9962f1e8a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,8 +18,9 @@ get "metadata", to: "metadata#show" delete "metadata/:doi_id", to: "metadata#destroy", constraints: { doi_id: /.+/ } - resources :dois, path: "/doi", constraints: { id: /.+/ } do - resources :media + resources :dois, path: "/doi", only: %i[index show update destroy], + constraints: { id: /.+/ } do + resources :media, only: %i[index show create destroy] end match "*path", to: "application#route_not_found", via: :all diff --git a/spec/requests/mds/dois_spec.rb b/spec/requests/mds/dois_spec.rb index 186575f4f..f88aa1d14 100644 --- a/spec/requests/mds/dois_spec.rb +++ b/spec/requests/mds/dois_spec.rb @@ -131,6 +131,40 @@ expect(last_response.status).to eq(400) expect(last_response.body).to eq("Not a valid HTTP(S) or FTP URL") end + + it "rejects path DOI that does not match body doi parameter" do + body = "doi=10.14454/body-doi\nurl=https://example.org/landing" + put "/doi/10.14454/path-doi", + body, + basic_headers.merge("CONTENT_TYPE" => "text/plain;charset=UTF-8") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq(Mds::PATH_BODY_MISMATCH) + end + end + + describe "GET /doi/:id Handle failures" do + it "returns 403 when Handle is not responsible for the DOI" do + findable_doi + allow_any_instance_of(DataciteDoi).to receive(:resolve_landing_url). + and_return(LandingUrlResolution.forbidden_handle) + + get "/doi/#{findable_doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(403) + expect(last_response.body).to eq("SERVER NOT RESPONSIBLE FOR HANDLE") + end + + it "returns an error status when Handle upstream fails" do + findable_doi + allow_any_instance_of(DataciteDoi).to receive(:resolve_landing_url). + and_return(LandingUrlResolution.upstream(status: 503, body: {})) + + get "/doi/#{findable_doi.doi}", nil, basic_headers + + expect(last_response.status).to eq(503) + expect(last_response.body).to eq("Handle service error") + end end describe "DELETE /doi/:id" do diff --git a/spec/requests/mds/media_spec.rb b/spec/requests/mds/media_spec.rb index 27e671af2..9ac0d2a1b 100644 --- a/spec/requests/mds/media_spec.rb +++ b/spec/requests/mds/media_spec.rb @@ -83,6 +83,52 @@ expect(last_response.body).to eq("Media type and URL missing") expect(doi.media.count).to eq(0) end + + it "rejects non-http(s)/ftp media URLs" do + post "/media/#{doi.doi}", + "application/pdf=not-a-url", + basic_headers.merge("CONTENT_TYPE" => "text/plain") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq("Not a valid HTTP(S) or FTP URL") + expect(doi.media.count).to eq(0) + end + end + + describe "cross-client media visibility" do + let(:other_client) do + create( + :client, + provider: provider, + symbol: "DATACITE.OTHER", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:other_client_prefix) do + create(:client_prefix, client: other_client, prefix: prefix) + end + let!(:other_draft) do + create( + :doi, + client: other_client, + doi: "10.14454/other-draft-media", + aasm_state: "draft", + ) + end + + it "does not list media for another repository's draft DOI" do + create( + :media, + doi: other_draft, + media_type: "application/pdf", + url: "https://example.org/secret.pdf", + ) + + get "/media/#{other_draft.doi}", nil, basic_headers + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("DOI is unknown to MDS") + end end describe "GET /media/:doi_id" do diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb index 892e52b40..11bea73ef 100644 --- a/spec/requests/mds/metadata_spec.rb +++ b/spec/requests/mds/metadata_spec.rb @@ -125,6 +125,71 @@ expect(last_response.status).to eq(404) expect(last_response.body).to eq("DOI is unknown to MDS") end + + context "cross-client visibility" do + let(:other_client) do + create( + :client, + provider: provider, + symbol: "DATACITE.OTHER", + password: encrypt_password_sha256(ENV["MDS_PASSWORD"]), + ) + end + let!(:other_client_prefix) do + create(:client_prefix, client: other_client, prefix: prefix) + end + + it "does not leak another repository's draft metadata" do + draft = + create( + :doi, + client: other_client, + doi: "10.14454/other-draft-meta", + aasm_state: "draft", + ) + draft.update_columns(xml: xml) + + get "/metadata/#{draft.doi}", + nil, + basic_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(404) + expect(last_response.body).to eq("DOI is unknown to MDS") + end + + it "allows reading another repository's findable metadata" do + findable = + create( + :doi, + client: other_client, + doi: "10.14454/other-findable-meta", + aasm_state: "findable", + url: "https://example.org/public", + ) + findable.update_columns(xml: xml) + + get "/metadata/#{findable.doi}", + nil, + basic_headers.except("CONTENT_TYPE") + + expect(last_response.status).to eq(200) + expect(last_response.body).to include("resource") + end + end + end + + describe "PUT /metadata path vs non-DataCite body identifier" do + it "rejects path DOI that does not match schema.org @id" do + body = file_fixture("schema_org.json").read + # fixture @id is 10.5438/4K3M-NYVG — detected as schema_org by body shape + put "/metadata/10.14454/other-doi", + body, + basic_headers.merge("CONTENT_TYPE" => "application/json") + + expect(last_response.status).to eq(400) + expect(last_response.body).to eq(Mds::PATH_BODY_MISMATCH) + expect(DataciteDoi.where(doi: "10.14454/other-doi").count).to eq(0) + end end describe "DELETE /metadata/:doi_id" do diff --git a/spec/services/mds/doi_minter_spec.rb b/spec/services/mds/doi_minter_spec.rb new file mode 100644 index 000000000..c6e4159e7 --- /dev/null +++ b/spec/services/mds/doi_minter_spec.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Mds::DoiMinter do + subject(:minter) { described_class.new } + + describe "#resolve_doi_id" do + it "rejects path DOI that does not match schema_org body identifier" do + body = file_fixture("schema_org.json").read + + expect { + minter.resolve_doi_id( + "10.14454/other-doi", + data: body, + from: "schema_org", + ) + }.to raise_error(IdentifierError, Mds::PATH_BODY_MISMATCH) + end + + it "accepts matching path and schema_org body identifier" do + body = file_fixture("schema_org.json").read + # fixture DOI is 10.5438/4K3M-NYVG + doi = + minter.resolve_doi_id( + "10.5438/4K3M-NYVG", + data: body, + from: "schema_org", + ) + + expect(doi).to eq("10.5438/4k3m-nyvg") + end + end +end From ab11371b0992e0875672eba3d1e193dff1997f6f Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 17:11:41 +0200 Subject: [PATCH 30/31] Fix MDS cross-client specs: need a free prefix for other_client. Client create runs assign_prefix/check_prefix; with prefix_pool_size 1 the first client consumes the only free prefix and DATACITE.OTHER fails validation. --- spec/requests/mds/media_spec.rb | 3 ++- spec/requests/mds/metadata_spec.rb | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/spec/requests/mds/media_spec.rb b/spec/requests/mds/media_spec.rb index 9ac0d2a1b..f17874085 100644 --- a/spec/requests/mds/media_spec.rb +++ b/spec/requests/mds/media_spec.rb @@ -3,7 +3,8 @@ require "rails_helper" include Passwordable -describe "MDS Media API", type: :request, vcr: true, prefix_pool_size: 1 do +# prefix_pool_size 2: primary client + other_client for cross-repo visibility specs +describe "MDS Media API", type: :request, vcr: true, prefix_pool_size: 2 do let(:provider) do create( :provider, diff --git a/spec/requests/mds/metadata_spec.rb b/spec/requests/mds/metadata_spec.rb index 11bea73ef..ed2082fbc 100644 --- a/spec/requests/mds/metadata_spec.rb +++ b/spec/requests/mds/metadata_spec.rb @@ -3,7 +3,8 @@ require "rails_helper" include Passwordable -describe "MDS Metadata API", type: :request, vcr: true, prefix_pool_size: 1 do +# prefix_pool_size 2: primary client + other_client for cross-repo visibility specs +describe "MDS Metadata API", type: :request, vcr: true, prefix_pool_size: 2 do let(:provider) do create( :provider, From 9437d471becee10f690c6c33dbda5a5f3d7629c8 Mon Sep 17 00:00:00 2001 From: kaysiz Date: Thu, 13 Aug 2026 18:26:46 +0200 Subject: [PATCH 31/31] Constrain nested MDS media doi_id; document Poodle show event. Nested /doi/:doi_id/media needs doi_id: /./+ so slashy DOIs match, matching top-level media and Poodle intent. Keep metadata event show as in Poodle Metadatable#create_metadata. --- app/controllers/mds/metadata_controller.rb | 2 ++ config/routes.rb | 6 +++++- spec/routing/mds_routing_spec.rb | 5 ++++- 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/app/controllers/mds/metadata_controller.rb b/app/controllers/mds/metadata_controller.rb index 82ec1108e..9daf95477 100644 --- a/app/controllers/mds/metadata_controller.rb +++ b/app/controllers/mds/metadata_controller.rb @@ -40,6 +40,8 @@ def create fail Mds::Error.new(Mds::DOI_UNKNOWN_TO_MDS, status: 404) if doi_id.blank? xml_b64 = data.present? ? Base64.strict_encode64(data) : nil + # event: "show" matches Poodle Metadatable#create_metadata (REST attributes). + # Do not use "register" here — that would diverge from classic MDS/Poodle. attrs = ParamsSanitizer.new( { diff --git a/config/routes.rb b/config/routes.rb index 9962f1e8a..38516cb11 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -18,9 +18,13 @@ get "metadata", to: "metadata#show" delete "metadata/:doi_id", to: "metadata#destroy", constraints: { doi_id: /.+/ } + # Nested media parent param is :doi_id (not :id). Constrain it like top-level + # /media/:doi_id and like Poodle's intent for slashy DOIs. resources :dois, path: "/doi", only: %i[index show update destroy], constraints: { id: /.+/ } do - resources :media, only: %i[index show create destroy] + resources :media, + only: %i[index show create destroy], + constraints: { doi_id: /.+/ } end match "*path", to: "application#route_not_found", via: :all diff --git a/spec/routing/mds_routing_spec.rb b/spec/routing/mds_routing_spec.rb index c15e2b2bd..8c352bb46 100644 --- a/spec/routing/mds_routing_spec.rb +++ b/spec/routing/mds_routing_spec.rb @@ -66,10 +66,13 @@ def mds_url(path) ) end - it "routes nested media under /doi" do + it "routes nested media under /doi when DOI contains a slash" do expect(get: mds_url("/doi/10.14454/abc/media")).to route_to( "mds/media#index", doi_id: "10.14454/abc", ) + expect(post: mds_url("/doi/10.14454/abc/media")).to route_to( + "mds/media#create", doi_id: "10.14454/abc", + ) end it "routes GET /heartbeat to shared heartbeat#index (not MDS stub)" do