Question on User authentication - devise filters missing

Mirv commented 7 years ago

You've got a ton of controllers and not all of them have the devise authentication check ... is this by design? So people can read the site without logging in?

It came up because in many of your pages you are relying on "current_user" but you don't have a "current_user" if not logged in (or if you don't provision some special invisible default guest access user) ...

@jatwell93

Searching for before_action :authenticate_user! inyour entire workspace (Found 6 matches in 6 files)

/app/controllers/conversations_controller.rb: 2: before_action :authenticate_user!

/app/controllers/exercises_controller.rb: 2: before_action :authenticate_user!

/app/controllers/friendships_controller.rb: 2: before_action :authenticate_user!

/app/controllers/mailbox_controller.rb: 2: before_action :authenticate_user!

/app/controllers/recipes_controller.rb: 5: before_action :authenticate_user!, except: [:show, :index, :like, :search]

/app/controllers/workouts_controller.rb: 3: before_action :authenticate_user!, except: [:show, :index, :like, :search]

Found 6 matches in 6 files

jatwell93 commented 7 years ago

No thats not by design, thats purely an error on my end. Would it be best to go through and add the filter onto each controller? Although maybe we want to allow access to some parts of the application without logging in? such as; recipes page and workouts page?

Mirv commented 7 years ago

That's a user design question - I've seen those ones where you can see the preview of the receipes & then when you click it's like "BAM SIGN UP TO SEE FOR FREE!". It would be relatively easy to add the "before_filter" action to the application_controller", then insert except for ":index" and ":show" on each individual controller you want to permit the ones you want?

Mirv commented 7 years ago

Sorry didn't mean to close this one @jatwell93

jatwell93 commented 7 years ago

I think i've fixed this up, i've gone through each of the controllers and added the devise filter for the relevant actions. I wasn't quite sure what you meant by the "before_filter" for the application controller though?

Mirv commented 7 years ago

the before filter is beforeaction or ... doing something and filtering in the controllers is all it means really, sometimes I drop the "" in there to emphasize something & I shouldn't :)

jatwell93 commented 7 years ago

While we're on the topic, should this;

class RecipesController < ApplicationController
  include RecipesHelper
  before_action :set_user, except: [:index, :new]

be to this?

class RecipesController < ApplicationController
  include RecipesHelper
  before_action :set_user, except: [:index, :show, :new]

so users can view the recipes show page without logging in? Currently this generates the error undefined methodid' for nil:NilClass`, when a non-logged in user tries to view a recipe

However adding :show causes an error with the current helper

Mirv commented 7 years ago

I think this is a situation of separate concerns... set_user exists to get the current_user and apply it for making changes - this should not be required or related to index. Also, you are telling me in this post anyone whether they are signed in (identified) or not should see the page.

3 things to try

First, add that set_user & see if it fixes Second, it could be that record doesn't exist for real in the db, verify it via the rails console & do Recipe.find(x) Third, it's an extra on the webpage....start removing chunks of code & adding them back after you find working version

jatwell93 / rubyproject

Question on User authentication - devise filters missing #16