🧑🔬 Prototype: Encourage MFA for owners of top 100 downloaded gems (CLI)#5
Closed
jenshenny wants to merge 6 commits into
Closed
🧑🔬 Prototype: Encourage MFA for owners of top 100 downloaded gems (CLI)#5jenshenny wants to merge 6 commits into
jenshenny wants to merge 6 commits into
Conversation
**What does this change do:** Currently, a users profile can be queried by their id or handle (`api/v1/profiles/:id|:handle`). This PR adds the ability for an authenticated user to also query their own profile without needing to know their id or pass their handle by making an authenticated call to `api/v1/profile`. **Why is this neccessary:** Once MFA can be enabled on specific API keys through the UI, a user should also be able to enable it on keys that they create during `gem signin` in the CLI. However, we only want to ask a user if they would like to enable mfa on new keys if they have account mfa levels of `ui_only` or `ui_and_gem_sign`. Users that have MFA disabled or those that have it enabled for `ui_and_api ` should not be prompted, as it should be auto enabled or disabled for those levels. Once an owners mfa level can be queried through the API, then enabling an authed user to pull their profile will return their MFA level and help us determine if we should ask them to enable MFA on new keys created during gem signin.
jenshenny
commented
Dec 16, 2021
Comment on lines
+2
to
+22
| before_action :set_user, only: [:show] | ||
|
|
||
| def show | ||
| @user = User.find_by_slug!(params[:id]) | ||
| respond_to do |format| | ||
| format.json { render json: @user } | ||
| format.yaml { render yaml: @user } | ||
| end | ||
| end | ||
|
|
||
| private | ||
|
|
||
| def set_user | ||
| @user = | ||
| if params[:id] | ||
| User.find_by_slug!(params[:id]) | ||
| else | ||
| authenticate_or_request_with_http_basic do |username, password| | ||
| User.authenticate(username.strip, password) | ||
| end | ||
| end | ||
| end |
jchestershopify
approved these changes
Dec 16, 2021
jchestershopify
left a comment
jchestershopify
left a comment
There was a problem hiding this comment.
Some suggested tweaks, but otherwise LGTM.
bettymakes
changed the title
Author
|
Closing in favor of #9 which uses the most up to date version of how we store and determine |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This prototype relates to the stage of encouraging users to setup mfa in the proposed policy. The regular response is sent back with signin, push/yank gems, and add/remove owners actions along with a message (or warning) to encourage users to set up MFA.
Gem signin
This requires changes to the rubygems'
gem signin commandas the response from the rubygems.org API when creating an API key is the API key itself. Before creating an API key, an request is sent toapi/v1/profiledetermine if MFA is enabled on the account, and if not, a message gets shown (see Shopify/rubygems#1). This applies to all users and not the targetted ones.Gem push/yank gems, and add/remove owners
A message is appended to the end of the response body of these commands. A warning cannot be appended before since these commands can check the beginning of the response body to determine certain states (eg. mfa unauthorized). This applies to users that have been targetted via the
mfa_required?method (ie. most downloaded gem owners) on the user.