Skip to content

Commit

Permalink
OTWO-7311 Remove Firebase phone auth (#1802)
Browse files Browse the repository at this point in the history
* OTWO-7311 Relax Github oauth restrictions
* OTWO-7311 Remove Firebase phone auth
  • Loading branch information
alex-sig authored Oct 10, 2024
1 parent e005068 commit a9d6d82
Show file tree
Hide file tree
Showing 45 changed files with 119 additions and 854 deletions.
4 changes: 0 additions & 4 deletions .env
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,6 @@ TRAVIS_API_BASE_URL = 'https://api.travis-ci.org/'
CII_API_BASE_URL = 'https://bestpractices.coreinfrastructure.org/'
CII_PROJECTS_EMAIL_RECEIPIENT = '[email protected]'

FIREBASE_API_KEY =
FIREBASE_PROJECT_ID =
FIREBASE_AUTH_DOMAIN =

CODE_SUBDOMAIN = 'code'

FISBOT_CLIENT_REGISTRATION_ID =
Expand Down
4 changes: 0 additions & 4 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,9 @@ KB_PROJECT_SEARCH_URL = 'https://www.vcrlocalhost.org/kb?'
KB_AUTH_KEY = 'dummy_test_key'

OHLOH_CIPHER_KEY = 'ohlohuiapplicationcipherkeyforminitestcases'
FIREBASE_API_KEY = 'dummy_firebase_api_key'
GITHUB_CLIENT_ID = 'dummy_github_client_id'
GITHUB_CLIENT_SECRET = 'dummy_github_client_secret'

FIREBASE_PROJECT_ID = "openhub-test"
FIREBASE_AUTH_DOMAIN = "openhub-test.firebaseapp.com"

GITHUB_REDIRECT_URI = "http://lvh.me:3000/authentications/github_callback"

FISBOT_API_URL = 'http://vcrlocalhost.org:4004'
Expand Down
9 changes: 7 additions & 2 deletions app/assets/javascripts/accounts.js.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ $(document).on 'page:change', ->
if $('#account_affiliation_type').length
new App.OrganizationSelector('account')


$('.show-more-2').click ->
if $('.text-2').hasClass('show-more-height-2')
$(this).text 'Show Less'
Expand All @@ -15,4 +15,9 @@ $(document).on 'page:change', ->
$(this).text 'Show Less'
else
$(this).text 'Show More'
$('.text-1').toggleClass 'show-more-height-1'
$('.text-1').toggleClass 'show-more-height-1'

if $('#email-sign-up').length
$('#email-sign-up').click ->
$('#sign-up-options').remove()
$('#sign-up-fields').show()
42 changes: 0 additions & 42 deletions app/assets/javascripts/firebase.coffee

This file was deleted.

9 changes: 1 addition & 8 deletions app/assets/stylesheets/account.sass
Original file line number Diff line number Diff line change
Expand Up @@ -132,21 +132,14 @@
margin-left: auto
margin-right: auto
text-align: center
#digits-sign-up
#email-sign-up
width: 200px
.github-oauth
width: 200px

#accounts_unsubscribe_emails_page
padding: 0 30px 0 20px !important

#hidden-verified-label
display: none

.verified
display: block
margin-bottom: 10px

.grecaptcha-badge
display: none
@media only all and (min-width: 768px) and (max-width: 1024px)
Expand Down
23 changes: 0 additions & 23 deletions app/assets/stylesheets/static_forms.sass
Original file line number Diff line number Diff line change
Expand Up @@ -697,26 +697,3 @@ span.add-on
-moz-box-sizing: content-box
-ms-box-sizing: content-box
box-sizing: content-box

@media (min-width: 768px) and (max-width: 1024px)
#firebaseui-auth-container
width: 115%
margin-left: -10px

.firebaseui-country-selector
width: 65px

.firebaseui-flag
width: 15px

.firebaseui-button
width: 60px
padding: 0px 10px !important
margin-left: -15px !important

.firebaseui-title
font-size: 15px

@media (min-width: 320px) and (max-width: 480px)
.firebaseui-title
font-size: 15px
4 changes: 2 additions & 2 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,11 @@ class AccountsController < ApplicationController

def new
@account = Account.new
@account.build_firebase_verification
end

def create
@account = Account.new(account_params)
@account.build_manual_verification

if @account.save
clearance_session.sign_in @account
Expand Down Expand Up @@ -114,7 +114,7 @@ def account_params
params.require(:account).permit(
:login, :email, :password, :name, :country_code, :location, :latitude, :longitude,
:twitter_account, :organization_id, :organization_name, :affiliation_type, :invite_code,
:about_raw, :url, firebase_verification_attributes: [:credentials]
:about_raw, :url
)
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -307,7 +307,7 @@ def redirect_for_email_activation
end

def redirect_for_spammer_verification
return if current_user && !current_user.access.disabled? && current_user.access.mobile_or_oauth_verified?
return if current_user && !current_user.access.disabled? && current_user.access.manual_or_oauth_verified?

redirect_to new_authentication_path
end
Expand Down
22 changes: 2 additions & 20 deletions app/controllers/authentications_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,12 @@

class AuthenticationsController < ApplicationController
skip_before_action :store_location
before_action :session_required, only: %i[new firebase_callback]
before_action :redirect_invalid_github_account, only: :github_callback, unless: :github_api_account_is_verified?
before_action :session_required, only: :new
before_action :redirect_matching_account, only: :github_callback, unless: -> { current_user.present? }
before_action :redirect_if_current_user_verified

def new
@account = current_user
@account.build_firebase_verification
render partial: 'fields' if request.xhr?
end

Expand All @@ -18,11 +16,6 @@ def github_callback
create(github_verification_params)
end

def firebase_callback
statsd_increment('Openhub.Account.Signup.firebase')
create(firebase_verification_params)
end

private

def create(auth_params)
Expand Down Expand Up @@ -54,10 +47,6 @@ def create_account_using_github(auth_params)
end
end

def firebase_verification_params
params.require(:account).permit(firebase_verification_attributes: [:credentials])
end

def github_verification_params
{ github_verification_attributes: { unique_id: github_api.login, token: github_api.access_token } }
end
Expand Down Expand Up @@ -113,14 +102,7 @@ def github_api
def redirect_if_current_user_verified
return if current_user.nil?

redirect_to root_path if current_user.access.mobile_or_oauth_verified?
end

def redirect_invalid_github_account
return if github_api.created_at < ENV['GITHUB_REPO_AGE_LIMIT'].to_i.days.ago && github_api.repository_has_language?

redirect_path = current_user.present? ? new_authentication_path : new_account_path
redirect_to redirect_path, notice: t('.invalid_github_account')
redirect_to root_path if current_user.access.manual_or_oauth_verified?
end

def github_api_account_is_verified?
Expand Down
2 changes: 1 addition & 1 deletion app/models/account.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class Account < ApplicationRecord

serialize :reset_password_tokens, Hash

accepts_nested_attributes_for :github_verification, :firebase_verification
accepts_nested_attributes_for :github_verification, :manual_verification

def anonymous?
login == AnonymousAccount::LOGIN
Expand Down
4 changes: 2 additions & 2 deletions app/models/account/access.rb
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,13 @@ def bot!
account.update!(level: BOT)
end

def mobile_or_oauth_verified?
def manual_or_oauth_verified?
return if account.nil?

account.verifications.exists?
end

def verified?
mobile_or_oauth_verified? && email_verified?
manual_or_oauth_verified? && email_verified?
end
end
19 changes: 1 addition & 18 deletions app/models/firebase_verification.rb
Original file line number Diff line number Diff line change
@@ -1,22 +1,5 @@
# frozen_string_literal: true

# Retained for backwards compatibility with existing DB records.
class FirebaseVerification < Verification
attr_accessor :credentials

validates :credentials, presence: true

before_validation :generate_token, on: :create

private

def generate_token
firebase = FirebaseService.new(ENV['FIREBASE_PROJECT_ID'])
decoded_token = firebase.decode(credentials)
if decoded_token
self.token = decoded_token[0]['user_id']
self.unique_id = token
else
errors.add(:unique_id, 'Phone Verification failed please try again')
end
end
end
16 changes: 0 additions & 16 deletions app/models/github_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,6 @@ def email
fetch_private_email
end

def created_at
Time.zone.parse(user_response['created_at'])
end

def repository_has_language?
repositories_response.any? do |repository_hash|
repository_hash['language'].present?
end
end

def access_token
return @access_token if @access_token

Expand Down Expand Up @@ -63,12 +53,6 @@ def user_response
@user_response = get_response_using_token(user_uri)
end

def repositories_response
params = { type: :owner, sort: :pushed, per_page: REPO_LIMIT }
repositories_uri = URI(GITHUB_USER_URI + "/repos?#{params.to_query}")
get_response_using_token(repositories_uri)
end

def config
CGI.unescape({ code: @code, client_id: ENV['GITHUB_CLIENT_ID'], client_secret: ENV['GITHUB_CLIENT_SECRET'],
redirect_uri: ENV['GITHUB_REDIRECT_URI'] }.to_query)
Expand Down
2 changes: 2 additions & 0 deletions app/models/github_verification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,6 @@

class GithubVerification < Verification
validates :token, presence: true

validates :unique_id, presence: true, uniqueness: true
end
2 changes: 1 addition & 1 deletion app/models/manual_verification.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

class ManualVerification < Verification
belongs_to :account, optional: true
belongs_to :account
end
2 changes: 1 addition & 1 deletion app/models/reverification_tracker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ def remove_reverification_trackers_for_verified_accounts
next unless rev_tracker.account

rev_tracker.account.update!(level: 0) if rev_tracker.account.level == -10
rev_tracker.destroy if rev_tracker.account.access.mobile_or_oauth_verified?
rev_tracker.destroy if rev_tracker.account.access.manual_or_oauth_verified?
end
end

Expand Down
3 changes: 1 addition & 2 deletions app/models/verification.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,5 @@
class Verification < ApplicationRecord
belongs_to :account, optional: true

validates :unique_id, :type, presence: true
validates :unique_id, uniqueness: { scope: :type }
validates :type, presence: true
end
Loading

0 comments on commit a9d6d82

Please sign in to comment.