Skip to content

Commit

Permalink
Update GitHub omniauth
Browse files Browse the repository at this point in the history
Updates omniauth because GitHub is deprecating query params which is used by the old omniauth.

To do this I updated to Omniauth 2+ which also required an update of devise. There's a change to the omniauth API which is talked about here:

heartcombo/devise#5236

Basically:
- Omniauth2 requires post (instead of GET)
- Omniauth 2 also needs this `omniauth-rails_csrf_protection` gem.

I added the gem and updated all `link_to` and `button_to` to include a `method: :post`. There is one controller redirect which apparently still seems to work, but it might be broken. I'm not sure how we could possibly preserve the existing behavior since you cannot redirect to a post. 

This gets tests to pass though. So it's good enough for the short term.

## Deprecation Email from GitHub

```
[GitHub API] Deprecation notice for authentication via URL query parameters
GitHub

Mar 6, 2021, 4:37 AM (5 days ago)

to Richard
Hi @schneems,

On March 6th, 2021 at 10:37 (UTC) your application (CodeTriage) used an access token (with the User-Agent Faraday v0.17.3) as part of a query parameter to access an endpoint through the GitHub API:

https://api.github.com/user

Please use the Authorization HTTP header instead as using the `access_token` query parameter is deprecated.

Depending on your API usage, we'll be sending you this email reminder on a monthly basis.

Visit https://developer.github.com/changes/2020-02-10-deprecating-auth-through-query-param for more information about suggested workarounds and removal dates.

Thanks,
The GitHub Team
```

Need to use devise from GitHub due to this not being released yet heartcombo/devise#5327

# This is the commit message #2:

WIP Move omniauth links to post

Omniauth 2 requires post. heartcombo/devise#5236

```
Install the gem OmniAuth - Rails CSRF Protection
Add the link user_facebook_omniauth_authorize_path method: :post
```

TODO: Convert the rest of the links to `user_github_omniauth_authorize_path` to be post
  • Loading branch information
schneems committed Apr 4, 2021
1 parent 050f420 commit 4719819
Show file tree
Hide file tree
Showing 14 changed files with 70 additions and 55 deletions.
7 changes: 4 additions & 3 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ git_source(:github) do |repo_name|
"https://github.com/#{repo_name}.git"
end

ruby '2.7.2'
ruby ">= 2.7.2", "< 3.1"

git_source :github do |name|
"https://github.com/#{name}.git"
Expand All @@ -24,12 +24,13 @@ end

gem 'bluecloth'
gem 'dalli'
gem 'devise'
gem 'devise', github: "heartcombo/devise"
gem 'git_hub_bub'
gem 'jquery-rails'
gem 'local_time', '2.1.0'
gem 'maildown', '~> 3.3'
gem 'omniauth', '~> 1.9.1'
gem 'omniauth', '~> 2.0.3'
gem 'omniauth-rails_csrf_protection'
gem 'omniauth-github'
gem 'pg'
gem 'puma'
Expand Down
48 changes: 29 additions & 19 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,17 @@ GIT
activesupport (>= 3.0.0)
uniform_notifier (~> 1.11)

GIT
remote: https://github.com/heartcombo/devise.git
revision: 0cd72a56f984a7ff089246f87a8b259120545edd
specs:
devise (4.7.3)
bcrypt (~> 3.0)
orm_adapter (~> 0.1)
railties (>= 4.1.0)
responders
warden (~> 1.2.3)

GIT
remote: https://github.com/rails/sprockets.git
revision: 2c38d99930421c22896a65e1d384edf90830f4c3
Expand Down Expand Up @@ -154,12 +165,6 @@ GEM
rake (> 10, < 14)
ruby-statistics (>= 2.1)
thor (>= 0.19, < 2)
devise (4.7.3)
bcrypt (~> 3.0)
orm_adapter (~> 0.1)
railties (>= 4.1.0)
responders
warden (~> 1.2.3)
docile (1.3.5)
dotenv (2.7.6)
dotenv-rails (2.7.6)
Expand All @@ -170,7 +175,7 @@ GEM
execjs (2.7.0)
faker (2.17.0)
i18n (>= 1.6, < 2)
faraday (0.17.3)
faraday (0.17.4)
multipart-post (>= 1.2, < 3)
ffi (1.15.0)
flamegraph (0.9.5)
Expand All @@ -195,7 +200,7 @@ GEM
rails-dom-testing (>= 1, < 3)
railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
jwt (2.2.1)
jwt (2.2.2)
kramdown (2.3.1)
rexml
kramdown-parser-gfm (1.1.0)
Expand Down Expand Up @@ -226,7 +231,7 @@ GEM
minitest (5.14.4)
mocha (1.12.0)
msgpack (1.4.2)
multi_json (1.13.1)
multi_json (1.15.0)
multi_xml (0.6.0)
multipart-post (2.1.1)
mustermann (1.1.1)
Expand All @@ -239,22 +244,26 @@ GEM
mini_portile2 (~> 2.5.0)
racc (~> 1.4)
normalize-rails (8.0.1)
oauth2 (1.4.2)
oauth2 (1.4.4)
faraday (>= 0.8, < 2.0)
jwt (>= 1.0, < 3.0)
multi_json (~> 1.3)
multi_xml (~> 0.5)
rack (>= 1.2, < 3)
oj (3.11.2)
omniauth (1.9.1)
omniauth (2.0.3)
hashie (>= 3.4.6)
rack (>= 1.6.2, < 3)
omniauth-github (1.3.0)
omniauth (~> 1.5)
omniauth-oauth2 (>= 1.4.0, < 2.0)
omniauth-oauth2 (1.6.0)
oauth2 (~> 1.1)
omniauth (~> 1.9)
rack-protection
omniauth-github (2.0.0)
omniauth (~> 2.0)
omniauth-oauth2 (~> 1.7.1)
omniauth-oauth2 (1.7.1)
oauth2 (~> 1.4)
omniauth (>= 1.9, < 3)
omniauth-rails_csrf_protection (1.0.0)
actionpack (>= 4.2)
omniauth (~> 2.0)
optimist (3.0.1)
orm_adapter (0.5.0)
parallel (1.20.1)
Expand Down Expand Up @@ -461,7 +470,7 @@ DEPENDENCIES
coffee-rails (~> 5.0.0)
dalli
derailed_benchmarks
devise
devise!
dotenv-rails (= 2.7.6)
faker
flamegraph
Expand All @@ -480,8 +489,9 @@ DEPENDENCIES
neat (~> 1.7)
normalize-rails
oj
omniauth (~> 1.9.1)
omniauth (~> 2.0.3)
omniauth-github
omniauth-rails_csrf_protection
pg
prawn
premailer-rails
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/repos_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
module ReposHelper
def link_to_or_log_in(options = {})
path = user_signed_in? ? options[:path] : user_github_omniauth_authorize_path(origin: request.fullpath)
button_to options[:text], path, class: "button #{options[:html_class]}"
button_to options[:text], path, class: "button #{options[:html_class]}", method: :post
end
end
4 changes: 2 additions & 2 deletions app/views/application/_nav.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ nav.application-navigation
- else
li.application-navigation-list-item= link_to 'About', what_path
li.application-navigation-list-item= link_to 'University', university_index_path
li.application-navigation-list-item= link_to 'Log in', user_github_omniauth_authorize_path
li.application-navigation-list-item= link_to 'Sign Up', user_github_omniauth_authorize_path
li.application-navigation-list-item= link_to 'Log in', user_github_omniauth_authorize_path, method: :post
li.application-navigation-list-item= link_to 'Sign Up', user_github_omniauth_authorize_path, method: :post

div.application-logo
= link_to root_path
Expand Down
2 changes: 1 addition & 1 deletion app/views/pages/index.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
h2.hero-title-secondary =link_to "What is CodeTriage?", what_path

- unless user_signed_in?
= link_to user_github_omniauth_authorize_path, class: 'button'
= link_to user_github_omniauth_authorize_path, class: 'button', method: :post
| Sign up with GitHub

hr.section-break
Expand Down
10 changes: 5 additions & 5 deletions app/views/pages/what.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
</blockquote>
<br />
<center>
<%= link_to "Discover a new way to contribute", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Discover a new way to contribute", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>
<br />
<p>
Expand Down Expand Up @@ -46,7 +46,7 @@
</p>
<br />
<center>
<%= link_to "Get started now", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Get started now", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>
<br />
<p>
Expand All @@ -66,7 +66,7 @@
</p>
<br />
<center>
<%= link_to "Explore Open Source with CodeTriage", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Explore Open Source with CodeTriage", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>
<br />
<p>
Expand All @@ -82,11 +82,11 @@
CodeTriage is all about helping people get started and get more involved in Open Source. The service is a volunteer (and Open Source) effort. A huge thank you to <a href="https://github.com/codetriage/codetriage/graphs/contributors">all our contributors!</a>. If you have ideas for tools that can help people make impactful contributions to open source please let us know with an <a href="https://github.com/codetriage/codetriage/issues">issue</a>.
</p>
<p>
If you're not a <%= link_to "CodeTriage member already", user_github_omniauth_authorize_path %>, what are you waiting for? <%= link_to "Discover how to make your Open Source journey approachable and sustainable", user_github_omniauth_authorize_path %>. Get started!
If you're not a <%= link_to "CodeTriage member already", user_github_omniauth_authorize_path, method: :post %>, what are you waiting for? <%= link_to "Discover how to make your Open Source journey approachable and sustainable", user_github_omniauth_authorize_path %>. Get started!
</p>
<br />
<center>
<%= link_to "Start Contributing Today", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Start Contributing Today", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>
<br />
</div>
2 changes: 1 addition & 1 deletion app/views/repos/show.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ div class="subpage-content-wrapper #{ @repo.weight }"
= cache(:hero_secondary_title_repo_show, expires_in: 1.hour) do
h2.hero-title-secondary Pick your favorite repos to receive a different open issue in your inbox every day. Fix the issue and everybody wins. #{number_with_delimiter(User.count, delimiter: ',')} developers are working on #{number_with_delimiter(Repo.count, delimiter: ',')} open source repos using CodeTriage. #{link_to "What is CodeTriage?", what_path}.

= link_to user_github_omniauth_authorize_path, class: 'button'
= link_to user_github_omniauth_authorize_path, class: 'button', method: :post
| Sign up with GitHub

div class="subpage-content-wrapper #{ @repo.weight }"
Expand Down
2 changes: 1 addition & 1 deletion app/views/university/example_app.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ Sign up, subscribe to repos you want to help, and we send you contribution ideas

<br />
<center>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>

<% end %>
2 changes: 1 addition & 1 deletion app/views/university/rebase.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ Sign up, subscribe to repos you want to help, and we send you contribution ideas

<br />
<center>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>

<% end %>
2 changes: 1 addition & 1 deletion app/views/university/reproduction_code.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ Sign up, subscribe to repos you want to help, and we send you contribution ideas

<br />
<center>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>

<% end %>
2 changes: 1 addition & 1 deletion app/views/university/squash.md.erb
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ Sign up, subscribe to repos you want to help, and we send you contribution ideas

<br />
<center>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, class: 'button' %>
<%= link_to "Sign up via GitHub", user_github_omniauth_authorize_path, method: :post, class: 'button' %>
</center>

<% end %>
2 changes: 1 addition & 1 deletion app/views/users/show.html.slim
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
- if current_user.blank?
section.user-logged-out
h1= link_to 'Login to view your account', user_github_omniauth_authorize_path
h1= link_to 'Login to view your account', user_github_omniauth_authorize_path, method: :post

- if current_user && current_user == @user
.subpage-content-wrapper
Expand Down
3 changes: 2 additions & 1 deletion test/integration/adding_repos_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,12 @@ class AddingReposTest < ActionDispatch::IntegrationTest
end

test "supports URL without github.com" do
visit "/"
login_via_github

VCR.use_cassette('my_repos') do
visit "/"
click_link "Submit a Repo"

fill_in 'url', with: 'bemurphy/issue_triage_sandbox'

click_button "Add Repo"
Expand Down
37 changes: 20 additions & 17 deletions test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,24 @@ class ActionDispatch::IntegrationTest
end

OmniAuth.config.test_mode = true
OmniAuth.config.add_mock(:github, {
uid: 'mockstar',
credentials: {
token: ENV['GITHUB_API_KEY'] || "d401116495671f0a0ceca9276e677eff"
},
email: "[email protected]",
info: {
nickname: 'mockstar'
},
extra: {
raw_info: {
name: "Mock Star",
avatar_url: "http://gravatar.com/avatar/default"
}
}
})
OmniAuth.config.add_mock(
:github, {
uid: 'mockstar',
credentials: {
token: ENV['GITHUB_API_KEY'] || "d401116495671f0a0ceca9276e677eff"
},
email: "[email protected]",
info: {
nickname: 'mockstar'
},
extra: {
raw_info: {
name: "Mock Star",
avatar_url: "http://gravatar.com/avatar/default"
}
}
}
)

VCR.configure do |c|
# This 'allow' should be temporary, work towards covering
Expand All @@ -68,7 +70,8 @@ def login_via_github
# Works based off of omniauth's mock
# The user will be looked up from the database and updated
# based off of the info in the mock.
visit "/users/auth/github"
visit "/"
click_on "Log in"
end
end
end
Expand Down

0 comments on commit 4719819

Please sign in to comment.