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.

Sign up for GitHub

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

Add OpenID Connect auth support #2194

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
+164 −1
Open

Add OpenID Connect auth support #2194

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9311ae5
feat: add oidc support
thezzisu Sep 12, 2024
e499aaa
chore: update code
thezzisu Sep 12, 2024
2bcd230
fix: lint
thezzisu Sep 12, 2024
18b90f8
chore: update docs
thezzisu Oct 27, 2024
e4a2f10
chore: update docs/installation/oidc_integration.md
thezzisu Oct 29, 2024
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
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ gem 'omniauth-facebook', '>=2.0.0'
gem 'omniauth-google-oauth2', '>=0.2.5'
gem 'omniauth-rails_csrf_protection', '~> 1.0'
gem 'omniauth-shibboleth-redux', '~> 2.0', require: 'omniauth-shibboleth'
gem 'omniauth_openid_connect', '~> 0.8.0'

# OAuth2 authentication
gem 'oauth2'
Expand Down
50 changes: 50 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ GEM
zeitwerk (~> 2.3)
addressable (2.8.6)
public_suffix (>= 2.0.2, < 6.0)
aes_key_wrap (1.1.0)
afm (0.2.2)
ast (2.4.2)
attr_required (1.0.2)
autoprefixer-rails (10.4.16.0)
execjs (~> 2)
base64 (0.2.0)
Expand All @@ -80,6 +82,7 @@ GEM
erubi (~> 1.4)
parser (>= 2.4)
smart_properties
bindata (2.5.0)
binding_of_caller (1.0.0)
debug_inspector (>= 0.0.1)
bootstrap-sass (3.4.1)
Expand Down Expand Up @@ -139,6 +142,8 @@ GEM
dotenv-rails (2.8.1)
dotenv (= 2.8.1)
railties (>= 3.2)
email_validator (2.2.4)
activemodel
erb_lint (0.5.0)
activesupport
better_html (>= 2.0.1)
Expand All @@ -161,6 +166,8 @@ GEM
base64
faraday-net_http (>= 2.0, < 3.1)
ruby2_keywords (>= 0.0.4)
faraday-follow_redirects (0.3.0)
faraday (>= 1, < 3)
faraday-net_http (3.0.2)
ffi (1.16.3)
fomantic-ui-sass (2.8.8.1)
Expand Down Expand Up @@ -192,6 +199,13 @@ GEM
js_cookie_rails (2.2.0)
railties (>= 3.1)
json (2.7.1)
json-jwt (1.16.6)
activesupport (>= 4.2)
aes_key_wrap
base64
bindata
faraday (~> 2.0)
faraday-follow_redirects
jstz-rails3-plus (1.0.5)
railties (>= 3.1)
jwt (2.7.1)
Expand Down Expand Up @@ -279,6 +293,22 @@ GEM
omniauth (~> 2.0)
omniauth-shibboleth-redux (2.0.0)
omniauth (>= 2.0.0)
omniauth_openid_connect (0.8.0)
omniauth (>= 1.9, < 3)
openid_connect (~> 2.2)
openid_connect (2.3.0)
activemodel
attr_required (>= 1.0.0)
email_validator
faraday (~> 2.0)
faraday-follow_redirects
json-jwt (>= 1.16)
mail
rack-oauth2 (~> 2.2)
swd (~> 2.0)
tzinfo
validate_url
webfinger (~> 2.0)
orm_adapter (0.5.0)
overcommit (0.61.0)
childprocess (>= 0.6.3, < 5)
Expand Down Expand Up @@ -307,6 +337,13 @@ GEM
rack (2.2.9)
rack-attack (6.7.0)
rack (>= 1.0, < 4)
rack-oauth2 (2.2.1)
activesupport
attr_required
faraday (~> 2.0)
faraday-follow_redirects
json-jwt (>= 1.11.0)
rack (>= 2.1.0)
rack-protection (3.2.0)
base64 (>= 0.1.0)
rack (~> 2.2, >= 2.2.4)
Expand Down Expand Up @@ -432,6 +469,11 @@ GEM
mini_portile2 (~> 2.8.0)
stringio (3.1.0)
strscan (3.1.0)
swd (2.0.3)
activesupport (>= 3)
attr_required (>= 0.0.5)
faraday (~> 2.0)
faraday-follow_redirects
terser (1.1.20)
execjs (>= 0.3.0, < 3)
thin (1.8.2)
Expand All @@ -448,9 +490,16 @@ GEM
tzinfo (>= 1.0.0)
unicode-display_width (2.5.0)
uri (0.10.3)
validate_url (1.0.15)
activemodel (>= 3.0.0)
public_suffix
version_gem (1.1.3)
warden (1.2.9)
rack (>= 2.0.9)
webfinger (2.1.3)
activesupport
faraday (~> 2.0)
faraday-follow_redirects
webmock (3.19.1)
addressable (>= 2.8.0)
crack (>= 0.3.2)
Expand Down Expand Up @@ -518,6 +567,7 @@ DEPENDENCIES
omniauth-google-oauth2 (>= 0.2.5)
omniauth-rails_csrf_protection (~> 1.0)
omniauth-shibboleth-redux (~> 2.0)
omniauth_openid_connect (~> 0.8.0)
overcommit
populator (>= 1.0.0)
prawn (= 0.13.0)
Expand Down
22 changes: 22 additions & 0 deletions app/controllers/users/omniauth_callbacks_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,4 +99,26 @@ def shibboleth
set_flash_message(:notice, :success, kind: "Shibboleth") if is_navigational_format?
end
end

def openid_connect
if user_signed_in?
if (data = request.env["omniauth.auth"]) && current_user.authentications.where(provider: data["provider"],
uid: data["uid"]).empty?
current_user.authentications.create(provider: data["provider"],
uid: data["uid"])
end
redirect_to root_path
else
@user = User.find_for_openid_connect_oauth(request.env["omniauth.auth"], current_user)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

OpenID Connect implementation found but needs enhancement

The find_for_openid_connect_oauth method is implemented in the User model but appears to have minimal functionality:

  • It only looks up the user through Authentication model and returns the associated user
  • Missing error handling and user creation logic for new OpenID Connect users
  • The method should handle cases where the user doesn't exist yet

Suggested improvements:

  • Add user creation logic when authentication is not found
  • Implement proper error handling
  • Consider adding validation for required OpenID Connect claims
🔗 Analysis chain

Verify User model implementation for OpenID Connect.

Ensure that the User model implements the required methods for OIDC authentication.

Let me try a different approach to search for these methods using rg which might be more effective for Ruby files.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify User model implementation for OpenID Connect

# Check for the find_for_openid_connect_oauth method
echo "Checking for find_for_openid_connect_oauth method..."
ast-grep --pattern 'def self.find_for_openid_connect_oauth'

# Check for add_oauth_if_user_exists method
echo "Checking for add_oauth_if_user_exists method..."
ast-grep --pattern 'def self.add_oauth_if_user_exists'

# Check for new_with_session method
echo "Checking for new_with_session method..."
ast-grep --pattern 'def self.new_with_session'

Length of output: 476

Script:

#!/bin/bash
# Search for OIDC-related methods in User model and related files
echo "Searching for OpenID Connect related methods in User model..."
rg -A 5 "find_for_openid_connect_oauth" app/models/

echo -e "\nSearching for OAuth-related methods in User model..."
rg -A 5 "class User" app/models/user.rb

echo -e "\nChecking for any OpenID Connect related files..."
fd -t f openid

Length of output: 1163

unless @user
# automatic cleanup of devise.* after sign in
session["devise.openid_connect_data"] = request.env["omniauth.auth"].except("extra")
@user = User.add_oauth_if_user_exists session
@user ||= User.new_with_session(nil, session)
end
Comment on lines +112 to +118
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Consider adding error handling for authentication failures.

While the code handles the case when a user is not found, it doesn't handle potential failures during user creation or authentication.

     else
       @user = User.find_for_openid_connect_oauth(request.env["omniauth.auth"], current_user)
-      unless @user
+      if @user.nil?
         # automatic cleanup of devise.* after sign in
         session["devise.openid_connect_data"] = request.env["omniauth.auth"].except("extra")
         @user = User.add_oauth_if_user_exists session
         @user ||= User.new_with_session(nil, session)
+        
+        if @user&.new_record?
+          flash[:alert] = "Failed to create user from OpenID Connect data"
+          redirect_to new_user_registration_url and return
+        end
       end

Committable suggestion was skipped due to low confidence.


sign_in_and_redirect @user, event: :authentication
set_flash_message(:notice, :success, kind: "OpenID_Connect") if is_navigational_format?
end
end
thezzisu marked this conversation as resolved.
Show resolved Hide resolved
end
23 changes: 22 additions & 1 deletion app/models/user.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class User < ApplicationRecord
:recoverable, :rememberable, :trackable, :validatable,
:confirmable

devise :omniauthable, omniauth_providers: [:shibboleth, :google_oauth2]
devise :omniauthable, omniauth_providers: [:shibboleth, :google_oauth2, :openid_connect]

has_many :course_user_data, dependent: :destroy
has_many :courses, through: :course_user_data
Expand Down Expand Up @@ -99,6 +99,12 @@ def self.find_for_shibboleth_oauth(auth, _signed_in_resource = nil)
authentication.user if authentication&.user
end

def self.find_for_openid_connect_oauth(auth, _signed_in_resource = nil)
authentication = Authentication.find_by(provider: auth.provider,
uid: auth.uid)
authentication.user if authentication&.user
end

def self.assign_random_password(user)
return if user.nil?

Expand Down Expand Up @@ -127,6 +133,17 @@ def self.new_with_session(params, session)
user.email = data["uid"] # email is uid in our case
User.assign_random_password user
User.add_oauth_if_user_exists session
elsif (data = session["devise.openid_connect_data"])
if data["info"]["first_name"].present? && data["info"]["last_name"].present?
user.first_name = data["info"]["first_name"]
user.last_name = data["info"]["last_name"]
else
user.first_name = data["info"]["name"]
user.last_name = ""
end
user.email = data["info"]["email"]
User.assign_random_password user
User.add_oauth_if_user_exists session
thezzisu marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
Expand All @@ -145,6 +162,10 @@ def self.add_oauth_if_user_exists(session)
email = data["uid"] # email is uid in our case
provider = "CMU-Shibboleth"
uid = data["uid"]
elsif (data = session["devise.openid_connect_data"])
email = data["info"]["email"]
provider = data["provider"]
uid = data["uid"]
thezzisu marked this conversation as resolved.
Show resolved Hide resolved
end

user = User.find_by(email:)
Expand Down
9 changes: 9 additions & 0 deletions app/views/admins/_oauth_integration.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,12 @@
</div>
</li>
</ul>
<ul class="collapsible">
<li class="active">
<% openid_config = OauthConfigController.get_oauth_credentials :openid_connect %>
<div class="collapsible-header">OpenID
<% if @configured_oauth_providers.include? :openid_connect %><i class="material-icons">check</i><% end %>
</div>
Edit configuration on disk
</li>
</ul>
thezzisu marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 4 additions & 0 deletions config/initializers/devise.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,10 @@
config.omniauth :google_oauth2, "", "", setup: lambda{ |env|
env['omniauth.strategy'].options.merge!(OauthConfigController.get_oauth_credentials :google_oauth2)
}
# Set up OpenID Connect interceptor
config.omniauth :openid_connect, setup: lambda{ |env|
env['omniauth.strategy'].options.merge!(OauthConfigController.get_oauth_credentials :openid_connect)
}
Comment on lines +230 to +233
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Based on the implementation details found, I can now provide a final response:

OIDC configuration has security concerns that need addressing

The current implementation has several security gaps:

  • Credentials are stored in plaintext YAML files without encryption
  • No validation of OIDC configuration values before use
  • File permissions are not explicitly set for the config file
  • Cache is used but without encryption for sensitive data

Required changes:

  • Implement encryption for client_secret storage using Rails credentials or a secure key management solution
  • Add validation for OIDC configuration values in get_oauth_credentials
  • Set explicit restrictive file permissions (0600) when writing oauth_config.yml
  • Add input sanitization for provider names before using them as hash keys
  • Consider moving sensitive data to environment variables or Rails credentials instead of YAML files
🔗 Analysis chain

Verify OIDC configuration management security.

The credentials are loaded at runtime which is good for flexibility, but ensure:

  1. The configuration file has appropriate access restrictions
  2. Sensitive OIDC credentials are properly encrypted/secured
  3. The configuration is validated before use

Let me gather more information about the configuration validation and security measures.

Let me try a different approach to find the implementation details.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for potential security issues in configuration management

# Look for OIDC configuration files
echo "Searching for OIDC configuration files..."
fd -t f -e yml -e yaml | rg -i "oauth|oidc"

# Check for potential credential exposure in version control
echo "Checking for potential credential exposure..."
rg -i "client_secret|client_id|private_key"

Length of output: 10075

Script:

#!/bin/bash
# Check for OIDC configuration validation and security measures

# Look for OauthConfigController implementation
echo "Checking OauthConfigController implementation..."
ast-grep --pattern 'def get_oauth_credentials'

# Check for encryption/security related code
echo "Checking for encryption/security measures..."
rg -i "encrypt|secure|validate" app/controllers/oauth_config_controller.rb

# Check for configuration storage location and access controls
echo "Checking configuration storage..."
rg -i "config.*store|save.*config" app/controllers/oauth_config_controller.rb

Length of output: 488

Script:

#!/bin/bash
# Search for the entire OauthConfigController class and related files

echo "Searching for OauthConfigController implementation..."
rg -l "class OauthConfigController"

echo "Looking for the file content..."
fd -t f oauth_config_controller.rb -x cat {}

echo "Checking for configuration files handling..."
rg -g "*.rb" -i "yaml|yml.*load|credentials.*load" app/controllers/

Length of output: 6506



# ==> Warden configuration
Expand Down
Binary file added docs/images/openid_setup.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
56 changes: 56 additions & 0 deletions docs/installation/oidc_integration.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# OpenID Connect Integration Setup

OpenID Connect (OIDC) is a modern authentication protocol built on top of OAuth 2.0. It allows applications to verify user identities through an external identity provider (IdP). By implementing OIDC, you can enable students to sign into Autolab using a third-party identity provider (e.g., your institution's CAS).

This guide will walk you through setting up Autolab with your chosen IdP.

## Registration Process

First, register your Autolab instance with your IdP. While the registration process varies across different OIDC IdP implementations, there are several key parameters to configure:

- `redirect_url`: Must be set to `{scheme}://{your_autolab_instance}/auth/users/auth/openid_connect/callback`
- For production environments, always use `https` as the scheme. Only use `http` for local development.
- Carefully specify `your_autolab_instance` to exactly match your deployment configuration and access URL.
- `client_id`:
- Some IdPs allow you to specify a custom `client_id`, while others generate a unique identifier automatically. Either way, you'll need this ID for the next step.
- `client_secret`:
- Upon registration completion, the IdP typically generates a unique and secure `client_secret`.
- Store this secret securely (e.g., using environment variables or secure credential storage).
- Never commit the secret to version control or share it in public forums.

## Configuring Autolab for OpenID Connect

To enable OIDC in Autolab, create or modify the `Autolab/config/oauth_config.yml` file with the following configuration:

```yml
---
openid_connect:
# Example configuration for Auth0 with sample credentials
issuer: https://dev-s5lhkwr76zowpqbs.us.auth0.com/
discovery: true
uid_field: 'sub'
client_auth_method: other
scope: ['openid', 'email', 'profile']
send_nonce: false
client_options:
identifier: 4RfbkCTRdfxQYs7vVABzHbWDwkpq58u6
secret: O6vwmDkp31jE63r_VLR8SKvcIBFdrUqvnm1wv958DRJTFEiCOQsLU7haPobqmVwi
redirect_uri: http://localhost:3303/auth/users/auth/openid_connect/callback
```
thezzisu marked this conversation as resolved.
Show resolved Hide resolved

The configuration options under `openid_connect` correspond to the [`omniauth_openid_connect` package settings](https://github.com/omniauth/omniauth_openid_connect/tree/master?tab=readme-ov-file#options-overview).

Map the `client_options.identifier` and `client_options.secret` to the `client_id` and `client_secret` obtained during registration. For other fields, consult both the `omniauth_openid_connect` documentation and your IdP's documentation to determine the required fields and appropriate values.

### Required Claims

Ensure your IdP's ID token includes the following claims (configure via `scope` or your IdP's management console):

- `email`
- `name` OR `first_name` and `last_name`
- If both `first_name` and `last_name` are provided, they will be used for the user's name
- If only `name` is provided, it will be used as `first_name`, and `last_name` will be empty

After updating `oauth_config.yml` and restarting Autolab, you can verify the configuration in Autolab's **OAuth Integration** settings page:

![OIDC Setup Screenshot](/images/openid_setup.png)
thezzisu marked this conversation as resolved.
Show resolved Hide resolved