提交 a0835d44 编写于 作者: M Mislav Marohnić

Merge pull request #743 from github/imporve_auth_process

Improve authentication process for v2.2.0

Fixes #728, fixes #622
......@@ -2,7 +2,6 @@ package octokit
import (
"fmt"
"io/ioutil"
"net/http"
"regexp"
"strings"
......@@ -98,43 +97,42 @@ func (e *ResponseError) errorMessage() string {
func NewResponseError(resp *sawyer.Response) (err *ResponseError) {
err = &ResponseError{}
err.Response = resp.Response
err.Type = getResponseErrorType(resp.Response)
e := resp.Decode(&err)
if e != nil {
err.Message = fmt.Sprintf("Problems parsing error message: %s", e)
}
err.Response = resp.Response
err.Type = getResponseErrorType(err)
return
}
func getResponseErrorType(resp *http.Response) ResponseErrorType {
code := resp.StatusCode
func getResponseErrorType(err *ResponseError) ResponseErrorType {
code := err.Response.StatusCode
header := err.Response.Header
switch {
case code == http.StatusBadRequest:
return ErrorBadRequest
case code == http.StatusUnauthorized:
header := resp.Header.Get("X-GitHub-OTP")
otp := header.Get("X-GitHub-OTP")
r := regexp.MustCompile(`(?i)required; (\w+)`)
if r.MatchString(header) {
if r.MatchString(otp) {
return ErrorOneTimePasswordRequired
}
return ErrorUnauthorized
case code == http.StatusForbidden:
body, err := ioutil.ReadAll(resp.Body)
if err != nil {
return ErrorForbidden
}
msg := err.Message
rr := regexp.MustCompile("(?i)rate limit exceeded")
if rr.MatchString(string(body)) {
if rr.MatchString(msg) {
return ErrorTooManyRequests
}
lr := regexp.MustCompile("(?i)login attempts exceeded")
if lr.MatchString(string(body)) {
if lr.MatchString(msg) {
return ErrorTooManyLoginAttempts
}
......
......@@ -4,7 +4,7 @@ import (
"net/url"
)
var GitTreesURL = Hyperlink("repos/{owner}/{repo}/git/trees/{/sha}{?recursive}")
var GitTreesURL = Hyperlink("repos/{owner}/{repo}/git/trees/{sha}{?recursive}")
func (c *Client) GitTrees(url *url.URL) (trees *GitTreesService) {
trees = &GitTreesService{client: c, URL: url}
......
......@@ -5,12 +5,15 @@ Feature: OAuth authentication
Scenario: Ask for username & password, create authorization
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') { '[]' }
require 'socket'
require 'etc'
machine_id = "#{Etc.getlogin}@#{Socket.gethostname}"
post('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
assert :scopes => ['repo']
assert_basic_auth 'mislav', 'kitty'
assert :scopes => ['repo'],
:note => "hub for #{machine_id}",
:note_url => 'http://hub.github.com/'
json :token => 'OTOKEN'
}
get('/user') {
......@@ -32,50 +35,29 @@ Feature: OAuth authentication
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
And the file "../home/.config/hub" should have mode "0600"
Scenario: Ask for username & password, re-use existing authorization
Scenario: Rename & retry creating authorization if there's a token name collision
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
json [
{:token => 'SKIPPD', :note_url => 'http://example.com'},
{:token => 'OTOKEN', :note_url => 'http://hub.github.com/'}
]
}
get('/user') {
json :login => 'mislav'
}
post('/user/repos') {
json :full_name => 'mislav/dotfiles'
}
"""
When I run `hub create` interactively
When I type "mislav"
And I type "kitty"
Then the output should contain "github.com password for mislav (never stored):"
And the exit status should be 0
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
require 'socket'
require 'etc'
machine_id = "#{Etc.getlogin}@#{Socket.gethostname}"
Scenario: Re-use existing authorization with an old URL
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
json [
{:token => 'OTOKEN', :note => 'hub', :note_url => 'http://defunkt.io/hub/'}
]
}
post('/authorizations') {
status 422
json :message => "Validation Failed",
:errors => [{:resource => "OauthAccess", :code => "already_exists", :field => "description"}]
assert_basic_auth 'mislav', 'kitty'
if params[:note] == "hub for #{machine_id} 3"
json :token => 'OTOKEN'
else
status 422
json :message => 'Validation Failed',
:errors => [{
:resource => 'OauthAccess',
:code => 'already_exists',
:field => 'description'
}]
end
}
get('/user') {
json :login => 'mislav'
json :login => 'MiSlAv'
}
post('/user/repos') {
json :full_name => 'mislav/dotfiles'
......@@ -84,54 +66,43 @@ Feature: OAuth authentication
When I run `hub create` interactively
When I type "mislav"
And I type "kitty"
Then the output should contain "github.com password for mislav (never stored):"
Then the output should contain "github.com username:"
And the exit status should be 0
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
Scenario: Re-use existing authorization found on page 3
Scenario: Avoid getting caught up in infinite recursion while retrying token names
Given the GitHub API server:
"""
get('/authorizations') {
assert_basic_auth 'mislav', 'kitty'
page = (params[:page] || 1).to_i
if page < 3
response.headers['Link'] = %(<#{url}?page=#{page+1}>; rel="next")
json []
else
json [
{:token => 'OTOKEN', :note => 'hub', :note_url => 'http://hub.github.com/'}
]
end
}
tries = 0
post('/authorizations') {
tries += 1
halt 400, json(:message => "too many tries") if tries >= 10
status 422
json :message => "Validation Failed",
:errors => [{:resource => "OauthAccess", :code => "already_exists", :field => "description"}]
}
get('/user') {
json :login => 'mislav'
}
post('/user/repos') {
json :full_name => 'mislav/dotfiles'
json :message => 'Validation Failed',
:errors => [{
:resource => 'OauthAccess',
:code => 'already_exists',
:field => 'description'
}]
}
"""
When I run `hub create` interactively
When I type "mislav"
And I type "kitty"
Then the output should contain "github.com password for mislav (never stored):"
And the exit status should be 0
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
Then the output should contain:
"""
Error creating repository: Unprocessable Entity (HTTP 422)
Duplicate value for "description"
"""
And the exit status should be 1
And the file "../home/.config/hub" should not exist
Scenario: Credentials from GITHUB_USER & GITHUB_PASSWORD
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
json [
{:token => 'OTOKEN', :note_url => 'http://hub.github.com/'}
]
post('/authorizations') {
assert_basic_auth 'mislav', 'kitty'
json :token => 'OTOKEN'
}
get('/user') {
json :login => 'mislav'
......@@ -149,41 +120,54 @@ Feature: OAuth authentication
Scenario: Wrong password
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
post('/authorizations') {
assert_basic_auth 'mislav', 'kitty'
}
"""
When I run `hub create` interactively
When I type "mislav"
And I type "WRONG"
Then the stderr should contain "Error creating repository: Unauthorized (HTTP 401)"
Then the stderr should contain exactly:
"""
Error creating repository: Unauthorized (HTTP 401)
Bad credentials
"""
And the exit status should be 1
And the file "../home/.config/hub" should not exist
Scenario: Two-factor authentication, create authorization
Scenario: Personal access token used instead of password
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
if request.env['HTTP_X_GITHUB_OTP'] != "112233"
response.headers['X-GitHub-OTP'] = "required; app"
halt 401
end
json [ ]
post('/authorizations') {
status 403
json :message => "This API can only be accessed with username and password Basic Auth"
}
"""
When I run `hub create` interactively
When I type "mislav"
And I type "PERSONALACCESSTOKEN"
Then the stderr should contain exactly:
"""
Error creating repository: Forbidden (HTTP 403)
This API can only be accessed with username and password Basic Auth
"""
And the exit status should be 1
And the file "../home/.config/hub" should not exist
Scenario: Two-factor authentication, create authorization
Given the GitHub API server:
"""
post('/authorizations') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
halt 412 unless params[:scopes]
if request.env['HTTP_X_GITHUB_OTP'] != "112233"
response.headers['X-GitHub-OTP'] = "required; app"
halt 401
assert_basic_auth 'mislav', 'kitty'
if request.env['HTTP_X_GITHUB_OTP'] == '112233'
json :token => 'OTOKEN'
else
response.headers['X-GitHub-OTP'] = 'required; app'
status 401
json :message => "Must specify two-factor authentication OTP code."
end
json :token => 'OTOKEN'
}
get('/user') {
json :login => 'mislav'
......@@ -198,28 +182,25 @@ Feature: OAuth authentication
And I type "112233"
Then the output should contain "github.com password for mislav (never stored):"
Then the output should contain "two-factor authentication code:"
And the output should not contain "warning: invalid two-factor code"
And the exit status should be 0
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
Scenario: Two-factor authentication, re-use existing authorization
Scenario: Retry entering two-factor authentication code
Given the GitHub API server:
"""
token = 'OTOKEN'
previous_otp_code = nil
post('/authorizations') {
assert_basic_auth 'mislav', 'kitty'
token << 'SMS'
status 412
}
get('/authorizations') {
assert_basic_auth 'mislav', 'kitty'
if request.env['HTTP_X_GITHUB_OTP'] != "112233"
response.headers['X-GitHub-OTP'] = "required; app"
halt 401
if request.env['HTTP_X_GITHUB_OTP'] == '112233'
halt 400 unless '666' == previous_otp_code
json :token => 'OTOKEN'
else
previous_otp_code = request.env['HTTP_X_GITHUB_OTP']
response.headers['X-GitHub-OTP'] = 'required; app'
status 401
json :message => "Must specify two-factor authentication OTP code."
end
json [ {
:token => token,
:note_url => 'http://hub.github.com/'
} ]
}
get('/user') {
json :login => 'mislav'
......@@ -231,16 +212,15 @@ Feature: OAuth authentication
When I run `hub create` interactively
When I type "mislav"
And I type "kitty"
And I type "666"
And I type "112233"
Then the output should contain "github.com password for mislav (never stored):"
Then the output should contain "two-factor authentication code:"
Then the output should contain "warning: invalid two-factor code"
And the exit status should be 0
And the file "../home/.config/hub" should contain "oauth_token: OTOKENSMS"
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
Scenario: Special characters in username & password
Given the GitHub API server:
"""
get('/authorizations') { '[]' }
post('/authorizations') {
assert_basic_auth 'mislav@example.com', 'my pass@phrase ok?'
json :token => 'OTOKEN'
......@@ -257,3 +237,29 @@ Feature: OAuth authentication
And the exit status should be 0
And the file "../home/.config/hub" should contain "user: mislav"
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
Scenario: Enterprise fork authentication with username & password, re-using existing authorization
Given the GitHub API server:
"""
require 'rack/auth/basic'
post('/api/v3/authorizations', :host_name => 'git.my.org') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
json :token => 'OTOKEN', :note_url => 'http://hub.github.com/'
}
get('/api/v3/user', :host_name => 'git.my.org') {
json :login => 'mislav'
}
post('/api/v3/repos/evilchelu/dotfiles/forks', :host_name => 'git.my.org') { '' }
"""
And "git.my.org" is a whitelisted Enterprise host
And the "origin" remote has url "git@git.my.org:evilchelu/dotfiles.git"
When I run `hub fork` interactively
And I type "mislav"
And I type "kitty"
Then the output should contain "git.my.org password for mislav (never stored):"
And the exit status should be 0
And the file "../home/.config/hub" should contain "git.my.org"
And the file "../home/.config/hub" should contain "user: mislav"
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
And the url for "mislav" should be "git@git.my.org:mislav/dotfiles.git"
......@@ -151,32 +151,3 @@ Scenario: Related fork already exists
And "git.my.org" is a whitelisted Enterprise host
When I successfully run `hub fork`
Then the url for "mislav" should be "git@git.my.org:mislav/dotfiles.git"
Scenario: Enterprise fork authentication with username & password, re-using existing authorization
Given the GitHub API server:
"""
require 'rack/auth/basic'
get('/api/v3/authorizations', :host_name => 'git.my.org') {
auth = Rack::Auth::Basic::Request.new(env)
halt 401 unless auth.credentials == %w[mislav kitty]
json [
{:token => 'SKIPPD', :note_url => 'http://example.com'},
{:token => 'OTOKEN', :note_url => 'http://hub.github.com/'}
]
}
get('/api/v3/user', :host_name => 'git.my.org') {
json :login => 'mislav'
}
post('/api/v3/repos/evilchelu/dotfiles/forks', :host_name => 'git.my.org') { '' }
"""
And "git.my.org" is a whitelisted Enterprise host
And the "origin" remote has url "git@git.my.org:evilchelu/dotfiles.git"
When I run `hub fork` interactively
And I type "mislav"
And I type "kitty"
Then the output should contain "git.my.org password for mislav (never stored):"
And the exit status should be 0
And the file "../home/.config/hub" should contain "git.my.org"
And the file "../home/.config/hub" should contain "user: mislav"
And the file "../home/.config/hub" should contain "oauth_token: OTOKEN"
And the url for "mislav" should be "git@git.my.org:mislav/dotfiles.git"
......@@ -85,11 +85,7 @@ module Hub
require 'rack/auth/basic'
auth = Rack::Auth::Basic::Request.new(env)
if auth.credentials != expected
halt 401, json(
:message => "expected %p; got %p" % [
expected, auth.credentials
]
)
halt 401, json(:message => "Bad credentials")
end
end
......
......@@ -5,6 +5,7 @@ import (
"io"
"net/url"
"os"
"os/user"
"strings"
"github.com/github/hub/Godeps/_workspace/src/github.com/octokit/go-octokit/octokit"
......@@ -14,7 +15,6 @@ const (
GitHubHost string = "github.com"
GitHubApiHost string = "api.github.com"
UserAgent string = "Hub"
OAuthAppName string = "hub"
OAuthAppURL string = "http://hub.github.com/"
)
......@@ -27,18 +27,23 @@ func NewClientWithHost(host *Host) *Client {
}
type AuthError struct {
error
Err error
}
func (e *AuthError) Error() string {
return e.error.Error()
return e.Err.Error()
}
func (e *AuthError) Is2FAError() bool {
re, ok := e.error.(*octokit.ResponseError)
func (e *AuthError) IsRequired2FACodeError() bool {
re, ok := e.Err.(*octokit.ResponseError)
return ok && re.Type == octokit.ErrorOneTimePasswordRequired
}
func (e *AuthError) IsDuplicatedTokenError() bool {
re, ok := e.Err.(*octokit.ResponseError)
return ok && re.Type == octokit.ErrorUnprocessableEntity
}
type Client struct {
Host *Host
}
......@@ -455,55 +460,39 @@ func (client *Client) FindOrCreateToken(user, password, twoFactorCode string) (t
c := client.newOctokitClient(basicAuth)
authsService := c.Authorizations(client.requestURL(authUrl))
if twoFactorCode == "" {
// dummy request to trigger a 2FA SMS since a HTTP GET won't do it
authsService.Create(nil)
authParam := octokit.AuthorizationParams{
Scopes: []string{"repo"},
NoteURL: OAuthAppURL,
}
auths, result := authsService.All()
if result.HasError() {
err = &AuthError{result.Err}
return
}
var moreAuths []octokit.Authorization
for result.NextPage != nil {
authUrl, e := result.NextPage.Expand(nil)
count := 1
for {
note, e := authTokenNote(count)
if e != nil {
return "", e
}
authUrl, _ = url.Parse(authUrl.RequestURI())
as := c.Authorizations(authUrl)
moreAuths, result = as.All()
if result.HasError() {
err = &AuthError{result.Err}
err = e
return
}
auths = append(auths, moreAuths...)
}
for _, auth := range auths {
if auth.Note == OAuthAppName || auth.NoteURL == OAuthAppURL {
authParam.Note = note
auth, result := authsService.Create(authParam)
if !result.HasError() {
token = auth.Token
break
}
}
if token == "" {
authParam := octokit.AuthorizationParams{}
authParam.Scopes = append(authParam.Scopes, "repo")
authParam.Note = OAuthAppName
authParam.NoteURL = OAuthAppURL
auth, result := authsService.Create(authParam)
if result.HasError() {
err = &AuthError{result.Err}
return
authErr := &AuthError{result.Err}
if authErr.IsDuplicatedTokenError() {
if count >= 9 {
err = authErr
break
} else {
count++
continue
}
} else {
err = authErr
break
}
token = auth.Token
}
return
......@@ -577,6 +566,8 @@ func FormatError(action string, err error) (ee error) {
switch e := err.(type) {
default:
ee = err
case *AuthError:
return FormatError(action, e.Err)
case *octokit.ResponseError:
statusCode := e.Response.StatusCode
var reason string
......@@ -586,26 +577,33 @@ func FormatError(action string, err error) (ee error) {
errStr := fmt.Sprintf("Error %s: %s (HTTP %d)", action, reason, statusCode)
var messages []string
if statusCode == 422 {
if e.Message != "" {
messages = append(messages, e.Message)
var errorSentences []string
for _, err := range e.Errors {
switch err.Code {
case "custom":
errorSentences = append(errorSentences, err.Message)
case "missing_field":
errorSentences = append(errorSentences, fmt.Sprintf("Missing filed: \"%s\"", err.Field))
case "already_exists":
errorSentences = append(errorSentences, fmt.Sprintf("Duplicate value for \"%s\"", err.Field))
case "invalid":
errorSentences = append(errorSentences, fmt.Sprintf("Invalid value for \"%s\"", err.Field))
case "unauthorized":
errorSentences = append(errorSentences, fmt.Sprintf("Not allowed to change field \"%s\"", err.Field))
}
}
if len(e.Errors) > 0 {
for _, e := range e.Errors {
messages = append(messages, e.Error())
}
}
var errorMessage string
if len(errorSentences) > 0 {
errorMessage = strings.Join(errorSentences, "\n")
} else {
errorMessage = e.Message
}
if len(messages) > 0 {
errStr = fmt.Sprintf("%s\n%s", errStr, strings.Join(messages, "\n"))
if errorMessage != "" {
errStr = fmt.Sprintf("%s\n%s", errStr, errorMessage)
}
ee = fmt.Errorf(errStr)
case *AuthError:
errStr := fmt.Sprintf("Error %s: Unauthorized (HTTP 401)", action)
ee = fmt.Errorf(errStr)
}
......@@ -625,3 +623,22 @@ func warnExistenceOfRepo(project *Project, ee error) (err error) {
return
}
func authTokenNote(num int) (string, error) {
u, err := user.Current()
if err != nil {
return "", err
}
n := u.Username
h, err := os.Hostname()
if err != nil {
return "", err
}
if num > 1 {
return fmt.Sprintf("hub for %s@%s %d", n, h, num), nil
}
return fmt.Sprintf("hub for %s@%s", n, h), nil
}
......@@ -3,6 +3,7 @@ package github
import (
"fmt"
"net/http"
"regexp"
"testing"
"github.com/github/hub/Godeps/_workspace/src/github.com/bmizerany/assert"
......@@ -62,3 +63,18 @@ func TestClient_warnExistenceOfRepo(t *testing.T) {
err := warnExistenceOfRepo(project, e)
assert.Equal(t, "Are you sure that github.com/github/hub exists?", fmt.Sprintf("%s", err))
}
func TestAuthTokenNote(t *testing.T) {
note, err := authTokenNote(1)
assert.Equal(t, nil, err)
reg := regexp.MustCompile("hub for (.+)@(.+)")
assert.T(t, reg.MatchString(note))
note, err = authTokenNote(2)
assert.Equal(t, nil, err)
reg = regexp.MustCompile("hub for (.+)@(.+) 2")
assert.T(t, reg.MatchString(note))
}
......@@ -8,8 +8,8 @@ import (
"path/filepath"
"strconv"
"github.com/github/hub/utils"
"github.com/github/hub/Godeps/_workspace/src/github.com/howeyc/gopass"
"github.com/github/hub/utils"
)
var (
......@@ -45,13 +45,20 @@ func (c *Config) PromptForHost(host string) (h *Host, err error) {
pass := c.PromptForPassword(host, user)
client := NewClient(host)
token, e := client.FindOrCreateToken(user, pass, "")
if e != nil {
if ae, ok := e.(*AuthError); ok && ae.Is2FAError() {
code := c.PromptForOTP()
token, err = client.FindOrCreateToken(user, pass, code)
var code, token string
for {
token, err = client.FindOrCreateToken(user, pass, code)
if err == nil {
break
}
if ae, ok := err.(*AuthError); ok && ae.IsRequired2FACodeError() {
if (code != "") {
fmt.Fprintln(os.Stderr, "warning: invalid two-factor code")
}
code = c.PromptForOTP()
} else {
err = e
break
}
}
......
Markdown is supported
0% .
You are about to add 0 people to the discussion. Proceed with caution.
先完成此消息的编辑!
想要评论请 注册