-
Notifications
You must be signed in to change notification settings - Fork 119
Aaron/feature/v2 api token for internal users #3597
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: api_v2_dmponline
Are you sure you want to change the base?
Changes from all commits
1e5e00e
4220103
2f9e5e2
e556374
577da3b
15fb3ca
6f09aab
1c4d6e2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Api | ||
| module V2 | ||
| # Controller for managing the current user's internal V2 API access token. | ||
| # Provides token rotation for authenticated internal users. | ||
| # See Api::V2::InternalUserAccessTokenService for token implementation details. | ||
| class InternalUserAccessTokensController < ApplicationController | ||
| # POST "/api/v2/internal_user_access_token" | ||
| def create | ||
| authorize current_user, :internal_user_v2_access_token? | ||
| @token = Api::V2::InternalUserAccessTokenService.rotate!(current_user) | ||
| respond_to do |format| | ||
| format.js { render 'users/refresh_token' } | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible for this endpoint to be hit without JS? |
||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,79 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| module Api | ||
| module V2 | ||
| # Service responsible for user-scoped v2 API access tokens, strictly for | ||
| # internal users of this application. | ||
| # | ||
| # Tokens issued by this service are functionally equivalent to Personal Access | ||
| # Tokens (PATs) for first-party usage. They are minted directly for a user | ||
| # who is already authenticated in the application, bypassing the standard | ||
| # OAuth 2.0 authorization_code redirect and consent flow. | ||
| # | ||
| # This design is intentional: | ||
| # - tokens are internal to this application (first-party) | ||
| # - tokens are owned by a single user and scoped accordingly | ||
| # - token creation, rotation, and revocation happen entirely within the app UI | ||
| # | ||
| # Tokens are stored as Doorkeeper::AccessToken records to leverage existing | ||
| # scoping, expiry, and revocation mechanisms. | ||
| # | ||
| # This service does NOT support third-party OAuth clients or delegated consent flows. | ||
| class InternalUserAccessTokenService | ||
| READ_SCOPE = 'read' | ||
| INTERNAL_OAUTH_APP_NAME = Rails.application.config.x.application.internal_oauth_app_name | ||
|
|
||
| class << self | ||
| def for_user(user) | ||
| Doorkeeper::AccessToken.find_by(user_token_filter(user)) | ||
| end | ||
|
|
||
| def rotate!(user) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be possible for this to fail in the creation stage? If so, I'm just wondering if we have ample error handling for that scenario.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I will at least add some handling in case the internal OAuth application is not found. |
||
| revoke_existing!(user) | ||
|
|
||
| Doorkeeper::AccessToken.create!( | ||
| user_token_filter(user) | ||
| .merge(expires_in: nil) # Overrides Doorkeeper's `access_token_expires_in` | ||
| ) | ||
| end | ||
|
|
||
| # Used by views (e.g. devise/registrations/_v2_api_token.html.erb) to safely | ||
| # gate token UI if the internal OAuth application is missing. | ||
| def application_present? | ||
| application! | ||
| true | ||
| rescue StandardError => e | ||
| Rails.logger.error(e.message) | ||
| false | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def application! | ||
| @application ||= Doorkeeper::Application.find_by( | ||
| name: INTERNAL_OAUTH_APP_NAME | ||
| ) || raise( | ||
| StandardError, | ||
| "Required Doorkeeper application '#{INTERNAL_OAUTH_APP_NAME}' not found. " \ | ||
| 'Please ensure the application exists in the database.' | ||
| ) | ||
| end | ||
|
|
||
| def revoke_existing!(user) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just double checking, do revoked tokens remain in the DB?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, cleaning up expired tokens is something we'll have to address even beyond this PR. |
||
| Doorkeeper::AccessToken | ||
| .where(user_token_filter(user)) | ||
| .update_all(revoked_at: Time.current) | ||
| end | ||
|
|
||
| def user_token_filter(user) | ||
| { | ||
| resource_owner_id: user.id, | ||
| application_id: application!.id, | ||
| scopes: READ_SCOPE, | ||
| revoked_at: nil | ||
| } | ||
| end | ||
| end | ||
| end | ||
| end | ||
| end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,25 +1,11 @@ | ||
| <%# locals: user %> | ||
|
|
||
| <% api_wikis = Rails.configuration.x.application.api_documentation_urls %> | ||
| <div id="api-token" class="col-xs-12"> | ||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= label_tag(:api_token, _('Access token'), class: 'form-label') %> | ||
| <% if user.api_token.present? %> | ||
| <%= user.api_token %> | ||
| <% else %> | ||
| <%= _("Click the button below to generate an API token") %> | ||
| <% end %> | ||
| </div> | ||
| <div class="form-control mb-3 col-xs-12"> | ||
| <%= label_tag(:api_information, _('Documentation'), class: 'form-label') %> | ||
| <br> | ||
| <%= _('See the <a href="%{api_v0_wiki}">documentation for v0</a> for more details on the original API which includes access to statistics, the full text of plans and the ability to connect users with departments.').html_safe % { api_v0_wiki: api_wikis[:v0] } %></a> | ||
| <br><br> | ||
| <%= _('See the <a href="%{api_v1_wiki}">documentation for v1</a> for more details on the API that supports the <a href="%{rda_standard_url}">RDA Common metadata standard for DMPs.</a>').html_safe % { api_v1_wiki: api_wikis[:v1], rda_standard_url: 'https://github.com/RDA-DMP-Common/RDA-DMP-Common-Standard' } %></a> | ||
| </div> | ||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= link_to _("Regenerate token"), | ||
| refresh_token_user_path(user), | ||
| class: "btn btn-secondary", remote: true %> | ||
| </div> | ||
| <div id="api-tokens"> | ||
| <%# v2 API token %> | ||
| <%= render partial: "devise/registrations/v2_api_token", locals: { user: user } %> | ||
|
|
||
| <% if user.can_use_api? %> | ||
| <%# v0/v1 API token %> | ||
| <%= render partial: "devise/registrations/legacy_api_token", locals: { user: user } %> | ||
| <% end %> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| <%# locals: user %> | ||
|
|
||
| <% api_wikis = Rails.configuration.x.application.api_documentation_urls %> | ||
| <div id="legacy-api-token" class="card mb-4"> | ||
| <div class="card-heading"> | ||
| <%= _('Legacy API') %> | ||
| </div> | ||
| <div class="card-body"> | ||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= label_tag(:api_token, _('Access token'), class: 'form-label') %> | ||
| <% if user.api_token.present? %> | ||
| <code><%= user.api_token %></code> | ||
| <% else %> | ||
| <%= _("Click the button below to generate an API token") %> | ||
| <% end %> | ||
| </div> | ||
| <div class="form-control mb-3 col-xs-12"> | ||
| <%= label_tag(:api_information, _('Documentation'), class: 'form-label') %> | ||
| <br> | ||
| <%= _('See the <a href="%{api_v0_wiki}">documentation for v0</a> for more details on the original API which includes access to statistics, the full text of plans and the ability to connect users with departments.').html_safe % { api_v0_wiki: api_wikis[:v0] } %></a> | ||
| <br><br> | ||
| <%= _('See the <a href="%{api_v1_wiki}">documentation for v1</a> for more details on the API that supports the <a href="%{rda_standard_url}">RDA Common metadata standard for DMPs.</a>').html_safe % { api_v1_wiki: api_wikis[:v1], rda_standard_url: 'https://github.com/RDA-DMP-Common/RDA-DMP-Common-Standard' } %></a> | ||
| </div> | ||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= link_to _("Regenerate token"), | ||
| refresh_token_user_path(user), | ||
| class: "btn btn-secondary", remote: true %> | ||
| </div> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| <%# locals: user %> | ||
|
|
||
| <div id="v2-api-token" class="card mb-4"> | ||
| <div class="card-heading"> | ||
| <%= _('V2 API') %> | ||
| </div> | ||
| <div class="card-body"> | ||
| <% if Api::V2::InternalUserAccessTokenService.application_present? %> | ||
| <% token = Api::V2::InternalUserAccessTokenService.for_user(user) %> | ||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= label_tag(:api_token, _('Access token'), class: 'form-label') %> | ||
| <% if token.present? %> | ||
| <code><%= token.token %></code> | ||
| <% else %> | ||
| <%= _("Click the button below to generate an API token") %> | ||
| <% end %> | ||
| </div> | ||
|
|
||
| <div class="form-control mb-3 col-xs-8"> | ||
| <%= link_to _("Regenerate token"), | ||
| api_v2_internal_user_access_token_path(format: :js), | ||
| method: :post, | ||
| class: 'btn btn-secondary', | ||
| remote: true %> | ||
| </div> | ||
| <% else %> | ||
| <div class="alert alert-warning"> | ||
| <%= _("V2 API token service is currently unavailable. Please contact us for help.") %> | ||
| <%= mail_to Rails.application.config.x.organisation.helpdesk_email %> | ||
| </div> | ||
| <% end %> | ||
| </div> | ||
| </div> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,6 @@ | ||
| var msg = '<%= @success ? _("Successfully regenerate your API token.") : _("Unable to regenerate your API token.") %>'; | ||
|
|
||
| var context = $('#api-token'); | ||
| var context = $('#api-tokens'); | ||
| context.html('<%= escape_javascript(render partial: "/devise/registrations/api_token", locals: { user: current_user }) %>'); | ||
| renderNotice(msg); | ||
| toggleSpinner(false); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,8 @@ class Application < Rails::Application | |
|
|
||
| # Used throughout the system via ApplicationService.application_name | ||
| config.x.application.name = 'DMPRoadmap' | ||
| # Name of the internal Doorkeeper OAuth application for v2 API access tokens | ||
| config.x.application.internal_oauth_app_name = 'Internal v2 API Client' | ||
| # Used as the default domain when 'archiving' (aka anonymizing) a user account | ||
| # for example `[email protected]` becomes `1234@removed_accounts-example.org` | ||
| config.x.application.archived_accounts_email_suffix = '@removed_accounts-example.org' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| # frozen_string_literal: true | ||
|
|
||
| namespace :doorkeeper do | ||
| desc 'Ensure internal OAuth application exists' | ||
| task ensure_internal_app: :environment do | ||
| app = Doorkeeper::Application.find_or_create_by!( | ||
| name: Rails.application.config.x.application.internal_oauth_app_name | ||
| ) do |a| | ||
| a.scopes = 'read' | ||
| a.confidential = true | ||
| # OOB redirect URI used only as a placeholder. | ||
| # Tokens are minted server-side for already-authenticated first-party users. | ||
| # No redirect, authorization code, or third-party client is involved, | ||
| # so there is no security risk despite OOB deprecation. | ||
| a.redirect_uri = 'urn:ietf:wg:oauth:2.0:oob' | ||
| end | ||
|
|
||
| puts "Internal OAuth app ready (id=#{app.id}, uid=#{app.uid})" | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, but the class title has
Internal User Access Token*s*which doesn't matchinternal_user_access_token, would be good to just standardize it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to nitpick, I certainly do my share...
I was a little unsure about how to do this.
Here, the singular route
resource :internal_user_access_tokenis meant to signal that this is managing "the user's token" (one at a time), not a collection of tokens.However, even for singular resources, the Rails convention is to pluralise controller names (e.g.
InternalUserAccessTokensControllerrather thanInternalUserAccessTokenController). I could override that (pretty easy to do), but even things likeDevise::SessionsController(rather thanDevise::SessionController) are used, even though a user only has one session at a time; so it seems like the pluralisation convention is a strong one.