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

Feature/GitHub personal access token set #2060

Open
wants to merge 18 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 7 additions & 6 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,8 @@
#
FROM phusion/passenger-ruby32:2.6.1

MAINTAINER Autolab Development Team "autolab-dev@andrew.cmu.edu"

# Change to your time zone here
RUN ln -fs /usr/share/zoneinfo/America/New_York /etc/localtime
RUN ln -fs /usr/share/zoneinfo/America/Los_Angeles /etc/localtime

# Install dependencies
ENV DEBIAN_FRONTEND=noninteractive
Expand Down Expand Up @@ -55,11 +53,14 @@ RUN mkdir -p /home/app/webapp/log && \
chmod 0664 /home/app/webapp/log/production.log && \
chown -R app:app .

USER app
USER root
RUN chown -R app:app .

#WORKDIR /home/app/webapp

# precompile the Rails assets
ENV SECRET_KEY_BASE=dummy_secret_key_base
RUN RAILS_ENV=production bundle exec rails assets:precompile
ENV SECRET_KEY_BASE="ekz0nrn_DPG6ucv5ukw"
Copy link
Contributor

Choose a reason for hiding this comment

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

The SECRET_KEY_BASE environment variable has been hardcoded into the Dockerfile. This is a security risk as it exposes the secret key in the Docker image and thus to anyone who can access the image.

- ENV SECRET_KEY_BASE="ekz0nrn_DPG6ucv5ukw"
+ ENV SECRET_KEY_BASE=${SECRET_KEY_BASE}

And ensure that the SECRET_KEY_BASE is passed as a build argument or environment variable at runtime.


Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
ENV SECRET_KEY_BASE="ekz0nrn_DPG6ucv5ukw"
ENV SECRET_KEY_BASE=${SECRET_KEY_BASE}

#RUN RAILS_ENV=production bundle exec rails assets:precompile

# Clean up APT when done.
USER root
Expand Down
21 changes: 19 additions & 2 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ class UsersController < ApplicationController
skip_before_action :update_persistent_announcements
before_action :set_gh_oauth_client, only: [:github_oauth, :github_oauth_callback]
before_action :set_user,
only: [:github_oauth, :github_revoke, :lti_launch_initialize,
:lti_launch_link_course]
only: [:github_oauth, :github_revoke,
:show_github_update_form, :submit_github_update_form,
:lti_launch_initialize, :lti_launch_link_course]

# GET /users
action_auth_level :index, :student
Expand Down Expand Up @@ -249,6 +250,22 @@ def lti_launch_link_course
flash[:success] = "#{course.name} successfully linked."
redirect_to(course)
end

action_auth_level :github_update, :student
def show_github_update_form
@github_integration = GithubIntegration.find_by(user_id: @user.id)
render('github_update')
end

action_auth_level :github_update_form, :student
def submit_github_update_form
github_integration = GithubIntegration.find_by(user_id: @user.id)

access_token = params[:access_token]
github_integration.update!(access_token:)
flash[:success] = "Updated Github Token"
redirect_to(user_path)
end
Comment on lines +254 to +268
Copy link
Contributor

Choose a reason for hiding this comment

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

The new actions show_github_update_form and submit_github_update_form have been added with appropriate authorization levels. However, there is no error handling for the case where GithubIntegration.find_by(user_id: @user.id) returns nil, which could lead to a NoMethodError when calling update! on nil.

- github_integration.update!(access_token:)
+ if github_integration
+   github_integration.update!(access_token:)
+   flash[:success] = "Updated Github Token"
+ else
+   flash[:error] = "GitHub integration not found."
+ end

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
action_auth_level :github_update, :student
def show_github_update_form
@github_integration = GithubIntegration.find_by(user_id: @user.id)
render('github_update')
end
action_auth_level :github_update_form, :student
def submit_github_update_form
github_integration = GithubIntegration.find_by(user_id: @user.id)
access_token = params[:access_token]
github_integration.update!(access_token:)
flash[:success] = "Updated Github Token"
redirect_to(user_path)
end
action_auth_level :github_update, :student
def show_github_update_form
@github_integration = GithubIntegration.find_by(user_id: @user.id)
render('github_update')
end
action_auth_level :github_update_form, :student
def submit_github_update_form
github_integration = GithubIntegration.find_by(user_id: @user.id)
access_token = params[:access_token]
if github_integration
github_integration.update!(access_token:)
flash[:success] = "Updated Github Token"
else
flash[:error] = "GitHub integration not found."
end
redirect_to(user_path)
end


action_auth_level :github_oauth, :student
def github_oauth
Expand Down
2 changes: 1 addition & 1 deletion app/helpers/assessment_autograde_core.rb
Original file line number Diff line number Diff line change
Expand Up @@ -569,4 +569,4 @@ def extend_config_module(assessment, submission, cud)
raise error
end

end
end
13 changes: 13 additions & 0 deletions app/views/users/github_update.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<% @title = "Github Access Token" %>

<h2>Github Update Access Token</h2>
<% if @github_integration %>
<%= form_with url: github_update_user_path do |form| %>
<div class="input-field">
<%= form.text_field :access_token, required: true %>
<%= form.label :access_token, "Access Token" %>
<span class="helper-text">Helpful description here</span>
</div>
<%= form.submit "Update Token", { class: "btn primary" } %>
<% end %>
<% end %>
Comment on lines +1 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

The view template for updating the GitHub access token is straightforward and seems to follow Rails conventions. However, there is a placeholder text "Helpful description here" which should be replaced with an actual description to guide the user.

- <span class="helper-text">Helpful description here</span>
+ <span class="helper-text">Enter your GitHub personal access token.</span>

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
<% @title = "Github Access Token" %>
<h2>Github Update Access Token</h2>
<% if @github_integration %>
<%= form_with url: github_update_user_path do |form| %>
<div class="input-field">
<%= form.text_field :access_token, required: true %>
<%= form.label :access_token, "Access Token" %>
<span class="helper-text">Helpful description here</span>
</div>
<%= form.submit "Update Token", { class: "btn primary" } %>
<% end %>
<% end %>
<% @title = "Github Access Token" %>
<h2>Github Update Access Token</h2>
<% if @github_integration %>
<%= form_with url: github_update_user_path do |form| %>
<div class="input-field">
<%= form.text_field :access_token, required: true %>
<%= form.label :access_token, "Access Token" %>
<span class="helper-text">Enter your GitHub personal access token.</span>
</div>
<%= form.submit "Update Token", { class: "btn primary" } %>
<% end %>
<% end %>

6 changes: 6 additions & 0 deletions app/views/users/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,12 @@
<% end %>
<% if GithubIntegration.connected %>
<% if @user.github_integration && @user.github_integration.is_connected %>
<%= link_to github_update_user_path(@user) do %>
<span class="btn primary">
Update Github Token
</span>
<% end %>

<%= link_to github_revoke_user_path(@user), data: { method: "post" } do %>
<span class="btn primary">
Revoke Github Token
Expand Down
3 changes: 2 additions & 1 deletion config/initializers/assets.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
Rails.application.config.assets.precompile += %w( eventdrops.css )
Rails.application.config.assets.precompile += %w( metricsgraphics.scss )
Rails.application.config.assets.precompile += %w( users.css.scss )
Rails.application.config.assets.precompile += %w( bootstrap-datetimepicker.css )

Rails.application.config.assets.precompile += %w( *.js )
Rails.application.config.assets.precompile += %w( annotations.js )
Expand Down Expand Up @@ -68,4 +69,4 @@
Rails.application.config.assets.precompile += %w( initialize_datetimepickers.js )
Rails.application.config.assets.precompile += %w( jquery.stickytableheaders.js )
Rails.application.config.assets.precompile += %w( pdf.js )
Rails.application.config.assets.precompile += %w( course_users.js )
Rails.application.config.assets.precompile += %w( course_users.js )
2 changes: 2 additions & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@
get "lti_launch_initialize", on: :member
post "lti_launch_link_course", on: :member
post "github_revoke", on: :member
get "github_update", on: :member, to: 'users#show_github_update_form'
post 'github_update', on: :member, to: 'users#submit_github_update_form'
get "github_oauth_callback", on: :collection
match "update_password_for_user", on: :member, via: [:get, :put]
post "change_password_for_user", on: :member
Expand Down