From 5cc219c795afe9158e18b24a439abd2f08ea1eda Mon Sep 17 00:00:00 2001 From: Emi Matchu Date: Mon, 8 Apr 2024 05:33:58 -0700 Subject: [PATCH] Connect a NeoPass to an existing account including validation logic to make sure it's not already connected to another one! The `intent` param on the NeoPass form is part of the key! Thanks OmniAuth for making it easy to pass that data through! --- .../devise/omniauth_callbacks_controller.rb | 61 ++++++++++++++++--- app/models/auth_user.rb | 24 ++++++++ app/views/auth_users/edit.html.erb | 14 +++++ app/views/devise/sessions/new.html.erb | 2 +- ..._add_unique_index_for_omniauth_to_users.rb | 5 ++ db/openneo_id_schema.rb | 3 +- 6 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 db/openneo_id_migrate/20240408120359_add_unique_index_for_omniauth_to_users.rb diff --git a/app/controllers/devise/omniauth_callbacks_controller.rb b/app/controllers/devise/omniauth_callbacks_controller.rb index 0054337d..f2ef2638 100644 --- a/app/controllers/devise/omniauth_callbacks_controller.rb +++ b/app/controllers/devise/omniauth_callbacks_controller.rb @@ -1,8 +1,31 @@ class Devise::OmniauthCallbacksController < ApplicationController + rescue_from AuthUser::AuthAlreadyConnected, with: :auth_already_connected rescue_from AuthUser::MissingAuthInfoError, with: :missing_auth_info rescue_from ActiveRecord::RecordInvalid, with: :validation_failed def neopass + case intent_param + when "login" + sign_in_with_neopass + when "connect" + connect_with_neopass + else + flash[:alert] = "Hrm, the NeoPass form you used was missing some " + + "\"intent\" information. That's surprising! Maybe try again?" + redirect_to root_path + end + end + + def failure + flash[:warning] = + "Hrm, something went wrong in our connection to NeoPass, sorry! " + + "Maybe wait a moment and try again?" + redirect_to new_auth_user_session_path + end + + private + + def sign_in_with_neopass @auth_user = AuthUser.from_omniauth(request.env["omniauth.auth"]) if @auth_user.previously_new_record? @@ -18,11 +41,20 @@ class Devise::OmniauthCallbacksController < ApplicationController sign_in_and_redirect @auth_user, event: :authentication end - def failure - flash[:warning] = - "Hrm, something went wrong in our connection to NeoPass, sorry! " + - "Maybe wait a moment and try again?" - redirect_to new_auth_user_session_path + def connect_with_neopass + raise AccessDenied unless auth_user_signed_in? + + current_auth_user.connect_omniauth!(request.env["omniauth.auth"]) + + flash[:notice] = "We've successfully connected your NeoPass!" + redirect_to edit_auth_user_path + end + + def auth_already_connected(error) + flash[:alert] = "This NeoPass is already connected to another account. " + + "You'll need to log out of this account, log in with that NeoPass, " + + "then disconnect it from the other account." + redirect_to return_path end def missing_auth_info(error) @@ -33,7 +65,7 @@ class Devise::OmniauthCallbacksController < ApplicationController "Hrm, our connection with NeoPass didn't return the information we " + "usually expect, sorry! If this keeps happening, please email me at " + "matchu@openneo.net so I can help investigate!" - redirect_to new_auth_user_session_path + redirect_to return_path end def validation_failed(error) @@ -44,6 +76,21 @@ class Devise::OmniauthCallbacksController < ApplicationController "Hrm, the connection with NeoPass worked, but we had trouble saving " + "your account, sorry! If this keeps happening, please email me at " + "matchu@openneo.net so I can help investigate!" - redirect_to new_auth_user_session_path + redirect_to return_path + end + + def intent_param + request.env['omniauth.params']["intent"] + end + + def return_path + case intent_param + when "login" + new_auth_user_session_path + when "connect" + edit_auth_user_path + else + root_path + end end end diff --git a/app/models/auth_user.rb b/app/models/auth_user.rb index ec11cce1..d969228b 100644 --- a/app/models/auth_user.rb +++ b/app/models/auth_user.rb @@ -7,6 +7,8 @@ class AuthUser < AuthRecord validates :name, presence: true, uniqueness: {case_sensitive: false}, length: {maximum: 30} + + validates :uid, uniqueness: {scope: :provider, allow_nil: true} has_one :user, foreign_key: :remote_id, inverse_of: :auth_user @@ -79,6 +81,23 @@ class AuthUser < AuthRecord encrypted_password? end + def connect_omniauth!(auth) + raise MissingAuthInfoError, "Email missing" if auth.info.email.blank? + + begin + update!(provider: auth.provider, uid: auth.uid, + neopass_email: auth.info.email) + rescue ActiveRecord::RecordInvalid + # If this auth is already bound to another account, raise a specific + # error about it, instead of the normal error. + if errors.where(:uid).any? { |e| e.type == :taken } + raise AuthAlreadyConnected, "there's already an account with " + + "provider #{auth.provider}, uid #{auth.uid}" + end + raise + end + end + def disconnect_neopass # If there's no NeoPass, we're already done! return true if !uses_neopass? @@ -100,6 +119,10 @@ class AuthUser < AuthRecord end end + def self.find_by_omniauth(auth) + find_by(provider: auth.provider, uid: auth.uid) + end + def self.from_omniauth(auth) raise MissingAuthInfoError, "Email missing" if auth.info.email.blank? @@ -160,5 +183,6 @@ class AuthUser < AuthRecord "\"#{base_name}\" are taken??)" end + class AuthAlreadyConnected < ArgumentError;end class MissingAuthInfoError < ArgumentError;end end \ No newline at end of file diff --git a/app/views/auth_users/edit.html.erb b/app/views/auth_users/edit.html.erb index 29a6aaab..f6154033 100644 --- a/app/views/auth_users/edit.html.erb +++ b/app/views/auth_users/edit.html.erb @@ -96,6 +96,20 @@ <%= form.submit "Disconnect your NeoPass", disabled: !@auth_user.uses_password? && !@auth_user.email? %> <% end %> +<% else %> + <%= form_with url: auth_user_neopass_omniauth_authorize_path(intent: "connect"), + method: :post, class: "settings-form", data: {turbo: false} do |form| + %> +

Your NeoPass

+
+

+ If you connect a NeoPass, you can use it to log into this DTI account! + You'll still be able to use your password to log in too, and you can + disconnect this later if you'd like. +

+
+ <%= form.submit "Connect your NeoPass" %> + <% end %> <% end %> <% content_for :stylesheets do %> diff --git a/app/views/devise/sessions/new.html.erb b/app/views/devise/sessions/new.html.erb index 7ebdd49b..996e6a1b 100644 --- a/app/views/devise/sessions/new.html.erb +++ b/app/views/devise/sessions/new.html.erb @@ -4,7 +4,7 @@ 🌟✨🌟✨🌟✨🌟✨🌟
<%= button_to "Log in with NeoPass", - auth_user_neopass_omniauth_authorize_path, + auth_user_neopass_omniauth_authorize_path(intent: "login"), data: {turbo: false} # Turbo can't handle this redirect! %> 🌟✨🌟✨🌟✨🌟✨🌟 diff --git a/db/openneo_id_migrate/20240408120359_add_unique_index_for_omniauth_to_users.rb b/db/openneo_id_migrate/20240408120359_add_unique_index_for_omniauth_to_users.rb new file mode 100644 index 00000000..abaee21d --- /dev/null +++ b/db/openneo_id_migrate/20240408120359_add_unique_index_for_omniauth_to_users.rb @@ -0,0 +1,5 @@ +class AddUniqueIndexForOmniauthToUsers < ActiveRecord::Migration[7.1] + def change + add_index :users, [:provider, :uid], unique: true + end +end diff --git a/db/openneo_id_schema.rb b/db/openneo_id_schema.rb index 8ea0f0e8..3113d4ff 100644 --- a/db/openneo_id_schema.rb +++ b/db/openneo_id_schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_04_07_135246) do +ActiveRecord::Schema[7.1].define(version: 2024_04_08_120359) do create_table "users", id: { type: :integer, unsigned: true }, charset: "utf8mb3", collation: "utf8mb3_general_ci", force: :cascade do |t| t.string "name", limit: 30, null: false t.string "encrypted_password", limit: 64 @@ -33,6 +33,7 @@ ActiveRecord::Schema[7.1].define(version: 2024_04_07_135246) do t.string "uid" t.string "neopass_email" t.index ["email"], name: "index_users_on_email", unique: true + t.index ["provider", "uid"], name: "index_users_on_provider_and_uid", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true t.index ["unlock_token"], name: "index_users_on_unlock_token", unique: true end