{web,server}: use html/template and reduce use of auth request ID
Switch from using "text/template" to "html/template", which provides basic XSS preventions. We haven't identified any particular place where unsanitized user data is rendered to the frontend. This is just a preventative step. At the same time, make more templates take pure URL instead of forming an URL themselves using an "authReqID" argument. This will help us stop using the auth req ID in certain places, preventing garbage collection from killing login flows that wait too long at the login screen. Also increase the login session window (time between initial redirect and the user logging in) from 30 minutes to 24 hours, and display a more helpful error message when the session expires. How to test: 1. Spin up dex and example with examples/config-dev.yaml. 2. Login through both the password prompt and the direct redirect. 3. Edit examples/config-dev.yaml removing the "connectors" section. 4. Ensure you can still login with a password. (email/password is "admin@example.com" and "password")
This commit is contained in:
parent
a3ef8d26bc
commit
72a431dd4b
@ -155,14 +155,22 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
authReq.Expiry = s.now().Add(time.Minute * 30)
|
// TODO(ericchiang): Create this authorization request later in the login flow
|
||||||
|
// so users don't hit "not found" database errors if they wait at the login
|
||||||
|
// screen too long.
|
||||||
|
//
|
||||||
|
// See: https://github.com/coreos/dex/issues/646
|
||||||
|
authReq.Expiry = s.now().Add(24 * time.Hour) // Totally arbitrary value.
|
||||||
if err := s.storage.CreateAuthRequest(authReq); err != nil {
|
if err := s.storage.CreateAuthRequest(authReq); err != nil {
|
||||||
s.logger.Errorf("Failed to create authorization request: %v", err)
|
s.logger.Errorf("Failed to create authorization request: %v", err)
|
||||||
s.renderError(w, http.StatusInternalServerError, "Failed to connect to the database.")
|
s.renderError(w, http.StatusInternalServerError, "Failed to connect to the database.")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(s.connectors) == 1 {
|
if len(s.connectors) == 1 {
|
||||||
for id := range s.connectors {
|
for id := range s.connectors {
|
||||||
|
// TODO(ericchiang): Make this pass on r.URL.RawQuery and let something latter
|
||||||
|
// on create the auth request.
|
||||||
http.Redirect(w, r, s.absPath("/auth", id)+"?req="+authReq.ID, http.StatusFound)
|
http.Redirect(w, r, s.absPath("/auth", id)+"?req="+authReq.ID, http.StatusFound)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
@ -174,12 +182,14 @@ func (s *Server) handleAuthorization(w http.ResponseWriter, r *http.Request) {
|
|||||||
connectorInfos[i] = connectorInfo{
|
connectorInfos[i] = connectorInfo{
|
||||||
ID: id,
|
ID: id,
|
||||||
Name: conn.DisplayName,
|
Name: conn.DisplayName,
|
||||||
URL: s.absPath("/auth", id),
|
// TODO(ericchiang): Make this pass on r.URL.RawQuery and let something latter
|
||||||
|
// on create the auth request.
|
||||||
|
URL: s.absPath("/auth", id) + "?req=" + authReq.ID,
|
||||||
}
|
}
|
||||||
i++
|
i++
|
||||||
}
|
}
|
||||||
|
|
||||||
if err := s.templates.login(w, connectorInfos, authReq.ID); err != nil {
|
if err := s.templates.login(w, connectorInfos); err != nil {
|
||||||
s.logger.Errorf("Server template error: %v", err)
|
s.logger.Errorf("Server template error: %v", err)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@ -198,7 +208,11 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
|
|||||||
authReq, err := s.storage.GetAuthRequest(authReqID)
|
authReq, err := s.storage.GetAuthRequest(authReqID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
s.logger.Errorf("Failed to get auth request: %v", err)
|
s.logger.Errorf("Failed to get auth request: %v", err)
|
||||||
s.renderError(w, http.StatusInternalServerError, "Database error.")
|
if err == storage.ErrNotFound {
|
||||||
|
s.renderError(w, http.StatusBadRequest, "Login session expired.")
|
||||||
|
} else {
|
||||||
|
s.renderError(w, http.StatusInternalServerError, "Database error.")
|
||||||
|
}
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
scopes := parseScopes(authReq.Scopes)
|
scopes := parseScopes(authReq.Scopes)
|
||||||
@ -229,7 +243,7 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
http.Redirect(w, r, callbackURL, http.StatusFound)
|
http.Redirect(w, r, callbackURL, http.StatusFound)
|
||||||
case connector.PasswordConnector:
|
case connector.PasswordConnector:
|
||||||
if err := s.templates.password(w, authReqID, r.URL.String(), "", false); err != nil {
|
if err := s.templates.password(w, r.URL.String(), "", false); err != nil {
|
||||||
s.logger.Errorf("Server template error: %v", err)
|
s.logger.Errorf("Server template error: %v", err)
|
||||||
}
|
}
|
||||||
case connector.SAMLConnector:
|
case connector.SAMLConnector:
|
||||||
@ -277,7 +291,7 @@ func (s *Server) handleConnectorLogin(w http.ResponseWriter, r *http.Request) {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
if !ok {
|
if !ok {
|
||||||
if err := s.templates.password(w, authReqID, r.URL.String(), username, true); err != nil {
|
if err := s.templates.password(w, r.URL.String(), username, true); err != nil {
|
||||||
s.logger.Errorf("Server template error: %v", err)
|
s.logger.Errorf("Server template error: %v", err)
|
||||||
}
|
}
|
||||||
return
|
return
|
||||||
|
@ -2,6 +2,7 @@ package server
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
|
"html/template"
|
||||||
"io"
|
"io"
|
||||||
"io/ioutil"
|
"io/ioutil"
|
||||||
"net/http"
|
"net/http"
|
||||||
@ -9,7 +10,6 @@ import (
|
|||||||
"path/filepath"
|
"path/filepath"
|
||||||
"sort"
|
"sort"
|
||||||
"strings"
|
"strings"
|
||||||
"text/template"
|
|
||||||
)
|
)
|
||||||
|
|
||||||
const (
|
const (
|
||||||
@ -181,23 +181,20 @@ func (n byName) Len() int { return len(n) }
|
|||||||
func (n byName) Less(i, j int) bool { return n[i].Name < n[j].Name }
|
func (n byName) Less(i, j int) bool { return n[i].Name < n[j].Name }
|
||||||
func (n byName) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
|
func (n byName) Swap(i, j int) { n[i], n[j] = n[j], n[i] }
|
||||||
|
|
||||||
func (t *templates) login(w http.ResponseWriter, connectors []connectorInfo, authReqID string) error {
|
func (t *templates) login(w http.ResponseWriter, connectors []connectorInfo) error {
|
||||||
sort.Sort(byName(connectors))
|
sort.Sort(byName(connectors))
|
||||||
|
|
||||||
data := struct {
|
data := struct {
|
||||||
Connectors []connectorInfo
|
Connectors []connectorInfo
|
||||||
AuthReqID string
|
}{connectors}
|
||||||
}{connectors, authReqID}
|
|
||||||
return renderTemplate(w, t.loginTmpl, data)
|
return renderTemplate(w, t.loginTmpl, data)
|
||||||
}
|
}
|
||||||
|
|
||||||
func (t *templates) password(w http.ResponseWriter, authReqID, callback, lastUsername string, lastWasInvalid bool) error {
|
func (t *templates) password(w http.ResponseWriter, postURL, lastUsername string, lastWasInvalid bool) error {
|
||||||
data := struct {
|
data := struct {
|
||||||
AuthReqID string
|
PostURL string
|
||||||
PostURL string
|
Username string
|
||||||
Username string
|
Invalid bool
|
||||||
Invalid bool
|
}{postURL, lastUsername, lastWasInvalid}
|
||||||
}{authReqID, string(callback), lastUsername, lastWasInvalid}
|
|
||||||
return renderTemplate(w, t.passwordTmpl, data)
|
return renderTemplate(w, t.passwordTmpl, data)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -5,7 +5,7 @@
|
|||||||
<div>
|
<div>
|
||||||
{{ range $c := .Connectors }}
|
{{ range $c := .Connectors }}
|
||||||
<div class="theme-form-row">
|
<div class="theme-form-row">
|
||||||
<a href="{{ $c.URL }}?req={{ $.AuthReqID }}" target="_self">
|
<a href="{{ $c.URL }}" target="_self">
|
||||||
<button class="dex-btn theme-btn-provider">
|
<button class="dex-btn theme-btn-provider">
|
||||||
<span class="dex-btn-icon dex-btn-icon--{{ $c.ID }}"></span>
|
<span class="dex-btn-icon dex-btn-icon--{{ $c.ID }}"></span>
|
||||||
<span class="dex-btn-text">Log in with {{ $c.Name }}</span>
|
<span class="dex-btn-text">Log in with {{ $c.Name }}</span>
|
||||||
|
@ -15,7 +15,6 @@
|
|||||||
</div>
|
</div>
|
||||||
<input tabindex="2" required id="password" name="password" type="password" class="theme-form-input" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/>
|
<input tabindex="2" required id="password" name="password" type="password" class="theme-form-input" placeholder="password" {{ if .Invalid }} autofocus {{ end }}/>
|
||||||
</div>
|
</div>
|
||||||
<input type="hidden" name="req" value="{{ .AuthReqID }}"/>
|
|
||||||
|
|
||||||
{{ if .Invalid }}
|
{{ if .Invalid }}
|
||||||
<div class="dex-error-box">
|
<div class="dex-error-box">
|
||||||
|
Reference in New Issue
Block a user