Authorization setup

[coreypurcell]

use authorization to limit Users/Volunteers/Donors/Donations to signed in users. Allow anyone to access the volunteer signin page.

[coreypurcell]

refer to discussion in #6

[heatlill]

So based on that discussion you guys are looking to start out with adding 3 (boolean volunteer, super-volunteer and admin fields to the user table)? Then in each controller you would want logic validating the current user is logged in and has "true" for one of the predefined authorized roles? I just want to make sure I understand what you guys are looking for.

[semmons99]

I'd just create a new table access_levels with two fields: name and level. Then add the access_level_id to the users table so that it has_one :access_level. Then define <=> on AccessLevel to use #level and you're all set.

# migration
def change
  create_table :access_levels do |t|
    t.string :name, null: false
    t.integer :level, null: false
  end
  AccessLevel.create!(name: "Volunteer", level: 1)
  AccessLevel.create!(name: "Super-Volunteer", level: 2)
  AccessLevel.create!(name: "Admin", level: 3)

  add_column :user, :access_level_id, :integer
end

# model
class AccessLevel < ActiveRecord::Base
  def self.admin
    AccessLevel.where(name: "Admin")
  end

  def self.super_volunteer
    AccessLevel.where(name: "Super-Volunteer")
  end

  def <=>(other)
    self.level <=> other.level
  end
end

# usage
if user.access_level >= AccessLevel.super_volunteer
  # do stuff a super volunteer or higher can do
end

if user.access_level == AccessLevel.admin
  # do stuff only an admin could do
end

In the future, if more levels are needed, just write a migration the increases the existing levels as needed and inserts the new one(s) where appropriate.

@coreypurcell or anyone else; feel free to disagree with any of this. It's late and I'm not sure if it makes any sense.

[heatlill]

Makes sense to me, this just seems a little like roles and I was thinking about what booleans you meant. I like this though.

Thanks for the clarification.

[semmons99]

I was just thinking, we should probably put the creation of the access levels inside of db/seeds/access_level_seeds.rb (or whatever the naming convention is).

[coreypurcell]

I like it. @heatlill feel free to give it a shot. Just make sure you get some test coverage on this feature.

[heatlill]

I am still working on this, it's been a bit of a bit of trial and error with alot of things that won't work (I'm still learning rspec, cucumber, and capybara). Currently I'm looking at doing something like http://collectiveidea.com/blog/archives/2012/01/05/capybara-cucumber-and-how-the-cookie-crumbles/ as a testing solution. Let me know if you have any thoughts.

Thanks.

[coreypurcell]

Looks like a good start. You don't need to test all the edge cases at the cucumber spec level, leave that for lower level specs in rspec. Just ensure that the main paths are covered so that you prove that things are wired up correctly. I'm excited!

[heatlill]

So if a user is NOT an admin and they try to edit a user what should happen? I know we want a 401 response, I'm just not sure how it should be done.

With the structure outline above I have the user_controller.rb looking like...

class UsersController < ApplicationController
  expose(:user)
  expose(:users)
  before_filter :authenticate_user!

  def create 
    unless has_access?
      render_unauthorized 
    end
    if user.save
      redirect_to users_path, :notice => "User successfully created"
    else
      render :new
    end
  end

  def edit
  end

  def update
    unless has_access?
      render_unauthorized
    end
    if user.save
      redirect_to users_path, :notice => "Successfully updated the user."
    else
      render :edit
    end
  end

  private

  def has_access?
    current_user.admin?   
  end

  def allowable
    [
      :email, :password, :password_confirmation, :remember_me,
      :first_name, :last_name, :password_digest
    ]
  end

  def user_params
    params.require(:user).permit(*allowable)
  end

  def render_unauthorized
    head 401
  end

end

Looking for input. Thanks.

[semmons99]

FIrst thing I'd do is create another before_filter to handle the repeated unless has_access? block. You could also update #user_params to check for #has_access? and have it error there.

[coreypurcell]

@heatlill I gave you commit rights, if you want, you can create a pull request directly on this repo that way we can collaborate on the pull request. Or you can just create a pull request.

You might want to redirect_to instead of just head 401, so maybe

redirect_to root_url, :alert => "You are not authorized.", :status => :unauthorized

[heatlill]

@semmons99 I thought the has_access? call could be in the before_filter, I just wasn't sure, thanks for the suggestions I'll make it happen.

@coreypurcell Thanks for setting me up with rights, I've been holding off committing my changes because I want the tests to pass before I get too deep into a code review. However, I it's time for a review. Let me make the above adjustments and I'll make the request as I get time. Thanks again.