Skip to content

Commit

Permalink
Add standard, overcommit and gh action for linting
Browse files Browse the repository at this point in the history
Resolves #37
  • Loading branch information
mark-cooper committed Jun 17, 2024
1 parent ffb03e0 commit ae55275
Show file tree
Hide file tree
Showing 9 changed files with 86 additions and 52 deletions.
22 changes: 22 additions & 0 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
name: Lint

on: [pull_request]

jobs:
lint:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup Ruby and install gems
uses: ruby/setup-ruby@v1
with:
bundler-cache: true
ruby-version: 3.2

- name: Run standardrb
run: |
gem install standard
standardrb
4 changes: 4 additions & 0 deletions .overcommit.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
PreCommit:
StandardRB:
enabled: true
command: ["standardrb"]
10 changes: 5 additions & 5 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# frozen_string_literal: true

ASpaceGems.setup if defined? ASpaceGems
source 'https://rubygems.org'
source "https://rubygems.org"

gem 'omniauth', '~> 1.7', '>= 1.7.1'
gem 'omniauth-cas', '~> 2.0'
gem 'omniauth-saml', '~> 1.8', '>= 1.8.1'
gem 'addressable', '~> 2.8'
gem "omniauth", "~> 1.7", ">= 1.7.1"
gem "omniauth-cas", "~> 2.0"
gem "omniauth-saml", "~> 1.8", ">= 1.8.1"
gem "addressable", "~> 2.8"
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,15 @@ openssl req -new -x509 -key rsaprivkey.pem -out rsacert.pem
./build/run bundler -Dgemfile=../plugins/aspace-oauth/Gemfile
```

For linting:

```bash
# install overcommit for git precommit hooks
gem install overcommit && overcommit --install && overcommit --sign pre-commit
gem install standard
standardrb --fix
```

## License

This project is available as open source under the terms of the [MIT License](http://opensource.org/licenses/MIT).
Expand Down
31 changes: 15 additions & 16 deletions backend/model/asoauth.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,16 @@ def authenticate(username, password)
return nil
end

return nil unless username == info['username'].downcase
return nil unless username == info["username"].downcase

JSONModel(:user).from_hash(
username: username,
name: info['name'],
email: info['email'],
first_name: info['first_name'],
last_name: info['last_name'],
telephone: info['phone'],
additional_contact: info['description']
name: info["name"],
email: info["email"],
first_name: info["first_name"],
last_name: info["last_name"],
telephone: info["phone"],
additional_contact: info["description"]
)
end

Expand All @@ -57,7 +57,7 @@ def get_oauth_shared_secret
end
end

unless secret.is_a? String and secret.length > 0
unless secret.is_a?(String) && (secret.length > 0)
raise ASOauthException.new(":oauth_shared_secret config option is not set")
end
secret
Expand All @@ -66,16 +66,15 @@ def get_oauth_shared_secret
def validate_login_token_and_extract_user_info(login_token)
begin
unverified_token = JSON.parse(login_token)
rescue JSON::ParserError => e
rescue JSON::ParserError
raise InvalidLoginTokenException.new("login_token is not valid JSON")
end

signature = unverified_token["signature"] if unverified_token.is_a? Hash
json_payload = unverified_token["payload"] if unverified_token.is_a? Hash

unless signature.is_a? String and json_payload.is_a? String
unless signature.is_a?(String) && json_payload.is_a?(String)
raise InvalidLoginTokenException.new("login_token content is invalid")
return nil
end

secret = get_oauth_shared_secret
Expand All @@ -88,7 +87,7 @@ def validate_login_token_and_extract_user_info(login_token)

begin
payload = JSON.parse(json_payload)
rescue JSON::ParserError => e
rescue JSON::ParserError
raise InvalidLoginTokenException.new("login_token payload is not valid JSON")
end

Expand All @@ -109,7 +108,7 @@ def validate_login_token_and_extract_user_info(login_token)
end

user_info = payload["user_info"]
unless user_info.is_a? Hash and user_info.has_key? "username"
unless user_info.is_a?(Hash) && user_info.has_key?("username")
raise InvalidLoginTokenException.new(
"login_token's payload user_info is invalid"
)
Expand All @@ -119,12 +118,12 @@ def validate_login_token_and_extract_user_info(login_token)

def matching_usernames(query)
DB.open do |db|
query = query.gsub(/[%]/, '').downcase
query = query.gsub(/[%]/, "").downcase
db[:user]
.filter(Sequel.~(is_system_user: 1))
.filter(Sequel.like(
Sequel.function(:lower, :username), "#{query}%"
))
Sequel.function(:lower, :username), "#{query}%"
))
.filter(Sequel.like(:source, name))
.select(:username)
.limit(AppConfig[:max_usernames_per_source].to_i)
Expand Down
12 changes: 6 additions & 6 deletions frontend/controllers/oauth_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,15 @@ class OauthController < ApplicationController
# user-provided password can't be used to forge oauth logins.
def create
backend_session = nil
email = AspaceOauth.get_email(auth_hash)
username = AspaceOauth.use_uid? ? auth_hash.uid : email
email = AspaceOauth.get_email(auth_hash)
username = AspaceOauth.use_uid? ? auth_hash.uid : email

puts "Received callback for user: #{username}"

if email && username
username = username.split('@').first unless AspaceOauth.username_is_email?
username = username.split("@").first unless AspaceOauth.username_is_email?
auth_hash[:info][:username] = username.downcase # checked in backend
auth_hash[:info][:email] = email # ensure email is set in info
auth_hash[:info][:email] = email # ensure email is set in info
login_token = AspaceOauth.encode_user_login_token(auth_hash)
backend_session = User.login(username, login_token)
end
Expand All @@ -34,7 +34,7 @@ def create
User.establish_session(self, backend_session, username)
load_repository_list
else
flash[:error] = 'Authentication error, unable to login.'
flash[:error] = "Authentication error, unable to login."
end

redirect_to controller: :welcome, action: :index
Expand All @@ -58,6 +58,6 @@ def saml_logout
protected

def auth_hash
request.env['omniauth.auth']
request.env["omniauth.auth"]
end
end
24 changes: 12 additions & 12 deletions frontend/lib/aspace_oauth.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

require 'date'
require "date"

module AspaceOauth
def self.build_url(host, path, params = {})
Expand All @@ -12,12 +12,12 @@ def self.build_url(host, path, params = {})
end

def self.cas_logout_url
config = get_oauth_config_for('cas')
config = get_oauth_config_for("cas")
return unless config

host = config[:config][:url]
path = config[:config][:logout_url]
params = { service: AppConfig[:frontend_proxy_url] }
host = config[:config][:url]
path = config[:config][:logout_url]
params = {service: AppConfig[:frontend_proxy_url]}
build_url(host, path, params)
end

Expand All @@ -40,7 +40,7 @@ def self.get_oauth_config_for(strategy)
end

def self.saml_logout_url
config = get_oauth_config_for('saml')
config = get_oauth_config_for("saml")
return unless config

build_url(
Expand All @@ -60,7 +60,7 @@ def self.username_is_email?
def self.get_oauth_shared_secret
secret = AppConfig[:oauth_shared_secret] if AppConfig.has_key? :oauth_shared_secret

if not (secret.is_a? String and secret.length > 0)
if !(secret.is_a?(String) && (secret.length > 0))
raise ":oauth_shared_secret config option is not set"
end

Expand All @@ -69,11 +69,11 @@ def self.get_oauth_shared_secret

def self.encode_user_login_token(auth_hash)
payload = JSON.generate({
:created_by => "aspace-oauth-#{auth_hash[:provider]}",
:created_at => DateTime.now.rfc3339,
:user_info => auth_hash[:info]
created_by: "aspace-oauth-#{auth_hash[:provider]}",
created_at: DateTime.now.rfc3339,
user_info: auth_hash[:info]
})
signature = OpenSSL::HMAC.hexdigest("SHA256", self.get_oauth_shared_secret, payload)
JSON.generate({:signature => signature, :payload => payload})
signature = OpenSSL::HMAC.hexdigest("SHA256", get_oauth_shared_secret, payload)
JSON.generate({signature: signature, payload: payload})
end
end
16 changes: 8 additions & 8 deletions frontend/plugin_init.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
# frozen_string_literal: true

require_relative 'lib/aspace_oauth'
require_relative "lib/aspace_oauth"
oauth_definitions = AppConfig[:authentication_sources].find_all do |as|
as[:model] == 'ASOauth'
as[:model] == "ASOauth"
end
unless oauth_definitions.any?
raise 'OmniAuth plugin enabled but no definitions provided =('
raise "OmniAuth plugin enabled but no definitions provided =("
end

# oauth_shared_secret is used to authenticate internal login requests from the
# frontend to the backend. It needs to be explicitly specified if the backend is
# not running in the same JVM as the frontend. When they're in the same JVM the
# secret generated here is propagated between them automatically via the system
# property set here.
if not AppConfig.has_key? :oauth_shared_secret
require 'securerandom'
if !AppConfig.has_key? :oauth_shared_secret
require "securerandom"
AppConfig[:oauth_shared_secret] = SecureRandom.uuid
java.lang.System.set_property(
"aspace.config.oauth_shared_secret", AppConfig[:oauth_shared_secret]
Expand All @@ -24,17 +24,17 @@
# also used for ui [refactor]
AppConfig[:oauth_definitions] = oauth_definitions
ArchivesSpace::Application.extend_aspace_routes(
File.join(File.dirname(__FILE__), 'routes.rb')
File.join(File.dirname(__FILE__), "routes.rb")
)
require 'omniauth'
require "omniauth"

Rails.application.config.middleware.use OmniAuth::Builder do
oauth_definitions.each do |oauth_definition|
verify_ssl = oauth_definition.fetch(:verify_ssl, true)
config = oauth_definition[:config]
if oauth_definition.key? :metadata_parser_url
idp_metadata_parser = OneLogin::RubySaml::IdpMetadataParser.new
idp_metadata = idp_metadata_parser.parse_remote_to_hash(
idp_metadata = idp_metadata_parser.parse_remote_to_hash(
oauth_definition[:metadata_parser_url], verify_ssl
)
config = idp_metadata.merge(config)
Expand Down
10 changes: 5 additions & 5 deletions frontend/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,10 @@
# OMNIAUTH-SAML: /auth/saml/slo
# OMNIAUTH-SAML: /auth/saml/spslo

get '/auth/:provider/callback', to: 'oauth#create'
post '/auth/:provider/callback', to: 'oauth#create'
get '/auth/failure', to: 'oauth#failure'
get '/auth/cas_logout', to: 'oauth#cas_logout'
get '/auth/saml_logout', to: 'oauth#saml_logout'
get "/auth/:provider/callback", to: "oauth#create"
post "/auth/:provider/callback", to: "oauth#create"
get "/auth/failure", to: "oauth#failure"
get "/auth/cas_logout", to: "oauth#cas_logout"
get "/auth/saml_logout", to: "oauth#saml_logout"
end
end

0 comments on commit ae55275

Please sign in to comment.