Skip to content

Commit

Permalink
Pass HTTP status to Oktakit::Error.from_response() [description below]
Browse files Browse the repository at this point in the history
This fixes [Shopify#59](Shopify#59) by
modifying `Oktakit::Error.from_response()` method to accept and
process client response and HTTP status.

The reason behind these changes:
```ruby
response, http_status = client.get_user('[email protected]')

```
This usage method is listed in the repo's README and works in most
cases. However, it separates response from HTTP status. `response` is a
`Sawyer::Resource` object and doesn't contain any data about the request
(HTTP status, URL, method or headers). It only has a Hash with Okta
error code, error summary, internal error link, error id and error
causes, which causes a `TypeError` if we try to
`raise Oktakit::Error.from_response(response) unless http_status == 200`
in case the user is not found.

With these changes implemented, we will gain the ability to process
different response formats without causing errors on the client side.
  • Loading branch information
arharovets committed Jun 13, 2022
1 parent 1c58db5 commit 6bad449
Show file tree
Hide file tree
Showing 31 changed files with 696 additions and 602 deletions.
17 changes: 9 additions & 8 deletions Gemfile
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
# frozen_string_literal: true
source 'https://rubygems.org'

gem 'byebug'
gem 'rake'
gem 'yard'
source "https://rubygems.org"

gem "byebug"
gem "rake"
gem "yard"

group :test do
gem 'rubocop'
gem 'rubocop-shopify', require: false
gem 'rspec', '~> 3.10'
gem 'vcr', '~> 6.0'
gem "rubocop"
gem "rubocop-shopify", require: false
gem "rspec", "~> 3.10"
gem "vcr", "~> 6.0"
end

gemspec
14 changes: 7 additions & 7 deletions Rakefile
Original file line number Diff line number Diff line change
@@ -1,19 +1,19 @@
require 'bundler/gem_tasks'
require "bundler/gem_tasks"

require 'rspec/core/rake_task'
require "rspec/core/rake_task"
RSpec::Core::RakeTask.new(:spec)

require 'rubocop/rake_task'
require "rubocop/rake_task"
RuboCop::RakeTask.new

task test: :spec
task default: %i[spec rubocop]
task default: [:spec, :rubocop]

namespace :doc do
require 'yard'
require "yard"
YARD::Rake::YardocTask.new do |task|
task.files = %w[LICENSE.md lib/**/*.rb]
task.options = %w[--output-dir doc/yard --markup markdown]
task.files = ["LICENSE.md", "lib/**/*.rb"]
task.options = ["--output-dir", "doc/yard", "--markup", "markdown"]
end
task default: :yard
end
6 changes: 3 additions & 3 deletions bin/console
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/usr/bin/env ruby

require 'bundler/setup'
require 'oktakit'
require "bundler/setup"
require "oktakit"

# You can add fixtures and/or initialization code here to make experimenting
# with your gem easier. You can also use a different console, if you like.
Expand All @@ -10,5 +10,5 @@ require 'oktakit'
# require "pry"
# Pry.start

require 'irb'
require "irb"
IRB.start
4 changes: 2 additions & 2 deletions lib/oktakit.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require 'oktakit/version'
require 'oktakit/client'
require "oktakit/version"
require "oktakit/client"

module Oktakit
def self.new(**args)
Expand Down
32 changes: 16 additions & 16 deletions lib/oktakit/client.rb
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
require 'sawyer'
require 'oktakit/response/raise_error'
require 'oktakit/client/admin_roles'
require 'oktakit/client/apps'
require 'oktakit/client/events'
require 'oktakit/client/factors'
require 'oktakit/client/groups'
require 'oktakit/client/group_rules'
require 'oktakit/client/identity_providers'
require 'oktakit/client/schemas'
require 'oktakit/client/templates'
require 'oktakit/client/users'
require "sawyer"
require "oktakit/response/raise_error"
require "oktakit/client/admin_roles"
require "oktakit/client/apps"
require "oktakit/client/events"
require "oktakit/client/factors"
require "oktakit/client/groups"
require "oktakit/client/group_rules"
require "oktakit/client/identity_providers"
require "oktakit/client/schemas"
require "oktakit/client/templates"
require "oktakit/client/users"

module Oktakit
class Client
Expand Down Expand Up @@ -186,10 +186,10 @@ def request(method, path, data:, query:, headers:, accept:, content_type:, pagin

def sawyer_agent
@sawyer_agent ||= Sawyer::Agent.new(api_endpoint, sawyer_options) do |http|
http.headers[:accept] = 'application/json'
http.headers[:content_type] = 'application/json'
http.headers[:accept] = "application/json"
http.headers[:content_type] = "application/json"
http.headers[:user_agent] = "Oktakit v#{Oktakit::VERSION}"
http.authorization('SSWS ', @token) if @token
http.authorization("SSWS ", @token) if @token
http.authorization(:Bearer, @access_token) if @access_token
end
end
Expand All @@ -204,7 +204,7 @@ def sawyer_options
def absolute_to_relative_url(next_ref)
return unless next_ref

next_ref.href.sub(api_endpoint, '')
next_ref.href.sub(api_endpoint, "")
end
end
end
4 changes: 2 additions & 2 deletions lib/oktakit/client/apps.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Apps
# @example
# Oktakit.add_application
def add_application(options = {})
post('/apps', options)
post("/apps", options)
end

# Get Application
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_application(id, options = {})
# @example
# Oktakit.list_applications
def list_applications(options = {})
get('/apps', options)
get("/apps", options)
end

# Update Application
Expand Down
2 changes: 1 addition & 1 deletion lib/oktakit/client/events.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Events
# @example
# Oktakit.list_events
def list_events(options = {})
get('/events', options)
get("/events", options)
end
end
end
Expand Down
4 changes: 2 additions & 2 deletions lib/oktakit/client/group_rules.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module GroupRules
# @example
# Oktakit.add_group_rule
def add_group_rule(options = {})
post('/groups/rules', options)
post("/groups/rules", options)
end

# Get Group Rule
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_group_rule(id, options = {})
# @example
# Oktakit.list_group_rules
def list_group_rules(options = {})
get('/groups/rules', options)
get("/groups/rules", options)
end

# Update Group Rule
Expand Down
4 changes: 2 additions & 2 deletions lib/oktakit/client/groups.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Groups
# @example
# Oktakit.add_group
def add_group(options = {})
post('/groups', options)
post("/groups", options)
end

# Get Group
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_group(id, options = {})
# @example
# Oktakit.list_groups
def list_groups(options = {})
get('/groups', options)
get("/groups", options)
end

# Update Group
Expand Down
8 changes: 4 additions & 4 deletions lib/oktakit/client/identity_providers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module IdentityProviders
# @example
# Oktakit.add_identity_provider
def add_identity_provider(options = {})
post('/idps', options)
post("/idps", options)
end

# Get Identity Provider
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_identity_provider(id, options = {})
# @example
# Oktakit.list_identity_providers
def list_identity_providers(options = {})
get('/idps', options)
get("/idps", options)
end

# Update Identity Provider
Expand Down Expand Up @@ -220,7 +220,7 @@ def link_idp_user(transaction_id, user_id, options = {})
# @example
# Oktakit.add_x509_certificate_public_key
def add_x509_certificate_public_key(options = {})
post('/idps/credentials/keys', options)
post("/idps/credentials/keys", options)
end

# Get Key
Expand Down Expand Up @@ -251,7 +251,7 @@ def get_key(key_id, options = {})
# @example
# Oktakit.list_keys
def list_keys(options = {})
get('/idps/credentials/keys', options)
get("/idps/credentials/keys", options)
end

# Delete Key
Expand Down
8 changes: 4 additions & 4 deletions lib/oktakit/client/schemas.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Schemas
# @example
# Oktakit.get_user_schema
def get_user_schema(options = {})
get('/meta/schemas/user/default', options)
get("/meta/schemas/user/default", options)
end

# Add Property to User Profile Schema
Expand All @@ -28,7 +28,7 @@ def get_user_schema(options = {})
# @example
# Oktakit.add_property_to_user_profile_schema
def add_property_to_user_profile_schema(options = {})
post('/meta/schemas/user/default', options)
post("/meta/schemas/user/default", options)
end

# Update User Profile Schema Property
Expand All @@ -43,7 +43,7 @@ def add_property_to_user_profile_schema(options = {})
# @example
# Oktakit.update_user_profile_schema_property
def update_user_profile_schema_property(options = {})
post('/meta/schemas/user/default', options)
post("/meta/schemas/user/default", options)
end

# Remove Property from User Profile Schema
Expand All @@ -58,7 +58,7 @@ def update_user_profile_schema_property(options = {})
# @example
# Oktakit.remove_property_from_user_profile_schema
def remove_property_from_user_profile_schema(options = {})
post('/meta/schemas/user/default', options)
post("/meta/schemas/user/default", options)
end

# Get App User Schema
Expand Down
4 changes: 2 additions & 2 deletions lib/oktakit/client/templates.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Templates
# @example
# Oktakit.add_sms_template
def add_sms_template(options = {})
post('/templates/sms', options)
post("/templates/sms", options)
end

# Get SMS Template
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_sms_template(id, options = {})
# @example
# Oktakit.list_sms_templates
def list_sms_templates(options = {})
get('/templates/sms', options)
get("/templates/sms", options)
end

# Update Sms Template
Expand Down
4 changes: 2 additions & 2 deletions lib/oktakit/client/users.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module Users
# @example
# Oktakit.create_user
def create_user(options = {})
post('/users', options)
post("/users", options)
end

# Get User
Expand Down Expand Up @@ -44,7 +44,7 @@ def get_user(id, options = {})
# @example
# Oktakit.list_users
def list_users(options = {})
get('/users', options)
get("/users", options)
end

# Update User
Expand Down
48 changes: 25 additions & 23 deletions lib/oktakit/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@ class Error < StandardError
#
# @param [Hash] response HTTP response
# @return [Oktakit::Error]
def self.from_response(response)
status = response[:status].to_i
if (klass = error(status))
klass.new(response)
end
def self.from_response(response, status)
status = status.to_i

return if response.nil? || status.nil? || status.zero?

(klass = error(status)) ? klass.new(response, status) : nil
end

def self.error(status)
Expand All @@ -33,8 +34,10 @@ def self.error(status)
end
end

def initialize(response = nil)
def initialize(response = nil, status = nil)
@response = response
@status = status

super(build_error_message)
end

Expand All @@ -54,18 +57,16 @@ def data

private

attr_reader :response, :status

def parse_data
body = @response[:body]
return if body.empty?
return body unless body.is_a?(String)

headers = @response[:response_headers]
content_type = headers && headers[:content_type] || ''
if content_type =~ /json/
Sawyer::Agent.serializer.decode(body)
else
body
end
return "" if response.empty?
return response unless response.is_a?(String)

headers = response[:response_headers]
content_type = headers && headers[:content_type] || ""

content_type =~ /json/ ? Sawyer::Agent.serializer.decode(body) : body
end

def response_message
Expand All @@ -78,17 +79,18 @@ def response_message
end

def build_error_message
return nil if @response.nil?
message = ""

message << "#{response[:method].to_s.upcase} " unless response[:method].nil?
message << "#{redact_url(response[:url].to_s)} : " unless response[:url].nil?
message << status.to_s
message << " - #{response_message}" unless response_message.nil? || response_message.empty?

message = "#{@response[:method].to_s.upcase} "
message << redact_url(@response[:url].to_s) + ': '
message << "#{@response[:status]} - "
message << response_message.to_s unless response_message.nil?
message
end

def redact_url(url_string)
%w[client_secret access_token].each do |token|
["client_secret", "access_token"].each do |token|
url_string.gsub!(/#{token}=\S+/, "#{token}=(redacted)") if url_string.include?(token)
end
url_string
Expand Down
Loading

0 comments on commit 6bad449

Please sign in to comment.