Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Implement Osso #1

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ gem "oj", "~> 3.10" # JSON parser and object serializer
gem "omniauth", "~> 1.9" # A generalized Rack framework for multiple-provider authentication
gem "omniauth-facebook", "~> 8.0" # OmniAuth strategy for Facebook
gem "omniauth-github", "~> 1.3" # OmniAuth strategy for GitHub
gem "omniauth-osso", "0.1.8.pre" # OmniAuth strategy for Osso
gem "omniauth-twitter", "~> 1.4" # OmniAuth strategy for Twitter
gem "parallel", "~> 1.20" # Run any kind of code in parallel processes
gem "patron", "~> 0.13.3" # HTTP client library based on libcurl, used with Elasticsearch to support http keep-alive connections
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,8 @@ GEM
omniauth-oauth2 (1.7.0)
oauth2 (~> 1.4)
omniauth (~> 1.9)
omniauth-osso (0.1.8.pre)
omniauth-oauth2 (>= 1.6, < 1.8)
omniauth-twitter (1.4.0)
omniauth-oauth (~> 1.1)
rack
Expand Down Expand Up @@ -894,6 +896,7 @@ DEPENDENCIES
omniauth (~> 1.9)
omniauth-facebook (~> 8.0)
omniauth-github (~> 1.3)
omniauth-osso (= 0.1.8.pre)
omniauth-twitter (~> 1.4)
parallel (~> 1.20)
patron (~> 0.13.3)
Expand Down
2 changes: 2 additions & 0 deletions app/controllers/admin/configs_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ class ConfigsController < Admin::ApplicationController
github_secret
facebook_key
facebook_secret
osso_key
osso_secret
auth_providers_to_enable
invite_only_mode
allow_email_password_registration
Expand Down
10 changes: 10 additions & 0 deletions app/lib/constants/site_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,16 @@ module SiteConfig
description: "Used as the onboarding task-card image",
placeholder: IMAGE_PLACEHOLDER
},
osso_key: {
description:
"The \"Client ID\" for the OAuth client from your Osso instance",
placeholder: ""
},
osso_secret: {
description:
"The \"Client secret\" for the OAuth client from your Osso instance",
placeholder: ""
},
payment_pointer: {
description: "Used for site-wide web monetization. " \
"See: https://github.com/thepracticaldev/dev.to/pull/6345",
Expand Down
2 changes: 2 additions & 0 deletions app/models/site_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ class SiteConfig < RailsSettings::Base
field :twitter_secret, type: :string, default: ApplicationConfig["TWITTER_SECRET"]
field :github_key, type: :string, default: ApplicationConfig["GITHUB_KEY"]
field :github_secret, type: :string, default: ApplicationConfig["GITHUB_SECRET"]
field :osso_key, type: :string, default: ApplicationConfig["OSSO_KEY"]
field :osso_secret, type: :string, default: ApplicationConfig["OSSO_SECRET"]
field :facebook_key, type: :string
field :facebook_secret, type: :string

Expand Down
4 changes: 2 additions & 2 deletions app/services/authentication/authenticator.rb
Original file line number Diff line number Diff line change
Expand Up @@ -155,9 +155,9 @@ def account_less_than_a_week_old?(user, logged_in_identity)
def extract_created_at_from_payload(logged_in_identity)
raw_info = logged_in_identity.auth_data_dump.extra.raw_info

if raw_info.created_at.present?
if raw_info&.created_at.present?
Time.zone.parse(raw_info.created_at)
elsif raw_info.auth_time.present?
elsif raw_info&.auth_time.present?
Comment on lines +158 to +160
Copy link
Author

Choose a reason for hiding this comment

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

this was throwing a no method error with raw_info being nil

Time.zone.at(raw_info.auth_time)
else
Time.current
Expand Down
79 changes: 79 additions & 0 deletions app/services/authentication/providers/osso.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
module Authentication
module Providers
# Osso authentication provider, uses omniauth-osso as backend.
# Osso is an open source service for adding SAML based SSO to
# your application. The Osso team added Osso as a provider
# in order to serve as a real-world example of how simple it
# is to integrate Osso.
#
# Learn more about Osso at https://ossoapp.com

class Osso < Provider
OFFICIAL_NAME = "SAML SSO".freeze
Copy link
Author

Choose a reason for hiding this comment

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

I'd probably want to use ENV vars here if I were actually adding this to forem. if its an internal community, it might be "Sign in with EnterpriseCo Okta". but in multi-tenancy, we expect folks to be more generic and not use "Osso".

SETTINGS_URL = "https://ossoapp.com".freeze # TODO
Copy link
Author

Choose a reason for hiding this comment

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

this can read off ENV to send to admin ui


def new_user_data
name = info.name || info.email.to_s

{
email: info.email.to_s,
name: name,
osso_username: user_nickname
}
end

def existing_user_data
{
email: info.email.to_s
}
end

def user_email
info.email.to_s
end

# We're overriding this method because Osso doesn't have a concept nickname or username.
# Instead: we'll construct one based on the user's name with some randomization thrown in based
# on uid, which is guaranteed to be present and unique on Facebook.
def user_nickname
[
info.name.sub(" ", "_"),
Digest::SHA512.hexdigest(payload.uid),
].join("_")[0...25]
end

def self.official_name
OFFICIAL_NAME
end

def self.settings_url
SETTINGS_URL
end

# We're overriding these methods to add a little security with a random
# string for state
def self.authentication_path(state: SecureRandom.hex(32), **kwargs)
::Authentication::Paths.authentication_path(
provider_name,
state: state,
**kwargs,
)
end

# TODO: this suggests we can pass email in, rather than use hosted login
def self.sign_in_path(state: SecureRandom.hex(32), **_kwargs)
::Authentication::Paths.authentication_path(
provider_name,
email: "[email protected]",
state: state,
)
end

protected

def cleanup_payload(auth_payload)
auth_payload
end
end
end
end
7 changes: 7 additions & 0 deletions config/initializers/devise.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
env["omniauth.strategy"].options[:token_params][:parse] = :json
end

OSSO_OMNIAUTH_SETUP = lambda do |env|
env["omniauth.strategy"].options[:client_id] = SiteConfig.osso_key
env["omniauth.strategy"].options[:client_secret] = SiteConfig.osso_secret
env["omniauth.strategy"].options[:client_options].site = ENV["OSSO_BASE_URL"]
end

Devise.setup do |config|
# The secret key used by Devise. Devise uses this key to generate
# random tokens. Changing this key will render invalid all existing
Expand Down Expand Up @@ -303,6 +309,7 @@
# Fun fact, unless Twitter is last, it doesn't work for some reason.
config.omniauth :facebook, setup: FACEBOOK_OMNIAUTH_SETUP
config.omniauth :github, setup: GITHUB_OMNIUATH_SETUP
config.omniauth :osso, setup: OSSO_OMNIAUTH_SETUP
config.omniauth :twitter, setup: TWITTER_OMNIAUTH_SETUP

# ==> Warden configuration
Expand Down
6 changes: 6 additions & 0 deletions db/migrate/20201216025223_add_osso_login_fields.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
class AddOssoLoginFields < ActiveRecord::Migration[6.0]
def change
add_column :users, :osso_username, :string
add_column :users, :osso_created_at, :datetime
end
end
4 changes: 3 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2020_12_03_063435) do
ActiveRecord::Schema.define(version: 2020_12_16_025223) do

# These are extensions that must be enabled in order to support this database
enable_extension "citext"
Expand Down Expand Up @@ -1305,6 +1305,8 @@
t.string "old_username"
t.boolean "onboarding_package_requested", default: false
t.datetime "organization_info_updated_at"
t.datetime "osso_created_at"
t.string "osso_username"
t.string "payment_pointer"
t.boolean "permit_adjacent_sponsors", default: true
t.string "profile_image"
Expand Down
46 changes: 46 additions & 0 deletions spec/services/authentication/providers/osso_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require "rails_helper"

RSpec.describe Authentication::Providers::Osso, type: :service do
describe ".authentication_path" do
it "returns the correct authentication path" do
expected_path = Rails.application.routes.url_helpers.user_osso_omniauth_authorize_path
expect(described_class.authentication_path).to start_with(expected_path)
end

it "supports state parameter" do
path = described_class.authentication_path(state: "state")
expect(path).to include("state=state")
end

it "provides default state parameter" do
allow(SecureRandom).to receive(:hex).and_return("secure-state")
path = described_class.authentication_path
expect(path).to include("state=secure-state")
end

it "overrides the callback_url parameter" do
path = described_class.sign_in_path(callback_url: "https://example.com/callback")
expect(path).not_to include("callback_url")
end
end

describe ".sign_in_path" do
let(:expected_path) do
"/users/auth/osso"
end

it "returns the correct sign in path" do
expect(described_class.sign_in_path).to start_with(expected_path)
end

it "supports state parameter" do
path = described_class.sign_in_path(state: "state")
expect(path).to include("state=state")
end

it "overrides the callback_url parameter" do
path = described_class.sign_in_path(callback_url: "https://example.com/callback")
expect(path).not_to include("callback_url")
end
end
end