-
Notifications
You must be signed in to change notification settings - Fork 222
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
base: master
Are you sure you want to change the base?
Feature/GitHub personal access token set #2060
Conversation
… personal access token, PAT)
WalkthroughWalkthroughThe recent updates to the application focus on enhancing GitHub integration by allowing users to update their GitHub access tokens through new actions in the UsersController. The Dockerfile has been adjusted to change the server's time zone and user permissions. The Rails application's environment has been tweaked, and the assets precompilation list now includes a new CSS file. Additionally, the routing has been updated to support the new GitHub token update functionality. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
I have a fork with updates to the Dockerfile. Did not intend to commit those, only updates in /app and to routes. I am going to close this with comment in case there is discussion and prepare a better merge source for the weekend. |
<% @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 %> |
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.
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.
<% @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 %> |
|
||
# 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" |
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.
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.
ENV SECRET_KEY_BASE="ekz0nrn_DPG6ucv5ukw" | |
ENV SECRET_KEY_BASE=${SECRET_KEY_BASE} |
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 |
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.
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.
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 |
Does #2061 supersede this? |
Add minimal (PoC) "update github" token with text field. Allow user/student to manually enter an override AFTER the OAuth flow completes. Useful for user controlled access tokens, potentially at a "per assignment" level.
Summary
New button on user page (if Github integaration): "update github token". Single field form. Overwrite (ActiveRecord encrpting) existing token. These appear to be universal across the API and do nor require individual attention.
Incomplete tests. I am teaching an overload this term and already behind. Not able to build out the spec for the submission. Happy to assist others but I cannot be lead.