Rails like session/flash management for Amber. #138

eliasjpr · 2017-07-15T19:19:26Z

Description of the Change

This PR adds Rails like session/flash management to Amber.

I have used the following amber blog app to test the functionality https://github.com/eliasjpr/blog

The Session

A session store that uses an Amber:Router::Session::Store to store the sessions. This store is most useful if you don't store critical data in your sessions and you don't need them to live for extended periods of time.

This cookie-based session store is the Amber default. It is dramatically faster than the alternatives.

The cookie store used for storage is automatically configured to be the best possible option given your application's configuration.

Sessions typically contain at most a user_id and flash message; both fit within the 4K cookie size limit. A CookieOverflow exception is raised if you attempt to store more than 4K of data.

To configure the session:

#  Cookie Store
Amber::Server.instance.session = {
  :key     => "name.session",
  :store   => :cookie,
  :expires => 120, # 0, will make the session last as long as the browser is open, upon closing, session will be terminated
  :secret  => "secret"
}

# Redis Store
Amber::Server.instance.session = {
  :key       => "name.session",
  :store     => :redis,
  :expires   => 120,
  :secret    => "secret",
  :redis_url => "redis://localhost:6379",
}

Configure the Pipeline

 # Keep in mind the order of the Pipes. Session hash needs to be populated before trying to access the session flash scope, the flash depends on the session. 
 pipeline :web do
    plug Amber::Pipe::Session.new
    plug Amber::Pipe::Logger.new
    plug Amber::Pipe::Flash.new
    plug Amber::Pipe::CSRF.new
  end

Accessing the session

class ApplicationController < Amber::Controller::Base
 
  private
 
  # Finds the User with the ID stored in the session with the key
  # :current_user_id This is a common way to handle user login in
  # a Amber application; logging in sets the session value and
  # logging out removes it.
  def current_user
    @_current_user ||= session[:current_user_id] &&
      User.find_by(id: session[:current_user_id])
  end
end

To store something in the session, just assign it to the key like a hash:

class LoginsController < ApplicationController
  # "Create" a login, aka "log the user in"
  def create
    if user = User.authenticate(params[:username], params[:password])
      # Save the user ID in the session so it can be used in
      # subsequent requests
      session[:current_user_id] = user.id
      redirect_to root_url
    end
  end
end

To remove something from the session, assign that key to be nil or use session.delete(key)

class LoginsController < ApplicationController
  # "Delete" a login, aka "log the user out"
  def destroy
    # Remove the user id from the session
    @_current_user = session[:current_user_id] = nil
    redirect_to root_url
  end
end

The Flash

The flash is a special part of the session which is cleared with each request. This means that values stored there will only be available on the next request, which is useful for passing error messages etc.

It is accessed in much the same way as the session, as a hash.

Let's use the act of logging out as an example. The controller can send a message which will be displayed to the user on the next request:

class LoginsController < ApplicationController
  def destroy
    session[:current_user_id] = nil
    #  Alternatively, `flash.notice=` could be use.
    flash[:notice] = "You have successfully logged out."
    redirect_to root_url
  end
end

Rendering the flash message

<html>
  <!-- <head/> -->
  <body>
    <% flash.each do |name, msg| -%>
      <%= content_tag :div, msg, class: name %>
    <% end -%>
 
    <!-- more content -->
  </body>
</html>

If you want a flash value to be carried over to another request, use the keep method:

class MainController < ApplicationController
  # Let's say this action corresponds to root_url, but you want
  # all requests here to be redirected to UsersController#index.
  # If an action sets the flash and redirects here, the values
  # would normally be lost when another redirect happens, but you
  # can use 'keep' to make it persist for another request.
  def index
    # Will persist all flash values.
    flash.keep
 
    # You can also use a key to keep only some kind of value.
    # flash.keep(:notice)
    redirect_to users_url
  end
end

Flash.now

By default, adding values to the flash will make them available to the next request, but sometimes you may want to access those values in the same request. For example, if the create action fails to save a resource and you render the new template directly, that's not going to result in a new request, but you may still want to display a message using the flash. To do this, you can use flash.now in the same way you use the normal flash.

class ClientsController < ApplicationController
  def create
    @client = Client.new(params[:client])
    if @client.save
      # ...
    else
      flash.now[:error] = "Could not save client"
      render action: "new"
    end
  end
end

Possible Drawbacks

Current all sessions are encrypted. It has been proposed to sign and/or only encrypt by passing a config param.

- Adds Cookies Session using new cookies storage - Adds Redis Session storage

elorest · 2017-07-15T21:13:15Z

.travis.yml

@@ -1,4 +1,4 @@
-language: generic
+language: crystal


This should probably be in a different PR. I know Dru was in favor of using docker which was why I used this fix. Crystal Core team also uses docker for their travis tests.

I decided to remove it since the crystal v0.23.1 works on Travis. Redis server was not being installed with the docker option causing the specs for this branch to fail.

I recommend adding the redis service to dockercompose.yml and reenabling docker for running our tests. This will eliminate failures because of differences between environments and gives us finer control of the testing environment.

@drujensen I would not know how to do this. Let me know when you have some time I would like if you can walk me thru this setup.

elorest

Looks good. I'd definitely change the expires time though. A session isn't very useful if it expires in 2 hours.

elorest · 2017-07-15T21:14:13Z

src/amber.cr

+      @session = {
+        :key       => "session_id",
+        :store     => :cookie,
+        :expires   => 2 * 60 * 60, # 2 Hours


Session should last as long as the browser is open like the other MVC's instead of 2 hours.

elorest · 2017-07-15T21:26:34Z

src/amber/router/pipe/session.cr

-        def find_entry(key)
-          super(key.to_s)
-        end
+        context.session.set_session


Writing cookies is expensive time wise, now more than ever due to encryption. We should only save cookies if something has changed.

Let's iterate over this in a separate PR. On those upcoming PRs the signed and encryption should be addressed as well

elorest · 2017-07-15T21:29:01Z

src/amber/router/session/cookie_store.cr

@@ -0,0 +1,65 @@
+module Amber::Router::Session
+  class SessionHash < Hash(String, String)


I like that you utilized this.

elorest · 2017-07-16T01:52:06Z

spec/amber/controller/base_spec.cr

        html_output = <<-HTML
        <form action="/posts" method="post">
-          <input type="hidden" name="_csrf" value="#{Amber::Pipe::CSRF.new.token(context)}" />
+          <input type="hidden" name="_csrf" value="#{csrf_token}" />


Like this more.

fridgerator

will defer to @elorest comments, but I like where this is headed 👍

eliasjpr · 2017-07-16T12:57:25Z

Comments have been addressed. When setting expires: 0 to the session settings, sessions will only last until the browser is closed.

drujensen · 2017-07-16T13:52:52Z

LGTM

Please squash the commits before merging.

Add tickets for re-enabling cookie optimization and encryption options.

drujensen

I created a new amber project and scaffolding:

amber new blog -d mysql --deps
cd blog
amber g scaffold post title:string body:text

I updated amber to point to this branch:
branch: ep/session-storage
shards update

I added the config for cookie session to the config/application.cr

Amber::Server.instance.session = {
  :key     => "blog.session",
  :store   => :cookie,
  :expires => 120,
  :secret  => "secret"
}

I launched the app and created a new blog post.

The flash message for flash["success"] is not being displayed as expected.

eliasjpr · 2017-07-17T13:15:57Z

@drujensen the reason that the flash message is not being rendered for the new generated app is that the session pipe needs to come before the flash and csrf pipe, as stated above in the PR description.

Currently the amber new {project name} will add the session to the end of the pipeline.

 # Keep in mind the order of the Pipes. Session hash needs to be populatedd before trying to access the session flash scope, the flash depends of the session. 
 pipeline :web do
    plug Amber::Pipe::Session.new
    plug Amber::Pipe::Logger.new
    plug Amber::Pipe::Flash.new
    plug Amber::Pipe::CSRF.new
  end

drujensen · 2017-07-17T13:39:57Z

@eliasjpr moving the session fixed it. I also tested with redis and everything seems to be working correctly.

elorest

Ok we can deal with slow downs and options for encrypted vs signed later. Lets get this merged.

* Adds Session Management - Adds Cookies Session using new cookies storage - Adds Redis Session storage * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management * fixup! Adds Session Management

eliasjpr and others added 11 commits July 15, 2017 14:06

Adds Session Management

15c1ad7

- Adds Cookies Session using new cookies storage - Adds Redis Session storage

fixup! Adds Session Management

2f5d904

fixup! Adds Session Management

53dd419

fixup! Adds Session Management

bf57a86

fixup! Adds Session Management

dad1e33

fixup! Adds Session Management

460b5b5

fixup! Adds Session Management

5b109f2

fixup! Adds Session Management

8e57ba3

fixup! Adds Session Management

b216bec

fixup! Adds Session Management

f7422b7

fixup! Adds Session Management

d5aa79c

eliasjpr added kind:enhancement WIP labels Jul 15, 2017

eliasjpr assigned drujensen and bew Jul 15, 2017

eliasjpr requested a review from fridgerator July 15, 2017 19:19

eliasjpr unassigned drujensen and bew Jul 15, 2017

eliasjpr requested review from drujensen, mixflame and bew July 15, 2017 19:23

eliasjpr force-pushed the ep/session-storage branch from 9c2167a to fcbfeb9 Compare July 15, 2017 20:56

fixup! Adds Session Management

893ccae

eliasjpr force-pushed the ep/session-storage branch from fcbfeb9 to 893ccae Compare July 15, 2017 20:58

elorest reviewed Jul 15, 2017

View reviewed changes

elorest requested changes Jul 15, 2017

View reviewed changes

fixup! Adds Session Management

825d6de

eliasjpr force-pushed the ep/session-storage branch from b938056 to 825d6de Compare July 15, 2017 22:05

fixup! Adds Session Management

cd1ffb8

eliasjpr force-pushed the ep/session-storage branch from 8f31748 to 08cc2d4 Compare July 15, 2017 22:13

elorest reviewed Jul 16, 2017

View reviewed changes

fridgerator reviewed Jul 16, 2017

View reviewed changes

fixup! Adds Session Management

e9d49a1

eliasjpr force-pushed the ep/session-storage branch from 08cc2d4 to e9d49a1 Compare July 16, 2017 12:23

Merge branch 'master' into ep/session-storage

b0c7891

drujensen requested changes Jul 16, 2017

View reviewed changes

elorest approved these changes Jul 18, 2017

View reviewed changes

fixup! Adds Session Management

61c0ba0

eliasjpr merged commit c540fbb into master Jul 18, 2017

eliasjpr deleted the ep/session-storage branch July 18, 2017 15:13

eliasjpr mentioned this pull request Jul 18, 2017

Cookies: Look up Cookies is Set-Cookie header #135

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rails like session/flash management for Amber. #138

Rails like session/flash management for Amber. #138

eliasjpr commented Jul 15, 2017 •

edited

Loading

elorest Jul 15, 2017

eliasjpr Jul 16, 2017 •

edited

Loading

drujensen Jul 16, 2017

eliasjpr Jul 17, 2017

elorest left a comment

elorest Jul 15, 2017

elorest Jul 15, 2017

eliasjpr Jul 16, 2017 •

edited

Loading

elorest Jul 15, 2017

elorest Jul 16, 2017

fridgerator left a comment

eliasjpr commented Jul 16, 2017 •

edited

Loading

drujensen commented Jul 16, 2017

drujensen left a comment •

edited

Loading

eliasjpr commented Jul 17, 2017 •

edited

Loading

drujensen commented Jul 17, 2017

elorest left a comment

		@@ -0,0 +1,65 @@
		module Amber::Router::Session
		class SessionHash < Hash(String, String)

Rails like session/flash management for Amber. #138

Rails like session/flash management for Amber. #138

Conversation

eliasjpr commented Jul 15, 2017 • edited Loading

Description of the Change

The Session

The Flash

Flash.now

Possible Drawbacks

elorest Jul 15, 2017

Choose a reason for hiding this comment

eliasjpr Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

drujensen Jul 16, 2017

Choose a reason for hiding this comment

eliasjpr Jul 17, 2017

Choose a reason for hiding this comment

elorest left a comment

Choose a reason for hiding this comment

elorest Jul 15, 2017

Choose a reason for hiding this comment

elorest Jul 15, 2017

Choose a reason for hiding this comment

eliasjpr Jul 16, 2017 • edited Loading

Choose a reason for hiding this comment

elorest Jul 15, 2017

Choose a reason for hiding this comment

elorest Jul 16, 2017

Choose a reason for hiding this comment

fridgerator left a comment

Choose a reason for hiding this comment

eliasjpr commented Jul 16, 2017 • edited Loading

drujensen commented Jul 16, 2017

drujensen left a comment • edited Loading

Choose a reason for hiding this comment

eliasjpr commented Jul 17, 2017 • edited Loading

drujensen commented Jul 17, 2017

elorest left a comment

Choose a reason for hiding this comment

eliasjpr commented Jul 15, 2017 •

edited

Loading

eliasjpr Jul 16, 2017 •

edited

Loading

eliasjpr Jul 16, 2017 •

edited

Loading

eliasjpr commented Jul 16, 2017 •

edited

Loading

drujensen left a comment •

edited

Loading

eliasjpr commented Jul 17, 2017 •

edited

Loading