Skip to content

Configurable token randomness #67

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

Merged
merged 4 commits into from
Aug 3, 2017
Merged

Configurable token randomness #67

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d0e403a
Configurable token randomness
mtsmfm Jun 13, 2017
cfe32eb
Add changelog entry for pull #67
joshbuker Jun 27, 2017
87526f2
Update config call in generate_random_token and move randomness confi…
joshbuker Jul 13, 2017
964a5cd
Merge branch 'master' into config-for-token-randomness
joshbuker Jul 13, 2017
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog
## HEAD

* Add configuration for token randomness [#67](https://github.com/Sorcery/sorcery/pull/67)
* Add facebook user_info_path option to initializer.rb [#63](https://github.com/Sorcery/sorcery/pull/63)

## 0.11.0
Expand Down
6 changes: 6 additions & 0 deletions lib/generators/sorcery/templates/initializer.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@
#
# config.remember_me_httponly =

# Set token randomness. (e.g. user activation tokens)
# The length of the result string is about 4/3 of `token_randomness`.
# Default: `15`
#
# config.token_randomness =

# -- session timeout --
# How long in seconds to keep the session alive.
# Default: `3600`
Expand Down
5 changes: 4 additions & 1 deletion lib/sorcery/model/config.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ class Config
attr_accessor :email_delivery_method
# an array of method names to call after configuration by user. used internally.
attr_accessor :after_config
# Set token randomness
attr_accessor :token_randomness

# change default encryption_provider.
attr_reader :encryption_provider
Expand All @@ -61,7 +63,8 @@ def initialize
:@subclasses_inherit_config => false,
:@before_authenticate => [],
:@after_config => [],
:@email_delivery_method => default_email_delivery_method
:@email_delivery_method => default_email_delivery_method,
:@token_randomness => 15
}
reset!
end
Expand Down
4 changes: 3 additions & 1 deletion lib/sorcery/model/temporary_token.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ module Model
# such as reseting password and activating the user by email.
module TemporaryToken
def self.included(base)
# FIXME: This may not be the ideal way of passing sorcery_config to generate_random_token.
@sorcery_config = base.sorcery_config
Copy link
Member

Choose a reason for hiding this comment

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

Mildly worried that this might cause @sorcery_config to be unintentionally exposed, but not familiar enough with how ruby inheritance/privacy works to be sure. This is how it gets saved in lib/sorcery/model.rb however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sorcery_config is owned by TemporaryToken.
It will be overridden if we include TemporaryToken.

module Foo
  def self.included(base)
    @var = base.name
  end

  def self.foo
    puts @var
  end
end

class Bar
  include Foo
end

Foo.foo #=> Bar
puts Foo.instance_variable_get(:@var) #=> Bar

class Baz
  include Foo
end

Foo.foo #=> Baz
puts Foo.instance_variable_get(:@var) #=> Baz

Is this intentional behaviour?

Copy link
Member

Choose a reason for hiding this comment

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

As far as I can tell, Sorcery::Model is the only place where TemporaryToken is/should be included. Based on your description, it sounds like it should be working exactly as intended. What I was worried about was more along the lines of @sorcery_config accidentally becoming available inside of a controller/model class.

For example:

class User < ApplicationRecord
  authenticates_with_sorcery!

  def some_method
    # @sorcery_config should not be available here!
    @sorcery_config = nil # This hopefully shouldn't break things either!
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for late reply 💦

Don't worry, User instance can't override @sorcery_config

module Foo
  def self.included(base)
    @var = base.name
  end

  def self.foo
    @var
  end
end

class Bar
  include Foo

  def bar
    @var
  end

  def bar=(var)
    @var = var
  end
end

p Foo.foo #=> "Bar"
p Foo.instance_variable_get(:@var) #=> "Bar"
bar = Bar.new
p bar.bar #=> nil
bar.bar = :bar
p bar.bar #=> :bar
p Foo.foo #=> Bar
p Foo.instance_variable_get(:@var) #=> Bar

base.extend(ClassMethods)
end

# Random code, used for salt and temporary tokens.
def self.generate_random_token
SecureRandom.urlsafe_base64(15).tr('lIO0', 'sxyz')
SecureRandom.urlsafe_base64(@sorcery_config.token_randomness).tr('lIO0', 'sxyz')
end

module ClassMethods
Expand Down
27 changes: 27 additions & 0 deletions spec/sorcery_temporary_token_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
require 'spec_helper'
Copy link
Member

Choose a reason for hiding this comment

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

Yay! Thanks for writing tests for the new functionality. 👍


describe Sorcery::Model::TemporaryToken do
describe '.generate_random_token' do
before do
sorcery_reload!
end

subject { Sorcery::Model::TemporaryToken.generate_random_token.length }

context 'token_randomness is 3' do
before do
sorcery_model_property_set(:token_randomness, 3)
end

it { is_expected.to eq 4 }
end

context 'token_randomness is 15' do
before do
sorcery_model_property_set(:token_randomness, 15)
end

it { is_expected.to eq 20 }
end
end
end