-
Notifications
You must be signed in to change notification settings - Fork 0
IIRR-30 | IIRR-31: Fix auth bug and enable puzzle cloning #62
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
Changes from all commits
fe09489
470bfca
2cede1a
38ceffe
277248f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,9 +2,16 @@ class ApplicationController < ActionController::Base | |
| # Only allow modern browsers supporting webp images, web push, badges, import maps, CSS nesting, and CSS :has. | ||
| allow_browser versions: :modern | ||
| before_action :check_session_expiry | ||
| before_action :check_user_token | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure user is authenticated in all controllers that inherit from ApplicationController |
||
|
|
||
| private | ||
|
|
||
| def check_user_token | ||
| unless session[:user_token] | ||
| render "puzzles/login" | ||
| end | ||
| end | ||
|
|
||
| def check_session_expiry | ||
| if session[:expires_at].present? && Time.current > session[:expires_at] | ||
| reset_session | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| class Puzzles::ClonesController < ApplicationController | ||
| def create | ||
| original_puzzle = Puzzle.find(params[:puzzle_id]) | ||
| attributes = original_puzzle.attributes.slice("question", "answer", "explanation", "link", "suggested_by") | ||
| cloned_puzzle = Puzzle.new(attributes.merge(original_puzzle:, state: params.fetch(:state, "pending"))) | ||
|
|
||
| if cloned_puzzle.save | ||
| redirect_to puzzles_path, notice: "Puzzle cloned. You can now edit the new puzzle." | ||
| else | ||
| redirect_to puzzles_path, alert: "Failed to clone puzzle." | ||
| end | ||
| end | ||
| end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| class SessionsController < ApplicationController | ||
| skip_before_action :check_user_token | ||
|
|
||
| def create | ||
| auth = request.env["omniauth.auth"] | ||
| user_email = auth.info.email | ||
|
|
||
| domain_allowlist = ENV.fetch("DOMAIN_ALLOWLIST").split(",").map(&:strip) | ||
| domain_allowlist = ENV.fetch("DOMAIN_ALLOWLIST", "").split(",").map(&:strip) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix a CI failure where |
||
| if domain_allowlist.present? | ||
| unless domain_allowlist.any? { |domain| user_email.end_with?("@#{domain}") } | ||
| reset_session | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,7 @@ | ||
| class Slack::ApplicationController < ApplicationController | ||
| skip_before_action :verify_authenticity_token | ||
| skip_before_action :check_user_token | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Slack controllers don't relay on omniauth credentials. |
||
|
|
||
| before_action :valid_slack_request? | ||
|
|
||
| private | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,6 +42,7 @@ | |
| <%= button_to 'Pending', puzzle_state_path(puzzle, state: :pending), method: :patch, form_class: 'inline-form', class: 'btn pending-btn' %> | ||
| <% elsif actions == :archived %> | ||
| <%= button_to 'Un-Archive', puzzle_state_path(puzzle, state: :pending), method: :patch, form_class: 'inline-form', class: 'btn pending-btn' %> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we want to keep the Un-Archive button? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should since clone is being added, so we do not have to use un-archive. |
||
| <%= button_to 'Clone', puzzle_clone_path(puzzle, state: :pending), method: :post, form_class: 'inline-form', class: 'btn pending-btn' %> | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cloned puzzled state can be set here. |
||
| <% end %> | ||
| </td> | ||
| </tr> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| class AddOriginalPuzzleToPuzzlesTable < ActiveRecord::Migration[8.0] | ||
| def change | ||
| add_reference :puzzles, :original_puzzle, foreign_key: { to_table: :puzzles }, index: true, null: true | ||
| end | ||
| end |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| require "test_helper" | ||
|
|
||
| class Puzzles::ClonesControllerTest < ActionDispatch::IntegrationTest | ||
| test "creates a cloned puzzle and redirects to index" do | ||
| original = puzzles(:one) | ||
|
|
||
| sign_in | ||
| assert_difference("Puzzle.count", 1) do | ||
| post puzzle_clone_path(original) | ||
| end | ||
|
|
||
| cloned = Puzzle.order(:id).last | ||
| assert_redirected_to puzzles_path | ||
|
|
||
| assert_equal original.id, cloned.original_puzzle_id | ||
| assert_equal original.question, cloned.question | ||
| assert_equal original.answer, cloned.answer | ||
| assert_equal original.explanation, cloned.explanation | ||
| assert_equal original.link, cloned.link | ||
| assert_equal original.suggested_by, cloned.suggested_by | ||
| assert_nil cloned.sent_at | ||
| assert_equal "pending", cloned.state | ||
| end | ||
|
|
||
| test "does not allow unauthenticated users to create a clone" do | ||
| original = puzzles(:one) | ||
|
|
||
| assert_no_difference("Puzzle.count") do | ||
| post puzzle_clone_path(original) | ||
| end | ||
|
|
||
| assert_dom "p", "Log in to access the Ruby or Rails admin panel." | ||
| end | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For CI to pass