Upgrade deps, csrf check, few small improvements (#24)

* New amber, granite-orm, multi_auth

* Enable CSRF plug

* Fix type names in a dropdown on Announcement#new edit pages

* Encrypted cookie + csrf fixes

* CSRF post/put/delete actions specs

README.md

@@ -1,6 +1,6 @@
 # Crystal [ANN]
 [![Build Status](https://travis-ci.org/veelenga/crystal-ann.svg?branch=master)](https://travis-ci.org/veelenga/crystal-ann)
-[![Amber Framework](https://img.shields.io/badge/using-amber%20framework-orange.svg)](https://github.com/veelenga/crystal-ann)
+[![Amber Framework](https://img.shields.io/badge/using-amber%20framework-orange.svg)](http://www.ambercr.io/)
 [![Gitter](https://badges.gitter.im/veelenga/crystal-ann.svg)](https://gitter.im/veelenga/crystal-ann?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge)
 
 Source code for https://crystal-ann.com.

config/application.cr

@@ -15,7 +15,7 @@ end
 
 Amber::Server.instance.session = {
   :key     => "crystal.ann.session",
-  :store   => :cookie,
+  :store   => :encrypted_cookie,
   :expires => 0,
   :secret  => ENV["AMBER_SESSION_SECRET"]? || "",
 }

config/routes.cr

@@ -2,11 +2,10 @@ Amber::Server.instance.config do |app|
   pipeline :web do
     # Plug is the method to use connect a pipe (middleware)
     # A plug accepts an instance of HTTP::Handler
-    # plug Amber::Pipe::Params.new
     plug Amber::Pipe::Logger.new unless app.env == "test"
     plug Amber::Pipe::Flash.new
     plug Amber::Pipe::Session.new
-    # plug Amber::Pipe::CSRF.new
+    plug Amber::Pipe::CSRF.new
   end
 
   # All static content will run these transformations

public/javascripts/main.js

@@ -18,13 +18,22 @@ function signoutListener() {
   if (button) {
     button.addEventListener("click", (e) => {
       e.preventDefault();
+
       var form = document.createElement("form");
       form.method = "POST";
       form.action = "/sessions";
+
       var method = document.createElement("input");
       method.name = "_method";
-      method.value = "DELETE";
+      method.value = "delete";
       form.appendChild(method);
+
+      var csrf = document.createElement("input");
+      csrf.type = "hidden";
+      csrf.name = "_csrf";
+      csrf.value = button.getAttribute("csrf-token");
+      form.appendChild(csrf);
+
       document.body.appendChild(form);
       form.submit();
     });

shard.lock

@@ -2,7 +2,7 @@ version: 1.0
 shards:
   amber:
     github: Amber-Crystal/amber
-    commit: f3f9e3905b0c8a3d435109d10a09ceffe134d5f0
+    version: 0.1.6
 
   baked_file_system:
     github: schovi/baked_file_system
@@ -14,7 +14,7 @@ shards:
 
   granite_orm:
     github: kemalyst/granite-orm
-    version: 0.6.2
+    version: 0.6.4
 
   hashids:
     github: splattael/hashids.cr
@@ -46,7 +46,7 @@ shards:
 
   multi_auth:
     github: msa7/multi_auth
-    commit: 83f7c7bf8cbab7d716daf27caffbb5720df0de26
+    commit: 2fc4db9b60e63d535289550c62eab4ece94aaf71
 
   pg:
     github: will/crystal-pg

shard.yml

@@ -1,8 +1,9 @@
 name: crystal-ann
-version: 0.1.0
+version:
 
 authors:
-  - Amber <contact@ambercr.io>
+  - V. Elenhaupt <velenhaupt@gmail.com>
+  - Hugo Abonizio <hugo_abonizio@hotmail.com>
 
 crystal: 0.23.1
 
@@ -11,11 +12,11 @@ license: MIT
 dependencies:
   amber:
     github: Amber-Crystal/amber
-    branch: master
+    version: 0.1.6
 
   granite_orm:
     github: kemalyst/granite-orm
-    version: ~> 0.6.0
+    version: ~> 0.6.4
 
   pg:
     github: will/crystal-pg
@@ -36,7 +37,6 @@ dependencies:
 
   hashids:
     github: splattael/hashids.cr
-    version: 0.2.1
 
 development_dependencies:
   spec2:

spec/controllers/announcement_controller_spec.cr

@@ -77,6 +77,12 @@ describe AnnouncementController do
         post "/announcements", body: "title=test-title"
         expect(Announcement.all.size).to eq 0
       end
+
+      it "does not create an announcement if csrf token is invalid" do
+        post "/announcements", body: "title=test-title&description=test-description&type=0&_csrf=invalid-token"
+        expect(response.status_code).to eq 403
+        expect(Announcement.all.size).to eq 0
+      end
     end
 
     context "when user not signed-in" do
@@ -194,6 +200,15 @@ describe AnnouncementController do
           patch "/announcements/#{announcement.id}", body: HTTP::Params.encode(invalid_params)
           expect(response.status_code).to eq 200
         end
+
+        it "does not update announcement if csrf token is invalid" do
+          patch "/announcements/#{announcement.id}", body: HTTP::Params.encode(valid_params) + "&_csrf=invalid-token"
+          expect(response.status_code).to eq 403
+          a = Announcement.find(announcement.try &.id).not_nil!
+          expect(a.title).to eq announcement.title
+          expect(a.description).to eq announcement.description
+          expect(a.type).to eq announcement.type
+        end
       end
 
       context "when user cannot update announcement" do
@@ -262,6 +277,12 @@ describe AnnouncementController do
           expect(response.status_code).to eq 302
           expect(response).to redirect_to "/"
         end
+
+        it "does not delete announcement if csrf token is invalid" do
+          delete "/announcements/#{announcement.id}", body: "_csrf=invalid-token"
+          expect(response.status_code).to eq 403
+          expect(Announcement.find announcement.id).not_to be_nil
+        end
       end
 
       context "when user cannot update announcement" do

spec/controllers/sessions_controller_spec.cr

@@ -43,7 +43,7 @@ describe SessionsController do
     end
   end
 
-  describe "POST create" do
+  describe "GET create" do
     before { Announcement.clear; User.clear }
     before { stub_github_authorize_request }
 
@@ -98,6 +98,12 @@ describe SessionsController do
       expect(session["user_id"]).to be_nil
     end
 
+    it "does not sign out user if csrf is invalid" do
+      delete "/sessions", body: "_csrf=invalid-token"
+      expect(response.status_code).to eq 403
+      expect(session["user_id"]).not_to be_nil
+    end
+
     it "redirects to root url" do
       delete "/sessions"
       expect(response.status_code).to eq 302

spec/controllers/spec_helper.cr

@@ -32,7 +32,7 @@ end
 
 {% for method in %w(get head post put patch delete) %}
   def {{method.id}}(path, headers : HTTP::Headers? = nil, body : String? = nil)
-    request = HTTP::Request.new("{{method.id}}".upcase, path, headers, body )
+    request = HTTP::Request.new("{{method.id}}".upcase, path, headers, body)
     request.headers["Content-Type"] = Amber::Router::Params::URL_ENCODED_FORM
     Global.response = process_request request
   end
@@ -43,6 +43,7 @@ def process_request(request)
   response = HTTP::Server::Response.new(io)
   context = HTTP::Server::Context.new(request, response)
   context.session = Global.session if Global.session
+  context.params["_csrf"] ||= Amber::Pipe::CSRF.token(context).to_s
   main_handler = build_main_handler
   main_handler.call context
   response.close

src/models/announcement.cr

@@ -62,6 +62,10 @@ class Announcement < Granite::ORM
     Announcement.all("WHERE created_at > $1 ORDER BY created_at DESC", from)
   end
 
+  def self.typename(type)
+    TYPES[type].split("_").map(&.capitalize).join(" ")
+  end
+
   def next
     Announcement.all("WHERE created_at > $1 ORDER BY created_at LIMIT 1", created_at).first?
   end
@@ -85,7 +89,7 @@ class Announcement < Granite::ORM
   end
 
   def typename
-    TYPES[type].split("_").map(&.capitalize).join(" ")
+    Announcement.typename(type)
   end
 
   def user

src/views/announcement/_form.slang

@@ -6,6 +6,8 @@
 - action = (announcement.id ? "/announcements/#{ announcement.id.to_s }" : "/announcements")
 
 form action="#{ action }" method="post"
+  == csrf_tag
+
   - if announcement.id
     input type="hidden" name="_method" value="patch"
 
@@ -15,11 +17,12 @@ form action="#{ action }" method="post"
 
   select name="type"
     option value="-1" selected="selected" = "-- select type --"
-    - Announcement::TYPES.each do |type, name|
+    - Announcement::TYPES.each do |type, _|
+      - typename = Announcement.typename type
       - if type == announcement.type
-        option value="#{ type }" selected="selected" = name
+        option value="#{ type }" selected="selected" = typename
       - else
-        option value="#{ type }" = name
+        option value="#{ type }" = typename
 
   div.actions
     button type="submit" Submit

src/views/layouts/_nav.slang

@@ -14,6 +14,6 @@ nav#site-navigration.main-navigation
         | About
     - if signed_in?
       li
-        a href="/sessions" id="sign-out-btn"
+        a href="/sessions" id="sign-out-btn" csrf-token="#{csrf_token}"
           i.fa.fa-sign-out
           | Sign out

src/views/layouts/_search.slang

@@ -2,4 +2,4 @@
 
 aside.widget.search
   form name="search" action="/"
-    input.search-input name="query" type="text" pattern=".{3,}" placeholder="Search..." value="#{query}"
+    input.search-input name="query" type="text" placeholder="Search..." value="#{query}"