zrhoffman commented on code in PR #7110:
URL: https://github.com/apache/trafficcontrol/pull/7110#discussion_r1131409786
##########
traffic_ops/traffic_ops_golang/login/login.go:
##########
@@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` +
"\r\n\r" + `
</html>
`))
+// LoginHandler first attempts to verify and parse user information from an
optionally
+// provided client TLS certificate. If it fails at any point, it will fall
back and
+// continue with the standard submitted form authentication.
func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
defer r.Body.Close()
authenticated := false
form := auth.PasswordForm{}
- if err := json.NewDecoder(r.Body).Decode(&form); err != nil {
- api.HandleErr(w, r, nil, http.StatusBadRequest, err,
nil)
- return
- }
- if form.Username == "" || form.Password == "" {
- api.HandleErr(w, r, nil, http.StatusBadRequest,
errors.New("username and password are required"), nil)
- return
- }
- resp := struct {
- tc.Alerts
- }{}
+ var resp tc.Alerts
dbCtx, cancelTx := context.WithTimeout(r.Context(),
time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second)
defer cancelTx()
- userAllowed, err, blockingErr :=
auth.CheckLocalUserIsAllowed(form, db, dbCtx)
- if blockingErr != nil {
- api.HandleErr(w, r, nil, http.StatusServiceUnavailable,
nil, fmt.Errorf("error checking local user password: %s\n",
blockingErr.Error()))
- return
- }
- if err != nil {
- log.Errorf("checking local user: %s\n", err.Error())
+
+ // Attempt to perform client certificate authentication. If
fails, goto standard form auth. If the
+ // certificate was verified, has a UID, and the UID matches an
existing user we consider this to
+ // be a successful login.
+ {
+ // No certs provided by the client. Skip to form
authentication
+ if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
+ goto FormAuth
+ }
+
+ // If no configuration is set, skip to form auth
+ if cfg.ClientCertAuth == nil ||
len(cfg.ClientCertAuth.RootCertsDir) == 0 {
+ goto FormAuth
+ }
+
+ // Perform certificate verification to ensure it is
valid against Root CAs
+ err := auth.VerifyClientCertificate(r,
cfg.ClientCertAuth.RootCertsDir)
+ if err != nil {
+ log.Warnf("ClientCertAuth: error attempting to
verify client provided TLS certificate. err: %s\n", err)
+ goto FormAuth
+ }
+
+ // Client provided a verified certificate. Extract UID
value.
+ form.Username =
auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0])
+ if len(form.Username) == 0 {
+ log.Infoln("ClientCertAuth: client provided
certificate did not contain a UID object identifier or value")
+ goto FormAuth
+ }
+
+ // Check if user exists locally (TODB) and has a role.
+ var blockingErr error
+ authenticated, err, blockingErr =
auth.CheckLocalUserIsAllowed(form.Username, db, dbCtx)
+ if blockingErr != nil {
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user has
role: %s", blockingErr.Error()))
+ return
+ }
+ if err != nil {
+ log.Warnf("ClientCertAuth: checking local user:
%s\n", err.Error())
+ }
+
+ // Check LDAP if enabled
+ if !authenticated && cfg.LDAPEnabled {
+ _, authenticated, err =
auth.LookupUserDN(form.Username, cfg.ConfigLDAP)
+ if err != nil {
+ log.Warnf("ClientCertAuth: checking
ldap user: %s\n", err.Error())
+ }
+ }
}
- if userAllowed {
+
+ FormAuth:
+ // Failed certificate-based auth, perform standard form auth
+ if !authenticated {
+ // Perform form authentication
+ if err := json.NewDecoder(r.Body).Decode(&form); err !=
nil {
+ api.HandleErr(w, r, nil, http.StatusBadRequest,
err, nil)
+ return
+ }
+ if form.Username == "" || form.Password == "" {
+ api.HandleErr(w, r, nil, http.StatusBadRequest,
errors.New("username and password are required"), nil)
+ return
+ }
+
+ // Check if user exists and has a role
+ userAllowed, err, blockingErr :=
auth.CheckLocalUserIsAllowed(form.Username, db, dbCtx)
+ if blockingErr != nil {
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user has
role: %s", blockingErr.Error()))
+ return
+ }
+ if err != nil {
+ log.Errorf("checking local user: %s\n",
err.Error())
+ }
+
+ // User w/ role does not exist, return unauthorized
+ if !userAllowed {
+ resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid
username or password.")
+ w.WriteHeader(http.StatusUnauthorized)
+ api.WriteRespRaw(w, r, resp)
+ return
+ }
+
+ // Check local DB or LDAP
authenticated, err, blockingErr =
auth.CheckLocalUserPassword(form, db, dbCtx)
if blockingErr != nil {
- api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user
password: %s\n", blockingErr.Error()))
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user
password: %s", blockingErr.Error()))
return
}
if err != nil {
log.Errorf("checking local user password:
%s\n", err.Error())
}
var ldapErr error
- if !authenticated {
- if cfg.LDAPEnabled {
- authenticated, ldapErr =
auth.CheckLDAPUser(form, cfg.ConfigLDAP)
- if ldapErr != nil {
- log.Errorf("checking ldap user:
%s\n", ldapErr.Error())
- }
+ if !authenticated && cfg.LDAPEnabled {
+ authenticated, ldapErr =
auth.CheckLDAPUser(form, cfg.ConfigLDAP)
+ if ldapErr != nil {
+ log.Errorf("checking ldap user: %s\n",
ldapErr.Error())
}
}
- if authenticated {
- httpCookie := tocookie.GetCookie(form.Username,
defaultCookieDuration, cfg.Secrets[0])
- http.SetCookie(w, httpCookie)
-
- var jwtToken jwt.Token
- var jwtSigned []byte
- jwtBuilder := jwt.NewBuilder()
-
- emptyConf := config.CdniConf{}
- if cfg.Cdni != nil && *cfg.Cdni != emptyConf {
- ucdn, err := auth.GetUserUcdn(form, db,
dbCtx)
- if err != nil {
- // log but do not error out
since this is optional in the JWT for CDNi integration
- log.Errorf("getting ucdn for
user %s: %v", form.Username, err)
- }
- jwtBuilder.Claim("iss", ucdn)
- jwtBuilder.Claim("aud", cfg.Cdni.DCdnId)
- }
+ }
- jwtBuilder.Claim("exp",
httpCookie.Expires.Unix())
- jwtBuilder.Claim(api.MojoCookie,
httpCookie.Value)
- jwtToken, err = jwtBuilder.Build()
- if err != nil {
- api.HandleErr(w, r, nil,
http.StatusInternalServerError, nil, fmt.Errorf("building token: %s", err))
- return
- }
+ // Failed to authenticate in either local DB or LDAP, return
unauthorized
+ if !authenticated {
+ resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username
or password.")
+ w.WriteHeader(http.StatusUnauthorized)
+ api.WriteRespRaw(w, r, resp)
+ return
+ }
- jwtSigned, err = jwt.Sign(jwtToken, jwa.HS256,
[]byte(cfg.Secrets[0]))
- if err != nil {
- api.HandleErr(w, r, nil,
http.StatusInternalServerError, nil, err)
- return
- }
+ // Successful authentication, write cookie and return
+ httpCookie := tocookie.GetCookie(form.Username,
defaultCookieDuration, cfg.Secrets[0])
+ http.SetCookie(w, httpCookie)
- http.SetCookie(w, &http.Cookie{
- Name: api.AccessToken,
- Value: string(jwtSigned),
- Path: "/",
- MaxAge: httpCookie.MaxAge,
- Expires: httpCookie.Expires,
- HttpOnly: true, // prevents the cookie
being accessed by Javascript. DO NOT remove, security vulnerability
- })
-
- // If all's well until here, then update last
authenticated time
- tx, txErr := db.BeginTx(dbCtx, nil)
- if txErr != nil {
- api.HandleErr(w, r, tx,
http.StatusInternalServerError, nil, fmt.Errorf("beginning transaction: %w",
txErr))
- return
- }
- defer func() {
- if err := tx.Commit(); err != nil &&
err != sql.ErrTxDone {
- log.Errorln("committing
transaction: " + err.Error())
- }
- }()
- _, dbErr := tx.Exec(UpdateLoginTimeQuery,
form.Username)
- if dbErr != nil {
- log.Errorf("unable to update
authentication time for a given user: %s\n", dbErr.Error())
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel,
"Unable to update authentication time for a given user")}
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.SuccessLevel,
"Successfully logged in.")}
- }
+ var jwtToken jwt.Token
+ var jwtSigned []byte
+ jwtBuilder := jwt.NewBuilder()
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel, "Invalid
username or password.")}
+ emptyConf := config.CdniConf{}
+ if cfg.Cdni != nil && *cfg.Cdni != emptyConf {
+ ucdn, err := auth.GetUserUcdn(form, db, dbCtx)
+ if err != nil {
+ // log but do not error out since this is
optional in the JWT for CDNi integration
+ log.Errorf("getting ucdn for user %s: %v",
form.Username, err)
}
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or
password.")}
+ jwtBuilder.Claim("iss", ucdn)
+ jwtBuilder.Claim("aud", cfg.Cdni.DCdnId)
Review Comment:
Fixed in #7110
##########
traffic_ops/traffic_ops_golang/login/login.go:
##########
@@ -108,135 +108,184 @@ Subject: {{.InstanceName}} Password Reset Request` +
"\r\n\r" + `
</html>
`))
+// LoginHandler first attempts to verify and parse user information from an
optionally
+// provided client TLS certificate. If it fails at any point, it will fall
back and
+// continue with the standard submitted form authentication.
func LoginHandler(db *sqlx.DB, cfg config.Config) http.HandlerFunc {
return func(w http.ResponseWriter, r *http.Request) {
defer r.Body.Close()
authenticated := false
form := auth.PasswordForm{}
- if err := json.NewDecoder(r.Body).Decode(&form); err != nil {
- api.HandleErr(w, r, nil, http.StatusBadRequest, err,
nil)
- return
- }
- if form.Username == "" || form.Password == "" {
- api.HandleErr(w, r, nil, http.StatusBadRequest,
errors.New("username and password are required"), nil)
- return
- }
- resp := struct {
- tc.Alerts
- }{}
+ var resp tc.Alerts
dbCtx, cancelTx := context.WithTimeout(r.Context(),
time.Duration(cfg.DBQueryTimeoutSeconds)*time.Second)
defer cancelTx()
- userAllowed, err, blockingErr :=
auth.CheckLocalUserIsAllowed(form, db, dbCtx)
- if blockingErr != nil {
- api.HandleErr(w, r, nil, http.StatusServiceUnavailable,
nil, fmt.Errorf("error checking local user password: %s\n",
blockingErr.Error()))
- return
- }
- if err != nil {
- log.Errorf("checking local user: %s\n", err.Error())
+
+ // Attempt to perform client certificate authentication. If
fails, goto standard form auth. If the
+ // certificate was verified, has a UID, and the UID matches an
existing user we consider this to
+ // be a successful login.
+ {
+ // No certs provided by the client. Skip to form
authentication
+ if r.TLS == nil || len(r.TLS.PeerCertificates) == 0 {
+ goto FormAuth
+ }
+
+ // If no configuration is set, skip to form auth
+ if cfg.ClientCertAuth == nil ||
len(cfg.ClientCertAuth.RootCertsDir) == 0 {
+ goto FormAuth
+ }
+
+ // Perform certificate verification to ensure it is
valid against Root CAs
+ err := auth.VerifyClientCertificate(r,
cfg.ClientCertAuth.RootCertsDir)
+ if err != nil {
+ log.Warnf("ClientCertAuth: error attempting to
verify client provided TLS certificate. err: %s\n", err)
+ goto FormAuth
+ }
+
+ // Client provided a verified certificate. Extract UID
value.
+ form.Username =
auth.ParseClientCertificateUID(r.TLS.PeerCertificates[0])
+ if len(form.Username) == 0 {
+ log.Infoln("ClientCertAuth: client provided
certificate did not contain a UID object identifier or value")
+ goto FormAuth
+ }
+
+ // Check if user exists locally (TODB) and has a role.
+ var blockingErr error
+ authenticated, err, blockingErr =
auth.CheckLocalUserIsAllowed(form.Username, db, dbCtx)
+ if blockingErr != nil {
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user has
role: %s", blockingErr.Error()))
+ return
+ }
+ if err != nil {
+ log.Warnf("ClientCertAuth: checking local user:
%s\n", err.Error())
+ }
+
+ // Check LDAP if enabled
+ if !authenticated && cfg.LDAPEnabled {
+ _, authenticated, err =
auth.LookupUserDN(form.Username, cfg.ConfigLDAP)
+ if err != nil {
+ log.Warnf("ClientCertAuth: checking
ldap user: %s\n", err.Error())
+ }
+ }
}
- if userAllowed {
+
+ FormAuth:
+ // Failed certificate-based auth, perform standard form auth
+ if !authenticated {
+ // Perform form authentication
+ if err := json.NewDecoder(r.Body).Decode(&form); err !=
nil {
+ api.HandleErr(w, r, nil, http.StatusBadRequest,
err, nil)
+ return
+ }
+ if form.Username == "" || form.Password == "" {
+ api.HandleErr(w, r, nil, http.StatusBadRequest,
errors.New("username and password are required"), nil)
+ return
+ }
+
+ // Check if user exists and has a role
+ userAllowed, err, blockingErr :=
auth.CheckLocalUserIsAllowed(form.Username, db, dbCtx)
+ if blockingErr != nil {
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user has
role: %s", blockingErr.Error()))
+ return
+ }
+ if err != nil {
+ log.Errorf("checking local user: %s\n",
err.Error())
+ }
+
+ // User w/ role does not exist, return unauthorized
+ if !userAllowed {
+ resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid
username or password.")
+ w.WriteHeader(http.StatusUnauthorized)
+ api.WriteRespRaw(w, r, resp)
+ return
+ }
+
+ // Check local DB or LDAP
authenticated, err, blockingErr =
auth.CheckLocalUserPassword(form, db, dbCtx)
if blockingErr != nil {
- api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user
password: %s\n", blockingErr.Error()))
+ api.HandleErr(w, r, nil,
http.StatusServiceUnavailable, nil, fmt.Errorf("error checking local user
password: %s", blockingErr.Error()))
return
}
if err != nil {
log.Errorf("checking local user password:
%s\n", err.Error())
}
var ldapErr error
- if !authenticated {
- if cfg.LDAPEnabled {
- authenticated, ldapErr =
auth.CheckLDAPUser(form, cfg.ConfigLDAP)
- if ldapErr != nil {
- log.Errorf("checking ldap user:
%s\n", ldapErr.Error())
- }
+ if !authenticated && cfg.LDAPEnabled {
+ authenticated, ldapErr =
auth.CheckLDAPUser(form, cfg.ConfigLDAP)
+ if ldapErr != nil {
+ log.Errorf("checking ldap user: %s\n",
ldapErr.Error())
}
}
- if authenticated {
- httpCookie := tocookie.GetCookie(form.Username,
defaultCookieDuration, cfg.Secrets[0])
- http.SetCookie(w, httpCookie)
-
- var jwtToken jwt.Token
- var jwtSigned []byte
- jwtBuilder := jwt.NewBuilder()
-
- emptyConf := config.CdniConf{}
- if cfg.Cdni != nil && *cfg.Cdni != emptyConf {
- ucdn, err := auth.GetUserUcdn(form, db,
dbCtx)
- if err != nil {
- // log but do not error out
since this is optional in the JWT for CDNi integration
- log.Errorf("getting ucdn for
user %s: %v", form.Username, err)
- }
- jwtBuilder.Claim("iss", ucdn)
- jwtBuilder.Claim("aud", cfg.Cdni.DCdnId)
- }
+ }
- jwtBuilder.Claim("exp",
httpCookie.Expires.Unix())
- jwtBuilder.Claim(api.MojoCookie,
httpCookie.Value)
- jwtToken, err = jwtBuilder.Build()
- if err != nil {
- api.HandleErr(w, r, nil,
http.StatusInternalServerError, nil, fmt.Errorf("building token: %s", err))
- return
- }
+ // Failed to authenticate in either local DB or LDAP, return
unauthorized
+ if !authenticated {
+ resp = tc.CreateAlerts(tc.ErrorLevel, "Invalid username
or password.")
+ w.WriteHeader(http.StatusUnauthorized)
+ api.WriteRespRaw(w, r, resp)
+ return
+ }
- jwtSigned, err = jwt.Sign(jwtToken, jwa.HS256,
[]byte(cfg.Secrets[0]))
- if err != nil {
- api.HandleErr(w, r, nil,
http.StatusInternalServerError, nil, err)
- return
- }
+ // Successful authentication, write cookie and return
+ httpCookie := tocookie.GetCookie(form.Username,
defaultCookieDuration, cfg.Secrets[0])
+ http.SetCookie(w, httpCookie)
- http.SetCookie(w, &http.Cookie{
- Name: api.AccessToken,
- Value: string(jwtSigned),
- Path: "/",
- MaxAge: httpCookie.MaxAge,
- Expires: httpCookie.Expires,
- HttpOnly: true, // prevents the cookie
being accessed by Javascript. DO NOT remove, security vulnerability
- })
-
- // If all's well until here, then update last
authenticated time
- tx, txErr := db.BeginTx(dbCtx, nil)
- if txErr != nil {
- api.HandleErr(w, r, tx,
http.StatusInternalServerError, nil, fmt.Errorf("beginning transaction: %w",
txErr))
- return
- }
- defer func() {
- if err := tx.Commit(); err != nil &&
err != sql.ErrTxDone {
- log.Errorln("committing
transaction: " + err.Error())
- }
- }()
- _, dbErr := tx.Exec(UpdateLoginTimeQuery,
form.Username)
- if dbErr != nil {
- log.Errorf("unable to update
authentication time for a given user: %s\n", dbErr.Error())
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel,
"Unable to update authentication time for a given user")}
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.SuccessLevel,
"Successfully logged in.")}
- }
+ var jwtToken jwt.Token
+ var jwtSigned []byte
+ jwtBuilder := jwt.NewBuilder()
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel, "Invalid
username or password.")}
+ emptyConf := config.CdniConf{}
+ if cfg.Cdni != nil && *cfg.Cdni != emptyConf {
+ ucdn, err := auth.GetUserUcdn(form, db, dbCtx)
+ if err != nil {
+ // log but do not error out since this is
optional in the JWT for CDNi integration
+ log.Errorf("getting ucdn for user %s: %v",
form.Username, err)
}
- } else {
- resp = struct {
- tc.Alerts
- }{tc.CreateAlerts(tc.ErrorLevel, "Invalid username or
password.")}
+ jwtBuilder.Claim("iss", ucdn)
+ jwtBuilder.Claim("aud", cfg.Cdni.DCdnId)
}
- respBts, err := json.Marshal(resp)
+
+ jwtBuilder.Claim("exp", httpCookie.Expires.Unix())
Review Comment:
Fixed in #7110
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
