-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Allow usersFile to be specified for basic or digest auth #1189
Changes from all commits
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,6 +2,7 @@ package middlewares | |
|
||
import ( | ||
"fmt" | ||
"io/ioutil" | ||
"net/http" | ||
"strings" | ||
|
||
|
@@ -25,7 +26,7 @@ func NewAuthenticator(authConfig *types.Auth) (*Authenticator, error) { | |
var err error | ||
authenticator := Authenticator{} | ||
if authConfig.Basic != nil { | ||
authenticator.users, err = parserBasicUsers(authConfig.Basic.Users) | ||
authenticator.users, err = parserBasicUsers(authConfig.Basic) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -43,7 +44,7 @@ func NewAuthenticator(authConfig *types.Auth) (*Authenticator, error) { | |
} | ||
}) | ||
} else if authConfig.Digest != nil { | ||
authenticator.users, err = parserDigestUsers(authConfig.Digest.Users) | ||
authenticator.users, err = parserDigestUsers(authConfig.Digest) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
@@ -64,9 +65,17 @@ func NewAuthenticator(authConfig *types.Auth) (*Authenticator, error) { | |
return &authenticator, nil | ||
} | ||
|
||
func parserBasicUsers(users types.Users) (map[string]string, error) { | ||
func parserBasicUsers(basic *types.Basic) (map[string]string, error) { | ||
var userStrs []string | ||
if basic.UsersFile != "" { | ||
var err error | ||
if userStrs, err = getLinesFromFile(basic.UsersFile); err != nil { | ||
return nil, err | ||
} | ||
} | ||
userStrs = append(basic.Users, userStrs...) | ||
userMap := make(map[string]string) | ||
for _, user := range users { | ||
for _, user := range userStrs { | ||
split := strings.Split(user, ":") | ||
if len(split) != 2 { | ||
return nil, fmt.Errorf("Error parsing Authenticator user: %v", user) | ||
|
@@ -76,9 +85,17 @@ func parserBasicUsers(users types.Users) (map[string]string, error) { | |
return userMap, nil | ||
} | ||
|
||
func parserDigestUsers(users types.Users) (map[string]string, error) { | ||
func parserDigestUsers(digest *types.Digest) (map[string]string, error) { | ||
var userStrs []string | ||
if digest.UsersFile != "" { | ||
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. Check for 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. As with 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. 👍 |
||
var err error | ||
if userStrs, err = getLinesFromFile(digest.UsersFile); err != nil { | ||
return nil, err | ||
} | ||
} | ||
userStrs = append(digest.Users, userStrs...) | ||
userMap := make(map[string]string) | ||
for _, user := range users { | ||
for _, user := range userStrs { | ||
split := strings.Split(user, ":") | ||
if len(split) != 3 { | ||
return nil, fmt.Errorf("Error parsing Authenticator user: %v", user) | ||
|
@@ -88,6 +105,23 @@ func parserDigestUsers(users types.Users) (map[string]string, error) { | |
return userMap, nil | ||
} | ||
|
||
func getLinesFromFile(filename string) ([]string, error) { | ||
dat, err := ioutil.ReadFile(filename) | ||
if err != nil { | ||
return nil, err | ||
} | ||
// Trim lines and filter out blanks | ||
rawLines := strings.Split(string(dat), "\n") | ||
var filteredLines []string | ||
for _, rawLine := range rawLines { | ||
line := strings.TrimSpace(rawLine) | ||
if line != "" { | ||
filteredLines = append(filteredLines, line) | ||
} | ||
} | ||
return filteredLines, nil | ||
} | ||
|
||
func (a *Authenticator) secretBasic(user, realm string) string { | ||
if secret, ok := a.users[user]; ok { | ||
return secret | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -241,12 +241,14 @@ type Users []string | |
|
||
// Basic HTTP basic authentication | ||
type Basic struct { | ||
Users `mapstructure:","` | ||
Users `mapstructure:","` | ||
UsersFile string | ||
} | ||
|
||
// Digest HTTP authentication | ||
type Digest struct { | ||
Users `mapstructure:","` | ||
Users `mapstructure:","` | ||
UsersFile string | ||
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. Basic and Digest seem type identical. Any particular reason we're not using a single type only? 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. Neither of those types are introduced by this PR. They existed previously. Your question may be valid, but this PR probably isn't the right venue for revisiting a design decision not made by this PR. 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. Agreed, this case might be too heavy to justify in terms of the boy scout rule. |
||
} | ||
|
||
// CanonicalDomain returns a lower case domain with trim space | ||
|
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.
Should we check
basic
fornil
?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.
If you look at where this function is invoked,
basic
is known not to benil
before this function is called.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.
You're right.