Skip to content

Commit 8140a04

Browse files
Jonathan S. Katzjkatz
Jonathan S. Katz
authored andcommitted
Ensure disabling BasicAuth skips Basic Authentication check.
Presently, disabling BasicAuth in the pgo.yaml configuration file would not actually disable the HTTP Basic Authentication from occuring, as the check against authorization headers provided by the HTTP requests would still be scanned. This ensures this check is skipped when BasicAuth is set to `"false"`. However, skipping Basic Authentication does not skip authorization, as the Operator heavily leverages RBAC checks, and as such, a valid username is required at all times even if BasicAuth is skipped. As such, this fix only solves one type of error, i.e. the case where no HTTP Authorization headers are sent to the Operator apiserver. And by "fix," I mean it just moves the failure from the authentication check to the authorization check. Issue: [ch6162]
1 parent 3edd580 commit 8140a04

File tree

2 files changed

+21
-5
lines changed

2 files changed

+21
-5
lines changed

apiserver/root.go

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -313,19 +313,35 @@ func GetNamespace(clientset *kubernetes.Clientset, username, requestedNS string)
313313
return requestedNS, errors.New("requested Namespace was not found to be in the list of Namespaces being watched.")
314314
}
315315

316+
// Authn performs HTTP Basic Authentication against a user if "BasicAuth" is set
317+
// to "true" (which it is by default).
318+
//
319+
// ...it also performs Authorization (Authz) against the user that is attempting
320+
// to authenticate, and as such, to truly "authenticate/authorize," one needs
321+
// at least a valid Operator User account.
316322
func Authn(perm string, w http.ResponseWriter, r *http.Request) (string, error) {
317323
var err error
318324
w.Header().Set("WWW-Authenticate", `Basic realm="Restricted"`)
319325

326+
// Need to run the HTTP library `BasicAuth` even if `BasicAuth == false`, as
327+
// this function currently encapsulates authorization as well, and this is
328+
// the call where we get the username to check the RBAC settings
320329
username, password, authOK := r.BasicAuth()
321330
if AuditFlag {
322331
log.Infof("[audit] %s username=[%s] method=[%s] ip=[%s] ok=[%t] ", perm, username, r.Method, r.RemoteAddr, authOK)
323332
}
324333

325-
log.Debugf("Authentication Attempt %s username=[%s]", perm, username)
326-
if authOK == false {
327-
http.Error(w, "Not authorized", 401)
328-
return "", errors.New("Not Authorized")
334+
// Check to see if this user is authenticated
335+
// If BasicAuth is "disabled", skip the authentication; o/w/ check if the
336+
// authentication passed
337+
if !BasicAuth {
338+
log.Debugf("BasicAuth disabled, Skipping Authentication %s username=[%s]", perm, username)
339+
} else {
340+
log.Debugf("Authentication Attempt %s username=[%s]", perm, username)
341+
if !authOK {
342+
http.Error(w, "Not Authorized. Basic Authentication credentials must be provided according to RFC 7617, Section 2.", 401)
343+
return "", errors.New("Not Authorized: Credentials do not comply with RFC 7617")
344+
}
329345
}
330346

331347
if !BasicAuthCheck(username, password) {

hugo/content/Configuration/pgo-yaml-configuration.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ The *pgo.yaml* file is broken into major sections as described below:
1414

1515
| Setting |Definition |
1616
|---|---|
17-
|BasicAuth | if set to *true* will enable Basic Authentication
17+
|BasicAuth | If set to `"true"` will enable Basic Authentication. If set to `"false"`, will allow a valid Operator user to successfully authenticate regardless of the value of the password provided for Basic Authentication. Defaults to `"true".`
1818
|PrimaryNodeLabel |newly created primary deployments will specify this node label if specified, unless you override it using the --node-label command line flag, if not set, no node label is specifed
1919
|ReplicaNodeLabel |newly created replica deployments will specify this node label if specified, unless you override it using the --node-label command line flag, if not set, no node label is specifed
2020
|CCPImagePrefix |newly created containers will be based on this image prefix (e.g. crunchydata), update this if you require a custom image prefix

0 commit comments

Comments
 (0)