Commit 220755f5 authored by Robert Speicher's avatar Robert Speicher Committed by Ruben Davila

Merge branch '2fa-api-check' into 'master'

2FA checks for API workflows

## What does this MR do?

It adds a check to the API `/session` endpoint that will deny authentication requests to users that have 2FA enabled. In the error message it will instruct them to use a Personal Access Token instead.

It adds a check to the `/oauth/token` endpoint, when `grant_type: 'password'` is used, so that no OAuth2 access token can be generated if the user has 2FA enabled. This endpoint should not be used by OAuth applications, anyway. OAuth apps should follow the flow of redirecting the user to GitLab, where 2FA access restrictions apply and logging them in there. Once successfully authenticated, the OAuth token is passed to the client.

## Why was this MR needed?

No 2FA check on API endpoints.

## What are the relevant issue numbers?

Fixes #2979

See merge request !5820
parent 0ff39331
...@@ -47,6 +47,10 @@ v 8.11.0 (unreleased) ...@@ -47,6 +47,10 @@ v 8.11.0 (unreleased)
- Various redundant database indexes have been removed - Various redundant database indexes have been removed
- Update `timeago` plugin to use multiple string/locale settings - Update `timeago` plugin to use multiple string/locale settings
- Remove unused images (ClemMakesApps) - Remove unused images (ClemMakesApps)
- Get issue and merge request description templates from repositories
- Add hover state to todos !5361 (winniehell)
- Fix icon alignment of star and fork buttons !5451 (winniehell)
- Enforce 2FA restrictions on API authentication endpoints !5820
- Limit git rev-list output count to one in forced push check - Limit git rev-list output count to one in forced push check
- Show deployment status on merge requests with external URLs - Show deployment status on merge requests with external URLs
- Clean up unused routes (Josef Strzibny) - Clean up unused routes (Josef Strzibny)
......
...@@ -12,7 +12,8 @@ Doorkeeper.configure do ...@@ -12,7 +12,8 @@ Doorkeeper.configure do
end end
resource_owner_from_credentials do |routes| resource_owner_from_credentials do |routes|
Gitlab::Auth.find_with_user_password(params[:username], params[:password]) user = Gitlab::Auth.find_with_user_password(params[:username], params[:password])
user unless user.try(:two_factor_enabled?)
end end
# If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below. # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
......
...@@ -90,7 +90,7 @@ curl --header "Authorization: Bearer OAUTH-TOKEN" https://localhost:3000/api/v3/ ...@@ -90,7 +90,7 @@ curl --header "Authorization: Bearer OAUTH-TOKEN" https://localhost:3000/api/v3/
## Deprecation Notice ## Deprecation Notice
1. Starting in GitLab 9.0, the Resource Owner Password Credentials will be *disabled* for users with two-factor authentication turned on. 1. Starting in GitLab 8.11, the Resource Owner Password Credentials has been *disabled* for users with two-factor authentication turned on.
2. These users can access the API using [personal access tokens] instead. 2. These users can access the API using [personal access tokens] instead.
--- ---
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
## Deprecation Notice ## Deprecation Notice
1. Starting in GitLab 9.0, this feature will be *disabled* for users with two-factor authentication turned on. 1. Starting in GitLab 8.11, this feature has been *disabled* for users with two-factor authentication turned on.
2. These users can access the API using [personal access tokens] instead. 2. These users can access the API using [personal access tokens] instead.
--- ---
......
...@@ -14,6 +14,7 @@ module API ...@@ -14,6 +14,7 @@ module API
user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password]) user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password])
return unauthorized! unless user return unauthorized! unless user
return render_api_error!('401 Unauthorized. You have 2FA enabled. Please use a personal access token to access the API', 401) if user.two_factor_enabled?
present user, with: Entities::UserLogin present user, with: Entities::UserLogin
end end
end end
......
require 'spec_helper'
describe API::API, api: true do
include ApiHelpers
context 'Resource Owner Password Credentials' do
def request_oauth_token(user)
post '/oauth/token', username: user.username, password: user.password, grant_type: 'password'
end
context 'when user has 2FA enabled' do
it 'does not create an access token' do
user = create(:user, :two_factor)
request_oauth_token(user)
expect(response).to have_http_status(401)
expect(json_response['error']).to eq('invalid_grant')
end
end
context 'when user does not have 2FA enabled' do
it 'creates an access token' do
user = create(:user)
request_oauth_token(user)
expect(response).to have_http_status(200)
expect(json_response['access_token']).not_to be_nil
end
end
end
end
...@@ -17,6 +17,17 @@ describe API::API, api: true do ...@@ -17,6 +17,17 @@ describe API::API, api: true do
expect(json_response['can_create_project']).to eq(user.can_create_project?) expect(json_response['can_create_project']).to eq(user.can_create_project?)
expect(json_response['can_create_group']).to eq(user.can_create_group?) expect(json_response['can_create_group']).to eq(user.can_create_group?)
end end
context 'with 2FA enabled' do
it 'rejects sign in attempts' do
user = create(:user, :two_factor)
post api('/session'), email: user.email, password: user.password
expect(response).to have_http_status(401)
expect(response.body).to include('You have 2FA enabled.')
end
end
end end
context 'when email has case-typo and password is valid' do context 'when email has case-typo and password is valid' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment