Aaron/feature/v2 api token for internal users#3597
Aaron/feature/v2 api token for internal users#3597aaronskiba wants to merge 10 commits intoapi_v2_dmponlinefrom
Conversation
Generated by 🚫 Danger |
|
|
||
| module Api | ||
| module V2 | ||
| class InternalUserAccessTokensController < ApplicationController |
There was a problem hiding this comment.
Nitpick, but the class title has Internal User Access Token*s* which doesn't match internal_user_access_token, would be good to just standardize it.
There was a problem hiding this comment.
Good to nitpick, I certainly do my share...
I was a little unsure about how to do this.
# config/routes.rb
resource :internal_user_access_token, only: :createHere, the singular route resource :internal_user_access_token is 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. InternalUserAccessTokensController rather than InternalUserAccessTokenController). I could override that (pretty easy to do), but even things like Devise::SessionsController (rather than Devise::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.
| 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' } |
There was a problem hiding this comment.
Would it be possible for this endpoint to be hit without JS?
| Doorkeeper::AccessToken.find_by(user_token_filter(user)) | ||
| end | ||
|
|
||
| def rotate!(user) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
I will at least add some handling in case the internal OAuth application is not found.
|
|
||
| private | ||
|
|
||
| def revoke_existing!(user) |
There was a problem hiding this comment.
Just double checking, do revoked tokens remain in the DB?
There was a problem hiding this comment.
Yes, cleaning up expired tokens is something we'll have to address even beyond this PR.
|
There's two documentation related Rubocop errors
|
- Creates a first-party Doorkeeper client for issuing internal v2 API tokens - Sets redirect_uri to OOB, scopes to 'read', and marks it as confidential - Ensures the internal application exists in all environments before token service is used
aaronskiba
force-pushed
the
aaron/feature/v2-api-token-for-internal-users
branch
from
250078d to
5d0e68c
Compare
This service manages user-scoped v2 API access tokens for internal app users. - Tokens are equivalent to first-party Personal Access Tokens (PATs) and are issued directly to authenticated users, bypassing the full OAuth 2.0 authorization_code flow. - Supports token creation, rotation, and revocation. - Uses Doorkeeper::AccessToken records for consistent scoping, expiry, and revocation handling. - Designed strictly for internal usage; third-party OAuth clients are not supported.
Adds `Api::V2::InternalUserAccessTokensController#create` with Pundit authorization and routing. Also reuses the existing `users/refresh_token.js.erb` response to update the UI via JS.
This change updates `app/views/devise/registrations/_api_token.html.erb` to include support for the v2 API access token. Existing v0/v1 token support is retained. - Introduce V2 token lookup via `Api::V2::InternalUserAccessTokenService` - Display a dedicated V2 API access token section with its own regeneration action
This change breaks refactors `_api_token.html.erb` into additional separate partials: 1) app/views/devise/registrations/_legacy_api_token.html.erb 2) app/views/devise/registrations/_v2_api_token.html.erb In addition to the refactor, the following changes have been made: - `<div id="api-token"` has been renamed to `<div id="legacy-api-token"` - A `<div id="api-tokens">` wrapper has been added in app/views/devise/registrations/_api_token.html.erb. - `app/views/users/refresh_token.js.erb` now references the '#api-tokens' wrapper.
The API Access tab is now visible to all users to support the new v2 API token, which is accessible to everyone. The existing v0/v1 legacy token remains restricted and continues to use the previous authorization and rendering logic within the tab.
Styling changes can be viewed at /users/edit#api-details
aaronskiba
force-pushed
the
aaron/feature/v2-api-token-for-internal-users
branch
from
5d0e68c to
6f09aab
Compare
`InternalUserAccessTokenService`: add `application!` (memoized lookup + raise) and `application_present?` (safe check with logging) `_v2_api_token.html.erb`: gate token UI on `application_present?` and show a warning when missing.
Add request specs for InternalUserAccessTokensController - Include both authenticated & unauthenticated user scenarios - Include both present & absent internal OAuth app scenarios Add service specs for InternalUserAccessTokenService - Test token retrieval, rotation, and OAuth app presence - Verify old token revocation when rotating Add view specs for API token partials - Test legacy partial rendering based on `user.can_use_api?` - Test OAuth application availability scenarios
Add `defaults: { format: :js }` to the internal_user_access_token route, allowing callers to omit the explicit format parameter.
aaronskiba
force-pushed
the
aaron/feature/v2-api-token-for-internal-users
branch
from
a5c24c6 to
23a2230
Compare
2 participants
Fixes # .
Changes proposed in this PR: