diff --git a/app/helpers/gov_one_helper.rb b/app/helpers/gov_one_helper.rb index 658d5d42c..f24c16022 100644 --- a/app/helpers/gov_one_helper.rb +++ b/app/helpers/gov_one_helper.rb @@ -1,31 +1,34 @@ module GovOneHelper - # @return [String] + # @return [URI] def login_uri params = { + redirect_uri: GovOneAuthService::CALLBACKS[:login], + client_id: Rails.application.config.gov_one_client_id, response_type: 'code', scope: 'email openid', - client_id: Rails.application.config.gov_one_client_id, nonce: SecureRandom.uuid, state: SecureRandom.uuid, } session[:gov_one_auth_state] = params[:state] - "#{gov_one_uri(:login, params)}&redirect_uri=#{GovOneAuthService::CALLBACKS[:login]}" + gov_one_uri(:login, params) end - # @return [String] + # @return [URI] def logout_uri params = { + post_logout_redirect_uri: GovOneAuthService::CALLBACKS[:logout], id_token_hint: session[:id_token], state: SecureRandom.uuid, } - "#{gov_one_uri(:logout, params)}&post_logout_redirect_uri=#{GovOneAuthService::CALLBACKS[:logout]}" + + gov_one_uri(:logout, params) end # @return [String] def login_button - govuk_button_link_to t('gov-one-info.sign-in-button'), login_uri + govuk_button_link_to t('gov_one_info.sign_in_button'), login_uri.to_s end private diff --git a/app/services/gov_one_auth_service.rb b/app/services/gov_one_auth_service.rb index 7f1d2809a..786ec439c 100644 --- a/app/services/gov_one_auth_service.rb +++ b/app/services/gov_one_auth_service.rb @@ -59,13 +59,6 @@ def response(request, http) http.request(request) end - # @param request [Net::HTTP::Get, Net::HTTP::Post] - # @return [Hash] - def handle_request(request) - response = http.request(request) - JSON.parse(response.body) - end - # @param token [String] # @return [Array] def decode_id_token(token) diff --git a/app/views/gov_one/info.html.slim b/app/views/gov_one/info.html.slim index 3522640ec..a6e57f1d4 100644 --- a/app/views/gov_one/info.html.slim +++ b/app/views/gov_one/info.html.slim @@ -5,23 +5,23 @@ .govuk-grid-row class='govuk-!-padding-top-9 govuk-!-padding-bottom-0' .govuk-grid-column-three-quarters h1.dfe-heading-xl class='govuk-!-margin-bottom-4' - = t('gov-one-info.hero-header') - p.govuk-body-l = t('gov-one-info.hero-body') + = t('gov_one_info.hero_header') + p.govuk-body-l = t('gov_one_info.hero_body') .govuk-grid-row .govuk-grid-column-three-quarters . class='govuk-!-margin-bottom-5' - = m('gov-one-info.body') + = m('gov_one_info.body') hr - = govuk_button_link_to t('gov-one-info.sign-in-button'), login_uri + = login_button . class='govuk-!-margin-top-6 govuk-!-margin-bottom-9' details.govuk-details data-module='govuk-details' summary.govuk-details__summary span.govuk-details__summary-text - = t('gov-one-info.details-summary') + = t('gov_one_info.details_summary') .govuk-details__text - = t('gov-one-info.details-text') + = t('gov_one_info.details_text') \ No newline at end of file diff --git a/app/views/home/_chevron.html.slim b/app/views/home/_chevron.html.slim new file mode 100644 index 000000000..8546e136b --- /dev/null +++ b/app/views/home/_chevron.html.slim @@ -0,0 +1,2 @@ +svg.govuk-button__start-icon xmlns='http://www.w3.org/2000/svg' width='17.5' height='19' viewBox='0 0 33 40' aria-hidden='true' focusable='false' + path fill='currentColor' d='M0 0h13l20 20-20 20H0l20-20z' \ No newline at end of file diff --git a/app/views/home/_cta.html.slim b/app/views/home/_cta.html.slim deleted file mode 100644 index 1c114a821..000000000 --- a/app/views/home/_cta.html.slim +++ /dev/null @@ -1,5 +0,0 @@ - .govuk-button-group - = govuk_button_link_to gov_one_info_path, class: "govuk-button--start" do - | #{t('home.gov-one-button')} - svg.govuk-button__start-icon xmlns='http://www.w3.org/2000/svg' width='17.5' height='19' viewBox='0 0 33 40' aria-hidden='true' focusable='false' - path fill='currentColor' d='M0 0h13l20 20-20 20H0l20-20z' \ No newline at end of file diff --git a/app/views/home/index.html.slim b/app/views/home/index.html.slim index 5c4543f0a..8e805cf00 100644 --- a/app/views/home/index.html.slim +++ b/app/views/home/index.html.slim @@ -20,4 +20,7 @@ = m('home.prompt', headings_start_with: 'xl') - unless current_user - = render 'cta' + .govuk-button-group + = govuk_button_link_to gov_one_info_path, class: "govuk-button--start" do + | #{t('home.gov_one_button')} + = render 'chevron' diff --git a/config/application.rb b/config/application.rb index 9979b9a54..42c3e1478 100644 --- a/config/application.rb +++ b/config/application.rb @@ -96,11 +96,6 @@ def debug? Types::Params::Bool[ENV.fetch('DEBUG', false)] end - # @return [Boolean] - def gov_one_login? - Types::Params::Bool[ENV.fetch('GOV_ONE_LOGIN', false)] || !live? - end - # @return [Boolean] def maintenance? Types::Params::Bool[ENV.fetch('MAINTENANCE', false)] diff --git a/config/locales/en.yml b/config/locales/en.yml index 19aa95298..65fd41cef 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -532,6 +532,7 @@ en: ## Return to your training Sign in to continue learning, see your progress and download certificates. + gov_one_button: Start your training now # /about-training about: @@ -572,6 +573,16 @@ en: %{criteria} + # /gov-one/info + gov_one_info: + title: Gov One Info + hero_header: How to access this training course + hero_body: This service uses GOV.UK One Login which is managed by the Government Digital Service. + body: You will be asked to sign in to your account, or create a One Login account, in this service + sign_in_button: Continue to GOV.UK One Login + details_summary: How to access an existing training account + details_text: If you have an existing early years child development training account but you do not yet have a GOV.UK One Login, you must use the same email address for both accounts. This will ensure that any progress you have made through the training is retained. + # /settings/cookie-policy cookie_policy: title: Cookie policy diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 7287b9819..11001e395 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -19,7 +19,8 @@ services: # app config - DATABASE_URL=postgres://postgres:password@db:5432/early_years_foundation_recovery_development - RAILS_ENV=development - - DOMAIN=app:3000 + # - DOMAIN=app:3000 + - DOMAIN=localhost:3000 - RAILS_SERVE_STATIC_FILES=true volumes: - .:/srv diff --git a/spec/controllers/users/omniauth_callbacks_controller_spec.rb b/spec/controllers/users/omniauth_callbacks_controller_spec.rb index c28512af5..0cf73f971 100644 --- a/spec/controllers/users/omniauth_callbacks_controller_spec.rb +++ b/spec/controllers/users/omniauth_callbacks_controller_spec.rb @@ -1,58 +1,66 @@ require 'rails_helper' RSpec.describe Users::OmniauthCallbacksController, type: :controller do + let(:auth_service) { instance_double(GovOneAuthService) } + let(:access_token) { 'mock_access_token' } + let(:id_token) { 'mock_id_token' } + let(:email) { 'test@example.com' } + let(:params) do + { 'code' => 'mock_code', 'state' => 'mock_state' } + end + before do request.env['devise.mapping'] = Devise.mappings[:user] + session[:gov_one_auth_state] = 'mock_state' + + allow(GovOneAuthService).to receive(:new).and_return(auth_service) + allow(auth_service).to receive(:tokens).and_return({ 'access_token' => access_token, 'id_token' => id_token }) + allow(auth_service).to receive(:user_info).and_return({ 'email' => email }) + allow(auth_service).to receive(:jwt_assertion).and_return('mock_jwt_assertion') + allow(auth_service).to receive(:decode_id_token).and_return([{ 'sub' => 'mock_sub' }]) + end + + context 'with a new user' do + before do + get :openid_connect, params: params + end + + it 'creates an account' do + expect(User.find_by(email: email)).to be_truthy + expect(User.find_by(gov_one_id: 'mock_sub')).to be_truthy + end + + it 'redirects to complete registration' do + expect(session[:id_token]).to eq id_token + expect(response).to redirect_to edit_registration_name_path + end end - describe '#openid_connect' do - let(:auth_service) { instance_double(GovOneAuthService) } - let(:access_token) { 'mock_access_token' } - let(:id_token) { 'mock_id_token' } - let(:email) { 'test@example.com' } + context 'with an existing non-gov-one user' do + before do + create :user, :registered, email: email + get :openid_connect, params: params + end + + it 'updates the account' do + expect(User.find_by(gov_one_id: 'mock_sub')).to be_truthy + end + + it 'redirects to /my-modules' do + expect(session[:id_token]).to eq id_token + expect(response).to redirect_to my_modules_path + end + end + context 'with an existing gov-one user' do before do - allow(GovOneAuthService).to receive(:new).and_return(auth_service) - allow(auth_service).to receive(:tokens).and_return({ 'access_token' => access_token, 'id_token' => id_token }) - allow(auth_service).to receive(:user_info).and_return({ 'email' => email }) - allow(auth_service).to receive(:jwt_assertion).and_return('mock_jwt_assertion') - allow(auth_service).to receive(:decode_id_token).and_return([{ 'sub' => 'mock_sub' }]) - session[:gov_one_auth_state] = 'mock_state' - end - - context 'when a User does not exist' do - it 'creates the user with the email and id_token' do - get :openid_connect, params: { 'code' => 'mock_code', 'state' => 'mock_state' } - expect(User.find_by(email: email)).to be_truthy - expect(User.find_by(gov_one_id: 'mock_sub')).to be_truthy - expect(response).to redirect_to(edit_registration_name_path) - expect(session[:id_token]).to eq(id_token) - end - end - - context 'when a User email exists' do - before do - create :user, :registered, email: email - end - - it 'updates the user id_token and signs them in' do - get :openid_connect, params: { 'code' => 'mock_code', 'state' => 'mock_state' } - expect(User.find_by(gov_one_id: 'mock_sub')).to be_truthy - expect(response).to redirect_to(my_modules_path) - expect(session[:id_token]).to eq(id_token) - end - end - - context 'when a User id_token exists' do - before do - create :user, :registered, gov_one_id: 'mock_sub' - end - - it 'signs the user in' do - get :openid_connect, params: { 'code' => 'mock_code', 'state' => 'mock_state' } - expect(response).to redirect_to(my_modules_path) - expect(session[:id_token]).to eq(id_token) - end + create :user, :registered, gov_one_id: 'mock_sub' + get :openid_connect, params: params + end + + it 'redirects to /my-modules' do + expect(session[:id_token]).to eq id_token + expect(response).to redirect_to my_modules_path end end end diff --git a/spec/helpers/gov_one_helper_spec.rb b/spec/helpers/gov_one_helper_spec.rb index 829e6f149..c3e333e24 100644 --- a/spec/helpers/gov_one_helper_spec.rb +++ b/spec/helpers/gov_one_helper_spec.rb @@ -5,8 +5,13 @@ subject(:login_uri) { helper.login_uri } it 'encodes the authorize endpoint params' do - expect(login_uri).to start_with 'https://oidc.test.account.gov.uk/authorize?response_type=code&scope=email+openid&client_id=some_client_id&' - expect(login_uri).to end_with 'redirect_uri=http://recovery.app/users/auth/openid_connect/callback' + expect(login_uri.host).to eq 'oidc.test.account.gov.uk' + expect(login_uri.path).to eq '/authorize' + expect(login_uri.query).to include 'redirect_uri=http%3A%2F%2Frecovery.app%2Fusers%2Fauth%2Fopenid_connect%2Fcallback' + expect(login_uri.query).to include 'client_id=some_client_id' + expect(login_uri.query).to include 'response_type=code' + expect(login_uri.query).to include 'scope=email+openid' + expect(login_uri.query).to include 'nonce=' end end @@ -14,8 +19,11 @@ subject(:logout_uri) { helper.logout_uri } it 'encodes the logout endpoint params' do - expect(logout_uri).to start_with 'https://oidc.test.account.gov.uk/logout?id_token_hint&state=' - expect(logout_uri).to end_with '&post_logout_redirect_uri=http://recovery.app/users/sign_out' + expect(logout_uri.host).to eq 'oidc.test.account.gov.uk' + expect(logout_uri.path).to eq '/logout' + expect(logout_uri.query).to include 'post_logout_redirect_uri=http%3A%2F%2Frecovery.app%2Fusers%2Fsign_out' + expect(logout_uri.query).to include 'id_token_hint' + expect(logout_uri.query).to include 'state=' end end @@ -25,7 +33,7 @@ it 'returns a button link to the gov one login uri' do expect(login_button).to include 'govuk-button' expect(login_button).to include 'Continue to GOV.UK One Login' - expect(login_button).to include 'href="https://oidc.test.account.gov.uk/authorize?response_type=code&scope=email+openid&client_id=some_client_id&' + expect(login_button).to include 'href="https://oidc.test.account.gov.uk/authorize?redirect_uri=http%3A%2F%2Frecovery.app%2Fusers%2Fauth%2Fopenid_connect%2Fcallback&client_id=some_client_id&response_type=code&scope=email+openid&nonce=' end end end diff --git a/spec/lib/seed_snippets_spec.rb b/spec/lib/seed_snippets_spec.rb index 75a4ac39a..649cddb91 100644 --- a/spec/lib/seed_snippets_spec.rb +++ b/spec/lib/seed_snippets_spec.rb @@ -5,7 +5,7 @@ subject(:locales) { described_class.new.call } it 'converts all translations' do - expect(locales.count).to be 187 + expect(locales.count).to be 195 end it 'dot separated key -> Page::Resource#name' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 1c1d0afa7..c1b68455c 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -208,33 +208,40 @@ end describe '.find_or_create_from_gov_one' do + subject(:find_or_create) { described_class.find_or_create_from_gov_one(email: email, gov_one_id: gov_one_id) } + let(:email) { 'test@test.com' } let(:gov_one_id) { '123' } + let(:user) { create(:user) } context 'with an existing user having an email but no gov_one_id' do - let!(:user) { create(:user, email: email) } + before do + user.update_column(:email, email) + find_or_create + end it 'updates the user gov_one_id' do - described_class.find_or_create_from_gov_one(email: email, gov_one_id: gov_one_id) expect(user.reload.gov_one_id).to eq(gov_one_id) end end context 'with an existing user having a gov_one_id' do - let!(:user) { create(:user, gov_one_id: gov_one_id) } + before do + user.update_column(:gov_one_id, gov_one_id) + find_or_create + end it 'updates the user email' do - described_class.find_or_create_from_gov_one(email: email, gov_one_id: gov_one_id) expect(user.reload.email).to eq(email) end end context 'without an existing user' do - let(:email) { 'some_new_email@test.com' } - let(:gov_one_id) { '321' } + before do + find_or_create + end it 'creates a new user' do - described_class.find_or_create_from_gov_one(email: email, gov_one_id: gov_one_id) expect(described_class.count).to eq(1) expect(described_class.first.email).to eq(email) expect(described_class.first.gov_one_id).to eq(gov_one_id) diff --git a/spec/services/gov_one_auth_service_spec.rb b/spec/services/gov_one_auth_service_spec.rb index 564ccfa14..66b65187b 100644 --- a/spec/services/gov_one_auth_service_spec.rb +++ b/spec/services/gov_one_auth_service_spec.rb @@ -8,25 +8,24 @@ before do allow(auth_service).to receive(:response).and_return(mock_response) + allow(mock_response).to receive(:body).and_return(payload.to_json) end describe '#tokens' do let(:result) { auth_service.tokens } - context 'when the request is successful' do + context 'when successful' do it 'returns a hash of tokens' do - allow(mock_response).to receive(:body).and_return(payload.to_json) - expect(result).to eq(payload) expect(auth_service).to have_received(:response).with(an_instance_of(Net::HTTP::Post), an_instance_of(Net::HTTP)) end end - context 'when the request is unsuccessful' do - it 'returns an empty hash' do - allow(mock_response).to receive(:body).and_return({}.to_json) + context 'when unsuccessful' do + let(:payload) { {} } - expect(result).to eq({}) + it 'returns an empty hash' do + expect(result).to eq(payload) expect(auth_service).to have_received(:response).with(an_instance_of(Net::HTTP::Post), an_instance_of(Net::HTTP)) end end @@ -36,31 +35,49 @@ let(:access_token) { 'mock_access_token' } let(:result) { auth_service.user_info(access_token) } - context 'when the request is successful' do + context 'when successful' do it 'returns a hash of user info' do - allow(mock_response).to receive(:body).and_return(payload.to_json) - expect(result).to eq(payload) expect(auth_service).to have_received(:response).with(an_instance_of(Net::HTTP::Get), an_instance_of(Net::HTTP)) end end - context 'when the request is unsuccessful' do - it 'returns an empty hash' do - allow(mock_response).to receive(:body).and_return({}.to_json) + context 'when unsuccessful' do + let(:payload) { {} } - expect(result).to eq({}) + it 'returns an empty hash' do + expect(result).to eq(payload) expect(auth_service).to have_received(:response).with(an_instance_of(Net::HTTP::Get), an_instance_of(Net::HTTP)) end end end - describe 'CALLBACKS' do - subject(:callbacks) { described_class::CALLBACKS } + describe '#token_body' do + let(:token_body) { auth_service.send(:token_body) } - specify 'callbacks' do - expect(callbacks).to be_frozen + it 'returns a hash of correct token body' do + expect(token_body[:grant_type]).to eq('authorization_code') + expect(token_body[:code]).to eq(code) + expect(token_body[:redirect_uri]).to end_with('/users/auth/openid_connect/callback') + expect(token_body[:client_assertion_type]).to eq('urn:ietf:params:oauth:client-assertion-type:jwt-bearer') + end + end + + describe '#jwt_payload' do + let(:jwt_payload) { auth_service.send(:jwt_payload) } + + it 'returns a hash of correct jwt payload' do + expect(jwt_payload[:aud]).to eq 'https://oidc.test.account.gov.uk/token' + expect(jwt_payload[:iss]).to eq 'some_client_id' + expect(jwt_payload[:sub]).to eq 'some_client_id' + expect(jwt_payload[:exp]).to be_between(Time.zone.now.to_i + 4 * 60, Time.zone.now.to_i + 6 * 60) + expect(jwt_payload[:jti]).to be_a String + expect(jwt_payload[:iat]).to be_a Integer end + end + + describe 'Internal callbacks' do + subject(:callbacks) { described_class::CALLBACKS } specify 'login' do expect(callbacks[:login]).to eq 'http://recovery.app/users/auth/openid_connect/callback' @@ -71,55 +88,27 @@ end end - describe 'ENDPOINTS' do + describe 'OIDC endpoints' do subject(:endpoints) { described_class::ENDPOINTS } - specify 'endpoints' do - expect(endpoints).to be_frozen - end - - specify 'login' do + specify 'login endpoint for starting gov one user session and redirecting back to service' do expect(endpoints[:login]).to eq 'https://oidc.test.account.gov.uk/authorize' end - specify 'logout' do + specify 'logout endpoint for ending gov one user session and redirecting back to service' do expect(endpoints[:logout]).to eq 'https://oidc.test.account.gov.uk/logout' end - specify 'token' do + specify 'token endpoint for retrieving user access token and id token' do expect(endpoints[:token]).to eq 'https://oidc.test.account.gov.uk/token' end - specify 'userinfo' do + specify 'userinfo endpoint for retrieving user email' do expect(endpoints[:userinfo]).to eq 'https://oidc.test.account.gov.uk/userinfo' end - specify 'jwks' do + specify 'jwks endpoint for retrieving public key for verifying user id token' do expect(endpoints[:jwks]).to eq 'https://oidc.test.account.gov.uk/.well-known/jwks.json' end end - - describe '#token_body' do - let(:token_body) { auth_service.send(:token_body) } - - it 'returns a hash of correct token body' do - expect(token_body[:grant_type]).to eq('authorization_code') - expect(token_body[:code]).to eq(code) - expect(token_body[:redirect_uri]).to end_with('/users/auth/openid_connect/callback') - expect(token_body[:client_assertion_type]).to eq('urn:ietf:params:oauth:client-assertion-type:jwt-bearer') - end - end - - describe '#jwt_payload' do - let(:jwt_payload) { auth_service.send(:jwt_payload) } - - it 'returns a hash of correct jwt payload' do - expect(jwt_payload[:aud]).to eq('https://oidc.test.account.gov.uk/token') - expect(jwt_payload[:iss]).to eq('some_client_id') - expect(jwt_payload[:sub]).to eq('some_client_id') - expect(jwt_payload[:exp]).to be_between(Time.zone.now.to_i + 4 * 60, Time.zone.now.to_i + 6 * 60) - expect(jwt_payload[:jti]).to be_a(String) - expect(jwt_payload[:iat]).to be_a(Integer) - end - end end