Skip to content

Adding option to explicitly set token, token_name on initialization #11

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

Open
misfir3 wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Adding option to explicitly set token, token_name on initialization #11

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a0440f2
intial pass at adding explicit token and token_name to initialize the…
misfir3 Feb 12, 2025
fae9a11
finishing the renaming I apparently didn't finish
misfir3 Feb 12, 2025
14296d2
got a few more tests working with copilots help and a little replumbi…
misfir3 Feb 14, 2025
de48956
updated with all tests passing in spec/hackerone/client_spec.rb
misfir3 Feb 14, 2025
f979c25
asset_spec tests passing, using web mock primarily now
misfir3 Feb 15, 2025
c48079f
program spec update for client, tests passing
misfir3 Feb 15, 2025
81e3947
report spec updated for API, tests passing
misfir3 Feb 15, 2025
01a17a5
reporter spec updated, passing
misfir3 Feb 15, 2025
03774d1
all tests passing
misfir3 Feb 15, 2025
62627fb
appeasing rubocop
misfir3 Feb 15, 2025
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
60 changes: 6 additions & 54 deletions fixtures/vcr_cassettes/create_report.yml
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion fixtures/vcr_cassettes/report.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions fixtures/vcr_cassettes/report_list.yml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

25 changes: 19 additions & 6 deletions lib/hackerone/client.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ class NotConfiguredError < StandardError; end
DEFAULT_CRITICAL_RANGE = 5000...100_000_000

LENIENT_MODE_ENV_VARIABLE = "HACKERONE_CLIENT_LENIENT_MODE"

REPORT_STATES = %w(
new
triaged
Expand All @@ -48,7 +47,7 @@ class NotConfiguredError < StandardError; end

class << self
ATTRS = [:low_range, :medium_range, :high_range, :critical_range].freeze
attr_accessor :program
attr_accessor :program, :token, :token_name
attr_reader *ATTRS

ATTRS.each do |attr|
Expand All @@ -60,14 +59,28 @@ class << self
end

class Api
def initialize(program = nil)
def initialize(program = nil, token: nil, token_name: nil)
@program = program
@token = token || ENV["HACKERONE_TOKEN"]
@token_name = token_name || ENV["HACKERONE_TOKEN_NAME"]
# Set class-level token and token_name if provided

@rzhade3 rzhade3 Feb 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ehhhhh, if we're setting this token at a class level then this is no better logically than if we just continue to use environment variables; part of the impetus to use tokens as variables is such that there is instance level encapsulation. This adds side effects across entire classes which defeats the purpose of this in the first place. This would clobber any calls to HackerOne::Client methods anytime after a new API object is created, which doesn't seem ideal. I think 🤔

It is also tough because that class is designed to operate as a class, which makes encapsulation really tough.

Some alternatives:

  • Accept this as tech debt, and accept that you can only have one instance of HackerOne::Client running
  • Put an instance of the HackerOne::Cient inside the HackerOne::Api object, and operate on that object. This would certainly not be backwards compatible.
  • Do not add this side effect, in which case HackerOne::Client and all subclasses would only be able to respond to the environment variable
  • Deprecate the HackerOne::Client class. I would prefer this, but this is also certainly not backwards compatible
  • Rewrite the HackerOne::Client object such that it becomes a class instead of an object

@misfir3 misfir3 Feb 18, 2025

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Appreciate the feedback ...

Deprecate the HackerOne::Client class. I would prefer this, but this is also certainly not backwards compatible

I think I prefer this too, but timeline changes with that. Gonna have to think on it a little more 🙇🏼

if token
HackerOne::Client.token = token
end

if token_name
HackerOne::Client.token_name = token_name
end
end

def program
@program || HackerOne::Client.program
end

def token_name
@token_name || ENV["HACKERONE_TOKEN_NAME"]

@rzhade3 rzhade3 Feb 18, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

When would this be needed? As far as I can tell, this conditional would only ever evaluate if the value of the HACKERONE_TOKEN_NAME token wasn't set when the client was initialized?

I suppose it would help from a backwards compatibility perspective, but just want to make sure this line earns its place in the code.

end

def reporters
raise ArgumentError, "Program cannot be nil" unless program
response = self.class.hackerone_api_connection.get do |req|
Expand Down Expand Up @@ -200,12 +213,12 @@ def self.parse_response(response, extract_data: true)
end

def self.hackerone_api_connection
unless ENV["HACKERONE_TOKEN_NAME"] && ENV["HACKERONE_TOKEN"]
raise NotConfiguredError, "HACKERONE_TOKEN_NAME HACKERONE_TOKEN environment variables must be set"
unless HackerOne::Client.token_name && HackerOne::Client.token
raise NotConfiguredError, "Either set token_name and token or HACKERONE_TOKEN_NAME and HACKERONE_TOKEN environment variables"
end

@connection ||= Faraday.new(url: "https://api.hackerone.com/v1") do |faraday|
faraday.request(:authorization, :basic, ENV["HACKERONE_TOKEN_NAME"], ENV["HACKERONE_TOKEN"])
faraday.request(:authorization, :basic, HackerOne::Client.token_name, HackerOne::Client.token)
faraday.adapter Faraday.default_adapter
end
end
Expand Down
38 changes: 30 additions & 8 deletions spec/hackerone/client/asset_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
# frozen_string_literal: true

require "spec_helper"
require "webmock/rspec"

RSpec.describe HackerOne::Client::Asset do
before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
end
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:each) do
# Initialize the API client before running tests
api

# Stub the HTTP request for fetching the program
stub_request(:get, "https://api.hackerone.com/v1/programs/18969").
to_return(body: <<~JSON)
{
Expand Down Expand Up @@ -36,6 +39,7 @@
}
JSON

# Stub the HTTP request for fetching the assets
stub_request(:get, "https://api.hackerone.com/v1/organizations/14/assets?page%5Bnumber%5D=1&page%5Bsize%5D=100").
to_return(body: <<~JSON2)
{
Expand Down Expand Up @@ -114,17 +118,35 @@
"links": {}
}
JSON2

# Stub the HTTP request for fetching the user's programs
stub_request(:get, "https://api.hackerone.com/v1/me/programs").
to_return(body: <<~JSON3)
{
"data": [
{
"id": "18969",
"type": "program",
"attributes": {
"handle": "github",
"created_at": "2016-02-02T04:05:06.000Z",
"updated_at": "2016-02-02T04:05:06.000Z"
}
}
]
}
JSON3
end

after(:each) do
# clear cached programs to prevent contaminatin between tests
# Clear both cached programs and configuration
HackerOne::Client::Program.instance_variable_set(:@my_programs, nil)
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

let(:program) do
VCR.use_cassette(:programs) do
HackerOne::Client::Program.find("github")
end
HackerOne::Client::Program.find("github")
end

let(:organization) do
Expand Down
15 changes: 12 additions & 3 deletions spec/hackerone/client/program_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,18 @@
require "spec_helper"

RSpec.describe HackerOne::Client::Program do
before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:each) do
# Initialize the API client before running tests
api
end

after(:each) do
# Clear both cached programs and configuration
HackerOne::Client::Program.instance_variable_set(:@my_programs, nil)
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

let(:program) do
Expand Down
14 changes: 10 additions & 4 deletions spec/hackerone/client/report_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
require "spec_helper"

RSpec.describe HackerOne::Client::Report do
let(:api) { HackerOne::Client::Api.new("github") }
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
before(:each) do
# Initialize the API client before running tests
api
end

after(:each) do
# Clear both cached programs and configuration
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

let(:report) do
Expand Down
14 changes: 10 additions & 4 deletions spec/hackerone/client/reporter_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
require "spec_helper"

RSpec.describe HackerOne::Client::Report do
let(:api) { HackerOne::Client::Api.new("github") }
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
before(:each) do
# Initialize the API client before running tests
api
end

after(:each) do
# Clear both cached programs and configuration
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

let(:reporters) do
Expand Down
15 changes: 10 additions & 5 deletions spec/hackerone/client/swag_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,17 @@
require "spec_helper"

RSpec.describe HackerOne::Client::Swag do
let(:api) { HackerOne::Client::Api.new("github") }
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
before(:each) do
# Initialize the API client before running tests
api
end

after(:each) do
# Clear both cached programs and configuration
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

let(:program) do
Expand Down Expand Up @@ -51,7 +57,6 @@
end
end


describe "user" do
it "returns a user" do
user = swag.user
Expand Down
14 changes: 11 additions & 3 deletions spec/hackerone/client/user_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,17 @@
require "spec_helper"

RSpec.describe HackerOne::Client::User do
before(:all) do
ENV["HACKERONE_TOKEN_NAME"] = "foo"
ENV["HACKERONE_TOKEN"] = "bar"
let(:api) { HackerOne::Client::Api.new("github", token: "bar", token_name: "foo") }

before(:each) do
# Initialize the API client before running tests
api
end

after(:each) do
# Clear both cached programs and configuration
HackerOne::Client.instance_variable_set(:@token, nil)
HackerOne::Client.instance_variable_set(:@token_name, nil)
end

describe "find" do
Expand Down
Loading