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

Implement #login! helper #332

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
# Changelog
## HEAD

* Add a `#login!` helper that raises an exception if the login fails [#332](https://github.com/Sorcery/sorcery/pull/322)

## 0.16.4

* Adapt to open request protection strategy of rails 7.0 [#318](https://github.com/Sorcery/sorcery/pull/318)
Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ explaining and the rest are commented:
```ruby
require_login # This is a before action
login(email, password, remember_me = false)
login!(email, password, remember_me = false) # Raises an `Sorcery::InvalidCredentials` exception on failure
auto_login(user) # Login without credentials
logout
logged_in? # Available in views
Expand Down
12 changes: 12 additions & 0 deletions lib/sorcery/controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
module Sorcery
class InvalidCredentials < StandardError; end
Spone marked this conversation as resolved.
Show resolved Hide resolved

module Controller
def self.included(klass)
klass.class_eval do
Expand Down Expand Up @@ -63,6 +65,16 @@ def login(*credentials)
end
end

def login!(*credentials)
Copy link
Member

Choose a reason for hiding this comment

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

We'll need to test this and see how it interacts when given a code block to yield to.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure what the behavior should be when this method is called with a block. What do you suggest?

Copy link
Member

Choose a reason for hiding this comment

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

Essentially, sanity checks that the method performs as expected when passing a block.

This wiki page has an example of using the block for raising different error messages during login: https://github.com/Sorcery/sorcery/wiki/Distinguish-login-failure-reasons

When given a code block, we can test that it succeeds when given valid credentials, fails when given invalid credentials, and raises your new error when given a blank string or nil value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@joshbuker I added specs for the block version of #login!. I have a few notes:

  • I can't find tests for the block version of #login
  • I'm not sure what the behavior should be when calling #login! with a block and with wrong credentials. I would expect it to always raise the InvalidCredentials exception, else I don't see the value in using #login!

user = login(*credentials)

if user.nil?
raise Sorcery::InvalidCredentials
else
user
end
Spone marked this conversation as resolved.
Show resolved Hide resolved
end

def reset_sorcery_session
reset_session # protect from session fixation attacks
end
Expand Down
33 changes: 33 additions & 0 deletions spec/controllers/controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,39 @@
end
end

describe '#login!' do
context 'when succeeds' do
before do
expect(User).to receive(:authenticate).with('[email protected]', 'secret') { |&block| block.call(user, nil) }
joshbuker marked this conversation as resolved.
Show resolved Hide resolved
get :test_login_bang, params: { email: '[email protected]', password: 'secret' }
end

it 'assigns user to @user variable' do
expect(assigns[:user]).to eq user
end

it 'writes user id in session' do
expect(session[:user_id]).to eq user.id.to_s
end

it 'sets csrf token in session' do
expect(session[:_csrf_token]).not_to be_nil
end
end

context 'when fails' do
before do
expect(User).to receive(:authenticate).with('[email protected]', 'opensesame!').and_return(nil)
end

it 'raises Sorcery::InvalidCredentials exception' do
expect do
get :test_login_bang, params: { email: '[email protected]', password: 'opensesame!' }
end.to raise_error(Sorcery::InvalidCredentials)
end
end
end

describe '#logout' do
it 'clears the session' do
cookies[:remember_me_token] = nil
Expand Down
5 changes: 5 additions & 0 deletions spec/rails_app/app/controllers/sorcery_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ def test_login
head :ok
end

def test_login_bang
@user = login!(params[:email], params[:password])
head :ok
end

def test_auto_login
@user = User.first
auto_login(@user)
Expand Down
1 change: 1 addition & 0 deletions spec/rails_app/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

controller :sorcery do
get :test_login
get :test_login_bang
get :test_logout
get :some_action
post :test_return_to
Expand Down