Add API V2 from DMPonline #3587

gjacob24 · 2025-11-26T14:57:52Z

PR for issue #3586

This issue branch uses the doorkeeper gem to implement the Authorization Code Grant Flow (ACGF) part of the v2 API.
(The Client Credentials Flow (CCF) part of the v2 API (which is what the org-admins will use) will be implemented separately in a future piece of work.)

github-actions · 2025-11-26T14:58:54Z

</tr>

	1 Error
🚫	Please include a CHANGELOG entry. `You can find it at [CHANGELOG.md](https://github.com/DMPRoadmap/roadmap/blob/main/CHANGELOG.md).`

	2 Warnings
⚠️	There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior. \n Ignore this warning if the PR ONLY contains translation.io synced updates.
⚠️	This PR is too big! Consider breaking it down into smaller PRs.

Generated by 🚫 Danger

aaronskiba · 2025-11-26T17:14:31Z

Thank you for this PR.

During today's DMPRoadmap Monthly all-team meeting I mentioned how there are also related DMPTool v2 API test files. Would you like me to add them to this PR?

momo3404 · 2025-11-27T18:14:59Z

I'm seeing a lot of rubocop errors in here. This might be a difference in the configuration of Rubocop between the Roadmap repo and DMP Online. Let me know how you'd want to approach this. If possible, I could start a PR attempting to fix these.

aaronskiba · 2025-11-28T16:11:02Z

app/views/api/v2/datasets/_show.json.jbuilder

+
+  json.type output.output_type
+  json.title output.title
+  json.doi_url output.doi_url


It looks like output.doi_url will raise *** NoMethodError Exception: undefined method 'doi_url' for #<ResearchOutput ....

Does DMPonline have a doi_url method defined? If not, I'm also seeing the following code comment in app/services/external_apis/doi_service.rb:

def mint(plan:) SecureRandom.uuid # Minted DOIs should be stored as an Identifier. For example: # doi_url = "#{landing_page_url}#{doi}" # Identifier.new(identifiable: plan, value: doi_url) # When this service is active and the above identifier is available, # the link to the DOI will appear on the Project Details page, in plan # exports and will become the `dmp_id` in this system's API responses end

Hi @aaronskiba, apologies I missed this. Not sure why it didn't throw an error when I tested it. Please remove line 10 json.doi_url output.doi_url from app/views/api/v2/datasets/_show.json.jbuilder because it's from the DOI in research outputs feature that we added in DMPonline. The DOI feature was meant to be shared back to roadmap by now in a different PR, that's why it's missing.

Regarding the file app/services/external_apis/doi_service.rb, this is not part of the API V2 code. It's part of something Brian worked on for DOI minting.

martaribeiro · 2025-11-28T16:34:27Z

Here is the document with the API calls.
API_v2.pdf

Please let us know if needs changing

gjacob24 · 2025-11-28T16:41:19Z

I'm seeing a lot of rubocop errors in here. This might be a difference in the configuration of Rubocop between the Roadmap repo and DMP Online. Let me know how you'd want to approach this. If possible, I could start a PR attempting to fix these.

@momo3404 apologies we missed this. I can do this and push it back next week, if that's ok.

martaribeiro · 2025-11-28T16:47:25Z

These two docs are more for developers support. It might be useful
[Authorization_Code_Grant_Flow.pdf](https://github.com/user-attachments/files/23827
doorkeeper_gem_for_OAuth_2.0_flows.pdf
178/Authorization_Code_Grant_Flow.pdf)

aaronskiba · 2025-11-28T17:02:27Z

These two docs are more for developers support. It might be useful [Authorization_Code_Grant_Flow.pdf](https://github.com/user-attachments/files/23827 doorkeeper_gem_for_OAuth_2.0_flows.pdf 178/Authorization_Code_Grant_Flow.pdf)

Awesome, thank you. Sorry, the links got a little bungled up in the comment. :P
https://github.com/user-attachments/files/23827180/doorkeeper_gem_for_OAuth_2.0_flows.pdf works.
Could you just provide the Authorization_Code_Grant_Flow.pdf link again?

gjacob24 · 2025-12-01T12:27:53Z

These two docs are more for developers support. It might be useful [Authorization_Code_Grant_Flow.pdf](https://github.com/user-attachments/files/23827 doorkeeper_gem_for_OAuth_2.0_flows.pdf 178/Authorization_Code_Grant_Flow.pdf)

Awesome, thank you. Sorry, the links got a little bungled up in the comment. :P https://github.com/user-attachments/files/23827180/doorkeeper_gem_for_OAuth_2.0_flows.pdf works. Could you just provide the Authorization_Code_Grant_Flow.pdf link again?

@aaronskiba Here you go:
Authorization_Code_Grant_Flow.pdf

Let us know if this doesn't work.

aaronskiba · 2025-12-08T19:17:49Z

I'm encountering the following when I test the GET /api/v2/heartbeat endpoint (I can also see that the endpoint is returning a 500 for DMPOnline):

$ curl http://127.0.0.1:3000/api/v2/heartbeat
NoMethodError at /api/v2/heartbeat
==================================

undefined method `name' for nil:NilClass

> To access an interactive console with this error, point your browser to: /__better_errors


app/views/api/v2/_standard_response.json.jbuilder, line 18
----------------------------------------------------------

``` ruby
   13   json.ignore_nil!
   14   
   15   json.server @server
   16   json.source "#{request.method} #{request.path}"
   17   json.time Time.now.to_formatted_s(:iso8601)
>  18   json.client @client.name
   19   json.code response.status
   20   json.message Rack::Utils::HTTP_STATUS_CODES[response.status]
   21   
   22   if response.status == 200
   23

App backtrace

app/views/api/v2/_standard_response.json.jbuilder:18:in `_app_views_api_v___standard_response_json_jbuilder__1455696549825964062_81440'
app/views/api/v2/error.json.jbuilder:3:in `_app_views_api_v__error_json_jbuilder__3591388735008983784_81420'
app/controllers/api/v2/base_api_controller.rb:69:in `handle_internal_server_error'
app/controllers/api/v2/base_api_controller.rb:58:in `handle_exception'

Full backtrace
...

aaronskiba · 2025-12-08T20:50:30Z

db/migrate/20251119131055_create_doorkeeper_tables.rb

+      # Remove `null: false` if you are planning to use grant flows
+      # that doesn't require redirect URI to be used during authorization
+      # like Client Credentials flow or Resource Owner Password.
+      t.text    :redirect_uri, null: false


Since we will be implementing the v2 API Client Credentials Flow in a future piece of work, maybe we should remove null: false on redirect_uri, as the code comment advises.

aaronskiba · 2025-12-08T22:02:56Z

app/controllers/api/v2/templates_controller.rb

+      respond_to :json
+
+      # GET /api/v2/templates
+      def index


There is no route defined for this in config/routes.rb. I think we need something like resources :templates, only: :index inside of namespace :v2 do.

aaronskiba · 2025-12-10T22:34:39Z

app/views/api/v2/error.json.jbuilder

+
+# json.items []
+json.message @payload[:message]
+json.details @payload[:details]


I can see @payload[:message] being assigned within app/controllers/api/v2/base_api_controller.rb. But is @payload[:details] being set anywhere? Maybe it can be removed.

aaronskiba · 2025-12-11T18:10:10Z

app/controllers/api/v2/base_api_controller.rb

+        @client = OauthApplication.find(doorkeeper_token.application_id) if doorkeeper_token
+        @scopes = doorkeeper_token.scopes.to_a if doorkeeper_token
+        return unless doorkeeper_token&.resource_owner_id
+
+        @resource_owner = User.find(doorkeeper_token.resource_owner_id)


I guess this could be refactored. An outer if doorkeeper_token could be used to replace the repeated if doorkeeper_token checks.

aaronskiba · 2025-12-11T19:08:53Z

app/models/oauth_application.rb

+# created_at:         :datetime
+# updated_at:         :datetime
+
+class OauthApplication < ApplicationRecord


I'm a little worried about directly inheriting from ApplicationRecord and simply mapping to the Doorkeeper tables in app/models/oauth_access_grant.rb, app/models/oauth_access_token.rb, and app/models/oauth_application.rb. These classes will not include Doorkeeper’s behaviour (validations, associations, etc.).

Currently, @client = OauthApplication.find(doorkeeper_token.application_id) if doorkeeper_token appears to be the only code that references these models (see app/controllers/api/v2/base_api_controller.rb), which should be safe. But I worry about issues arising in the future.

Unless this was implemented for a specific reason, I might suggest one of the following:

Remove these classes altogether

These tables/models can instead be referenced via Doorkeeper::Application, Doorkeeper::AccessToken;, and Doorkeeper::AccessGrant. (e.g. @client = Doorkeeper::Application.find(doorkeeper_token.application_id) if doorkeeper_token)

Update each of the classes to inherit from the appropriate Doorkeeper::Application, Doorkeeper::AccessToken;, or Doorkeeper::AccessGrant.

class OauthApplication < Doorkeeper::Application; end class OauthAccessToken < Doorkeeper::AccessToken; end class OauthAccessGrant < Doorkeeper::AccessGrant; end

aaronskiba · 2025-12-12T16:14:10Z

app/policies/api/v2/plans_policy.rb

+          Plan.joins(:roles)
+              .where(roles: { user_id: @resource_owner.id, active: true })
+              .distinct


It might be even better to reuse the Plan.active(@resource_owner) scope for this query. Note that a bit of refactoring could be performed in that scope to make this more efficient (i.e. don't use includes(:template, :roles))

momo3404 · 2025-12-12T20:36:26Z

Please let me know if I've done something wrong here, but I keep getting 302s when executing the GET command from the Authorization Code Grant Flow.
curl -i -X GET "http://127.0.0.1:3000/oauth/authorize?response_type=code&client_id=[my_client_id]&redirect_uri=[redirect_uri]" HTTP/1.1 302 Found x-frame-options: SAMEORIGIN x-xss-protection: 0 x-content-type-options: nosniff x-permitted-cross-domain-policies: none referrer-policy: strict-origin-when-cross-origin location: http://127.0.0.1:3000/users/sign_in content-type: text/html; charset=utf-8 cache-control: no-cache

But the command still worked, because after calling Doorkeeper::AccessGrant.first I found my authorization which I used for the POST command, and that worked perfectly. So is this a known error, or something I'm missing? I was also signed in when making the request.

aaronskiba · 2025-12-15T21:53:09Z

app/views/api/v2/templates/index.json.jbuilder

+    end
+
+    json.template_id do
+      identifier = Api::V2::ConversionService.to_identifier(context: @application,


I think @application will evaluate to nil here.
I can see that app/controllers/api/v1/base_api_controller.rb sets @application = ApplicationService.application_name, whereas app/controllers/api/v2/base_api_controller.rb is setting @server = ApplicationService.application_name.

gjacob24 and others added 2 commits November 19, 2025 15:06

Add API V2 code from DMPonline service

7a2b797

add doorkeeper to routes

63b65c2

aaronskiba reviewed Nov 28, 2025

View reviewed changes

Add rubocop fixes

10380fa

aaronskiba reviewed Dec 8, 2025

View reviewed changes

aaronskiba reviewed Dec 10, 2025

View reviewed changes

aaronskiba reviewed Dec 11, 2025

View reviewed changes

aaronskiba reviewed Dec 12, 2025

View reviewed changes

aaronskiba reviewed Dec 15, 2025

View reviewed changes

gjacob24 requested a review from johnpinto1 December 18, 2025 11:17

Add API V2 from DMPonline #3587

Are you sure you want to change the base?

Add API V2 from DMPonline #3587

Uh oh!

Conversation

gjacob24 commented Nov 26, 2025

Uh oh!

github-actions bot commented Nov 26, 2025

Uh oh!

aaronskiba commented Nov 26, 2025

Uh oh!

momo3404 commented Nov 27, 2025

Uh oh!

aaronskiba Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gjacob24 Nov 28, 2025

Choose a reason for hiding this comment

Uh oh!

martaribeiro commented Nov 28, 2025

Uh oh!

gjacob24 commented Nov 28, 2025

Uh oh!

martaribeiro commented Nov 28, 2025

Uh oh!

aaronskiba commented Nov 28, 2025

Uh oh!

gjacob24 commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronskiba commented Dec 8, 2025

App backtrace

Uh oh!

aaronskiba Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aaronskiba Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

aaronskiba Dec 10, 2025

Choose a reason for hiding this comment

Uh oh!

aaronskiba Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

aaronskiba Dec 11, 2025

Choose a reason for hiding this comment

Uh oh!

aaronskiba Dec 12, 2025

Choose a reason for hiding this comment

Uh oh!

momo3404 commented Dec 12, 2025

Uh oh!

aaronskiba Dec 15, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aaronskiba Nov 28, 2025 •

edited

Loading

gjacob24 commented Dec 1, 2025 •

edited

Loading

aaronskiba Dec 8, 2025 •

edited

Loading