Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force new private users to accept the privacy policy before creating an account #3859

Merged
merged 15 commits into from
Aug 18, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class ApplicationController < ActionController::Base
protect_from_forgery with: :null_session

before_action :store_current_location,
except: %i[media sign_in institution_not_supported],
except: %i[media sign_in institution_not_supported privacy_prompt accept_privacy_policy],
unless: -> { devise_controller? || remote_request? }

before_action :skip_session,
Expand Down
61 changes: 53 additions & 8 deletions app/controllers/auth/omniauth_callbacks_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,16 @@ def smartschool
generic_oauth
end

# ==> Privacy agreement acceptance before new account creation

def privacy_prompt
render 'auth/privacy_prompt'
end

def accept_privacy_policy
sign_in_new_user_from_session!
end

private

# ==> Authentication logic.
Expand Down Expand Up @@ -96,10 +106,8 @@ def try_login!
return redirect_to_known_provider!(user) if user.present?

# No existing user was found
# Create a new user
user = User.new institution: provider&.institution
# Create a new identity for the newly created user
identity = user.identities.build identifier: auth_uid, provider: provider
# Redirect to privacy prompt before we create a new user
return redirect_to_privacy_prompt
end

# Validation.
Expand All @@ -111,6 +119,12 @@ def try_login!
user.update_from_provider(auth_hash, provider)
return redirect_with_errors!(user) if user.errors.any?

sign_in!(user)
end

# ==> Utilities.

def sign_in!(user)
# If the session contains credentials for another identity, add this identity to the signed in user
create_identity_from_session!(user)

Expand All @@ -122,7 +136,38 @@ def try_login!
redirect_to_target!(user)
end

# ==> Utilities.
def redirect_to_privacy_prompt
session[:new_user_identifier] = auth_hash.uid
session[:new_user_email] = auth_hash.info.email
session[:new_user_first_name] = auth_hash.info.first_name
session[:new_user_last_name] = auth_hash.info.last_name
session[:new_user_provider_id] = provider&.id
session[:new_user_auth_target] = auth_target

redirect_to privacy_prompt_path
end

def sign_in_new_user_from_session!
identifier = session.delete(:new_user_identifier)
email = session.delete(:new_user_email)
first_name = session.delete(:new_user_first_name)
last_name = session.delete(:new_user_last_name)
provider_id = session.delete(:new_user_provider_id)

provider = Provider.find(provider_id)

# Create a new user
user = User.new institution: provider&.institution, email: email, first_name: first_name, last_name: last_name,
username: identifier

# Create a new identity for the newly created user
user.identities.build identifier: identifier, provider: provider
user.save

return redirect_with_errors!(user) if user.errors.any?

sign_in!(user)
end

def create_identity_from_session!(user)
# Find the original provider and uid in the session.
Expand Down Expand Up @@ -189,7 +234,7 @@ def find_or_create_oauth_provider
institution_created
provider
else
institution_creation_failed institution.errors
institution_create_failed institution.errors
nil
end
end
Expand Down Expand Up @@ -297,7 +342,7 @@ def redirect_to_target!(user)
# ==> Shorthands.

def target_path(user)
auth_target || after_sign_in_path_for(user)
auth_target || session.delete(:new_user_auth_target) || after_sign_in_path_for(user)
end

def auth_hash
Expand All @@ -319,7 +364,7 @@ def auth_redirect_params
end

def auth_target
return nil if auth_hash.extra[:target].blank?
return nil if auth_hash.blank? || auth_hash.extra[:target].blank?

"#{auth_hash.extra[:target]}?#{auth_redirect_params.to_param}"
end
Expand Down
22 changes: 22 additions & 0 deletions app/views/auth/privacy_prompt.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<div class="row justify-content-center">
<div class="col-12 col-lg-8">
<div class="card home-summary-card privacy-disclaimer">
<div class="card-supporting-text">
<div class="privacy-disclaimer-icon">
<i class="mdi mdi-48 mdi-shield-lock"></i>
</div>
<div class="privacy-disclaimer-text">
<%= t ".text_html", your_data: data_url, privacy_statement: privacy_url %>
</div>
</div>
<div class="card-actions card-border">
<%= link_to root_path, class: "btn btn-danger" do %>
<%= t ".decline_button" %>
<% end %>
<%= link_to privacy_prompt_path, method: :post, class: "btn btn-primary" do %>
<%= t ".accept_button" %>
<% end %>
</div>
</div>
</div>
</div>
10 changes: 0 additions & 10 deletions app/views/pages/_privacy_disclaimer.html.erb

This file was deleted.

1 change: 0 additions & 1 deletion app/views/pages/home.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
<div class="col-12 col-md-6 col-lg-8">
<% if @subscribed_courses.empty? %>
<%= render "getting_started_card" %>
<%= render 'privacy_disclaimer' %>
<% else %>
<div class="row favorites-row">
<% unless @favorite_courses.empty? %>
Expand Down
4 changes: 4 additions & 0 deletions config/locales/views/auth/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ en:
providers_title: "Link accounts"
"contact_support_html": "If this isn't you or you keep having issues? Fill in the %{form} and we will assist you as soon as possible."
contact_form: "contact form"
privacy_prompt:
text_html: "In order to use Dodona, you need to accept our privacy statement. On the <a href=\"%{your_data}\" target=\"_blank\">your data</a> page we explain in clear and understandable language what data we collect and how we use it. Our <a href=\"%{privacy_statement}\" target=\"_blank\">privacy statement</a> contains the legally binding version."
accept_button: Accept the privacy statement
decline_button: Decline
4 changes: 4 additions & 0 deletions config/locales/views/auth/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,7 @@ nl:
providers_title: "Accounts koppelen"
"contact_support_html": "Ben jij dit niet of blijf je problemen hebben? Vul het %{form} in en we helpen je zo snel mogelijk verder."
contact_form: "contactformulier"
privacy_prompt:
text_html: "Om Dodona te gebruiken moet je onze privacyverklaring accepteren. Op de <a href=\"%{your_data}\" target=\"_blank\">jouw data</a> pagina leggen we in mensentaal uit welke data we verzamelen en hoe we die gebruiken. De juridisch bindende versie kan je in onze <a href=\"%{privacy_statement}\" target=\"_blank\">privacyverklaring</a> vinden."
jorg-vr marked this conversation as resolved.
Show resolved Hide resolved
accept_button: Accepteer de privacyverklaring
jorg-vr marked this conversation as resolved.
Show resolved Hide resolved
decline_button: Weigeren
2 changes: 0 additions & 2 deletions config/locales/views/pages/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ en:
create_contact:
mail_sent: "Your message has been sent. Thanks for getting in touch."
captcha_failed: HCaptcha could not be verified; please try again.
privacy_disclaimer:
text_html: "Your privacy is important to us. On the <a href=\"%{your_data}\">your data</a> page we explain in clear and understandable language what data we collect and how we use it. Our <a href=\"%{privacy_statement}\">privacy statement</a> contains the legally binding version."
support:
support_dodona: Support Dodona
title_l1: Learn to code for free
Expand Down
2 changes: 0 additions & 2 deletions config/locales/views/pages/nl.yml
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ nl:
create_contact:
captcha_failed: HCaptcha kon niet geverifieerd worden, probeer opnieuw.
mail_sent: "Je bericht werd verstuurd. Bedankt om contact op te nemen."
privacy_disclaimer:
text_html: "Jouw privacy is belangrijk voor ons. Op de <a href=\"%{your_data}\">jouw data</a> pagina leggen we in mensentaal uit welke data we verzamelen en hoe we die gebruiken. De juridisch bindende versie kan je in onze <a href=\"%{privacy_statement}\">privacyverklaring</a> vinden."
support:
support_dodona: Steun Dodona
title_l1: Gratis leren programmeren
Expand Down
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
devise_scope :user do
get '/sign_in' => 'authentication#sign_in', as: 'sign_in'
delete '/sign_out' => 'authentication#destroy', as: 'sign_out'
get '/privacy_prompt' => 'omniauth_callbacks#privacy_prompt'
post '/privacy_prompt' => 'omniauth_callbacks#accept_privacy_policy'
end

get '/users/saml/metadata' => 'saml#metadata'
Expand Down
24 changes: 24 additions & 0 deletions test/controllers/auth/omniauth_callbacks_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,11 @@ def omniauth_path(provider)
assert_difference 'Identity.count', 1 do
get omniauth_url(provider)
follow_redirect!

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path
end
end

Expand All @@ -115,6 +120,11 @@ def omniauth_path(provider)
assert_difference 'Identity.count', 1 do
get omniauth_url(provider)
follow_redirect!

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path
end
end

Expand All @@ -141,6 +151,11 @@ def omniauth_path(provider)
assert_difference 'Institution.count', 1 do
get omniauth_url(provider)
follow_redirect!

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path
end
end
end
Expand Down Expand Up @@ -276,6 +291,11 @@ def omniauth_path(provider)
assert_difference 'User.count', +1 do
get omniauth_url(provider)
follow_redirect!

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path
end

assert_equal @controller.current_user.username, user.username
Expand Down Expand Up @@ -315,6 +335,10 @@ def omniauth_path(provider)
assert_difference 'User.count', 0 do
get omniauth_url(second_provider)
follow_redirect!

assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path
end

assert_redirected_to root_path
Expand Down
10 changes: 10 additions & 0 deletions test/controllers/lti_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,11 @@ def teardown
state: params[:state]
}

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path

assert_response :found
target_uri = URI.parse(@response.header['Location'])
params = URI.decode_www_form(target_uri.query).to_h.symbolize_keys
Expand Down Expand Up @@ -171,6 +176,11 @@ def teardown
state: params[:state]
}

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path

assert_response :redirect
target_uri = URI.parse(@response.header['Location'])
params = URI.decode_www_form(target_uri.query).to_h.symbolize_keys
Expand Down
5 changes: 5 additions & 0 deletions test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ def stub_keys!(kid = nil)
assert_equal 'authorization_code', parameters[:grant_type].first
end

# assert privacy prompt before successful sign in
assert_redirected_to privacy_prompt_path
assert_nil @controller.current_user
post privacy_prompt_path

# Validate that the user is correctly logged in.
current_user = @controller.current_user
assert_equal id_token_body[:given_name], current_user.first_name
Expand Down