From f722e625877553ba955568d46b7ecb05defa9ad1 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Mar 2024 13:34:42 +0100 Subject: [PATCH 01/13] Refactor ACM-IDM provider to support LeerID --- .gitignore | 2 + .../auth/omniauth_callbacks_controller.rb | 4 +- app/models/institution.rb | 2 +- app/models/provider.rb | 4 +- app/models/provider/{oidc.rb => acmidm.rb} | 6 +- app/views/auth/_provider_button.html.erb | 2 +- config/credentials.yml.enc | 2 +- config/credentials/staging.yml.enc | 2 +- config/initializers/devise.rb | 4 +- db/seeds.rb | 2 +- lib/{OIDC => ACMIDM}/client.rb | 4 +- lib/ACMIDM/setup.rb | 55 ++++++++++++++ lib/ACMIDM/strategy.rb | 46 ++++++++++++ lib/OIDC/auth.rb | 7 -- lib/OIDC/auth/settings.rb | 53 -------------- lib/OIDC/auth/setup.rb | 72 ------------------- lib/OIDC/auth/strategy.rb | 38 ---------- .../auth/vlaanderen/oidc_vlaanderen_test.rb | 2 +- 18 files changed, 121 insertions(+), 186 deletions(-) rename app/models/provider/{oidc.rb => acmidm.rb} (85%) rename lib/{OIDC => ACMIDM}/client.rb (86%) create mode 100644 lib/ACMIDM/setup.rb create mode 100644 lib/ACMIDM/strategy.rb delete mode 100644 lib/OIDC/auth.rb delete mode 100644 lib/OIDC/auth/settings.rb delete mode 100644 lib/OIDC/auth/setup.rb delete mode 100644 lib/OIDC/auth/strategy.rb diff --git a/.gitignore b/.gitignore index bd3671c72d..18561ea7d6 100644 --- a/.gitignore +++ b/.gitignore @@ -59,3 +59,5 @@ junit.xml /app/assets/builds/* !/app/assets/builds/.keep + +/config/credentials/production.key diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index f5ff5486f0..53f2293886 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -34,11 +34,11 @@ def lti end def oidc - try_login! + generic_oauth end def surf - try_login! + generic_oauth end def elixir diff --git a/app/models/institution.rb b/app/models/institution.rb index 442e16ebc5..595e5125cf 100644 --- a/app/models/institution.rb +++ b/app/models/institution.rb @@ -66,7 +66,7 @@ def uses_lti? end def uses_oidc? - providers.any? { |provider| provider.type == Provider::Oidc.name } + providers.any? { |provider| provider.type == Provider::Acmidm.name } end def uses_smartschool? diff --git a/app/models/provider.rb b/app/models/provider.rb index f87bc51213..3f5fc8e371 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -22,7 +22,7 @@ class Provider < ApplicationRecord enum mode: { prefer: 0, redirect: 1, link: 2, secondary: 3 } - PROVIDERS = [Provider::GSuite, Provider::Lti, Provider::Office365, Provider::Oidc, Provider::Saml, Provider::Smartschool, Provider::Surf, Provider::Elixir].freeze + PROVIDERS = [Provider::GSuite, Provider::Lti, Provider::Office365, Provider::Acmidm, Provider::Saml, Provider::Smartschool, Provider::Surf, Provider::Elixir].freeze belongs_to :institution, inverse_of: :providers, optional: true @@ -31,7 +31,7 @@ class Provider < ApplicationRecord scope :gsuite, -> { where(type: Provider::GSuite.name) } scope :lti, -> { where(type: Provider::Lti.name) } scope :office365, -> { where(type: Provider::Office365.name) } - scope :oidc, -> { where(type: Provider::Oidc.name) } + scope :oidc, -> { where(type: Provider::Acmidm.name) } scope :saml, -> { where(type: Provider::Saml.name) } scope :smartschool, -> { where(type: Provider::Smartschool.name) } scope :surf, -> { where(type: Provider::Surf.name) } diff --git a/app/models/provider/oidc.rb b/app/models/provider/acmidm.rb similarity index 85% rename from app/models/provider/oidc.rb rename to app/models/provider/acmidm.rb index 0bad766542..324f4e073b 100644 --- a/app/models/provider/oidc.rb +++ b/app/models/provider/acmidm.rb @@ -19,10 +19,10 @@ # issuer :string(255) # jwks_uri :string(255) # -class Provider::Oidc < Provider +class Provider::Acmidm < Provider validates :certificate, :entity_id, :sso_url, :slo_url, absence: true - validates :identifier, absence: true - validates :client_id, :issuer, presence: true + validates :identifier, uniqueness: { case_sensitive: false } + validates :client_id, :issuer, absence: true def self.sym :oidc diff --git a/app/views/auth/_provider_button.html.erb b/app/views/auth/_provider_button.html.erb index 95e48e7f11..f3cdb6df82 100644 --- a/app/views/auth/_provider_button.html.erb +++ b/app/views/auth/_provider_button.html.erb @@ -30,7 +30,7 @@ <% end %> <% elsif provider.type == "Provider::Oidc" %> - <%= link_to omniauth_authorize_path(:user, Provider::Oidc.sym, provider: provider), method: :post, class: 'institution-sign-in col-md-6 col-xl-4' do %> + <%= link_to omniauth_authorize_path(:user, Provider::Acmidm.sym, provider: provider), method: :post, class: 'institution-sign-in col-md-6 col-xl-4' do %>
<%= image_tag institution_logo(institution.logo), class: "img-fluid" %> diff --git a/config/credentials.yml.enc b/config/credentials.yml.enc index e87c55720e..33fcc6e09d 100644 --- a/config/credentials.yml.enc +++ b/config/credentials.yml.enc @@ -1 +1 @@ -HK5nEb7Obp66Y79JPxk8eDcvdMkiKxX49gZstAf3db6Da6bpOeuEr9biEKow07kOuhd/OqWdODtrk9Iz2Clilk2HbpaEqGXQxPPISnG3UY+r/vuLzkULbyPJlPxDVq0MsaSifVDqZAtkNhaO/p28YtNNl04+tDtYs/No00HvkEhbFwUOuPX6DQkOB5PG547v2rUVvLZ0K6O5PuNS+8VA4N21cgGrLtVTLPsjY0CghJ/A4fqLGkEypJuEQyaVUlEkaMb36lWTUZap4Xr6DHXOusGxX0698UlThK4GKmEhxZR0UsMtp5djZa6/v0YR1O5cjzl82Pe/3tOgrN3BupfSa7yeIuhH3DoHPwNHo3JrwyypW2FMcuhxjO8VCfAyYEBky/TJLqUdJnbx7++zjKTedC0UNLIxU9rKSsGsjDm+zK5UcF7dbpLFVBtH/ZdfeWnaZL/CiZPcBmOrtrVGMQkWp0921kLPbLWzxURA1H4cVJ3Zj/wOzqJhb253kFH9mTkN8+r8uKjcH4mp/bBk0R0USBmqy6ewEgb8NTHKPBJb9smHZywA2bVQGjFm3fUL0Tb0YCcZqs+CdRgaXzc/ShMj758YiXK6o5n6yrAlumbLoS1wMCaxzgbLmFcD8s3FeCI79fcpoiTkUQLzPAi7seUh5euU/W665vKe6Sc4LzRP5Tmh8Y2dz2TnQmCWYr9Z2QfUZlMRYCdUfILPinv1XkmfgZ+dSljTupaBLdSmcV/ssQbgGXUitEDHlN3ZYlIXALDckmL701E1/jM9xoMQg7QQ90NYQjcmYSuiORTBapLoy5lUMjSBXv8p8JXQ/zR3R26P9C9ODYMYJzePJB6+NC01zT2qfET6/XpU0M89kGxmahdopu/27YCe6DZFTPnIrQ7lFgWOdM8CJF9wdaVozfg0P8Ck/tS0EjZiQ1U6oC7NXBk67zr9+OZZixTOTUKBHO0tzmeX4/11kOAwg1FUAMeaP/CztdrDIMa/1olUMTPAzI4OY3+I+WoOOotOvv64lie/JJqm9LRQvjbGIQY6/T21wKR8K//rkKN53ODogAjBHEbLNFTjXTVkxKuXIkpJ+aSIxd9+CTANb44DKeLXB0FLz+fmMRoiZFbFMW1c45CaPAlROxiklqvOqyget/oAjbm6XCHEi4k9fruLqwWMsRgLkG2tUArBusybqsdKi6K7o0Esm0JrLVy1ACh2APadGhWMgtY2Wqwo+IQeZEdUCm2rLRYkuaXEKeNrSK2dJYFYoCEtzmiuTqIpocikMhLhVp/FW6xCxn+1PNqNIfw2fq52Zb/cEfIllDT9ZCEXTdAsPjy8/MY0jBAh/E1WvCBXaZSSfi3xLv79fEVq/eBKV7mmB7eVGCXT9gDn+A0mIuNkGlXjVaJCBbKnwByK0BF5Q1LhwJI8Nl+Y0FLiSmmW/Uf7GUnPcUoVeIBAiqj0LvYk3dmcjKWSqRGPwnjPa1qz--SMu6mv2AvOVU+3ZM--W7kqAiucIEpkAl4PMCk1sw== \ No newline at end of file +dl1376ElzC5rNNepOA36nOFqB6cB4mIK6FrbQ8DCUb/tqzFNZ+b0Pl1ttB9GP832ilatyNGYGY+ofSbAOU86b7fx7xACJ4Zembms8Yvg547YDuYlWwxr1uS1hJd+Jimp3Nhgxh2zxwgnc/VZYFM7jaWu1sFkld+IEn794tz1qUvvRgvQKaC4kazJii+apG39ccB6L/UE90kiQlR4gWNtG5EzPfSLuWe8QW1BCSDc9cjC0GOQB5Tw7julgwkGa3Zgq8qScQ3wavVe5a6w8B/QAppxBH4sZUCDnVdVXLk/0dmA/8C1IQeHaLrklllOC0KCF/YQrDVNArVQXorQoXEP5s2VQkaDG6vvxBFxcgS642hXJEvq6HT+sk4Pa2+WDiZKBGDvS8ZDQR5NpdDNeXRjgDrsoPw26VYDBqPD6SUkeB59bMDPSAyVhzkgbEpLsTQ8SF9Rw1W/D3Xo28JRefo19xDcTVCjLEJ+LCNpDc32qvOEact6Ss/KrtH+wVBKWMnzEOp0pcgaZTSwcGLR5cnh/XtYbtuuERVL5EiT7p+eJAXHSyH04nfsSpTzMDWTfLDx8yyEkG4Yv1rEtCZ5tVTHvJb2Hx2JDsF9tovqCD01AQwWComPxcnzOWzCl6zmA4kHyjMpkd/9dHYbHdJIGdsm18zxXQat6+148YVH8F2iiFpV+Hr/njlGGK99aFFlthrBvWI9JpNQ2+RBUDMuFqXEu8o9R9SAOzpkU3dtygGRptE3WYWOvUmP3cHQh8S22cM1gJRdcOXvzPujLE0IJzPtNLE1eF7/XeVMzQuBt3/ZtKaNaStXYD6eKYrqs7yWeGGQGAfx/aJpDvePVrpwIYR6n7B1aYfnFiWalsz9di3mu+lccn6gl6Nlayp6lQAQE7pAWtvH+OgQpVWoeShEFE3iQyLwTDQ1xyHgaNquqQ8xU6m6TrhuU0c4Yrt8+G6ucIVjBRWR5E7D7k6FnsD37E/Ad+FbDFjkVXZ7dLYES/ezczkQONvqzrbO6T0p2Kx9nJieZu7/0ZWxA8pwcDH70831O/htRxX+iU+kMmKm6D9jYZQ0WiEN5tlJKLUaK2PogDdZbkDT1T0bso2fXQv17z84PPsMMEYQHPln6vkCFonLBgBJvdDGvDC2IzZDNZ+OCHVtGLg0IoDWgqFQknsh+TgDz6c0esOox/ZFcrhINFyUrQc+AnNq0yhGL+29E89ly0mhpTsO9SEYBBOZj8GVvXYu0vrtENkwqVCRGS4AyREvBVGq0y8nXGYagnzETtUdDZt4ZnjDlZBa1WL/CVYOTJL6n1xkyHpVH+D3hX8X5Ofwzx49tVtbyAcFflGUrNIx03VEqfW6veRMGvjRun4hW+lpY3qSq/CTXf34di/6SqHzePWaQ4JConQ2BHt7gMEBNEe0j7z56Nrmpkya4yIx5sjdgDQiI5eQjQHgphvHemjPyzz/DOXmNEYJQROXS//anfV2CjFDVSUGbxrYxDV/KyD9mHTcqawwMk1cz62uSwvDdhoGZpe9t/3wRVg93C/jEGTw7qZAxaw5/A==--uSkQjmMwf1jNxea4--Wd/v5VDHQD+u86QjDZHWyw== \ No newline at end of file diff --git a/config/credentials/staging.yml.enc b/config/credentials/staging.yml.enc index 788b946c2d..4c7794cfde 100644 --- a/config/credentials/staging.yml.enc +++ b/config/credentials/staging.yml.enc @@ -1 +1 @@ -FtEacz67m78JlWej2mS79IyO8jDCtCb0q5VAgrfzcRMwUrhd/npLcaxUC5lju07PRmvS23o2yR4iv6twgZ++YuU52j3VUkrBA+tpjO/JQjZIxYmF3YH88OOgludhGJDsKO3Piz12Ha6PFup6dcX0sOJInJeA8OFnJgGB2L+j4xULDidBfVvN9qBRECmHIcx4DrCPBNa30CbtPWpnM8fxtCQdqI9w0gZdQTTLvf8AYD7fQHsJjZ/JQdkJlyixHlFK2nCwJht9zj6a4VK+JwXzNueKR1XolVijXgPo/vx9RK3ZFfvieVhoRx8A/yZtmHxdDMLRoWWdjq5ruudUSM91z+7a6IZTncuf4bx/Q9gGMdFI9P1Ly9JnfhqJ+6yeeqlbo9As3wrjPIdGD54OSJE/vpvmle9cbop2GU7pq/9YwQC9yKoJizPOGqdDsqZUXgU99mKP9UjFDs2e1w849rJn3IyPcYlWN4s/ha/Z4nQAMkXoU4SzGyImNZv1Ne2oLPVEUCHvPppsal/lS4EjOFoKOVQ9n98DUu2VryFUUW7EoBZdobdHlhE7+sMd7j7Knrd8o3kEiddK9aVPBny9wOzu1D1Bv+zM3dKE+tLUIbJxN/Y9upGwdDo3Lo2BIIYE/gyGIc4i5W4nG8aXqhRFrmdKhhY2iQRM5RXY1UGOCwjX9AZTGzm8k0ru4me6r8KbOAOs7DxrE1SzzjQuc036yEsCPb8bT2G5uWFf80D4ZEUaEQmBgNeoZ6GtSJt5Oi8wfl9ZpDD1gYHaXSDaDAXlH/8N1A2SqW2sOgBaGCvNdZQnOtfsvnwDblc/Cufz4oYp0IPgJL9usAVCsSRaJb+FE0E6lGBdTALcFQ4TRd3DGhHOOcPXRD4nvkuxcDQdijHYkPBz5mhUYZQqSURNopZPMA1dRh1Q6rXVDHdJTE+FeN9N7mUwJi7ALeFlyREODirSUW/Kls9oZKPTEDPDxIJ9fkbqtqXY+wKkXIJVmOsVERlh055HeYWMXB+nD3kyaqK41qcdVKb7uH7gtLDdxKL2oVZMEQb1vD/jN8AKv9kaSlnR3mV0SWEZVls8La+ZMlh7MI7DHcHgIc06gBO4yDGgzh6LDeaBwa5dMvvK--gylYIlQZnfRkadmD--50UxMH/J6luJOiKmTdDNfQ== \ No newline at end of file +R3/CJKHc7eg6SwhmKUg/iD3afrPlqR/QqLPDrzHK1W0K+fpawBmVTBvs+0E5xtZq0bDxYQUfV8dFPsnRzc8208tPtneg2tzXWojkxu7CaWKL63IgBnDRAj1mY6iRA398lZREa1xOnnrsBOUFMkHz2BxK8KzTWfiLMjn5zaPNq5ZpTIjqf+e78b+u/587aLKyfnHp2fKCtSIXwYZgK3iItTcxMybiX7jr3YXe65oRGV5mLsbcdr/N1WrhjVFD9m7NOnhjI7XUw2Y78/4YwJcHjKA7tSLwuAz9BHEBLqBzgpr4DNHTioK4iL3Bu321ia1ZK+aQI8D1tBrzNpH9w2Jr/3AwabTwwpIMtyVX+TslcuvmBQ62BRCCcVecdllNc6VV/KSG5pWy2yiRZ04eQOoti2S1P8X8fdwxzb4jQ/eiL4640Mfi6RZj+NLSRQUVt52TInC2fL9uIFMtk+xETOT8jGpFzH64rw8A+Bh6zq3vQQSedr8w4pGxRupFNE8zwpoXYiHGI3bkQFr7Mu5sRay8Aj+iGURGxnBU7r9Xz1Jq32iDqpRJjifI5iOhb/azEUJ/G+Z/TERPg31NgeYI7KlfPUqGo5Gfc/h9zq6wCFQ3UXe4NcJIJ3guMCFAknI5bgIChAIDr8UnLcTPf2PjA3R36L+NHLD8zvgxwKbcgkLpSHECYiKzbRifkkkYEdwLr9f7iJ5vkrFgrfwGdJMc94/doCckRAjUjsHD0D5tcK8QCjdmq5O47LajM41KG4/ADF7BXIfMdu7IFlNP1mTXTnLdGJ6epAjIbJ2WT55AeYqEKNFMcrIK1TOY+M4BqdcMX2+hC6pbAm4/m4bdfIsvOKlpkD73IfCt7sBFYltmMfyBph0uQGi8E35huXB59QXlwnB3NaHkYZTdSh1tUasi+qlCOWdmtaHiCUgiUH/4CQtz9HyDFYnGlk3WKKGoCvUhxN78eGA9Aai5Hrl9hQDKLHhXRlw53kWVUpDHTgWNG7FkGHysOXL9Efjqw2ktYUk1pfHxNgLYvM+A79rU+Meo9RGCoa7PXRAAHskMMSBKFOiKX7zOqdq5YQDnU1S5gAQnvYla0Z3xWMGAS0/5JtKeNvnzQTwzll3wLoWagYvQt/bxPF2nGIfPnSspehgcyUjuNFUwAiSbJcG8NIFqU23Z8pupIN7fyI5iGykDkA1VCEeaYbtF--4ZnDJyUUYZ4bBWXv--RAbKFPok4P+ERq999nDiiA== \ No newline at end of file diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 674afa09f4..305369f52a 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -2,7 +2,7 @@ require_relative '../../lib/LTI/auth.rb' ## OIDC. -require_relative '../../lib/OIDC/auth.rb' +require_relative '../../lib/ACMIDM/auth.rb' ## SAML. require_relative '../../lib/SAML/strategy.rb' @@ -272,7 +272,7 @@ Rails.application.credentials.office365_client_id, Rails.application.credentials.office365_client_secret - config.omniauth :oidc, setup: OIDC::Auth::OmniAuth::Setup + config.omniauth :acmidm, setup: ACMIDM::Auth::OmniAuth::Setup config.omniauth :surf, setup: Surf::Auth::OmniAuth::Setup diff --git a/db/seeds.rb b/db/seeds.rb index a1396cd274..c5d24d15aa 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -100,7 +100,7 @@ def fill_series_with_realistic_submissions(s) Provider::Smartschool.create institution: college_ieper, identifier: 'https://college-ieper.smartschool.be' # OIDC - Provider::Oidc.create institution: vlaanderen, client_id: '12345', issuer: 'https://authenticatie.vlaanderen.be/op' + Provider::Acmidm.create institution: vlaanderen, client_id: '12345', issuer: 'https://authenticatie.vlaanderen.be/op' # Personal providers Provider::Office365.create identifier: '9188040d-6c67-4c5b-b112-36a304b66dad', institution: nil diff --git a/lib/OIDC/client.rb b/lib/ACMIDM/client.rb similarity index 86% rename from lib/OIDC/client.rb rename to lib/ACMIDM/client.rb index cddaaa99f3..b983e48627 100644 --- a/lib/OIDC/client.rb +++ b/lib/ACMIDM/client.rb @@ -1,7 +1,9 @@ require "json/jwt" require "rack/oauth2/client/grant/authorization_code" -module OIDC +# ACMIDM is an expension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of the flemish government +module ACMIDM class Client < OpenIDConnect::Client # By default, the JWT grant will set the `grant type` to `jwtbearer`. # However, Vlaamse Overheid expects this to be authorization_code; hence we diff --git a/lib/ACMIDM/setup.rb b/lib/ACMIDM/setup.rb new file mode 100644 index 0000000000..8596f832d9 --- /dev/null +++ b/lib/ACMIDM/setup.rb @@ -0,0 +1,55 @@ +# ACMIDM is an expension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of the flemish government +module ACMIDM + module Auth + module OmniAuth + class Setup + KEY_PATH = '/home/dodona/key.pem'.freeze + + def self.call(env) + new(env).setup + end + + def initialize(env) + @env = env + end + + def setup + @env['omniauth.params'] ||= {} + @env['omniauth.strategy'].options.merge!(configure) + end + + def configure + { + client_options: { + identifier: Rails.application.credentials.acmidm_client_id, + private_key: private_key, + redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/oidc/callback" + }, + discovery: true, + response_mode: :form_post, + response_type: :code, + scope: [:openid, :profile, :vo, :ov_leerling], + client_auth_method: :jwt_bearer, + issuer: Rails.env.production? ? "https://authenticatie.vlaanderen.be/op" : "https://authenticatie-ti.vlaanderen.be/op", + } + end + + private + + def private_key_path + # This function allows to override the key path in tests. + KEY_PATH + end + + def private_key + # Only load the key if it exists (staging / production). + return nil unless File.file?(private_key_path) + + # Parse the key. + @private_key ||= OpenSSL::PKey::RSA.new File.read(private_key_path) + end + end + end + end +end diff --git a/lib/ACMIDM/strategy.rb b/lib/ACMIDM/strategy.rb new file mode 100644 index 0000000000..f75a973399 --- /dev/null +++ b/lib/ACMIDM/strategy.rb @@ -0,0 +1,46 @@ +require_relative '../ent.rb' +require 'openid_connect' +require 'openid_connect/response_object' + +# ACMIDM is an expension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of the flemish government +# This is used by both government officials and LeerID +module OmniAuth + module Strategies + class ACMIDM < OmniAuth::Strategies::OpenIDConnect + include Rails.application.routes.url_helpers + + option :name, 'oidc' + + info do + { + # No org is provided for flemish government accounts, so we default to 'vlaamse-overheid' + institution: user_info.raw_attributes['ov_orgcode'] || "vlaamse-overheid", + institution_name: user_info.raw_attributes['ov_orgnaam'] || "Vlaamse Overheid", + email: user_info.raw_attributes['vo_email'] # this will be nil for leerid accounts + } + end + + def client + # This logic was added specifically for Vlaamse Overheid. By default, + # the audience will be set to the token endpoint (which is compliant to + # the OIDC specification). However, Vlaamse Overheid wants this to be + # equal to the issuer. + # + # Token endpoint: https://authenticatie-ti.vlaanderen.be/op/v1/token. + # Vlaamse Overheid wants: https://authenticatie-ti.vlaanderen.be/op. + @client ||= ::ACMIDM::Client.new(client_options.merge(audience: options.issuer)) + end + + def uid + # Leerid accounts do not provide a sub, instead they provide 'ov_account_uuid', 'ov_leerid_uuid' and 'ov_historiek_account_uuid' + # We use 'ov_account_uuid' as it is always present + # It is important to note that 'ov_account_uuid' could change over time, but this should only happen in edge cases: eg. after merging accounts from foreign students + # Should we notice that this causes too much trouble, more complex logic should be implemented + # Probably in omniauth_callbacks_controller `find_identity_by_uid` + super || user_info.raw_attributes['ov_account_uuid'] + end + end + end +end + diff --git a/lib/OIDC/auth.rb b/lib/OIDC/auth.rb deleted file mode 100644 index 04c6965fed..0000000000 --- a/lib/OIDC/auth.rb +++ /dev/null @@ -1,7 +0,0 @@ -module OIDC - module Auth - require_relative 'auth/settings.rb' - require_relative 'auth/setup.rb' - require_relative 'auth/strategy.rb' - end -end diff --git a/lib/OIDC/auth/settings.rb b/lib/OIDC/auth/settings.rb deleted file mode 100644 index 74212b53ef..0000000000 --- a/lib/OIDC/auth/settings.rb +++ /dev/null @@ -1,53 +0,0 @@ -require 'omniauth' - -module OIDC - module Auth - module Settings - KEY_PATH = '/home/dodona/key.pem'.freeze - - def base_settings(host = Rails.configuration.default_host) - # Support only third-parties that are discoverable. - { - client_options: { - redirect_uri: "https://#{host}/users/auth/oidc/callback" - }, - discovery: true, - response_mode: :form_post, - response_type: :code, - scope: [:openid, :profile, :vo] - } - end - - def provider_settings(provider) - raise 'Not an OIDC provider.' unless provider.is_a?(Provider::Oidc) - - # The id of the provider is encoded into the state, so that we can know - # the provider during the callback phase. - { - client_auth_method: :jwt_bearer, - client_options: { - identifier: provider.client_id, - private_key: private_key - }, - issuer: provider.issuer, - state: lambda { format("%d-%s", provider.id, SecureRandom::hex(16)) } - } - end - - private - - def private_key_path - # This function allows to override the key path in tests. - KEY_PATH - end - - def private_key - # Only load the key if it exists (staging / production). - return nil unless File.file?(private_key_path) - - # Parse the key. - @private_key ||= OpenSSL::PKey::RSA.new File.read(private_key_path) - end - end - end -end diff --git a/lib/OIDC/auth/setup.rb b/lib/OIDC/auth/setup.rb deleted file mode 100644 index 313a088064..0000000000 --- a/lib/OIDC/auth/setup.rb +++ /dev/null @@ -1,72 +0,0 @@ -module OIDC - module Auth - module OmniAuth - class Setup - include OIDC::Auth::Settings - - def self.call(env) - new(env).setup - end - - def initialize(env) - @env = env - end - - def setup - @env['omniauth.params'] ||= {} - @env['omniauth.strategy'].options.merge!(base_settings(@env['HTTP_HOST'])) - @env['omniauth.strategy'].options.merge!(configure) - end - - private - - def configure - # Obtain the openid parameters for the provider. - _provider = provider - return failure! if _provider.blank? - - provider_settings(_provider) - end - - def failure! - raise "Invalid or unknown OIDC provider." - end - - def params - @params ||= Rack::Request.new(@env).params.symbolize_keys - end - - def provider - # Get the provider from the provider parameter. - return Provider::Oidc.find_by(id: params[:provider]) if params[:provider].present? - - # Get the provider from the issuer parameter. - return Provider::Oidc.find_by(issuer: params[:iss]) if params[:iss].present? - - # Get the provider from the state parameter. - return Provider::Oidc.find_by(id: provider_from_state) if provider_from_state.present? - - # If there is a JWT available, we can parse that to find the issuer. - return nil if params[:id_token].blank? - - # Parse the JWT token. - jwt_token = JSON::JWT.decode params[:id_token], :skip_verification - Provider::Oidc.find_by(issuer: jwt_token[:iss]) - end - - def provider_from_state - # If there is no state, we will not find anything. - return nil if params[:state].blank? - - # Attempt to split the state. - state = params[:state].split("-", 2) - return nil unless state.count == 2 - - # Attempt to parse the id as an integer. - parsed = state[0].to_i - parsed > 0 ? parsed : nil - end - end - end - end -end diff --git a/lib/OIDC/auth/strategy.rb b/lib/OIDC/auth/strategy.rb deleted file mode 100644 index 578607ebc6..0000000000 --- a/lib/OIDC/auth/strategy.rb +++ /dev/null @@ -1,38 +0,0 @@ -require_relative '../client.rb' -require 'openid_connect' -require 'openid_connect/response_object' - -# This strategy enables OIDC to be used by Dodona. -module OmniAuth - module Strategies - class OIDC < OmniAuth::Strategies::OpenIDConnect - include Rails.application.routes.url_helpers - - option :name, 'oidc' - - def client - # This logic was added specifically for Vlaamse Overheid. By default, - # the audience will be set to the token endpoint (which is compliant to - # the OIDC specification). However, Vlaamse Overheid wants this to be - # equal to the issuer. - # - # Token endpoint: https://authenticatie-ti.vlaanderen.be/op/v1/token. - # Vlaamse Overheid wants: https://authenticatie-ti.vlaanderen.be/op. - @client ||= ::OIDC::Client.new(client_options.merge(audience: options.issuer)) - end - - private - - def user_info - return @user_info if @user_info - - # Set the email address alias. This is specific to Vlaamse Overheid. - decoded = decode_id_token(access_token.id_token).raw_attributes - decoded["email"] = decoded["vo_email"] - @user_info = ::OpenIDConnect::ResponseObject::UserInfo.new(decoded) - end - end - end -end - -OmniAuth.config.add_camelization 'oidc', 'OIDC' diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index 06fbcbbe56..f3c7de1275 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -12,7 +12,7 @@ ### # Set the signing key. -module OIDC::Auth::Settings +module ACMIDM::Auth::Settings private def private_key_path From d91e1bec873f0b880e1848d5da123981036ac4c3 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Mar 2024 14:13:07 +0100 Subject: [PATCH 02/13] complete rename' --- app/controllers/application_controller.rb | 4 ++-- .../auth/authentication_controller.rb | 2 +- .../auth/omniauth_callbacks_controller.rb | 2 +- app/models/institution.rb | 2 +- app/models/provider.rb | 2 +- app/models/provider/acmidm.rb | 18 +++++++++++++++++- app/models/user.rb | 2 +- app/views/auth/_provider_button.html.erb | 14 -------------- config/initializers/devise.rb | 5 +++-- config/locales/models/en.yml | 2 +- config/locales/models/nl.yml | 2 +- db/seeds.rb | 4 ++-- lib/ACMIDM/setup.rb | 2 +- lib/ACMIDM/strategy.rb | 9 ++++----- test/factories/providers.rb | 2 +- 15 files changed, 37 insertions(+), 35 deletions(-) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 9e11c7e15a..488fae037e 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -71,9 +71,9 @@ def after_sign_out_path_for(_resource) end Warden::Manager.after_authentication do |user, _auth, _opts| - if user.email.blank? && !user.institution&.uses_lti? && !user.institution&.uses_oidc? && !user.institution&.uses_smartschool? + if user.email.blank? && !user.institution&.uses_lti? && !user.institution&.uses_acmidm? && !user.institution&.uses_smartschool? raise "User with id #{user.id} should not have a blank email " \ - 'if the provider is not LTI, OIDC or Smartschool' + 'if the provider is not LTI, ACM-IDM or Smartschool' end user.touch(:sign_in_at) end diff --git a/app/controllers/auth/authentication_controller.rb b/app/controllers/auth/authentication_controller.rb index b4a22f1bc7..cf1e540e73 100644 --- a/app/controllers/auth/authentication_controller.rb +++ b/app/controllers/auth/authentication_controller.rb @@ -26,7 +26,7 @@ def sign_in Provider.find_by(identifier: 'e638861b-15d9-4de6-a65d-b48789ae1f08') # UCLL ].compact @other = [ - Provider.find_by(issuer: 'https://authenticatie.vlaanderen.be/op'), + Provider::Acmidm, # Vlaamse Overheid Provider::Elixir # Elixir ].compact diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index 53f2293886..9d5e4e78df 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -33,7 +33,7 @@ def lti try_login! end - def oidc + def acmidm generic_oauth end diff --git a/app/models/institution.rb b/app/models/institution.rb index 595e5125cf..b10086c4ed 100644 --- a/app/models/institution.rb +++ b/app/models/institution.rb @@ -65,7 +65,7 @@ def uses_lti? providers.any? { |provider| provider.type == Provider::Lti.name } end - def uses_oidc? + def uses_acmidm? providers.any? { |provider| provider.type == Provider::Acmidm.name } end diff --git a/app/models/provider.rb b/app/models/provider.rb index 3f5fc8e371..03a7e7b3f0 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -31,7 +31,7 @@ class Provider < ApplicationRecord scope :gsuite, -> { where(type: Provider::GSuite.name) } scope :lti, -> { where(type: Provider::Lti.name) } scope :office365, -> { where(type: Provider::Office365.name) } - scope :oidc, -> { where(type: Provider::Acmidm.name) } + scope :acmidm, -> { where(type: Provider::Acmidm.name) } scope :saml, -> { where(type: Provider::Saml.name) } scope :smartschool, -> { where(type: Provider::Smartschool.name) } scope :surf, -> { where(type: Provider::Surf.name) } diff --git a/app/models/provider/acmidm.rb b/app/models/provider/acmidm.rb index 324f4e073b..3395055c6a 100644 --- a/app/models/provider/acmidm.rb +++ b/app/models/provider/acmidm.rb @@ -25,6 +25,22 @@ class Provider::Acmidm < Provider validates :client_id, :issuer, absence: true def self.sym - :oidc + :acmidm + end + + def self.logo + 'vlaamse-overheid.png' + end + + def self.readable_name + 'Vlaamse Overheid' + end + + def self.extract_institution_name(auth_hash) + institution_name = auth_hash&.info&.institution_name + + return Provider.extract_institution_name(auth_hash) if institution_name.nil? + + [institution_name, institution_name] end end diff --git a/app/models/user.rb b/app/models/user.rb index bc3fc08e4f..bd0b98edd8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -118,7 +118,7 @@ class User < ApplicationRecord has_many :annotations, dependent: :restrict_with_error has_many :questions, dependent: :restrict_with_error - devise :omniauthable, omniauth_providers: %i[google_oauth2 lti office365 oidc saml smartschool surf elixir] + devise :omniauthable, omniauth_providers: %i[google_oauth2 lti office365 acmidm saml smartschool surf elixir] validates :username, uniqueness: { case_sensitive: false, allow_blank: true, scope: :institution } validates :email, uniqueness: { case_sensitive: false, allow_blank: true, scope: :institution } diff --git a/app/views/auth/_provider_button.html.erb b/app/views/auth/_provider_button.html.erb index f3cdb6df82..4000401103 100644 --- a/app/views/auth/_provider_button.html.erb +++ b/app/views/auth/_provider_button.html.erb @@ -29,20 +29,6 @@
<% end %> - <% elsif provider.type == "Provider::Oidc" %> - <%= link_to omniauth_authorize_path(:user, Provider::Acmidm.sym, provider: provider), method: :post, class: 'institution-sign-in col-md-6 col-xl-4' do %> -
-
- <%= image_tag institution_logo(institution.logo), class: "img-fluid" %> -
-
-

<%= institution.short_name %>
- <%= institution.name %> -

-
-
-
- <% end %> <% else %> <%= link_to omniauth_authorize_path(:user, provider.class.sym), method: :post, class: 'institution-sign-in col-md-6 col-xl-4' do %>
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 305369f52a..a73d855eac 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -1,8 +1,9 @@ ## LTI. require_relative '../../lib/LTI/auth.rb' -## OIDC. -require_relative '../../lib/ACMIDM/auth.rb' +## ACM-IDM. +require_relative '../../lib/ACMIDM/strategy.rb' +require_relative '../../lib/ACMIDM/setup.rb' ## SAML. require_relative '../../lib/SAML/strategy.rb' diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 68e0057cc8..034a8eca94 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -159,7 +159,7 @@ en: office365: Office 365 smartschool: Smartschool google_oauth2: Google Workspace - oidc: OpenID Connect + acmidm: ACM-IDM lti: LTI surf: SURFconext elixir: Elixir diff --git a/config/locales/models/nl.yml b/config/locales/models/nl.yml index e8bfdd6616..1b6ad58d99 100644 --- a/config/locales/models/nl.yml +++ b/config/locales/models/nl.yml @@ -160,7 +160,7 @@ nl: office365: Office 365 smartschool: Smartschool google_oauth2: Google Workspace - oidc: OpenID Connect + acmidm: ACM-IDM lti: LTI surf: SURFconext elixir: Elixir diff --git a/db/seeds.rb b/db/seeds.rb index c5d24d15aa..2ae3037e9f 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -99,8 +99,8 @@ def fill_series_with_realistic_submissions(s) Provider::Smartschool.create institution: slo, identifier: 'https://slow.smartschool.be' Provider::Smartschool.create institution: college_ieper, identifier: 'https://college-ieper.smartschool.be' - # OIDC - Provider::Acmidm.create institution: vlaanderen, client_id: '12345', issuer: 'https://authenticatie.vlaanderen.be/op' + # ACM-IDM + Provider::Acmidm.create institution: vlaanderen, identifier: 'vlaamse-overheid' # Personal providers Provider::Office365.create identifier: '9188040d-6c67-4c5b-b112-36a304b66dad', institution: nil diff --git a/lib/ACMIDM/setup.rb b/lib/ACMIDM/setup.rb index 8596f832d9..c5ff04c695 100644 --- a/lib/ACMIDM/setup.rb +++ b/lib/ACMIDM/setup.rb @@ -24,7 +24,7 @@ def configure client_options: { identifier: Rails.application.credentials.acmidm_client_id, private_key: private_key, - redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/oidc/callback" + redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/acmidm/callback" }, discovery: true, response_mode: :form_post, diff --git a/lib/ACMIDM/strategy.rb b/lib/ACMIDM/strategy.rb index f75a973399..fa1ec8e367 100644 --- a/lib/ACMIDM/strategy.rb +++ b/lib/ACMIDM/strategy.rb @@ -1,6 +1,5 @@ -require_relative '../ent.rb' +require_relative 'client.rb' require 'openid_connect' -require 'openid_connect/response_object' # ACMIDM is an expension upon the OpenIDConnect Protocol # Changes are applied to support the specific requirements of the flemish government @@ -8,9 +7,7 @@ module OmniAuth module Strategies class ACMIDM < OmniAuth::Strategies::OpenIDConnect - include Rails.application.routes.url_helpers - - option :name, 'oidc' + option :name, 'acmidm' info do { @@ -44,3 +41,5 @@ def uid end end +OmniAuth.config.add_camelization 'acmidm', 'ACMIDM' + diff --git a/test/factories/providers.rb b/test/factories/providers.rb index db77e2e4d9..207609dc9b 100644 --- a/test/factories/providers.rb +++ b/test/factories/providers.rb @@ -39,7 +39,7 @@ identifier { SecureRandom.uuid } end - factory :oidc_provider, class: 'Provider::Oidc' do + factory :acmidm_provider, class: 'Provider::Acmidm' do institution client_id { SecureRandom.uuid } From 280c16e3e19a82eac985dceb2a38e7a069b42316 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 25 Mar 2024 10:34:01 +0100 Subject: [PATCH 03/13] Rename ACM-IDM to flemish government --- app/controllers/application_controller.rb | 2 +- app/controllers/auth/authentication_controller.rb | 2 +- app/controllers/auth/omniauth_callbacks_controller.rb | 2 +- app/models/institution.rb | 4 ++-- app/models/provider.rb | 4 ++-- app/models/provider/{acmidm.rb => flemish_government.rb} | 4 ++-- app/models/user.rb | 2 +- config/initializers/devise.rb | 6 +++--- config/locales/models/en.yml | 2 +- config/locales/models/nl.yml | 2 +- db/seeds.rb | 2 +- lib/{ACMIDM => FlemishGovernment}/client.rb | 6 +++--- lib/{ACMIDM => FlemishGovernment}/setup.rb | 8 ++++---- lib/{ACMIDM => FlemishGovernment}/strategy.rb | 8 +++----- test/factories/providers.rb | 2 +- test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb | 2 +- 16 files changed, 28 insertions(+), 30 deletions(-) rename app/models/provider/{acmidm.rb => flemish_government.rb} (94%) rename lib/{ACMIDM => FlemishGovernment}/client.rb (86%) rename lib/{ACMIDM => FlemishGovernment}/setup.rb (87%) rename lib/{ACMIDM => FlemishGovernment}/strategy.rb (86%) diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 488fae037e..402d6f216a 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -71,7 +71,7 @@ def after_sign_out_path_for(_resource) end Warden::Manager.after_authentication do |user, _auth, _opts| - if user.email.blank? && !user.institution&.uses_lti? && !user.institution&.uses_acmidm? && !user.institution&.uses_smartschool? + if user.email.blank? && !user.institution&.uses_lti? && !user.institution&.uses_flemish_government? && !user.institution&.uses_smartschool? raise "User with id #{user.id} should not have a blank email " \ 'if the provider is not LTI, ACM-IDM or Smartschool' end diff --git a/app/controllers/auth/authentication_controller.rb b/app/controllers/auth/authentication_controller.rb index cf1e540e73..3d0ad5332e 100644 --- a/app/controllers/auth/authentication_controller.rb +++ b/app/controllers/auth/authentication_controller.rb @@ -26,7 +26,7 @@ def sign_in Provider.find_by(identifier: 'e638861b-15d9-4de6-a65d-b48789ae1f08') # UCLL ].compact @other = [ - Provider::Acmidm, # Vlaamse Overheid + Provider::FlemishGovernment, # Vlaamse Overheid Provider::Elixir # Elixir ].compact diff --git a/app/controllers/auth/omniauth_callbacks_controller.rb b/app/controllers/auth/omniauth_callbacks_controller.rb index 9d5e4e78df..bee5e184d2 100644 --- a/app/controllers/auth/omniauth_callbacks_controller.rb +++ b/app/controllers/auth/omniauth_callbacks_controller.rb @@ -33,7 +33,7 @@ def lti try_login! end - def acmidm + def flemish_government generic_oauth end diff --git a/app/models/institution.rb b/app/models/institution.rb index b10086c4ed..5db806d31f 100644 --- a/app/models/institution.rb +++ b/app/models/institution.rb @@ -65,8 +65,8 @@ def uses_lti? providers.any? { |provider| provider.type == Provider::Lti.name } end - def uses_acmidm? - providers.any? { |provider| provider.type == Provider::Acmidm.name } + def uses_flemish_government? + providers.any? { |provider| provider.type == Provider::FlemishGovernment.name } end def uses_smartschool? diff --git a/app/models/provider.rb b/app/models/provider.rb index 03a7e7b3f0..c95bd1c70e 100644 --- a/app/models/provider.rb +++ b/app/models/provider.rb @@ -22,7 +22,7 @@ class Provider < ApplicationRecord enum mode: { prefer: 0, redirect: 1, link: 2, secondary: 3 } - PROVIDERS = [Provider::GSuite, Provider::Lti, Provider::Office365, Provider::Acmidm, Provider::Saml, Provider::Smartschool, Provider::Surf, Provider::Elixir].freeze + PROVIDERS = [Provider::GSuite, Provider::Lti, Provider::Office365, Provider::FlemishGovernment, Provider::Saml, Provider::Smartschool, Provider::Surf, Provider::Elixir].freeze belongs_to :institution, inverse_of: :providers, optional: true @@ -31,7 +31,7 @@ class Provider < ApplicationRecord scope :gsuite, -> { where(type: Provider::GSuite.name) } scope :lti, -> { where(type: Provider::Lti.name) } scope :office365, -> { where(type: Provider::Office365.name) } - scope :acmidm, -> { where(type: Provider::Acmidm.name) } + scope :flemish_government, -> { where(type: Provider::FlemishGovernment.name) } scope :saml, -> { where(type: Provider::Saml.name) } scope :smartschool, -> { where(type: Provider::Smartschool.name) } scope :surf, -> { where(type: Provider::Surf.name) } diff --git a/app/models/provider/acmidm.rb b/app/models/provider/flemish_government.rb similarity index 94% rename from app/models/provider/acmidm.rb rename to app/models/provider/flemish_government.rb index 3395055c6a..1f786c11cf 100644 --- a/app/models/provider/acmidm.rb +++ b/app/models/provider/flemish_government.rb @@ -19,13 +19,13 @@ # issuer :string(255) # jwks_uri :string(255) # -class Provider::Acmidm < Provider +class Provider::FlemishGovernment < Provider validates :certificate, :entity_id, :sso_url, :slo_url, absence: true validates :identifier, uniqueness: { case_sensitive: false } validates :client_id, :issuer, absence: true def self.sym - :acmidm + :flemish_government end def self.logo diff --git a/app/models/user.rb b/app/models/user.rb index bd0b98edd8..77cb7c1cb8 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -118,7 +118,7 @@ class User < ApplicationRecord has_many :annotations, dependent: :restrict_with_error has_many :questions, dependent: :restrict_with_error - devise :omniauthable, omniauth_providers: %i[google_oauth2 lti office365 acmidm saml smartschool surf elixir] + devise :omniauthable, omniauth_providers: %i[google_oauth2 lti office365 flemish_government saml smartschool surf elixir] validates :username, uniqueness: { case_sensitive: false, allow_blank: true, scope: :institution } validates :email, uniqueness: { case_sensitive: false, allow_blank: true, scope: :institution } diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index a73d855eac..18455d0c91 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -2,8 +2,8 @@ require_relative '../../lib/LTI/auth.rb' ## ACM-IDM. -require_relative '../../lib/ACMIDM/strategy.rb' -require_relative '../../lib/ACMIDM/setup.rb' +require_relative '../../lib/FlemishGovernment/strategy.rb' +require_relative '../../lib/FlemishGovernment/setup.rb' ## SAML. require_relative '../../lib/SAML/strategy.rb' @@ -273,7 +273,7 @@ Rails.application.credentials.office365_client_id, Rails.application.credentials.office365_client_secret - config.omniauth :acmidm, setup: ACMIDM::Auth::OmniAuth::Setup + config.omniauth :flemish_government, setup: FlemishGovernment::Auth::OmniAuth::Setup config.omniauth :surf, setup: Surf::Auth::OmniAuth::Setup diff --git a/config/locales/models/en.yml b/config/locales/models/en.yml index 034a8eca94..b322c517f9 100644 --- a/config/locales/models/en.yml +++ b/config/locales/models/en.yml @@ -159,7 +159,7 @@ en: office365: Office 365 smartschool: Smartschool google_oauth2: Google Workspace - acmidm: ACM-IDM + flemish_government: Flemish Government lti: LTI surf: SURFconext elixir: Elixir diff --git a/config/locales/models/nl.yml b/config/locales/models/nl.yml index 1b6ad58d99..700fe4a289 100644 --- a/config/locales/models/nl.yml +++ b/config/locales/models/nl.yml @@ -160,7 +160,7 @@ nl: office365: Office 365 smartschool: Smartschool google_oauth2: Google Workspace - acmidm: ACM-IDM + flemish_government: Vlaamse Overheid lti: LTI surf: SURFconext elixir: Elixir diff --git a/db/seeds.rb b/db/seeds.rb index 2ae3037e9f..275299b503 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -100,7 +100,7 @@ def fill_series_with_realistic_submissions(s) Provider::Smartschool.create institution: college_ieper, identifier: 'https://college-ieper.smartschool.be' # ACM-IDM - Provider::Acmidm.create institution: vlaanderen, identifier: 'vlaamse-overheid' + Provider::FlemishGovernment.create institution: vlaanderen, identifier: 'vlaamse-overheid' # Personal providers Provider::Office365.create identifier: '9188040d-6c67-4c5b-b112-36a304b66dad', institution: nil diff --git a/lib/ACMIDM/client.rb b/lib/FlemishGovernment/client.rb similarity index 86% rename from lib/ACMIDM/client.rb rename to lib/FlemishGovernment/client.rb index b983e48627..32e0810d5b 100644 --- a/lib/ACMIDM/client.rb +++ b/lib/FlemishGovernment/client.rb @@ -1,9 +1,9 @@ require "json/jwt" require "rack/oauth2/client/grant/authorization_code" -# ACMIDM is an expension upon the OpenIDConnect Protocol -# Changes are applied to support the specific requirements of the flemish government -module ACMIDM +# Flemish government is an extension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of ACM IDM. +module FlemishGovernment class Client < OpenIDConnect::Client # By default, the JWT grant will set the `grant type` to `jwtbearer`. # However, Vlaamse Overheid expects this to be authorization_code; hence we diff --git a/lib/ACMIDM/setup.rb b/lib/FlemishGovernment/setup.rb similarity index 87% rename from lib/ACMIDM/setup.rb rename to lib/FlemishGovernment/setup.rb index c5ff04c695..ee7cdfdfa8 100644 --- a/lib/ACMIDM/setup.rb +++ b/lib/FlemishGovernment/setup.rb @@ -1,6 +1,6 @@ -# ACMIDM is an expension upon the OpenIDConnect Protocol -# Changes are applied to support the specific requirements of the flemish government -module ACMIDM +# Flemish government is an extension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of ACM IDM. +module FlemishGovernment module Auth module OmniAuth class Setup @@ -24,7 +24,7 @@ def configure client_options: { identifier: Rails.application.credentials.acmidm_client_id, private_key: private_key, - redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/acmidm/callback" + redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/flemish_government/callback" }, discovery: true, response_mode: :form_post, diff --git a/lib/ACMIDM/strategy.rb b/lib/FlemishGovernment/strategy.rb similarity index 86% rename from lib/ACMIDM/strategy.rb rename to lib/FlemishGovernment/strategy.rb index fa1ec8e367..0621aade47 100644 --- a/lib/ACMIDM/strategy.rb +++ b/lib/FlemishGovernment/strategy.rb @@ -1,8 +1,8 @@ require_relative 'client.rb' require 'openid_connect' -# ACMIDM is an expension upon the OpenIDConnect Protocol -# Changes are applied to support the specific requirements of the flemish government +# Flemish government is an extension upon the OpenIDConnect Protocol +# Changes are applied to support the specific requirements of ACM IDM. # This is used by both government officials and LeerID module OmniAuth module Strategies @@ -26,7 +26,7 @@ def client # # Token endpoint: https://authenticatie-ti.vlaanderen.be/op/v1/token. # Vlaamse Overheid wants: https://authenticatie-ti.vlaanderen.be/op. - @client ||= ::ACMIDM::Client.new(client_options.merge(audience: options.issuer)) + @client ||= ::FlemishGovernment::Client.new(client_options.merge(audience: options.issuer)) end def uid @@ -41,5 +41,3 @@ def uid end end -OmniAuth.config.add_camelization 'acmidm', 'ACMIDM' - diff --git a/test/factories/providers.rb b/test/factories/providers.rb index 207609dc9b..c4f9c4ad5c 100644 --- a/test/factories/providers.rb +++ b/test/factories/providers.rb @@ -39,7 +39,7 @@ identifier { SecureRandom.uuid } end - factory :acmidm_provider, class: 'Provider::Acmidm' do + factory :flemish_government_provider, class: 'Provider::FlemishGovernment' do institution client_id { SecureRandom.uuid } diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index f3c7de1275..017455f79c 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -12,7 +12,7 @@ ### # Set the signing key. -module ACMIDM::Auth::Settings +module FlemishGovernment::Auth::Settings private def private_key_path From 195be841a7709bbaabe4f737f2909ad86b37baa6 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 25 Mar 2024 10:39:09 +0100 Subject: [PATCH 04/13] Fix rename --- lib/FlemishGovernment/strategy.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/FlemishGovernment/strategy.rb b/lib/FlemishGovernment/strategy.rb index 0621aade47..6e40b4a232 100644 --- a/lib/FlemishGovernment/strategy.rb +++ b/lib/FlemishGovernment/strategy.rb @@ -6,8 +6,8 @@ # This is used by both government officials and LeerID module OmniAuth module Strategies - class ACMIDM < OmniAuth::Strategies::OpenIDConnect - option :name, 'acmidm' + class FlemishGovernment < OmniAuth::Strategies::OpenIDConnect + option :name, 'flemish_government' info do { From f29cd1cf59bcbf9b2c9571b1e2fe455a6b397d1c Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Mon, 25 Mar 2024 11:31:05 +0100 Subject: [PATCH 05/13] Fix some tests --- app/models/user.rb | 2 +- test/factories/providers.rb | 3 +-- .../auth/vlaanderen/oidc_vlaanderen_test.rb | 17 ++++++----------- 3 files changed, 8 insertions(+), 14 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 77cb7c1cb8..4278a47e54 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -212,7 +212,7 @@ class User < ApplicationRecord } def provider_allows_blank_email - return if institution&.uses_lti? || institution&.uses_oidc? || institution&.uses_smartschool? + return if institution&.uses_lti? || institution&.uses_flemish_government? || institution&.uses_smartschool? errors.add(:email, 'should not be blank') if email.blank? end diff --git a/test/factories/providers.rb b/test/factories/providers.rb index c4f9c4ad5c..bb3404770a 100644 --- a/test/factories/providers.rb +++ b/test/factories/providers.rb @@ -42,8 +42,7 @@ factory :flemish_government_provider, class: 'Provider::FlemishGovernment' do institution - client_id { SecureRandom.uuid } - issuer { Faker::Internet.url } + identifier { institution.short_name } end factory :provider, aliases: [:saml_provider], class: 'Provider::Saml' do diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index 017455f79c..bd1a8306c2 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -31,7 +31,7 @@ class AuthOIDCVlaanderenTest < ActionDispatch::IntegrationTest TOKEN_URL = format('%s/v1/token', ISSUER).freeze def setup - @provider = create :oidc_provider, issuer: ISSUER + @provider = create :flemish_government_provider # Disable the test mode so that the whole flow is executed. OmniAuth.config.test_mode = false @@ -48,11 +48,11 @@ def teardown def omniauth_callback_url # Strip the trailing slash. - user_oidc_omniauth_callback_url(locale: nil, protocol: 'https').to_s.chomp('/') + user_flemish_government_omniauth_callback_url(locale: nil, protocol: 'https').to_s.chomp('/') end def omniauth_url(provider) - user_oidc_omniauth_authorize_url(provider: provider) + user_flemish_government_omniauth_authorize_url(provider: provider) end def stub_discovery! @@ -91,8 +91,8 @@ def stub_keys!(kid = nil) # Validate the parameters. parameters = CGI.parse(redirect_url.query).symbolize_keys - # Client id must be equal to the one set in the provider. - assert_equal @provider.client_id, parameters[:client_id].first + # Client id must be equal to the one set in the secrets. + assert_equal Rails.application.credentials.acmidm_client_id, parameters[:client_id].first # Nonce must not be empty. assert_not_empty parameters[:nonce].first @@ -108,12 +108,7 @@ def stub_keys!(kid = nil) assert_equal 'code', parameters[:response_type].first # Scope must contain openid and profile. - assert_equal 'openid profile vo', parameters[:scope].first - - # State must not be empty and must start with the id of the provider so that - # we can reconstruct this in the callback phase. - assert_not_empty parameters[:state].first - assert parameters[:state].first.to_s.start_with?(format('%s-', @provider.id)) + assert_equal 'openid profile vo ov_leerling', parameters[:scope].first end test 'should handle the callback phase' do From 6725eb37827355066b2c77b2ae9213f9b5fb8f86 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 10:33:10 +0100 Subject: [PATCH 06/13] Fix tests --- .../auth/vlaanderen/oidc_vlaanderen_test.rb | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index bd1a8306c2..d4e44cd81e 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -12,7 +12,7 @@ ### # Set the signing key. -module FlemishGovernment::Auth::Settings +class FlemishGovernment::Auth::OmniAuth::Setup private def private_key_path @@ -29,6 +29,7 @@ class AuthOIDCVlaanderenTest < ActionDispatch::IntegrationTest KEYS_URL = format('%s/v1/keys', ISSUER).freeze KEY_ID = format('OIDC_VLAANDEREN_%d', Time.now.to_i).freeze TOKEN_URL = format('%s/v1/token', ISSUER).freeze + USER_INFO_URL = format('%s/v1/userinfo', ISSUER).freeze def setup @provider = create :flemish_government_provider @@ -121,8 +122,8 @@ def stub_keys!(kid = nil) # Build an id token. id_token_body = { at_hash: Faker::Alphanumeric.alphanumeric, - aud: @provider.client_id, - azp: @provider.client_id, + aud: Rails.application.credentials.acmidm_client_id, + azp: Rails.application.credentials.acmidm_client_id, exp: Time.now.to_i + 3600, family_name: Faker::Name.last_name, given_name: Faker::Name.first_name, @@ -141,6 +142,9 @@ def stub_keys!(kid = nil) access_token_response = { access_token: Faker::Crypto.md5, expires_in: 3600, id_token: id_token, scope: 'profile', token_type: 'Bearer' } stub_request(:post, TOKEN_URL).to_return(body: access_token_response.to_json, headers: { 'Content-Type': 'application/json' }, status: 200) + # Stub the user info call. + stub_request(:get, USER_INFO_URL).to_return(body: {}.to_json, headers: { 'Content-Type': 'application/json' }, status: 200) + # Call the callback url. authorization_response = { code: Faker::Alphanumeric.alpha, state: session['omniauth.state'] } post omniauth_callback_url, params: authorization_response @@ -166,8 +170,8 @@ def stub_keys!(kid = nil) client_assertion = decode_jwt(client_assertion_encoded).symbolize_keys assert_equal ISSUER, client_assertion[:aud] - assert_equal @provider.client_id, client_assertion[:iss] - assert_equal @provider.client_id, client_assertion[:sub] + assert_equal Rails.application.credentials.acmidm_client_id, client_assertion[:iss] + assert_equal Rails.application.credentials.acmidm_client_id, client_assertion[:sub] # Code must be equal to the code received from the provider. assert_equal authorization_response[:code], parameters[:code].first From 8a774343027e4ff1aa20166fca78c98513a1e6e4 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 10:40:03 +0100 Subject: [PATCH 07/13] Noop --- test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index d4e44cd81e..dc6725eeb7 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -20,6 +20,7 @@ def private_key_path end end + class AuthOIDCVlaanderenTest < ActionDispatch::IntegrationTest include JwksHelper From b7c0be211e7f265481a7e395228ef13aef5c2065 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 10:44:44 +0100 Subject: [PATCH 08/13] Fix linting --- test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index dc6725eeb7..d4e44cd81e 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -20,7 +20,6 @@ def private_key_path end end - class AuthOIDCVlaanderenTest < ActionDispatch::IntegrationTest include JwksHelper From 07f92f1028d28269dbde0f0b50abb038a9f351e3 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 10:54:18 +0100 Subject: [PATCH 09/13] Try to fix tests again --- lib/FlemishGovernment/setup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/FlemishGovernment/setup.rb b/lib/FlemishGovernment/setup.rb index ee7cdfdfa8..fcc7406a23 100644 --- a/lib/FlemishGovernment/setup.rb +++ b/lib/FlemishGovernment/setup.rb @@ -31,7 +31,7 @@ def configure response_type: :code, scope: [:openid, :profile, :vo, :ov_leerling], client_auth_method: :jwt_bearer, - issuer: Rails.env.production? ? "https://authenticatie.vlaanderen.be/op" : "https://authenticatie-ti.vlaanderen.be/op", + issuer: "https://authenticatie-ti.vlaanderen.be/op", } end From 6731e6167384ce3887e7bf837dd89b10591e552b Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 10:58:13 +0100 Subject: [PATCH 10/13] Revert "Try to fix tests again" This reverts commit 07f92f1028d28269dbde0f0b50abb038a9f351e3. --- lib/FlemishGovernment/setup.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/FlemishGovernment/setup.rb b/lib/FlemishGovernment/setup.rb index fcc7406a23..ee7cdfdfa8 100644 --- a/lib/FlemishGovernment/setup.rb +++ b/lib/FlemishGovernment/setup.rb @@ -31,7 +31,7 @@ def configure response_type: :code, scope: [:openid, :profile, :vo, :ov_leerling], client_auth_method: :jwt_bearer, - issuer: "https://authenticatie-ti.vlaanderen.be/op", + issuer: Rails.env.production? ? "https://authenticatie.vlaanderen.be/op" : "https://authenticatie-ti.vlaanderen.be/op", } end From 1957a952dcd3cdb2eb9d8ebc92b0d47cc0135e37 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 16:47:14 +0100 Subject: [PATCH 11/13] Try potential test fix --- lib/FlemishGovernment/setup.rb | 7 ++++++- .../auth/vlaanderen/oidc_vlaanderen_test.rb | 16 +++++++++++----- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/FlemishGovernment/setup.rb b/lib/FlemishGovernment/setup.rb index ee7cdfdfa8..ad8a6372e3 100644 --- a/lib/FlemishGovernment/setup.rb +++ b/lib/FlemishGovernment/setup.rb @@ -22,7 +22,7 @@ def setup def configure { client_options: { - identifier: Rails.application.credentials.acmidm_client_id, + identifier: client_id, private_key: private_key, redirect_uri: "https://#{@env['HTTP_HOST']}/users/auth/flemish_government/callback" }, @@ -37,6 +37,11 @@ def configure private + def client_id + # This function allows to override the key path in tests. + Rails.application.credentials.acmidm_client_id + end + def private_key_path # This function allows to override the key path in tests. KEY_PATH diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index d4e44cd81e..32983d4937 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -10,6 +10,8 @@ # https://authenticatie.vlaanderen.be/docs/beveiligen-van-toepassingen/integratie-methoden/oidc/technische-info/discovery-url/ # https://authenticatie.vlaanderen.be/docs/beveiligen-van-toepassingen/integratie-methoden/oidc/technische-info/scope-claims/ ### +CLIENT_ID = "foo".freeze + # Set the signing key. class FlemishGovernment::Auth::OmniAuth::Setup @@ -18,6 +20,10 @@ class FlemishGovernment::Auth::OmniAuth::Setup def private_key_path JwksHelper.private_key_path end + + def client_id + CLIENT_ID + end end class AuthOIDCVlaanderenTest < ActionDispatch::IntegrationTest @@ -93,7 +99,7 @@ def stub_keys!(kid = nil) parameters = CGI.parse(redirect_url.query).symbolize_keys # Client id must be equal to the one set in the secrets. - assert_equal Rails.application.credentials.acmidm_client_id, parameters[:client_id].first + assert_equal CLIENT_ID, parameters[:client_id].first # Nonce must not be empty. assert_not_empty parameters[:nonce].first @@ -122,8 +128,8 @@ def stub_keys!(kid = nil) # Build an id token. id_token_body = { at_hash: Faker::Alphanumeric.alphanumeric, - aud: Rails.application.credentials.acmidm_client_id, - azp: Rails.application.credentials.acmidm_client_id, + aud: CLIENT_ID, + azp: CLIENT_ID, exp: Time.now.to_i + 3600, family_name: Faker::Name.last_name, given_name: Faker::Name.first_name, @@ -170,8 +176,8 @@ def stub_keys!(kid = nil) client_assertion = decode_jwt(client_assertion_encoded).symbolize_keys assert_equal ISSUER, client_assertion[:aud] - assert_equal Rails.application.credentials.acmidm_client_id, client_assertion[:iss] - assert_equal Rails.application.credentials.acmidm_client_id, client_assertion[:sub] + assert_equal CLIENT_ID, client_assertion[:iss] + assert_equal CLIENT_ID, client_assertion[:sub] # Code must be equal to the code received from the provider. assert_equal authorization_response[:code], parameters[:code].first From 01d15c1c692a96b991721d01b3b0aeb2f4bd19a6 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 16:48:21 +0100 Subject: [PATCH 12/13] Fix linting --- test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index 32983d4937..9ec39c3201 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -10,7 +10,7 @@ # https://authenticatie.vlaanderen.be/docs/beveiligen-van-toepassingen/integratie-methoden/oidc/technische-info/discovery-url/ # https://authenticatie.vlaanderen.be/docs/beveiligen-van-toepassingen/integratie-methoden/oidc/technische-info/scope-claims/ ### -CLIENT_ID = "foo".freeze +CLIENT_ID = 'foo'.freeze # Set the signing key. From 9ed183fb16ec8a6b7ef25349fc6eef3c477f4d62 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Tue, 26 Mar 2024 16:51:28 +0100 Subject: [PATCH 13/13] Fix linting --- test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb index 9ec39c3201..cc4f4c3d58 100644 --- a/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb +++ b/test/integration/auth/vlaanderen/oidc_vlaanderen_test.rb @@ -12,7 +12,6 @@ ### CLIENT_ID = 'foo'.freeze - # Set the signing key. class FlemishGovernment::Auth::OmniAuth::Setup private