Skip to content
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.

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

wire up passwordless authentication #1

Open
wants to merge 1 commit into
base: docker
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ gem 'rails_admin'
gem 'redcarpet'
gem 'erubis'
gem 'rack-attack'
gem 'openid_connect', '~> 1.1', '>= 1.1.8'

group :development, :test do
gem 'rspec-rails', '4.0.0.beta3'
Expand All @@ -38,4 +39,3 @@ group :test do
gem 'rails-controller-testing'
gem 'simplecov', require: false
end

38 changes: 38 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@ GEM
activerecord (>= 5.0, < 6.1)
addressable (2.7.0)
public_suffix (>= 2.0.2, < 5.0)
aes_key_wrap (1.0.1)
attr_required (1.0.1)
backports (3.15.0)
bindata (2.4.6)
bindex (0.8.1)
builder (3.2.4)
byebug (11.1.1)
Expand Down Expand Up @@ -124,6 +127,7 @@ GEM
haml (5.1.2)
temple (>= 0.8.0)
tilt
httpclient (2.8.3)
i18n (1.8.2)
concurrent-ruby (~> 1.0)
jquery-rails (4.3.5)
Expand All @@ -133,6 +137,10 @@ GEM
jquery-ui-rails (6.0.1)
railties (>= 3.2.16)
json (2.3.0)
json-jwt (1.11.0)
activesupport (>= 4.2)
aes_key_wrap
bindata
kaminari (1.1.1)
activesupport (>= 4.1.0)
kaminari-actionview (= 1.1.1)
Expand Down Expand Up @@ -169,13 +177,29 @@ GEM
nio4r (2.5.2)
nokogiri (1.10.8)
mini_portile2 (~> 2.4.0)
openid_connect (1.1.8)
activemodel
attr_required (>= 1.0.0)
json-jwt (>= 1.5.0)
rack-oauth2 (>= 1.6.1)
swd (>= 1.0.0)
tzinfo
validate_email
validate_url
webfinger (>= 1.0.1)
pg (1.2.2)
public_suffix (4.0.2)
puma (4.3.3)
nio4r (~> 2.0)
rack (2.0.8)
rack-attack (6.2.2)
rack (>= 1.0, < 3)
rack-oauth2 (1.10.1)
activesupport
attr_required
httpclient
json-jwt (>= 1.11.0)
rack
rack-pjax (1.1.0)
nokogiri (~> 1.5)
rack (>= 1.1)
Expand Down Expand Up @@ -276,6 +300,10 @@ GEM
actionpack (>= 4.0)
activesupport (>= 4.0)
sprockets (>= 3.0.0)
swd (1.1.2)
activesupport (>= 3)
attr_required (>= 0.0.5)
httpclient (>= 2.4)
temple (0.8.2)
thor (1.0.1)
thread_safe (0.3.6)
Expand All @@ -287,11 +315,20 @@ GEM
thread_safe (~> 0.1)
uglifier (4.2.0)
execjs (>= 0.3.0, < 3)
validate_email (0.1.6)
activemodel (>= 3.0)
mail (>= 2.2.5)
validate_url (1.0.8)
activemodel (>= 3.0.0)
public_suffix
web-console (3.7.0)
actionview (>= 5.0)
activemodel (>= 5.0)
bindex (>= 0.4.0)
railties (>= 5.0)
webfinger (1.1.0)
activesupport
httpclient (>= 2.4)
websocket-driver (0.7.1)
websocket-extensions (>= 0.1.0)
websocket-extensions (0.1.4)
Expand All @@ -313,6 +350,7 @@ DEPENDENCIES
faker
jquery-rails
listen
openid_connect (~> 1.1, >= 1.1.8)
pg (= 1.2.2)
puma
rack-attack
Expand Down
45 changes: 45 additions & 0 deletions app/controllers/session_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@

class SessionController < ApplicationController

def authenticate
config = OpenIDConnect::Discovery::Provider::Config.discover! "https://did.app"

client = OpenIDConnect::Client.new({
Copy link
Owner Author

Choose a reason for hiding this comment

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

Work out where to put the client and config.
Is the correct place as an initializer? Or is it somewhere in boot.
The discover! function makes a webcall but there would be no harm hardcoding the authorization_endpoint etc.

Choose a reason for hiding this comment

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

I think we can put it in an ENV variable still, no?

Copy link
Owner Author

Choose a reason for hiding this comment

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

That doesn't make sense to me.
I mentioned the discover! call makes a web request. and should be done in some startup step

Choose a reason for hiding this comment

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

Hey Peter — what's your specific question here? I can't tell if you're asking me something or telling me something. 😅

Copy link
Owner Author

Choose a reason for hiding this comment

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

I am asking, where do I put code that makes a web request during startup?

"https://did.app" can go in the env variable. But then there is a call that should be performed only once to lookup the authorization endpoint etc. that is what discover does. The config = line should not be part of the http handler but some boot or startup code

Choose a reason for hiding this comment

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

Ah, got it!

Well, I think you want a new file under config/initializers. See more here: https://guides.rubyonrails.org/configuring.html#using-initializer-files.

identifier: ENV["CLIENT_ID"],
secret: ENV["CLIENT_SECRET"],
redirect_uri: "http://localhost:3000/session/callback",
Copy link
Owner Author

Choose a reason for hiding this comment

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

How do we get the host url so that the redirect is dynamic?

Choose a reason for hiding this comment

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

I think we may need to set default_url_options:

https://edgeguides.rubyonrails.org/action_controller_overview.html#default-url-options
https://www.benpickles.com/articles/70-dynamically-setting-rails-default_url_options-in-heroku-review-apps.

And then either interpolate in the host (and port) or make a route in config/routes.rb, maybe something like:

get '/session/callback', to:  'session#callback', as: 'session_callback'

...and then refer to session_callback_url.

issuer: config.issuer,
authorization_endpoint: config.authorization_endpoint,
jwks_uri: config.jwks_uri,
token_endpoint: config.token_endpoint,
})
redirect_to client.authorization_uri()
end

def callback
config = OpenIDConnect::Discovery::Provider::Config.discover! "https://did.app"

client = OpenIDConnect::Client.new({
identifier: ENV["CLIENT_ID"],
Copy link
Owner Author

Choose a reason for hiding this comment

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

Is calling straight into ENV the right thing to do here?

Choose a reason for hiding this comment

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

Yes!

secret: ENV["CLIENT_SECRET"],
redirect_uri: "http://localhost:3000/session/callback",
issuer: config.issuer,
authorization_endpoint: config.authorization_endpoint,
jwks_uri: config.jwks_uri,
token_endpoint: config.token_endpoint,
})
code = params["code"]
client.authorization_code = code
tokens = client.access_token!

id_token = OpenIDConnect::ResponseObject::IdToken.decode tokens.id_token, config.jwks
id_token.verify!(issuer: config.issuer, client_id: client.identifier)
session[:current_user_id] = id_token.subject
redirect_to root_path
end
def terminate
session[:current_user_id] = nil
Copy link
Owner Author

Choose a reason for hiding this comment

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

Is this a satisfactory way to end a session

Choose a reason for hiding this comment

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

Looks like it! Or you can remove the entire session with reset_session.

https://api.rubyonrails.org/classes/ActionController/Base.html#M000668

redirect_to root_path
end

end
6 changes: 6 additions & 0 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@
<li><%= link_to 'Speakers', speakers_path %></li>
<li><%= link_to 'Proposals', proposals_path %></li>
<li><%= link_to 'Events', events_path %></li>
<% if session[:current_user_id] %>
<li><%= session[:current_user_id] %></li>
<li><%= link_to 'Sign out', sign_out_path %></li>
<% else %>
<li><%= link_to 'Sign in', sign_in_path %></li>
<% end %>
</div>
</ul>
</div>
Expand Down
4 changes: 4 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,8 @@
resources :events, only: [:index, :show]
resources :event_instances, only: [:new, :create, :show]
resources :tags, only: [:show]

get '/session/authenticate', to: 'session#authenticate', as: 'sign_in'
get '/session/callback', to: 'session#callback'
get '/session/terminate', to: 'session#terminate', as: 'sign_out'
end
3 changes: 2 additions & 1 deletion docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,14 @@ services:
- 3000:3000
volumes:
- ./:/opt/app
- rails_tmp:/opt/app/tmp
depends_on:
- db
environment:
- RECAPTCHA_SITE_KEY=aaaaa
- RAILS_ADMIN_USERNAME=rails
- RAILS_ADMIN_PASSWORD=rails
- CLIENT_ID=test_0xKvM6N9
Copy link
Owner Author

Choose a reason for hiding this comment

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

These are values for a test application so there is no harm them being in source control

- CLIENT_SECRET=test_y2650o6pLoxNejD9
db:
image: postgres:11.2
ports:
Expand Down