Merge a2a34fbadd into fbe9728f36

app/controllers/api/v1/apps_controller.rb

@@ -2,6 +2,9 @@
 
 class Api::V1::AppsController < Api::BaseController
   skip_before_action :require_authenticated_user!
+  before_action :validate_token_endpoint_auth_method!
+
+  TOKEN_ENDPOINT_AUTH_METHODS = %w(none client_secret_basic client_secret_post).freeze
 
   def create
     @app = Doorkeeper::Application.create!(application_options)
@@ -16,14 +19,25 @@ class Api::V1::AppsController < Api::BaseController
       redirect_uri: app_params[:redirect_uris],
       scopes: app_scopes_or_default,
       website: app_params[:website],
+      confidential: !app_public?,
     }
   end
 
+  def validate_token_endpoint_auth_method!
+    return unless app_params.include? :token_endpoint_auth_method
+
+    bad_request unless TOKEN_ENDPOINT_AUTH_METHODS.include? app_params[:token_endpoint_auth_method]
+  end
+
+  def app_public?
+    app_params[:token_endpoint_auth_method] == 'none'
+  end
+
   def app_scopes_or_default
     app_params[:scopes] || Doorkeeper.configuration.default_scopes
   end
 
   def app_params
-    params.permit(:client_name, :scopes, :website, :redirect_uris, redirect_uris: [])
+    params.permit(:client_name, :scopes, :website, :token_endpoint_auth_method, :redirect_uris, redirect_uris: [])
   end
 end

app/controllers/oauth/authorizations_controller.rb

@@ -19,7 +19,13 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController
   end
 
   def render_success
-    if skip_authorization? || (matching_token? && !truthy_param?('force_login'))
+    # FIXME: Find a better way to apply this validation: if the scopes only
+    # includes offline_access, then it's not valid, since offline_access doesn't
+    # actually give access to resources:
+    if pre_auth.scopes.all?('offline_access')
+      error = Doorkeeper::OAuth::InvalidRequestResponse.new(reason: :offline_access_only, missing_param: nil)
+      render :error, locals: { error_response: error }, status: 400
+    elsif skip_authorization? || (matching_token? && !truthy_param?('force_login'))
       redirect_or_render authorize_response
     elsif Doorkeeper.configuration.api_only
       render json: pre_auth

app/lib/scope_transformer.rb

@@ -14,6 +14,8 @@ class ScopeTransformer < Parslet::Transform
 
       # # override for profile scope which is read only
       @access = %w(read) if @term == 'profile'
+      # Override offline_access since it doesn't imply read or write access:
+      @access = %w(offline) if @term == 'offline_access'
     end
 
     def key

app/models/session_activation.rb

@@ -65,12 +65,22 @@ class SessionActivation < ApplicationRecord
   end
 
   def access_token_attributes
+    app = Doorkeeper::Application.find_by(superapp: true)
+    scopes = Doorkeeper::OAuth::Scopes.from_array(DEFAULT_SCOPES)
+
+    context = Doorkeeper::OAuth::Authorization::Token.build_context(
+      app,
+      Doorkeeper::OAuth::AUTHORIZATION_CODE,
+      scopes,
+      user_id
+    )
+
     {
-      application_id: Doorkeeper::Application.find_by(superapp: true)&.id,
-      resource_owner_id: user_id,
-      scopes: DEFAULT_SCOPES.join(' '),
-      expires_in: Doorkeeper.configuration.access_token_expires_in,
-      use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?,
+      application_id: context.client.id,
+      resource_owner_id: context.resource_owner,
+      scopes: context.scopes,
+      expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context),
+      use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context),
     }
   end
 end

app/models/user.rb

@@ -310,10 +310,12 @@ class User < ApplicationRecord
   def token_for_app(app)
     return nil if app.nil? || app.owner != self
 
-    Doorkeeper::AccessToken.find_or_create_by(application_id: app.id, resource_owner_id: id) do |t|
-      t.scopes            = app.scopes
-      t.expires_in        = Doorkeeper.configuration.access_token_expires_in
-      t.use_refresh_token = Doorkeeper.configuration.refresh_token_enabled?
+    context = Doorkeeper::OAuth::Authorization::Token.build_context(app, Doorkeeper::OAuth::AUTHORIZATION_CODE, app.scopes, app.owner.id)
+
+    Doorkeeper::AccessToken.find_or_create_by(application_id: context.client.id, resource_owner_id: context.resource_owner) do |t|
+      t.scopes            = context.scopes
+      t.expires_in        = Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context)
+      t.use_refresh_token = Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context)
     end
   end
 

app/presenters/oauth_metadata_presenter.rb

@@ -61,7 +61,7 @@ class OauthMetadataPresenter < ActiveModelSerializers::Model
   end
 
   def token_endpoint_auth_methods_supported
-    %w(client_secret_basic client_secret_post)
+    %w(none client_secret_basic client_secret_post)
   end
 
   def code_challenge_methods_supported

app/serializers/rest/credential_application_serializer.rb

@@ -8,7 +8,7 @@ class REST::CredentialApplicationSerializer < REST::ApplicationSerializer
   end
 
   def client_secret
-    object.secret
+    object.secret if object.confidential?
   end
 
   # Added for future forwards compatibility when we may decide to expire OAuth

app/services/app_sign_up_service.rb

@@ -27,12 +27,14 @@ class AppSignUpService < BaseService
   end
 
   def create_access_token!
+    context = Doorkeeper::OAuth::Authorization::Token.build_context(@app, Doorkeeper::OAuth::AUTHORIZATION_CODE, @app.scopes, @user.id)
+
     @access_token = Doorkeeper::AccessToken.create!(
-      application: @app,
-      resource_owner_id: @user.id,
-      scopes: @app.scopes,
-      expires_in: Doorkeeper.configuration.access_token_expires_in,
-      use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?
+      application: context.client,
+      resource_owner_id: context.resource_owner,
+      scopes: context.scopes,
+      expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context),
+      use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context)
     )
   end
 

config/initializers/doorkeeper.rb

@@ -31,10 +31,30 @@ Doorkeeper.configure do
   # If you want to disable expiration, set this to nil.
   access_token_expires_in nil
 
-  # Assign a custom TTL for implicit grants.
-  # custom_access_token_expires_in do |oauth_client|
-  #   oauth_client.application.additional_settings.implicit_oauth_expiration
-  # end
+  # context.grant_type to compare with Doorkeeper::OAUTH grant type constants
+  # context.client for client (Doorkeeper::Application)
+  # context.scopes for scopes
+  custom_access_token_expires_in do |context|
+    # If the client is confidential (all clients pre 4.3) and it hasn't
+    # requested offline_access, then we don't want to expire access tokens.
+    # Applications created by users are also considered confidential.
+    if context.client.confidential? && !context.scopes.exists?('offline_access')
+      nil
+    else
+      15.minutes.to_i
+    end
+  end
+
+  use_refresh_token do |context|
+    context.scopes.exists?('offline_access')
+  end
+
+  after_successful_strategy_response do |request, _response|
+    if request.is_a? Doorkeeper::OAuth::RefreshTokenRequest
+      Web::PushSubscription.where(access_token_id: request.refresh_token.id).update!(access_token_id: request.access_token.id)
+      SessionActivation.where(access_token_id: request.refresh_token.id).update!(access_token_id: request.access_token.id)
+    end
+  end
 
   # Use a custom class for generating the access token.
   # https://github.com/doorkeeper-gem/doorkeeper#custom-access-token-generator
@@ -71,6 +91,7 @@ Doorkeeper.configure do
   # https://github.com/doorkeeper-gem/doorkeeper/wiki/Using-Scopes
   default_scopes  :read
   optional_scopes :profile,
+                  :offline_access,
                   :write,
                   :'write:accounts',
                   :'write:blocks',
@@ -120,7 +141,9 @@ Doorkeeper.configure do
   # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
   # falls back to the `:client_id` and `:client_secret` params from the `params` object.
   # Check out the wiki for more information on customization
-  # client_credentials :from_basic, :from_params
+  #
+  # This is the default value:
+  client_credentials :from_basic, :from_params
 
   # Change the way access token is authenticated from the request object.
   # By default it retrieves first from the `HTTP_AUTHORIZATION` header, then
@@ -165,7 +188,17 @@ Doorkeeper.configure do
   #   http://tools.ietf.org/html/rfc6819#section-4.4.3
   #
 
-  grant_flows %w(authorization_code client_credentials)
+  grant_flows %w(authorization_code client_credentials refresh_token)
+
+  # If the client is not a confidential client, it should not be able to use the
+  # client_credentials grant flow, since it cannot keep a secret.
+  allow_grant_flow_for_client do |grant_flow, client|
+    if grant_flow == Doorkeeper::OAuth::CLIENT_CREDENTIALS
+      client.confidential?
+    else
+      true
+    end
+  end
 
   # Under some circumstances you might want to have applications auto-approved,
   # so that the user skips the authorization step.

config/locales/doorkeeper.en.yml

@@ -89,6 +89,7 @@ en:
         invalid_request:
           missing_param: 'Missing required parameter: %{value}.'
           request_not_authorized: Request need to be authorized. Required parameter for authorizing request is missing or invalid.
+          offline_access_only: The offline_access scope can only be used with other scopes.
           unknown: The request is missing a required parameter, includes an unsupported parameter value, or is otherwise malformed.
         invalid_resource_owner: The provided resource owner credentials are not valid, or resource owner cannot be found
         invalid_scope: The requested scope is invalid, unknown, or malformed.
@@ -118,6 +119,7 @@ en:
         read: Read-only access
         read/write: Read and write access
         write: Write-only access
+        offline: Access for an extended period of time
       title:
         accounts: Accounts
         admin/accounts: Administration of accounts
@@ -138,6 +140,7 @@ en:
         notifications: Notifications
         profile: Your Mastodon profile
         push: Push notifications
+        offline_access: Offline access
         reports: Reports
         search: Search
         statuses: Posts

spec/controllers/oauth/authorizations_controller_spec.rb

@@ -30,12 +30,19 @@ RSpec.describe Oauth::AuthorizationsController do
 
       context 'when app is already authorized' do
         before do
+          context = Doorkeeper::OAuth::Authorization::Token.build_context(
+            app,
+            Doorkeeper::OAuth::AUTHORIZATION_CODE,
+            app.scopes,
+            user.id
+          )
+
           Doorkeeper::AccessToken.find_or_create_for(
-            application: app,
-            resource_owner: user.id,
-            scopes: app.scopes,
-            expires_in: Doorkeeper.configuration.access_token_expires_in,
-            use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?
+            application: context.client,
+            resource_owner: context.resource_owner,
+            scopes: context.scopes,
+            expires_in: Doorkeeper::OAuth::Authorization::Token.access_token_expires_in(Doorkeeper.config, context),
+            use_refresh_token: Doorkeeper::OAuth::Authorization::Token.refresh_token_enabled?(Doorkeeper.config, context)
           )
         end
 

spec/fabricators/application_fabricator.rb

@@ -3,5 +3,5 @@
 Fabricator(:application, from: Doorkeeper::Application) do
   name         'Example'
   website      'http://example.com'
-  redirect_uri 'http://example.com/callback'
+  redirect_uri 'urn:ietf:wg:oauth:2.0:oob'
 end

spec/lib/scope_transformer_spec.rb

@@ -23,6 +23,12 @@ RSpec.describe ScopeTransformer do
       it_behaves_like 'a scope', nil, 'profile', 'read'
     end
 
+    context 'with scope "offline_access"' do
+      let(:input) { 'offline_access' }
+
+      it_behaves_like 'a scope', nil, 'offline_access', 'offline'
+    end
+
     context 'with scope "read"' do
       let(:input) { 'read' }
 

spec/requests/api/v1/apps_spec.rb

@@ -36,6 +36,7 @@ RSpec.describe 'Apps' do
         expect(app).to be_present
         expect(app.scopes.to_s).to eq scopes
         expect(app.redirect_uris).to eq redirect_uris
+        expect(app.confidential).to be true
 
         expect(response.parsed_body).to match(
           a_hash_including(
@@ -55,6 +56,76 @@ RSpec.describe 'Apps' do
       end
     end
 
+    context 'without being a confidential application' do
+      let(:client_name) { 'Test confidential app' }
+      let(:params) do
+        {
+          client_name: client_name,
+          redirect_uris: redirect_uris,
+          scopes: scopes,
+          website: website,
+          token_endpoint_auth_method: 'none',
+        }
+      end
+
+      it 'creates an public OAuth app', :aggregate_failures do
+        subject
+
+        expect(response).to have_http_status(200)
+        expect(response.content_type)
+          .to start_with('application/json')
+
+        app = Doorkeeper::Application.find_by(name: client_name)
+
+        expect(app).to be_present
+        expect(app.scopes.to_s).to eq scopes
+        expect(app.redirect_uris).to eq redirect_uris
+        expect(app.confidential).to be false
+
+        expect(response.parsed_body).to match(
+          a_hash_including(
+            id: app.id.to_s,
+            client_id: app.uid,
+            client_secret: nil,
+            client_secret_expires_at: 0,
+            name: client_name,
+            website: website,
+            scopes: ['read', 'write'],
+            redirect_uris: redirect_uris,
+            # Deprecated properties as of 4.3:
+            redirect_uri: redirect_uri,
+            vapid_key: Rails.configuration.x.vapid_public_key
+          )
+        )
+      end
+    end
+
+    context 'when token_endpoint_auth_method is unknown' do
+      let(:client_name) { 'Test unknown auth app' }
+      let(:params) do
+        {
+          client_name: client_name,
+          redirect_uris: redirect_uris,
+          scopes: scopes,
+          website: website,
+          # Not yet supported:
+          token_endpoint_auth_method: 'private_key_jwt',
+        }
+      end
+
+      it 'does not create an OAuth app', :aggregate_failures do
+        subject
+
+        expect(response).to have_http_status(400)
+        expect(response.content_type)
+          .to start_with('application/json')
+
+        app = Doorkeeper::Application.find_by(name: client_name)
+
+        expect(app).to_not be_present
+      end
+    end
+
     context 'without scopes being supplied' do
       let(:scopes) { nil }
 

spec/requests/oauth/token_spec.rb

@@ -5,17 +5,19 @@ require 'rails_helper'
 RSpec.describe 'Managing OAuth Tokens' do
   describe 'POST /oauth/token' do
     subject do
-      post '/oauth/token', params: params
+      post '/oauth/token', params: params, headers: {
+        # This is using the OAuth client_secret_basic client authentication method
+        Authorization: ActionController::HttpAuthentication::Basic.encode_credentials(application.uid, application.secret),
+      }
     end
 
     let(:application) do
       Fabricate(:application, scopes: 'read write follow', redirect_uri: 'urn:ietf:wg:oauth:2.0:oob')
     end
+
     let(:params) do
       {
         grant_type: grant_type,
-        client_id: application.uid,
-        client_secret: application.secret,
         redirect_uri: 'urn:ietf:wg:oauth:2.0:oob',
         code: code,
         scope: scope,
@@ -103,6 +105,53 @@ RSpec.describe 'Managing OAuth Tokens' do
         end
       end
     end
+
+    context "with grant_type 'refresh_token'" do
+      let(:grant_type) { 'refresh_token' }
+
+      let!(:user) { Fabricate(:user) }
+      let!(:application) { Fabricate(:application, scopes: 'read offline_access') }
+      let!(:access_token) do
+        Fabricate(
+          :accessible_access_token,
+          resource_owner_id: user.id,
+          application: application,
+          # Even though the `application` uses the `offline_access` scope, the
+          # generation of a refresh token only happens when the model is created
+          # with `use_refresh_token: true`.
+          #
+          # This is normally determined from the request to create the access
+          # token, but here we are just creating the access token model, so we
+          # need to force the `access_token` to have `use_refresh_token: true`
+          use_refresh_token: true
+        )
+      end
+
+      let!(:web_push_subscription) { Fabricate(:web_push_subscription, user: user, access_token: access_token) }
+
+      let(:params) do
+        {
+          grant_type: grant_type,
+          refresh_token: access_token.refresh_token,
+        }
+      end
+
+      it 'updates the Web::PushSubscription when refreshed' do
+        expect { subject }
+          .to change { access_token.reload.revoked_at }.from(nil).to(be_present)
+
+        expect(response).to have_http_status(200)
+
+        new_token = Doorkeeper::AccessToken.by_token(response.parsed_body[:access_token])
+
+        expect(web_push_subscription.reload.access_token_id).to eq(new_token.id)
+
+        # Assert that there are definitely no subscriptions left for the
+        # previous access token:
+        expect(Web::PushSubscription.where(access_token: access_token.id).count)
+          .to eq(0)
+      end
+    end
   end
 
   describe 'POST /oauth/revoke' do

spec/requests/well_known/oauth_metadata_spec.rb

@@ -25,7 +25,7 @@ RSpec.describe 'The /.well-known/oauth-authorization-server request' do
       scopes_supported: Doorkeeper.configuration.scopes.map(&:to_s),
       response_types_supported: Doorkeeper.configuration.authorization_response_types,
       response_modes_supported: Doorkeeper.configuration.authorization_response_flows.flat_map(&:response_mode_matches).uniq,
-      token_endpoint_auth_methods_supported: %w(client_secret_basic client_secret_post),
+      token_endpoint_auth_methods_supported: %w(none client_secret_basic client_secret_post),
       grant_types_supported: grant_types_supported,
       code_challenge_methods_supported: Doorkeeper.configuration.pkce_code_challenge_methods_supported,
       # non-standard extension: