Skip to content
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.

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

Edges Chantelle MediaRanker_Revisited #21

Open
wants to merge 25 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3e9d835
added Oauth login
BASIC-Belic Oct 16, 2018
4938e3e
temp solution for resolving if username does not exist in User.build_…
BASIC-Belic Oct 16, 2018
cdfb2d2
changed sessionscontroller#create to match the flash logic in layouts…
BASIC-Belic Oct 16, 2018
33b633e
changed sessions#destroy flash to flash instead of flash.now bc styli…
BASIC-Belic Oct 16, 2018
e76ecaf
also changed sessions#create else statement from flash.now to flash
BASIC-Belic Oct 16, 2018
590f875
replaced User.find(session[:user_id]) with @login_user in application…
BASIC-Belic Oct 16, 2018
154f990
changed users.yml file to include uid and provider
BASIC-Belic Oct 16, 2018
e23cb4a
root test working, first test for work#index
BASIC-Belic Oct 16, 2018
c606b74
added new and create workscontroller tests
BASIC-Belic Oct 16, 2018
e298e80
added workscontroller edit test
BASIC-Belic Oct 16, 2018
8542b50
added test for workscontroller #update
BASIC-Belic Oct 16, 2018
da50420
destroy testing done
BASIC-Belic Oct 16, 2018
67722b1
changed else in User self.build_from_github(auth_hash) method
BASIC-Belic Oct 16, 2018
eceafc0
changed application html to show logged in username not name
BASIC-Belic Oct 16, 2018
c24d05e
uncommented read me
BASIC-Belic Oct 28, 2018
06a926e
added uid and provider to failing model test
BASIC-Belic Oct 29, 2018
4e897b9
fixed typos in omniauth
BASIC-Belic Oct 29, 2018
403012c
added authorization, flash not working
BASIC-Belic Oct 29, 2018
9371420
got works test to green again with auth
BASIC-Belic Oct 29, 2018
6849579
upvote controller tests
BASIC-Belic Oct 30, 2018
1c19d40
user controller tests
BASIC-Belic Oct 30, 2018
6bb24d8
sessions controller test #create
BASIC-Belic Oct 30, 2018
a518c65
sessions #destro
BASIC-Belic Oct 30, 2018
2a9a6c8
more works controller testing for guest
BASIC-Belic Oct 30, 2018
53b48de
Modified the edit and delete functionality to ensure that users can o…
BASIC-Belic Oct 30, 2018
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
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,6 @@

# Ignore Byebug command history file.
.byebug_history

#Storing Credentials
.env
6 changes: 6 additions & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ group :development do
# Spring speeds up development by keeping your application running in the background. Read more: https://github.com/rails/spring
gem 'spring'
gem 'spring-watcher-listen', '~> 2.0.0'
#add .env to dev group
gem 'dotenv-rails'
end

# Windows does not include zoneinfo files, so bundle the tzinfo-data gem
gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby]

#Installing OmniAuth
gem "omniauth"
gem "omniauth-github"
28 changes: 28 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -70,11 +70,18 @@ GEM
concurrent-ruby (1.0.5)
crass (1.0.4)
debug_inspector (0.0.3)
dotenv (2.5.0)
dotenv-rails (2.5.0)
dotenv (= 2.5.0)
railties (>= 3.2, < 6.0)
erubi (1.7.1)
execjs (2.7.0)
faraday (0.15.3)
multipart-post (>= 1.2, < 3)
ffi (1.9.25)
globalid (0.4.1)
activesupport (>= 4.2.0)
hashie (3.5.7)
i18n (1.1.0)
concurrent-ruby (~> 1.0)
jbuilder (2.7.0)
Expand All @@ -84,6 +91,7 @@ GEM
rails-dom-testing (>= 1, < 3)
railties (>= 4.2.0)
thor (>= 0.14, < 2.0)
jwt (2.1.0)
listen (3.0.8)
rb-fsevent (~> 0.9, >= 0.9.4)
rb-inotify (~> 0.9, >= 0.9.7)
Expand Down Expand Up @@ -113,9 +121,26 @@ GEM
minitest (~> 5.0)
rails (>= 4.1)
multi_json (1.13.1)
multi_xml (0.6.0)
multipart-post (2.0.0)
nio4r (2.3.1)
nokogiri (1.8.4)
mini_portile2 (~> 2.3.0)
oauth2 (1.4.1)
faraday (>= 0.8, < 0.16.0)
jwt (>= 1.0, < 3.0)
multi_json (~> 1.3)
multi_xml (~> 0.5)
rack (>= 1.2, < 3)
omniauth (1.8.1)
hashie (>= 3.4.6, < 3.6.0)
rack (>= 1.6.2, < 3)
omniauth-github (1.3.0)
omniauth (~> 1.5)
omniauth-oauth2 (>= 1.4.0, < 2.0)
omniauth-oauth2 (1.5.0)
oauth2 (~> 1.1)
omniauth (~> 1.2)
pg (0.21.0)
popper_js (1.14.3)
pry (0.11.3)
Expand Down Expand Up @@ -207,13 +232,16 @@ DEPENDENCIES
bootstrap (~> 4.1.3)
byebug
coffee-rails (~> 4.2)
dotenv-rails
jbuilder (~> 2.5)
jquery-rails
listen (~> 3.0.5)
minitest-rails
minitest-reporters
minitest-skip
minitest-spec-rails
omniauth
omniauth-github
pg (~> 0.18)
pry-rails
puma (~> 3.0)
Expand Down
32 changes: 16 additions & 16 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ Take some time to understand what each controller is doing. Add tests to the exi
- Tests custom controller logic and custom routes when appropriate
- Tests positive, negative, nominal and edge cases

<!-- ## Wave 2: Authentication via OAuth
## Wave 2: Authentication via OAuth

Following the steps in the Textbook curriculum, add OAuth to your Media Ranker Application and enable a user to log in.

Expand All @@ -53,33 +53,33 @@ Following the steps in the Textbook curriculum, add OAuth to your Media Ranker A
- All other requirements from in-class notes apply:
- Managed via `session`
- `SessionsController`
- `User` model -->
- `User` model


<!-- ## Wave 3: Basic Authorization (Page Access)
## Wave 3: Basic Authorization (Page Access)

In this wave we will create authorization logic to enforce rules that govern what pages on the site users and guests (unauthenticated browsers) can view. The rule we'll use is that guests can only access the main page, and all logged-in users can access the show and index pages for all categories of work.
In this wave we will create authorization logic to enforce rules that govern what pages on the site users and guests (unauthenticated browsers) can view. The rule we'll use is that guests can only access the main page, and all logged-in users can access the show and index pages for all categories of work.

### Requirements
- Ensure that users who are not logged in can see *only* the main page with the spotlight and top 10 items. No other pages should be viewable by the guest user.
- Ensure that users who are logged in can see the rest of the pages.
- Full unit testing around authentication using mocks
### Requirements
- Ensure that users who are not logged in can see *only* the main page with the spotlight and top 10 items. No other pages should be viewable by the guest user.
- Ensure that users who are logged in can see the rest of the pages.
- Full unit testing around authentication using mocks


## Optional Wave 4: Advanced Authorization (Ownership)
## Optional Wave 4: Advanced Authorization (Ownership)

Create advanced authorization logic to enforce rules that govern what _changes_ users can make to the site's data. The rules here are more complex than for accessing pages:
- Guests cannot change any data on the site
- All logged-in users can add new works to the site
Create advanced authorization logic to enforce rules that govern what _changes_ users can make to the site's data. The rules here are more complex than for accessing pages:
- Guests cannot change any data on the site
- All logged-in users can add new works to the site
- Those works are owned by the user that created them
- The user who owns a given work can:
- The user who owns a given work can:
- Edit that work
- Delete that work

### Tasks
- Modify the edit and delete functionality to ensure that users can only change works they are associated with.
### Tasks
- Modify the edit and delete functionality to ensure that users can only change works they are associated with.
- Consider how this could be implemented at the model layer.
- Do some research into how to use Google or another OAuth provider for authentication and use that provider. -->
- Do some research into how to use Google or another OAuth provider for authentication and use that provider.

## Due Date
This project is due before class Monday October 29 via PR against Ada-C10/MediaRanker-Revisited.
Expand Down
3 changes: 2 additions & 1 deletion app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ def render_404
raise ActionController::RoutingError.new('Not Found')
end

private
private

def find_user
if session[:user_id]
@login_user = User.find_by(id: session[:user_id])
Expand Down
72 changes: 44 additions & 28 deletions app/controllers/sessions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,49 @@ class SessionsController < ApplicationController
def login_form
end

def login
username = params[:username]
if username and user = User.find_by(username: username)
session[:user_id] = user.id
flash[:status] = :success
flash[:result_text] = "Successfully logged in as existing user #{user.username}"
else
user = User.new(username: username)
if user.save
session[:user_id] = user.id
flash[:status] = :success
flash[:result_text] = "Successfully created new user #{user.username} with ID #{user.id}"
else
flash.now[:status] = :failure
flash.now[:result_text] = "Could not log in"
flash.now[:messages] = user.errors.messages
render "login_form", status: :bad_request
return
end
end
redirect_to root_path
end
def create
auth_hash = request.env['omniauth.auth']

def logout
session[:user_id] = nil
flash[:status] = :success
flash[:result_text] = "Successfully logged out"
redirect_to root_path
end

#UID specific to user for this provider
user = User.find_by(uid: auth_hash[:uid], provider: auth_hash[:provider])
if user
# User was found in the database
session[:user_id] = user.id
flash[:status] = :success
flash[:result_text] = "Logged in as returning user #{user.name}"
else
# User doesn't match anything in the DB
# Attempt to create a new user

user = User.build_from_github(auth_hash)

if user.save
flash[:status] = :success
flash[:result_text] = "Logged in as new user #{user.name}"
else
# Couldn't save the user for some reason. If we hit this it probably means there's a bug with the
# way we've configured GitHub. Our strategy will
# be to display error messages to make future
# debugging easier.
flash.now[:status] = :failure
flash[:result_text] = "Could not create new user account: "
flash[:messages] = user.errors.messages

redirect_to root_path
end
end

Choose a reason for hiding this comment

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

You redirect but don't return here, and then redirect again on line 40. This is a bug, and will cause Rails to throw an error! Your test coverage probably should have caught this...


# If we get here, we have a valid user instance
session[:user_id] = user.id
redirect_to root_path
end

def destroy
session[:user_id] = nil
flash[:status] = :success
flash[:result_text] = "Successfully logged out!"

redirect_to root_path
end
end
11 changes: 11 additions & 0 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
class UsersController < ApplicationController
before_action :require_login

def index
@users = User.all
end
Expand All @@ -8,3 +10,12 @@ def show
render_404 unless @user
end
end

private

def require_login
unless session[:user_id]
flash[:error] = "You must be logged in to do that."
redirect_to root_path
end
end
24 changes: 22 additions & 2 deletions app/controllers/works_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ class WorksController < ApplicationController
# We should always be able to tell what category
# of work we're dealing with
before_action :category_from_work, except: [:root, :index, :new, :create]
before_action :require_login, except: [:root, :show, :upvote]

def root
@albums = Work.best_albums
Expand All @@ -21,6 +22,7 @@ def new
def create
@work = Work.new(media_params)
@media_category = @work.category

if @work.save
flash[:status] = :success
flash[:result_text] = "Successfully created #{@media_category.singularize} #{@work.id}"
Expand All @@ -38,6 +40,11 @@ def show
end

def edit
if session[:user_id] != @work.user_id
redirect_back fallback_location: root_path
flash[:status] = :error
flash[:result_text] = "Sorry, can't edit a work that you don't own."
end
end

def update
Expand All @@ -50,11 +57,16 @@ def update
flash.now[:status] = :failure
flash.now[:result_text] = "Could not update #{@media_category.singularize}"
flash.now[:messages] = @work.errors.messages
render :edit, status: :not_found
render :edit, status: :bad_request
end
end

def destroy
if session[:user_id] != @work.user_id
redirect_back fallback_location: root_path
flash[:status] = :error
flash[:result_text] = "Sorry, can't edit a work that you don't own."
end
@work.destroy
flash[:status] = :success
flash[:result_text] = "Successfully destroyed #{@media_category.singularize} #{@work.id}"
Expand All @@ -81,7 +93,7 @@ def upvote
redirect_back fallback_location: work_path(@work)
end

private
private
def media_params
params.require(:work).permit(:title, :category, :creator, :description, :publication_year)
end
Expand All @@ -91,4 +103,12 @@ def category_from_work
render_404 unless @work
@media_category = @work.category.downcase.pluralize
end

def require_login
unless session[:user_id]

flash[:error] = "You must be logged in to do that."

Choose a reason for hiding this comment

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

I like that this is a controller filter, but you've got much the same code both here and in the users controller. Could you DRY it up further by putting this in the application controller?

redirect_to root_path
end
end
end
11 changes: 11 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,15 @@ class User < ApplicationRecord
has_many :ranked_works, through: :votes, source: :work

validates :username, uniqueness: true, presence: true

def self.build_from_github(auth_hash)
user = User.new(provider: auth_hash['provider'], uid: auth_hash['uid'], username: auth_hash['info']['nickname'], name: auth_hash['info']['name'], email: auth_hash['info']['email'], image_url: auth_hash['info']['image'])
if auth_hash['info']['nickname'] == nil
#temp solution for resolving if username does not exist
user.username = auth_hash['email']
# elsif user.username = auth_hash['email'] == nil
#method to generate random username
end
return user
end
end
1 change: 1 addition & 0 deletions app/models/work.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
class Work < ApplicationRecord
CATEGORIES = %w(album book movie)
belongs_to :user
has_many :votes, dependent: :destroy
has_many :ranking_users, through: :votes, source: :user

Expand Down
19 changes: 6 additions & 13 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -33,22 +33,15 @@
</li>
</ul>
<ul class="nav app-header__user-nav-container">
<% if @login_user %>

<li class="nav-item app-header__nav_item">
<%= link_to "Logged in as #{@login_user.username}", user_path(@login_user), class: "btn btn-primary" %>
</li>
<li class="nav-item app-header__nav_item">
<%= link_to "Log Out", logout_path, method: :post, class: "btn btn-primary" %>
</li>

<% if @login_user %>
<p>Currently logged in as user
<%= @login_user.username %>
<%= link_to "Log out", logout_path, method: "delete" %>
<% else %>

<li class="nav-item app-header__nav_item">
<%= link_to "Log In", login_path, class: "btn btn-primary" %>
</li>
<%= link_to "Login with Github", "/auth/github" %>
<% end %>

</p>
</ul>
</nav>
</header>
Expand Down
Binary file added config/initializers/404_page_template.jpg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
4 changes: 4 additions & 0 deletions config/initializers/omniauth.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# config/initializers/omniauth.rb
Rails.application.config.middleware.use OmniAuth::Builder do
provider :github, ENV["GITHUB_CLIENT_ID"], ENV["GITHUB_CLIENT_SECRET"], scope: "user:email"
end
Loading