diff --git a/app/controllers/oauth/authorizations_controller.rb b/app/controllers/oauth/authorizations_controller.rb index e714cbb5b7091caaec41c09c065705ca24bf4e17..0817813f967e1363ab8a484dbf6cbd0ab3d7af12 100644 --- a/app/controllers/oauth/authorizations_controller.rb +++ b/app/controllers/oauth/authorizations_controller.rb @@ -5,7 +5,7 @@ class Oauth::AuthorizationsController < Doorkeeper::AuthorizationsController include InitializesCurrentUserMode include Gitlab::Utils::StrongMemoize - before_action :verify_confirmed_email!, :verify_confidential_application! + before_action :verify_confirmed_email! layout 'profile' @@ -77,18 +77,6 @@ def application_has_api_scope? doorkeeper_application&.includes_scope?(*::Gitlab::Auth::API_SCOPES) end - # Confidential apps require the client_secret to be sent with the request. - # Doorkeeper allows implicit grant flow requests (response_type=token) to - # work without client_secret regardless of the confidential setting. - # This leads to security vulnerabilities and we want to block it. - def verify_confidential_application! - render 'doorkeeper/authorizations/error' if authorizable_confidential? - end - - def authorizable_confidential? - pre_auth.authorizable? && pre_auth.response_type == 'token' && pre_auth.client.application.confidential - end - def verify_confirmed_email! return if current_user&.confirmed? diff --git a/config/initializers/doorkeeper.rb b/config/initializers/doorkeeper.rb index 58dd19a82709ec5f54bcebfb0e200c7032f3a401..7827ad8f168ad161e6194910a9d56534d350aa1b 100644 --- a/config/initializers/doorkeeper.rb +++ b/config/initializers/doorkeeper.rb @@ -90,11 +90,10 @@ # strings and the flows they enable are: # # "authorization_code" => Authorization Code Grant Flow - # "implicit" => Implicit Grant Flow # "password" => Resource Owner Password Credentials Grant Flow # "client_credentials" => Client Credentials Grant Flow # - grant_flows %w(authorization_code implicit password client_credentials) + grant_flows %w(authorization_code password client_credentials) # Under some circumstances you might want to have applications auto-approved, # so that the user skips the authorization step. diff --git a/doc/api/oauth2.md b/doc/api/oauth2.md index ad93d8033d03d72240485f8966c91f0804ab98bd..aa9a86f33d5bba2d90bad663e7938a665d357df5 100644 --- a/doc/api/oauth2.md +++ b/doc/api/oauth2.md @@ -33,7 +33,7 @@ Implicit grant and Resource Owner Password Credentials flows. Refer to the [OAuth RFC](https://tools.ietf.org/html/rfc6749) to find out how all those flows work and pick the right one for your use case. -Both **authorization code** (with or without PKCE) and **implicit grant** flows require `application` to be +Authorization code (with or without PKCE) flow requires `application` to be registered first via the `/profile/applications` page in your user's account. During registration, by enabling proper scopes, you can limit the range of resources which the `application` can access. Upon creation, you obtain the @@ -59,8 +59,6 @@ For development, GitLab allows insecure HTTP redirect URIs. As OAuth 2.0 bases its security entirely on the transport layer, you should not use unprotected URIs. For more information, see the [OAuth 2.0 RFC](https://tools.ietf.org/html/rfc6749#section-3.1.2.1) and the [OAuth 2.0 Threat Model RFC](https://tools.ietf.org/html/rfc6819#section-4.4.2.1). -These factors are particularly important when using the -[Implicit grant flow](#implicit-grant-flow-deprecated), where actual credentials are included in the `redirect_uri`. In the following sections you can find detailed instructions on how to obtain authorization with each flow. @@ -319,12 +317,13 @@ access_token = client.password.get_token('user@example.com', 'secret') puts access_token.token ``` -### Implicit grant flow (DEPRECATED) +<!--- start_remove The following content will be removed on remove_date: '2022-08-22' --> + +### Implicit grant flow (removed) -WARNING: Implicit grant flow is inherently insecure and the IETF has removed it in [OAuth 2.1](https://oauth.net/2.1/). -It is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/288516) in GitLab 14.0, and is planned for -[removal](https://gitlab.com/gitlab-org/gitlab/-/issues/344609) in GitLab 15.0. +It is [deprecated](https://gitlab.com/gitlab-org/gitlab/-/issues/288516) in GitLab 14.0 and is +[removed](https://gitlab.com/gitlab-org/gitlab/-/issues/344609) in GitLab 15.0. We recommend that you use [Authorization code with PKCE](#authorization-code-with-proof-key-for-code-exchange-pkce) instead. @@ -353,6 +352,8 @@ parameters, for example: https://example.com/oauth/redirect#access_token=ABCDExyz123&state=YOUR_UNIQUE_STATE_HASH&token_type=bearer&expires_in=3600 ``` +<!--- end_remove --> + ## Access GitLab API with `access token` The `access token` allows you to make requests to the API on behalf of a user. diff --git a/spec/controllers/oauth/authorizations_controller_spec.rb b/spec/controllers/oauth/authorizations_controller_spec.rb index e6553c027d6d14b66f0fc979a1a8d743932fcf26..7489f50667418b4e990e65f5d762dbaaca269a73 100644 --- a/spec/controllers/oauth/authorizations_controller_spec.rb +++ b/spec/controllers/oauth/authorizations_controller_spec.rb @@ -56,27 +56,9 @@ end end - shared_examples "Implicit grant can't be used in confidential application" do - context 'when application is confidential' do - before do - application.update!(confidential: true) - params[:response_type] = 'token' - end - - it 'does not allow the implicit flow' do - subject - - expect(response).to have_gitlab_http_status(:ok) - expect(response).to render_template('doorkeeper/authorizations/error') - end - end - end - describe 'GET #new' do subject { get :new, params: params } - include_examples "Implicit grant can't be used in confidential application" - context 'when the user is confirmed' do context 'when there is already an access token for the application with a matching scope' do before do @@ -219,14 +201,12 @@ subject { post :create, params: params } include_examples 'OAuth Authorizations require confirmed user' - include_examples "Implicit grant can't be used in confidential application" end describe 'DELETE #destroy' do subject { delete :destroy, params: params } include_examples 'OAuth Authorizations require confirmed user' - include_examples "Implicit grant can't be used in confidential application" end it 'includes Two-factor enforcement concern' do diff --git a/spec/features/oauth_login_spec.rb b/spec/features/oauth_login_spec.rb index 99e4c680548ac63b419d36edb28ce25b4aa174d3..fca8972b56cd01fceae86b55cc9c64a61a89459f 100644 --- a/spec/features/oauth_login_spec.rb +++ b/spec/features/oauth_login_spec.rb @@ -166,16 +166,6 @@ def login_with_provider(provider, enter_two_factor: false, additional_info: {}) expect(page).to have_current_path(Gitlab::Routing.url_helpers.root_url, ignore_query: true) end - - it 'does not include the fragment for an implicit grant' do - implicit_grant_params = params.merge(response_type: 'token') - escaped_url = Regexp.escape(Gitlab::Routing.url_helpers.root_url) - auth_params_fragment = '#[a-zA-Z0-9&=_]+' - - visit "#{Gitlab::Routing.url_helpers.oauth_authorization_url(implicit_grant_params)}#a_test-hash" - - expect(page).to have_current_path(%r{\A#{escaped_url}#{auth_params_fragment}\z}, ignore_query: true, url: true) - end end context 'when JS is disabled' do