Merge pull request #3892 from dodona-edu/feature/personal-accounts

Allow sign in using personal accounts

app/controllers/application_controller.rb

@@ -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,

app/controllers/auth/authentication_controller.rb

@@ -38,6 +38,8 @@ def sign_in
       Provider::Surf
     ]
 
+    @private_providers = Provider.where(institution: nil)
+
     @providers = Provider.all
     @title = I18n.t('auth.sign_in.sign_in')
     @oauth_providers = apply_scopes(@providers

app/controllers/auth/omniauth_callbacks_controller.rb

@@ -47,13 +47,21 @@ 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.
 
   def generic_oauth
-    return provider_missing! if oauth_provider_id.blank?
-
     # Find the provider for the current institution. If no provider exists yet,
     # a new one will be created.
     return redirect_with_flash!(I18n.t('auth.sign_in.errors.institution-creation')) if provider.blank?
@@ -96,7 +104,11 @@ def try_login!
       return redirect_to_known_provider!(user) if user.present?
 
       # No existing user was found
-      # Create a new user
+      # Redirect to privacy prompt before we create a new private user
+      return redirect_to_privacy_prompt if provider&.institution.nil?
+
+      # Institutional users don't need to accept the privacy policy
+      # Thus we can immediately 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
@@ -111,6 +123,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)
 
@@ -122,7 +140,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.
@@ -155,7 +204,20 @@ def create_identity_from_session!(user)
 
   def find_identity_by_uid
     # In case of provider without uids, don't return any identity (As it won't be matching a unique user)
-    Identity.find_by(identifier: auth_uid, provider: provider) if auth_uid.present?
+    return nil if auth_uid.nil?
+
+    identity = Identity.find_by(identifier: auth_uid, provider: provider)
+
+    return identity unless identity.nil? && provider.class.sym == :office365 && auth_email.present?
+
+    # This code supports a migration of the office365 oauth api from v1 to v2
+    # Try to find the user by the legacy identifier
+    identity = Identity.find_by(identifier: auth_email.split('@').first, provider: provider, identifier_based_on_email: true)
+    return nil if identity.nil?
+
+    # Update the identifier to the new uid
+    identity.update(identifier: auth_uid, identifier_based_on_email: false)
+    identity
   end
 
   def find_identity_by_user(user)
@@ -297,7 +359,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
@@ -319,7 +381,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
@@ -366,9 +428,4 @@ def institution_create_failed(errors)
                      .institution_creation_failed
                      .deliver_later
   end
-
-  def provider_missing!
-    flash_failure I18n.t('auth.sign_in.errors.missing-provider')
-    redirect_to sign_in_path
-  end
 end

app/controllers/courses_controller.rb

@@ -342,26 +342,14 @@ def subscribe
           success_method = method(:signup_succeeded_response)
         end
 
-        case @course.registration
-        when 'open_for_all'
+        if @course.open_for_user?(current_user)
           if try_to_subscribe_current_user status: status
             success_method.call(format)
           else
             subscription_failed_response format
           end
-        when 'open_for_institution'
-          if @course.institution == current_user.institution
-            if try_to_subscribe_current_user status: status
-              success_method.call(format)
-            else
-              subscription_failed_response format
-            end
-          else
-            format.html { redirect_to(course_url(@course, secret: params[:secret]), alert: I18n.t('courses.registration.closed')) }
-            format.json { render json: { errors: ['course closed'] }, status: :unprocessable_entity }
-          end
-        when 'closed'
-          format.html { redirect_to(@course, alert: I18n.t('courses.registration.closed')) }
+        else
+          format.html { redirect_to(course_url(@course, secret: params[:secret]), alert: I18n.t('courses.registration.closed')) }
           format.json { render json: { errors: ['course closed'] }, status: :unprocessable_entity }
         end
       end

app/helpers/courses_helper.rb

@@ -7,7 +7,7 @@ def registration_action_for(**args)
     membership = args[:membership]
 
     if membership.nil? || membership.unsubscribed?
-      if course.open_for_all? || (course.open_for_institution? && (course.institution == current_user&.institution || current_user.nil?))
+      if course.open_for_user?(current_user) || current_user.nil?
         if course.moderated
           link_to t('courses.show.request_registration'),
                   subscribe_course_path(@course, secret: secret),

app/models/activity.rb

@@ -280,7 +280,7 @@ def accessible?(user, course)
       return true if user&.member_of? course
       return false if course.moderated && access_private?
 
-      course.open_for_all? || (course.open_for_institution? && course.institution == user&.institution)
+      course.open_for_user?(user)
     else
       return true if user&.repository_admin? repository
 

app/models/course.rb

@@ -55,7 +55,7 @@ class Course < ApplicationRecord
   has_many :course_labels, dependent: :destroy
 
   enum visibility: { visible_for_all: 0, visible_for_institution: 1, hidden: 2 }
-  enum registration: { open_for_all: 0, open_for_institution: 1, closed: 2 }
+  enum registration: { open_for_all: 3, open_for_institutional_users: 0, open_for_institution: 1, closed: 2 }
 
   # TODO: Remove and use activities?
   has_many :content_pages,
@@ -168,7 +168,7 @@ class Course < ApplicationRecord
   # Default year & enum values
   after_initialize do |course|
     self.visibility ||= 'visible_for_all'
-    self.registration ||= 'open_for_all'
+    self.registration ||= 'open_for_institutional_users'
     unless year
       now = Time.zone.now
       y = now.year
@@ -214,6 +214,10 @@ def all_activities_accessible?
     activities.where(access: :private).where.not(repository_id: usable_repositories).count.zero?
   end
 
+  def open_for_user?(user)
+    open_for_all? || (open_for_institution? && institution == user&.institution) || (open_for_institutional_users? && user&.institutional?)
+  end
+
   def invalidate_subscribed_members_count_cache
     Rails.cache.delete(format(SUBSCRIBED_MEMBERS_COUNT_CACHE_STRING, id: id))
   end

app/models/identity.rb

@@ -2,12 +2,13 @@
 #
 # Table name: identities
 #
-#  id          :bigint           not null, primary key
-#  identifier  :string(255)      not null
-#  provider_id :bigint           not null
-#  user_id     :integer          not null
-#  created_at  :datetime         not null
-#  updated_at  :datetime         not null
+#  id                        :bigint           not null, primary key
+#  identifier                :string(255)      not null
+#  provider_id               :bigint           not null
+#  user_id                   :integer          not null
+#  created_at                :datetime         not null
+#  updated_at                :datetime         not null
+#  identifier_based_on_email :boolean          default(FALSE), not null
 #
 class Identity < ApplicationRecord
   belongs_to :provider, inverse_of: :identities

app/models/provider.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)
@@ -24,7 +24,7 @@ class Provider < ApplicationRecord
 
   PROVIDERS = [Provider::GSuite, Provider::Lti, Provider::Office365, Provider::Oidc, Provider::Saml, Provider::Smartschool, Provider::Surf].freeze
 
-  belongs_to :institution, inverse_of: :providers
+  belongs_to :institution, inverse_of: :providers, optional: true
 
   has_many :identities, inverse_of: :provider, dependent: :destroy
 

app/models/provider/g_suite.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)
@@ -22,7 +22,7 @@
 class Provider::GSuite < Provider
   validates :certificate, :entity_id, :sso_url, :slo_url, absence: true
   validates :authorization_uri, :client_id, :issuer, :jwks_uri, absence: true
-  validates :identifier, uniqueness: { case_sensitive: false }, presence: true
+  validates :identifier, uniqueness: { case_sensitive: false }
 
   def self.sym
     :google_oauth2
@@ -46,4 +46,11 @@ def self.extract_institution_name(auth_hash)
       Provider.extract_institution_name(auth_hash)
     end
   end
+
+  def readable_name
+    # We want to display gmail for private accounts
+    return 'Gmail' if institution.nil?
+
+    super.readable_name
+  end
 end

app/models/provider/lti.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/provider/office365.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/provider/oidc.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/provider/saml.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/provider/smartschool.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/provider/surf.rb

@@ -4,7 +4,7 @@
 #
 #  id                :bigint           not null, primary key
 #  type              :string(255)      default("Provider::Saml"), not null
-#  institution_id    :bigint           not null
+#  institution_id    :bigint
 #  identifier        :string(255)
 #  certificate       :text(16777215)
 #  entity_id         :string(255)

app/models/user.rb

@@ -273,6 +273,14 @@ def repository_admin?(repository)
     @repository_admin.include?(repository.id)
   end
 
+  def personal?
+    institution.nil?
+  end
+
+  def institutional?
+    institution.present?
+  end
+
   def attempted_exercises(options)
     s = submissions.judged
     s = s.in_course(options[:course]) if options[:course].present?
@@ -351,13 +359,13 @@ def update_from_provider(auth_hash, auth_provider)
   end
 
   def self.from_email_and_institution(email, institution_id)
-    return nil if email.blank? || institution_id.nil?
+    return nil if email.blank?
 
     find_by(email: email, institution_id: institution_id)
   end
 
   def self.from_username_and_institution(username, institution_id)
-    return nil if username.blank? || institution_id.nil?
+    return nil if username.blank?
 
     find_by(username: username, institution_id: institution_id)
   end
@@ -453,7 +461,7 @@ def merge_into(other, force: false, force_institution: false)
   private
 
   def set_token
-    if institution.present?
+    if identities.present?
       self.token = nil
     elsif token.blank?
       generate_token